-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[cxxmodules] Removed libXXX.so from C++ module names.
#878
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
Conversation
|
Starting build on |
|
Build failed on mac1012/native. Errors:
|
691bc4f to
96fb265
Compare
|
Starting build on |
|
@phsft-bot build just on slc6/clang_gcc62 with flags -Dcxxmodules=ON -Dccache=OFF |
|
Starting build on |
|
Historically, we had this to disambiguate between pcm files generated by rootcling and 'regular' module files (i.e. if somebody unwisely forwarded the compiler's module cache there). |
cmake/modules/RootNewMacros.cmake
Outdated
| # problem. | ||
| # FIXME: error: declaration of 'NSObject' must be imported from module 'ROOT.libBonjour.so.TBonjourBrowser.h' before it is required | ||
| if (${library} MATCHES "libKrb5Auth.so" OR ${library} MATCHES "(libGCocoa|libGQuartz)\\..*") | ||
| if (${library} MATCHES "Krb5Auth" OR ${library} MATCHES "(GCocoa|libGQuartz)") |
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.
Why does libGQuartz keep the lib prefix?
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.
My bad, I think I forgot to save the latest changes in the text editor before I made the commit :)
cmake/modules/RootNewMacros.cmake
Outdated
| set(modulemap_entry "module \"${library}\" {") | ||
| # For modules GCocoa and GQuartz we need objc context. | ||
| if (${library} MATCHES "(libGCocoa|libGQuartz)\\..*") | ||
| if (${library} MATCHES "(libGCocoa|libGQuartz)") |
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.
Isn't this inconsistent with the change to ${library} MATCHES "(GCocoa|libGQuartz)" on line 483?
|
Build failed on slc6/clang_gcc62. Failing tests: |
The Core module is now just called `Core`, and no longer `libCore.so`, which leads to the confusing PCM file name `libCore.so.pcm` which is 75% just boilerplate prefixes and suffixes. This also make the error messages from clang more readable now as they use the same module name.
96fb265 to
f6d5d16
Compare
|
Starting build on |
|
@vgvassilev well, we use the same modulemap for building/runtime at the moment, so the names would be anyway equal :) Also if you try to trick root by placing other PCMs there you'll get an error because we check the clang version that generated the PCM before loading it, so you would have to do some serious work to get around this check. |
|
Build failed on slc6/clang_gcc62. Failing tests: |
|
LGTM, I assume the failures are due to the infamous clang/gcc ABI bug. |
The Core module is now just called
Core, and no longerlibCore.so,which leads to the confusing PCM file name
libCore.so.pcmwhich is 75%just boilerplate prefixes and suffixes. This also make the error
messages from clang more readable now as they use the same module name.