Skip to content
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

Add support for building with Mingw-gcc #17

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

hmartinez82
Copy link
Contributor

@hmartinez82 hmartinez82 commented Oct 3, 2023

Tune CMakeLists to use the right compiler switches when using MINGW

$ ctest
Test project D:/Dev/Github/CalcMySky/build
      Start  1: Spectrum test 1
 1/12 Test  #1: Spectrum test 1 .....................................................   Passed    0.07 sec
      Start  2: Spectrum test 2
 2/12 Test  #2: Spectrum test 2 .....................................................   Passed    0.01 sec
      Start  3: Spectrum test 3
 3/12 Test  #3: Spectrum test 3 .....................................................   Passed    0.01 sec
      Start  4: Spectrum test 4
 4/12 Test  #4: Spectrum test 4 .....................................................   Passed    0.01 sec
      Start  5: Fourier interpolation,  odd-length input, identity transformation
 5/12 Test  #5: Fourier interpolation,  odd-length input, identity transformation ...   Passed    0.01 sec
      Start  6: Fourier interpolation, even-length input, identity transformation
 6/12 Test  #6: Fourier interpolation, even-length input, identity transformation ...   Passed    0.01 sec
      Start  7: Fourier interpolation,  odd-length input, integral upsampling
 7/12 Test  #7: Fourier interpolation,  odd-length input, integral upsampling .......   Passed    0.01 sec
      Start  8: Fourier interpolation, even-length input, integral upsampling
 8/12 Test  #8: Fourier interpolation, even-length input, integral upsampling .......   Passed    0.01 sec
      Start  9: Fourier interpolation,  odd-length input, fractional upsampling
 9/12 Test  #9: Fourier interpolation,  odd-length input, fractional upsampling .....   Passed    0.01 sec
      Start 10: Fourier interpolation, even-length input, fractional upsampling
10/12 Test #10: Fourier interpolation, even-length input, fractional upsampling .....   Passed    0.01 sec
      Start 11: Spline interpolation
11/12 Test #11: Spline interpolation ................................................   Passed    0.01 sec
      Start 12: Catching exceptions from libShowMySky
12/12 Test #12: Catching exceptions from libShowMySky ...............................   Passed    0.03 sec

100% tests passed, 0 tests failed out of 12

Total Test time (real) =   0.20 sec

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

What is the issue with external only TUs? I've never heard of it. Any link?

@hmartinez82
Copy link
Contributor Author

hmartinez82 commented Oct 3, 2023

It's a known issue and I'm still working on a solution. It's only a problem with clang. GCC does not have this issue :(
9 yr old - https://stackoverflow.com/questions/25050575/basic-use-of-c-archives-libraries-with-clang-linker
2 yr old and still happening https://stackoverflow.com/questions/67045075/why-is-clang-reporting-an-undefined-symbol-for-an-extern-variable-referenced-in

@hmartinez82
Copy link
Contributor Author

So far I only got it locally building with clang if I remove the extern and convert it to a static and initialize its value in config.h instead of version.cpp

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

So far I only got it locally building with clang if I remove the extern and convert it to a static and initialize its value in config.h instead of version.cpp

Well this is not a good solution because it will force rebuilding of all sources from scratch on every commit (even CCache won't speed it up). Isolation of the change to a single file, version.cpp, was deliberate.

From your links I see that initializing the external definition helps the people with this problem. But here PROJECT_VERSION is already initialized, since it's a constant. Am I missing something?

@hmartinez82
Copy link
Contributor Author

I forgot to add "and removing its initialization from version.cpp"

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

One relatively clean solution would be to turn this constant into a function: const char* PROJECT_VERSION();. Then version.cpp would contain the definition of this function, while config.h would declare it.

@hmartinez82
Copy link
Contributor Author

Yes. I'm turning it into a function. After your comment about CCache, converting it into a function came to my mind

@hmartinez82
Copy link
Contributor Author

I'm at a loss. Even after the conversion to a function above, now I'm getting:

[2/4] Linking CXX executable CalcMySky\calcmysky.exe
FAILED: CalcMySky/calcmysky.exe
cmd.exe /C "cd . && C:\msys64\clang64\bin\c++.exe -Werror=return-type -Wall -Wextra -fvisibility=hidden  CalcMySky/CMakeFiles/calcmysky.dir/calcmysky_autogen/mocs_compilation.cpp.obj CalcMySky/CMakeFiles/calcmysky.dir/main.cpp.obj CalcMySky/CMakeFiles/calcmysky.dir/util.cpp.obj CalcMySky/CMakeFiles/calcmysky.dir/glinit.cpp.obj CalcMySky/CMakeFiles/calcmysky.dir/cmdline.cpp.obj CalcMySky/CMakeFiles/calcmysky.dir/shaders.cpp.obj CalcMySky/CMakeFiles/calcmysky.dir/interpolation-guides.cpp.obj -o CalcMySky\calcmysky.exe -Wl,--out-implib,CalcMySky\libcalcmysky.dll.a -Wl,--major-image-version,0,--minor-image-version,0  libversion.a  libcommon.a  C:/msys64/clang64/lib/libQt6OpenGL.dll.a  C:/msys64/clang64/lib/libQt6Widgets.dll.a  C:/msys64/clang64/lib/libQt6Gui.dll.a  C:/msys64/clang64/lib/libQt6Core.dll.a  -pthread  -lmpr  -luserenv  -ld3d11  -ldxgi  -ldxguid  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
ld.lld: error: undefined symbol: GetProjectVersion()
>>> referenced by CalcMySky/CMakeFiles/calcmysky.dir/main.cpp.obj:(main)
c++: error: linker command failed with exit code 1 (use -v to see invocation)
[4/4] Linking CXX executable ShowMySky\showmysky.exe
FAILED: ShowMySky/showmysky.exe
cmd.exe /C "cd . && C:\msys64\clang64\bin\c++.exe -Werror=return-type -Wall -Wextra -fvisibility=hidden  ShowMySky/CMakeFiles/showmysky-cmd.dir/showmysky-cmd_autogen/mocs_compilation.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/qrc_resources.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/win32res.rc.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/main.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/util.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/GLWidget.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/MainWindow.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/ToolsWidget.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/Manipulator.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/RadiancePlot.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/DockScrollArea.cpp.obj ShowMySky/CMakeFiles/showmysky-cmd.dir/GLSLCosineQualityChecker.cpp.obj -o ShowMySky\showmysky.exe -Wl,--out-implib,ShowMySky\libshowmysky.dll.a -Wl,--major-image-version,0,--minor-image-version,0  libversion.a  libcommon.a  C:/msys64/clang64/lib/libQt6OpenGLWidgets.dll.a  C:/msys64/clang64/lib/libQt6Widgets.dll.a  C:/msys64/clang64/lib/libQt6OpenGL.dll.a  C:/msys64/clang64/lib/libQt6Gui.dll.a  C:/msys64/clang64/lib/libQt6Core.dll.a  -pthread  -lmpr  -luserenv  -ld3d11  -ldxgi  -ldxguid  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
ld.lld: error: undefined symbol: GetProjectVersion()

But only with clang! gcc and msvc ware linking correctly, grr

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

Even after the conversion to a function above, now I'm getting

What if you avoid -fvisibility=hidden?

@hmartinez82
Copy link
Contributor Author

Same thing, removing -fvisibility=hidden was the very first thing I tried 😢

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

Interestingly, I experience no problem whatsoever when building with clang 10.0.0-4ubuntu1 on Ubuntu 20.04, even with the current master.

@hmartinez82
Copy link
Contributor Author

hmartinez82 commented Oct 3, 2023

I'm using 17.0.1 from MSYS2 I'm targeting x86_64-w64-windows-gnu

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

Well, what if you populate clang_dummy with the code to return the pointer, and call it instead of PROJECT_VERSION in the places of use? It seems strange that a simple renaming (and a change of return type) leads to a problem.

@hmartinez82
Copy link
Contributor Author

I already did that, (well, not called clang_dymmy). you'll have to refresh this PR

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

I already did that, (well, not called clang_dymmy). you'll have to refresh this PR

No, I mean, your clang_dummy, IIUC, fixed the problem. But now the GetProjectVersion reintroduces it. So I suggest trying to go from the working state, checking what exact change reintroduces the problem.

@hmartinez82
Copy link
Contributor Author

hmartinez82 commented Oct 3, 2023

The original PR was also failing with the clang_dummy() call. It was a draft to have others chime in to help :)
That's why this is a Draft and not a full PR

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

The original PR was also failing with the clang_dummy() call.

Well, this you didn't mention :)
Anyway, since I can't reproduce this, I can't try to fix.

@hmartinez82 hmartinez82 changed the title Add support for building with Mingw-gcc and Clang Add support for building with Mingw-gcc Oct 3, 2023
@hmartinez82
Copy link
Contributor Author

Ok. I've changed the title. Let's chew one thing at a time. I'll start working on the MSYS2 recipe for this project. INDIlib is next.

@hmartinez82 hmartinez82 marked this pull request as ready for review October 3, 2023 07:15
@10110111 10110111 merged commit 95e5d97 into 10110111:master Oct 3, 2023
@hmartinez82 hmartinez82 deleted the mingw_support branch October 3, 2023 07:16
@hmartinez82
Copy link
Contributor Author

Here we go msys2/MINGW-packages#18697

@hmartinez82
Copy link
Contributor Author

@10110111 where can I download .atmo files to test ShowMySky.exe ?

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

examples/sample.atmo

@10110111
Copy link
Owner

10110111 commented Oct 3, 2023

Ah, wait, it's for CalcMySky. For ShowMySky you can download the earth atmosphere model in the release assets list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants