RosettaCommons / rosetta

The Rosetta Bio-macromolecule modeling package.
https://www.rosettacommons.org
Other
116 stars 55 forks source link

build: use python3 as binary name, not python #37

Closed smoe closed 5 months ago

smoe commented 5 months ago

I apologize for this triviality, but if compatible with the other distros I kind request to adopt the "3". It just broke the build on Debian/Ubuntu where python2 can still be co-installed.

roccomoretti commented 5 months ago

Do we have a good sense of the current state for python3 installation -- not installation of Python 3 (which should be widespread enough now*), but installation of an executable named python3? There's a tickle in the back of my head that there were systems where this was complicated (Arch? Certain Mac installations?), but those might have been fixed by now, or at the very least might not be worth worrying about anymore.

smoe commented 5 months ago

My local compilation on Debian has progressed to PyRosetta, which encountered a few additional invocations of a "python" binary. It is still running, will add to this PR accordingly once that has completed.

My 14.4 macOS does not have a python binary but takes python3 from brew:

$ which python
$ which python3
/opt/homebrew/bin/python3

I also attempted to install on the mac but ran into a missing "imp" python module very early in the compilation process as triggered by scons.py. Would transition to conda if that was any urgent and will eventually at some long winter night contribute that imp module to homebrew :)

roccomoretti commented 5 months ago

We had another person who ran into imp issues with scons -- imp is a Python standard library module, so installation isn't an issue. I believe the issue is that imp was deprecated and Python 3.12 removed it.

We'll need to update the version of Scons we provide to fix it (assuming the issue is in scons modules). Until then, you'll likely have to use Python 3.11 or before, or attempt to use an externally installed Scons, rather than the Rosetta-provided version.

smoe commented 5 months ago

PyRosetta build now worked for me on Debian unstable.

smoe commented 5 months ago

We'll need to update the version of Scons we provide to fix it (assuming the issue is in scons modules). Until then, you'll likely have to use Python 3.11 or before, or attempt to use an externally installed Scons, rather than the Rosetta-provided version.

The local scons and the apps/public/ERRASER seem to be the culprits:

$ find . -name "*py" | xargs -r grep -r "import imp"
./build/prefix/llvm-6.0.1/tools/clang/utils/check_cfc/check_cfc.py:import imp
./external/rdkit/rdkit.upstream/Code/GraphMol/Wrap/rough_test.py:import importlib.util
./external/rdkit/rdkit.upstream/Code/GraphMol/ChemReactions/Wrap/testReactionWrapper.py:import importlib.util
./external/scons-local/sconsign.py:import imp
./external/scons-local/scons-local-3.0.4/SCons/Tool/packaging/__init__.py:import imp
./external/scons-local/scons-local-3.0.4/SCons/Tool/__init__.py:import imp
./external/scons-local/scons-local-3.0.4/SCons/Tool/__init__.py:import importlib
./external/scons-local/scons-local-3.0.4/SCons/Tool/__init__.py:            # import importlib.util
./external/scons-local/scons-local-3.0.4/SCons/Tool/__init__.py:            import importlib.util
./external/scons-local/scons-local-3.0.4/SCons/Platform/__init__.py:import imp
./external/scons-local/scons-local-3.0.4/SCons/compat/__init__.py:import imp  # Use the "imp" module to protect imports from fixers.
./external/scons-local/scons-local-3.0.4/SCons/Node/FS.py:            import imp
./external/scons-local/scons-local-3.0.4/SCons/Node/FS.py:            import importlib.util
./external/scons-local/scons-local-3.0.4/SCons/Script/Main.py:        import imp, re
./src/apps/public/ERRASER/rna_rosetta_to_pdb.py:import imp
./src/apps/public/ERRASER/erraser_analysis.py:import imp
./src/apps/public/ERRASER/erraser_util.py:import imp
./src/apps/public/ERRASER/SWA_rebuild.py:import imp
./src/apps/public/ERRASER/erraser.py:import imp
./src/apps/public/ERRASER/rna_rosetta_ready_set.py:import imp
./src/apps/public/ERRASER/erraser_single_res_analysis.py:import imp
./src/apps/public/ERRASER/erraser_wrapper.py:import imp
./src/apps/public/ERRASER/seq_rebuild.py:import imp
./src/apps/public/ERRASER/full_struct_minimize.py:import imp
./src/apps/public/ERRASER/erraser_single_res.py:import imp
./src/apps/public/ERRASER/erraser_option.py:import imp
./src/apps/public/ERRASER/rna_decompose.py:import imp
./scripts/python/public/generic_potential/Molecule.py:import importlib
./scripts/python/public/generic_potential/mol2genparams.py:import importlib`

Scons development (now at version 4.7 https://github.com/SCons/scons/tags) seems to have leapfroged since version 3.0.4 that Rosetta uses. An external installation of SCons (Debian's is at 4.5.2, brew's at 4.7, conda's at 4.1) I would personally prefer, but I am too remote to the scons community to have any strong preferences.

smoe commented 5 months ago

@roccomoretti , @lyskov , I can confirm that on my macOS 14.4.x with the scons distributed by homebrew (instead of invoking ./source/scons.py) , the missing imp module is not reported. Update: The build completes fine on the mac, which found clang by default, only a few warnings by the linker on duplicated libraries. What you may want to have a look at is:

clang -o build/external/debug/macos/14.4/64/arm/clang/15.0/default/rdkit/ML/Cluster/Murtagh/hc.os -c -std=c99 -isystem external/boost_submod/ -isystem external/ -isystem external/include/ -isystem external/dbio/ -isystem external/libxml2/include -isystem external/rdkit -mtune=generic -pipe -Qunused-arguments -DUNUSUAL_ALLOCATOR_DECLARATION -ftemplate-depth-256 -stdlib=libstdc++ -Wno-long-long -Wno-strict-aliasing -mtune=native -stdlib=libc++ -Wno-unused-variable -Wno-implicit-function-declaration -O0 -g -fPIC -DBOOST_ERROR_CODE_HEADER_ONLY -DBOOST_SYSTEM_NO_DEPRECATED -DBOOST_MATH_NO_LONG_DOUBLE_MATH_FUNCTIONS -DBOOST_DISABLE_THREADS -DPTR_STD -Iexternal/include external/rdkit/ML/Cluster/Murtagh/hc.c
external/rdkit/ML/Cluster/Murtagh/hc.c:93:20: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
            ind = ioffset_(n, &i__, &j);
                          ^
external/rdkit/ML/Cluster/Murtagh/hc.c:132:20: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
    ind1 = ioffset_(n, &i2, &j2);
                   ^
external/rdkit/ML/Cluster/Murtagh/hc.c:149:21: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
            ind1 = ioffset_(n, &i2, &k);
                           ^
external/rdkit/ML/Cluster/Murtagh/hc.c:151:21: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
            ind1 = ioffset_(n, &k, &i2);
                           ^
external/rdkit/ML/Cluster/Murtagh/hc.c:154:21: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
            ind2 = ioffset_(n, &j2, &k);
                           ^
external/rdkit/ML/Cluster/Murtagh/hc.c:156:21: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
            ind2 = ioffset_(n, &k, &j2);
                           ^
external/rdkit/ML/Cluster/Murtagh/hc.c:158:17: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
        ind3 = ioffset_(n, &i2, &j2);
                       ^
external/rdkit/ML/Cluster/Murtagh/hc.c:247:20: warning: passing arguments to 'ioffset_' without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
            ind = ioffset_(n, &i__, &j);
                          ^
external/rdkit/ML/Cluster/Murtagh/hc.c:29:22: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
/* Subroutine */ int hc_(n, len, iopt, ia, ib, crit, membr, nn, disnn, flag__,
                     ^
external/rdkit/ML/Cluster/Murtagh/hc.c:280:9: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
integer ioffset_(n, i__, j)
        ^
external/rdkit/ML/Cluster/Murtagh/hc.c:7:16: warning: a function declaration without a prototype is deprecated in all versions of C and is treated as a zero-parameter prototype in C2x, conflicting with a subsequent definition [-Wdeprecated-non-prototype]
extern integer ioffset_();
               ^
11 warnings generated.

This is harmless but looks bad :) I presume this to can get fixed by recreating external/rdkit/ML/Cluster/Murtagh/hc.c (autogenerated by f2c from hc.f this is, not part of the build process) and maybe by reordering the functions in hc.f. I can prepare a PR if that is helpful.

Used was the clang++ provided by Xcode:

$ clang++ --version
Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: arm64-apple-darwin23.4.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

with Python from homebrew and python executable not available:

$ which python
$ which python3
/opt/homebrew/bin/python3
$ python3 --version
Python 3.12.2