ANTsX / ANTs

Advanced Normalization Tools (ANTs)
Apache License 2.0
1.21k stars 381 forks source link

Test CXX_HAS_DISABLE_OPTIMIZATION_FLAG failed #898

Open yarikoptic opened 5 years ago

yarikoptic commented 5 years ago

originally reported long ago in #234, then was thought to be addressed, reported by me in #832 against 2.3.1, and didn't actually die for me with 2.3.2 release:

root@smaug:/tmp/buildd/ants-2.3.2# sed -n -e '/CXX_HAS_DISABLE_OPTIMIZATION_FLAG failed/,10000p' ./obj-x86_64-linux-gnu/CMakeFiles/CMakeError.log
Performing C++ SOURCE FILE Test CXX_HAS_DISABLE_OPTIMIZATION_FLAG failed with the following output:
Change Dir: /build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_3090e/fast"
make[2]: Entering directory '/build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp'
/usr/bin/make -f CMakeFiles/cmTC_3090e.dir/build.make CMakeFiles/cmTC_3090e.dir/build
make[3]: Entering directory '/build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_3090e.dir/src.cxx.o
/usr/bin/c++    -g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -DCXX_HAS_DISABLE_OPTIMIZATION_FLAG   -std=c++11 -o CMakeFiles/cmTC_3090e.dir/src.cxx.o -c /build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp/src.cxx
/build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp/src.cxx:1:1: error: expected unqualified-id before '-' token
    1 | -O0
      | ^
make[3]: *** [CMakeFiles/cmTC_3090e.dir/build.make:66: CMakeFiles/cmTC_3090e.dir/src.cxx.o] Error 1
make[3]: Leaving directory '/build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp'
make[2]: *** [Makefile:121: cmTC_3090e/fast] Error 2
make[2]: Leaving directory '/build/ants-2.3.2/obj-x86_64-linux-gnu/CMakeFiles/CMakeTmp'

Source file was:
-O0

When did the error occur? [x] CMake configuration (cmake / ccmake) [ ] Compilation (make) (initially I incorrectly specified that it was happening during make) [ ] Installation (make install)

Build environment

ANTs version 2.3.2 (as of the v2.3.2 tag)

Build configuration and logs

Please attach the following files (relative to your build directory)

Additional context

happens while building debian package in isolated environment, but some env variables (C*FLAGS etc might be set)

yarikoptic commented 5 years ago

BTW here is various *FLAGS set by debian packaging wrappers which might be relevant here:

# grep FLAG rules-export 
declare -x CFLAGS="-O2 -O2"
declare -x CPPFLAGS="-Wdate-time -D_FORTIFY_SOURCE=2"
declare -x CXXFLAGS="-g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong -Wformat -Werror=format-security"
declare -x DEB_LDFLAGS_MAINT_APPEND="-Wl,--as-needed"
declare -x DH_INTERNAL_BUILDFLAGS="1"
declare -x FCFLAGS="-g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong"
declare -x FFLAGS="-g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong"
declare -x GCJFLAGS="-g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong"
declare -x LDFLAGS="-Wl,-z,relro -Wl,--as-needed"
declare -x MAKEFLAGS="w"
declare -x MFLAGS="-w"
declare -x OBJCFLAGS="-g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong -Wformat -Werror=format-security"
declare -x OBJCXXFLAGS="-g -O2 -fdebug-prefix-map=/build/ants-2.3.2=. -fstack-protector-strong -Wformat -Werror=format-security"
gdevenyi commented 5 years ago
/usr/lib/cmake/ITK-4.12

I don't believe ITK4.x is supported for MASTER

yarikoptic commented 5 years ago
/usr/lib/cmake/ITK-4.12

I don't believe ITK4.x is supported for MASTER

isn't there a dedicated check somewhere in cmake files? FWIW, there is no ITK5 in my debian world yet unfortunately, ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=943677

gdevenyi commented 5 years ago

There isn't, the build system was in a pretty sorry state before I started hacking on it. The superbuild tracks ITK master pretty closely so its assumed right now that ANTs is a statically built set of binaries with the superbuild. I think packagers should be doing that to be considered in a supported configuration.

I'm almost ready to be releasing such packages automatically with release tags.

yarikoptic commented 5 years ago

For official debian packages, I wouldn't be able to use superbuild. It would be nice to know then what version of ITK which release is supposed to be built with -- I am ok to go for the most recent release of ANTs which is compatible with available ITK in Debian. So far, through the years we have managed to somehow find a balance ;-)

gdevenyi commented 5 years ago

This the version as of the v2.3.2 tag: https://github.com/ANTsX/ANTs/blob/v2.3.2/SuperBuild/External_ITKv5.cmake#L154

Closest ITK tag is https://github.com/InsightSoftwareConsortium/ITK/tree/v5.1b01

There are 95 commits between release and that tag: https://github.com/InsightSoftwareConsortium/ITK/compare/v5.1b01...39f1ba9b9540a5daa38ddb87d2c091d4db1fb45d

I don't believe we're taking advantage of any of those commits explicitly, we reverted the one: https://github.com/ANTsX/ANTs/commit/fbfe74d281fe3198ebb9f1732352b6a3b0742891

cookpa commented 5 years ago

You can find the exact ITK to build from the SuperBuild config:

https://github.com/ANTsX/ANTs/blob/master/SuperBuild/External_ITKv5.cmake#L154

The history of the SuperBuild directory will give clues as to when things change. The last release that uses ITK 4 is from 2017:

https://github.com/ANTsX/ANTs/releases/tag/v2.2.0

Looks like the switch over to ITK 5 happened in June 2018:

https://github.com/ANTsX/ANTs/commit/cfc8badd7524da1258385b956a5b11faad7235a6#diff-ac7bd306dd81aa183e37be355d0e0718

yarikoptic commented 5 years ago

The last release that uses ITK 4 is from 2017:

https://github.com/ANTsX/ANTs/releases/tag/v2.2.0

cool -- 2.2.0 is the one we made available in (Neuro)Debian land: http://neuro.debian.net/pkgs/ants.html

So it would be nice if upcoming ITK 5.1 (which I hope would enter Debian soonish) would be the one ANTs provided backward compatibility for a while (i.e. that there were no need for a bleeding edge 5.x.x to build a brand new ANTs). Meanwhile I will see if I could help Debian land to get ITK 5.x.

But regardless -- original issue should IMHO be resolved then with a dedicated error if it was triggered by outdated/incompatible ITK.

cookpa commented 5 years ago

It should generate an error, yes. I'm not sure why it's not.

BTW the last ITKv4 commit I could find is 9b51c8113e6dda7bdd59197099f9921d4bfbae88

cookpa commented 5 years ago

From your CMakeCache.txt:

//The directory containing a CMake configuration file for ITK.
ITK_DIR:PATH=/usr/lib/cmake/ITK-4.12

//Choose the expected ITK major version to build ANTS only version
// 5 allowed.
ITK_VERSION_MAJOR:STRING=5

I guess we need to check that the system ITK is the correct version. I don't get why the call to find_package doesn't produce some kind of error when the major version is wrong:

https://github.com/ANTsX/ANTs/blob/master/SuperBuild/External_ITKv5.cmake#L181