-
Notifications
You must be signed in to change notification settings - Fork 593
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
Update macos build #2638
Update macos build #2638
Conversation
@@ -6,7 +6,7 @@ $(project)_libraries += Glacier2CryptPermissionsVerif | |||
|
|||
Glacier2CryptPermissionsVerifier_targetdir := $(libdir) | |||
Glacier2CryptPermissionsVerifier_dependencies := Glacier2 Ice | |||
Glacier2CryptPermissionsVerifier[shared]_cppflags := -DCRYPT_PERMISSIONS_VERIFIER_API_EXPORTS | |||
Glacier2CryptPermissionsVerifier_cppflags := -DCRYPT_PERMISSIONS_VERIFIER_API_EXPORTS |
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.
Reverted an earlier commit that add "[shared]". We want to use exactly the same flags for static and shared builds on macos.
@@ -47,7 +47,7 @@ | |||
# endif | |||
# endif | |||
|
|||
# if !defined(__FreeBSD__) && defined(ICE_API_EXPORTS) |
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.
Not sure if we can remove this defined(ICE_API_EXPORTS)
. It seems to work fine on macos. Would be good to test on Linux with a static build.
Ice_excludes = src/Ice/DLLMain.cpp | ||
Ice[shared]_excludes = src/Ice/RegisterPluginsInit_min.cpp |
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.
That's how I can use the same flags for static/shared build and yet get a small configuration-specific behavior: include a different impl file depending on the configuration.
// Include the UDP and WS transport plugins with non-static builds. | ||
// | ||
#if defined(ICE_API_EXPORTS) | ||
// Include the UDP and WS transport plugins with "shared" builds. |
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.
That's the implementation (_all) we want to use/include in all builds, except the configuration=static build.
@@ -98,6 +98,7 @@ def getFilters(self, mapping, config): | |||
"IceSSL/configuration", | |||
"IceDiscovery/simple", | |||
"IceGrid/simple", | |||
"IceUtil/stacktrace", |
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.
Enable this test for static builds. Otherwise, it's hard to see if we have an issue with backtraces and static builds!
This PR reworks the build settings, primarily on macos.
On macos, -fPIC and -PIE are not necessary: they do nothing. This gives us the opportunity to use the compile static and shared
.o
with the same flags and reuse them in 'shared' and 'static' builds... and that's exactly what this PR does.On Linux, we keep (for now) fPIC, fPIE and separate directories as before.