compilation fails with GCC 7.3.0

[Kenneth Hoste]

g++ -o src/metabat2.o -c -Wall -g -std=c++11 -fopenmp -O3 -DNDEBUG -Wno-unknown-pragmas -Wno-deprecated-declarations -Wno-overflow -Wno-unused-variable -Iinclude -I/prefix/software/Boost/1.67.0-foss-2018b/include src/metabat2.cpp

src/metabat2.cpp: In function ‘int main(int, char**)’:
src/metabat2.cpp:382:54: error: expected ‘#pragma omp’ clause before ‘proc_bind’
     #pragma omp parallel for num_threads(numThreads) proc_bind(spread) schedule(dynamic)

I'm using GCC 7.3.0 + binutils 2.30 (on CentOS 7, but using custom GCC/binutils installations, not the ones provided via the OS).

Same error as #26, but with a very recent GCC...

Should the pragma be this instead?

#pragma omp for threads(numThreads) proc_bind(spread) schedule(dynamic)

[Rob Egan]

Please try with the standard build of binutils, as I believe your install is faulty. The code compiles fine on all versions of gcc >=4.9 where OpenMP is supported by the compiler and linking tools.

Doing your proposed change would produce incorrect code. That pragma:

#pragma omp parallel for
{...}

is a valid shorthand for:

#pragma omp parallel
#pragma omp for
{...}

regan@cori08:~/workspace/metabat-bitbucket> g++ --version g++ (GCC) 7.3.0 20180125 (Cray Inc.) Copyright (C) 2017 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

regan@cori08:~/workspace/metabat-bitbucket> g++ -o src/metabat2.o -c -Wall -g -std=c++11 -fopenmp -O3 -DNDEBUG -Wno-unknown-pragmas -Wno-deprecated-declarations -Wno-overflow -Wno-unused-variable -Iinclude -I${BOOST_ROOT}/include src/metabat2.cpp

[Kenneth Hoste]

Thank you for the reply @robegan21 !

Your reply made me realize what was going on: somehow the system compiler (g++ 4.8.5) was being picked up rather than the one we provide ourselves, despite it being made available via $PATH and the SConstruct file passing down $PATH into the SCons controlled environment...

Using "CXX='which g++; g++' scons ..." I found out that /bin/g++ was being used, rather than the intended /apps/.../GCC/7.3.0/bin/g++.

The culprit is that although the SConstruct file picks up the $PATH value and adds it to the controlled environment, it does so using SCons.Util.AppendPath, so any paths that are already in $PATH still get precedence. And /bin is one of those paths (it looks like the default $PATH value is /opt/bin:/bin).

So the fix was the following patch:

--- berkeleylab-metabat-37db58fe3fda/SConstruct.orig    2017-09-01 07:28:02.000000000 +0200
+++ berkeleylab-metabat-37db58fe3fda/SConstruct 2019-02-26 16:13:03.150132631 +0100
@@ -7,7 +7,7 @@
 def ENV_update(tgt_ENV, src_ENV):
     for K in src_ENV.keys():
         if K in tgt_ENV.keys() and K in [ 'PATH', 'LD_LIBRARY_PATH', 'LIB', 'INCLUDE' ]:
-            tgt_ENV[K]=SCons.Util.AppendPath(tgt_ENV[K], src_ENV[K])
+            tgt_ENV[K]=SCons.Util.PrependPath(tgt_ENV[K], src_ENV[K])
         else:
             tgt_ENV[K]=src_ENV[K]

If it makes sense to make this change, I'm happy to open a pull request for it...

[Rob Egan]

Sure that works and a pull request would be great. Thanks.

[Kenneth Hoste]

@robegan21 See pull request #9

[Rob Egan]

prepend $PATH & co to existing value in controlled environment (rather than append), to avoid picking up system g++ compiler instead of first g++ found via defined $PATH (fixes #52)

→ <<cset 7db8a1566e08>>