-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configuration fixes #153
Configuration fixes #153
Conversation
- Build RFIO using dpm libraries if castor libraries are not available - Add additional include and library search paths - Add missing glib header path in GFAL module - Search also for globus libraries wouthout the flavour in the name - Add missing io/hdfs/CMakeLists.txt - net/globusauth has no installed headers - remove ROOT_INSTALL_HEADERS()
- bin/pq2 - bin/rootd - bin/xpdtest - initd and xinitd start-up scripts - don't install some private headers
The configure script currently uses a simple grep to determine if an option is deprecated. This causes problems, since the enable-reflex option is deprecated and grepping for enable-r will result in a match. The configure script therefore considers enable-r deprecated too, even though it is not. This commit addresses this issue.
It is no longer used in gcc 6
The old rule also changed e.g. -Werror=format-security to =format-security, which causes the compilation to fail.
It leads to incompatible options like: $ g++ -Werror=format-security -c a.cxx cc1plus: error: -Wformat-security ignored without -Wformat [-Werror=format-security] Before -Wall was removed -Wall implied -Wformat, and he options were compatible: $ g++ -Wall -Werror=format-security -c a.cxx (no error) This commit makes sure also -Werror=* is removed when -Wall is removed.
CMAKE_Fortran_FLAGS is initialized from the FFLAGS environment variable. If the initial value of CMAKE_Fortran_FLAGS is overwritten, comfiguring the build using FFLAGS fails.
ROOT uses builtin unuran sources, but has no switch to use the system library instead. This adds this missing option.
ROOT uses builtin gl2ps sources, but has no switch to use the system library instead. This adds this missing option.
Hexfloat constants are part of c++17. They can not be used when compileing with -std=c++11 or -std=c++14. Doing so will result in "error: exponent has no digits". It would be possible to use them with GNU extensions enabled (i.e. with -std=gnu++11 or -std=gnu++14), but root compilation is not configured this way.
The linking of rootcling and libCling requires a lot of memory. Since these are linke form mostly the same objects, the build is ready to link them at the same time. If you make a parallel build this means that the two targets that require the most amount of memory are being linked in parallel. This exhausts the available memory, and the computer starts swapping. This adds a dependency of one of the targets to the other. The dependency is not really there since it is not needed for building, but it prevents the two memory consuming targets to be built in parallel.
Thank-you very much Mattias. This is great work. I have committed the changes to master branch (modulo few conflicts I had to revolve). I'll close the PR but if you have further changes don't hesitate to add another one. |
Many thanks for merging the proposed changes. It feels good to be able to contribute. |
@@ -372,6 +372,8 @@ elseif (LLVM_NATIVE_ARCH MATCHES "hexagon") | |||
set(LLVM_NATIVE_ARCH Hexagon) | |||
elseif (LLVM_NATIVE_ARCH MATCHES "s390x") | |||
set(LLVM_NATIVE_ARCH SystemZ) | |||
elseif (LLVM_NATIVE_ARCH MATCHES "s390") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Mattias @ellert - the llvm people tell us at https://reviews.llvm.org/D33465 that this is wrong. We'll back this out if you don't object within the next couple of days... I hope you understand - if llvm explicitly says "wrong" it doesn't make much sense for us to overrule them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pr #153 was written in order to make the cmake build do the same as the configure build, and at the time the configure build did have support for s390. This was the reason for the proposed changes here - not to loose functionality when switching to the cmake build. But you are right - llvm does not support s390. So a better way to remove the differences between configure and cmake would have been to drop the s390 support from the configure build rather than adding it to the cmake build. I was not aware of this detail at the time.
Just make sure that support for s390x (which is supported by llvm) is not lost when you remove the s390 support. I don't know if it still is the case, but it used to be the case that in same places in the code tree the substring match on "s390" was supposed to match both "s390" and "s390x".
Thanks a lot for your fast reply, Mattias! What you say makes perfect sense of course, we'll take care of that.
Cheers, Axel
|
Root has two different ways to configure the build - the traditional configure script and cmake. The builds generated by the two systems are similar, but far from equivalent. Historically the configure script has been more feature complete and some things that the configure script is able to do are either missing or broken in the cmake build. However, new features are often only added to the cmake build. This has resulted in that today neither of the two is able to build root with a complete set of features.
The cmake build is more standard and behaves in a more predictive way, e.g. it understands CFLAGS, LDFLAGS and friends which the configure script never did. It also integrates the test suite in the build and allows for running "make test", a feature that the configure script is missing.
So the cmake build is in many ways better, if it wasn't for those missing and broken things mentioned earlier. This pull request is an attempt to address those missing and broken issues (though it fixes a few things for the configure script as well).
Also contains a fix for ROOT-7326.