njoy / NJOY21

NJOY for the 21st Century
https://www.njoy21.io/NJOY21
Other
74 stars 38 forks source link

Compilation fails at link stage #110

Closed RokShox closed 4 years ago

RokShox commented 4 years ago

Context: Windows 10 64 bit Latest Cygwin (full install, just updated) gcc version 9.3.0 signature 1.1.1-NJOY21.json

Problem: At "make" stage of building per instructions from http://www.njoy21.io/Build/index.html, fails with following error: [ 84%] Building CXX object CMakeFiles/njoy21.dir/src/main.cpp.o /usr/lib/gcc/x86_64-pc-cygwin/9.3.0/../../../../x86_64-pc-cygwin/bin/as: CMakeFiles/njoy21.dir/src/main.cpp.o: too many sections (58183) /tmp/ccuu4fSO.s: Assembler messages: /tmp/ccuu4fSO.s: Fatal error: can't write 56 bytes to section .text of CMakeFiles/njoy21.dir/src/main.cpp.o: 'file too big' /usr/lib/gcc/x86_64-pc-cygwin/9.3.0/../../../../x86_64-pc-cygwin/bin/as: CMakeFiles/njoy21.dir/src/main.cpp.o: too many sections (58183) /tmp/ccuu4fSO.s: Fatal error: can't close CMakeFiles/njoy21.dir/src/main.cpp.o: file too big make[2]: [CMakeFiles/njoy21.dir/build.make:63: CMakeFiles/njoy21.dir/src/main.cpp.o] Error 1 make[1]: [CMakeFiles/Makefile2:115: CMakeFiles/njoy21.dir/all] Error 2 make: *** [Makefile:163: all] Error 2

Observations:

nathangibson14 commented 4 years ago

Thank you for reaching out. Unfortunately, our team does not often use NJOY on Windows, and I'm not sure the last time we attempted to build it on Windows. I will try to get my hands on a Windows machine and recreate the problem. A quick search tells me this error is Windows-specific and there may be some compile flags we could turn on to fix it. I don't really expect this to help, but you could try an older version of GCC in the meantime.

In any case, I suspect NJOY2016 won't run into the same issues. Thanks for telling us that our installation instructions for it are unclear. These are the commands I would run:

git clone https://github.com/njoy/NJOY2016
cd NJOY2016
mkdir bin && cd bin
cmake ..
make

That should give you version 2016.57 by default. But you could type git checkout 2016.57 or whatever other tag you are looking for after the second line above to be sure.

RokShox commented 4 years ago

Update The issue is related to two limitations in the Windows PE/COFF object file format:

The fix for the first limitation is to add the flag "-Wa,-mbig-obj" to the GCC compilation flags. Note this is a single argument which arranges for the -mbig-obj flag to be passed to the assembler (-Wa).

I modified CMakeLists.txt line 132: set( njoy21_GNU_CYGWIN_common_flags "-Wno-subobject-linkage" "-Wall" "-Wextra" "-Wpedantic" "-std=gnu++17" "-Wa,-mbig-obj" )

The second limitation I encountered when trying to build the debug version (cmake -DCMAKE_BUILD_TYPE=Debug ...) and is likely just too much debug info being put in the object file. The Release build, compiled with O3 optimization, does not encounter this limit.

So the Release version compiles with the addition of the "-Wa,-mbig-obj" flag.

But, all but the first test failed when I did a "make test". I reduced the optimization to O1 at CMakeLists.txt line 134:

set( njoy21_GNU_CYGWIN_RELEASE_flags "-O1" "-DNDEBUG" )

and it still compiles but fails the same tests. I'll look into this some more.

RokShox commented 4 years ago

Update 2

The tests were failing because shared libraries could not be located. I assume the cmake process is munging LD_LIBRARY_PATH during the "make test" to point at the dlls in ./subprojects, but Cygwin does not use LD_LIBRARY_PATH. Instead it looks for shared libraries on PATH directories.

I copied these dlls to /usr/local/bin (already on my PATH):

And all tests passed.

nathangibson14 commented 4 years ago

I'm glad that worked, those were the flags I was going to try. We'll hopefully get those implemented into our build system soon so that it works out of the box for future updates. Thanks for being willing to dig into the issue!

kahlerac commented 4 years ago

All, For what its worth, NJOY21.1.1.1 installs and passes all tests just fine under WSL/Ubuntu (at least after manually installing python2 since you only get python3 by default) so be careful you don't break something while fixing something else. And the soon to be May 2020 Win10 update is supposed to give us WSL2 so who knows what other "improvements" you'll have to work through! Skip

On Sat, May 30, 2020 at 5:23 PM Nathan A. Gibson notifications@github.com wrote:

I'm glad that worked, those were the flags I was going to try. We'll hopefully get those implemented into our build system soon so that it works out of the box for future updates. Thanks for being willing to dig into the issue!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/njoy/NJOY21/issues/110#issuecomment-636387138, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHJISPPD7N6JP5RXJSSN2LRUF2L7ANCNFSM4NOTEXJA .

-- Dr. A. C. (Skip) Kahler Kahler Nuclear Data Services, LLC kahler3ac@gmail.com +1 321 368 3645

jlconlin commented 4 years ago

So I'm glad that you were able to find a workaround for this. Officially, we do not support NJOY running in the Cygwin environment and suggest people use WSL. The only reason for this is that we don't have the resources to test every environment. We are working on improving the build system and hopefully that will improve our options for Windows environments.

nathangibson14 commented 4 years ago

Just to be clear, njoy21.io says: "Note that in Windows we currently support compiling and running under the Windows Subsystem for Linux or the Cygwin environment. If you have the necesssary components installed, this should also work in the DOS environment."

We should probably change that if we aren't going to officially support Cygwin :)

jlconlin commented 4 years ago

Oh

Just to be clear, njoy21.io says: "Note that in Windows we currently support compiling and running under the Windows Subsystem for Linux or the Cygwin environment. If you have the necesssary components installed, this should also work in the DOS environment."

We should probably change that if we aren't going to officially support Cygwin :)

Oops. I thought that it only said WSL. We are going to have to fix that.

RokShox commented 4 years ago

I'm not married to Cygwin wrt NJOY, but the support required was a simple compiler flag. I do use Cygwin apps for a lot of other stuff, particularly my precious Emacs.

nathangibson14 commented 4 years ago

@RokShox Thanks for the input. I agree, pushing your change to the CMakeLists to support Cygwin with the current version is easy enough.

That said, we're currently revamping our build system, so we'll have to have some internal discussions about how to approach the environments we support as part of that. The concern is if we make a big change (e.g., adding a new dependency, upgrading the C++ standard), we need to ensure it works on all officially-supported environments. And right now, we clearly aren't testing Cygwin!

RokShox commented 4 years ago

Yes, of course. I'm not pushing for it.