_fragments is referenced but not defined or used

[drew-parsons]

Avogadro version: (please complete the following information from the About box):

• Avogadrolibs: 1.99.0
• Qt: 5.15.10

Desktop version: (please complete the following information):

• OS: Linux
• Version debian unstable
• Compiler gcc 13.2.0

Describe the bug
Avogadrolibs: 1.99.0 has added fragments from https://github.com/openchemistry/fragments processed in avogadro/qtplugins/templatetool/CMakeLists.txt

The CMakeLists.txt pulls the data from the git repo if ${_fragments} does not exist. But _fragments is not defined anywhere, and more to the point there is no handling in place if it does exist. This means if it does exists, the install step

install(DIRECTORY "${AvogadroLibs_SOURCE_DIR}/../fragments"

will fail, unless _fragments happens to be set to ${AvogadroLibs_SOURCE_DIR}/../fragments.

This is a problem for debian builds, for instance, where fragments are kept in debian/data/fragments (via git subtree, since by Debian policy data cannot be pulled from the internet at build time). In this case git clone should not be needed (the data has already been cloned and is available locally).

To Reproduce
Steps to reproduce the behavior:

1. git clone https://github.com/openchemistry/fragments /tmp/fragments
2. mkdir build; cd build
3. cmake -D_fragments=/tmp/fragments -DUSE_MMTF=OFF -DUSE_LIBMSYM=OFF ..
4. See error

Expected behavior
Since _fragment is referenced in avogadro/qtplugins/templatetool/CMakeLists.txt, you'd expect it can be used to identify the location of the fragments dir, without required git.

Screenshots

[ 98%] Creating directories for 'fragments'
cd /projects/avogadrolibs/obj-x86_64-linux-gnu/avogadro/qtplugins/templatetool && /usr/bin/cmake -Dcfgdir= -P /projects/avogadrolibs/obj-x86_64-linux-gnu/avogadro/qtplugins/templatetool/fragments-prefix/tmp/fragments-mkdirs.cmake
cd /projects/avogadrolibs/obj-x86_64-linux-gnu/avogadro/qtplugins/templatetool && /usr/bin/cmake -E touch /projects/avogadrolibs/obj-x86_64-linux-gnu/avogadro/qtplugins/templatetool/fragments-prefix/src/fragments-stamp/fragments-mkdir
[ 98%] Performing download step (git clone) for 'fragments'
cd /projects && /usr/bin/cmake -P /projects/avogadrolibs/obj-x86_64-linux-gnu/avogadro/qtplugins/templatetool/fragments-prefix/tmp/fragments-gitclone.cmake
Cloning into 'fragments'...
fatal: unable to access 'https://github.com/openchemistry/fragments/': Could not resolve host: github.com
Cloning into 'fragments'...
fatal: unable to access 'https://github.com/openchemistry/fragments/': Could not resolve host: github.com
Cloning into 'fragments'...
fatal: unable to access 'https://github.com/openchemistry/fragments/': Could not resolve host: github.com
-- Had to git clone more than once: 3 times.
CMake Error at avogadrolibs/obj-x86_64-linux-gnu/avogadro/qtplugins/templatetool/fragments-prefix/tmp/fragments-gitclone.cmake:39 (message):
  Failed to clone repository: 'https://github.com/openchemistry/fragments'


make[3]: *** [avogadro/qtplugins/templatetool/CMakeFiles/fragments.dir/build.make:104: avogadro/qtplugins/templatetool/fragments-prefix/src/fragments-stamp/fragments-download] Error 1

[ghutchis]

See #1612

[ghutchis]

If you have the latest from openchemistry it's also defined there:
https://github.com/openchemistry/openchemistry

[drew-parsons]

#1612 does not fix the problem. It just hardcodes a value for _fragment as ${SRC}../fragments. It doesn't allow for alternative path to the dir set by cmake -D_fragments.

crystals and molecules are also maintained in git repos. But they don't have this problem with their paths. Wait, I see what's different, I patched them for the debian build with https://salsa.debian.org/debichem-team/avogadrolibs/-/blob/master/debian/patches/qtplugins_insertfragment.patch I'll need to update this patch for fragments. We could perhaps remove this patch if the data dirs could be set with cmake -D, e.g. only set them via ${AvogadroLibs_SOURCE_DIR}/.. if they are not already defined.

[drew-parsons]

I guess the configuration would be more consistent if the install step was set as

install(DIRECTORY "${_fragments}"

instead of

install(DIRECTORY "${AvogadroLibs_SOURCE_DIR}/../fragments"

likewise for _crystals and _molecules

[ghutchis]

Does it make sense to have a higher-level variable like _moleculardata_dir or "${AvogadroMolecules_SOURCE_DIR}/fragments" or somesuch? We're likely to have a few more pieces (e.g., polymer builder, peptide builder) in the near future.

[drew-parsons]

I think that would work. Then I'd only have to set the one variable to point at the debian/data dir containing these molecular data dirs.