Boost: Add embed_bitcode option #13970
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As of Xcode 14, Bitcode is deprecated and no longer accepted for AppStore submissions. This commit adds an `embed_bitcode` option with a default value of `True` to match the current behavior. When the compiler version is 14.0+ the option is deleted and the `-fembed-bitcode` flag is not added to the cxx_flags.
ghost
mentioned this pull request
|
I detected other pull requests that are modifying boost/all recipe:
This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prsso don't hesitate to report issues/improvements there. |
ghost
mentioned this pull request
4 tasks
|
I would have expected embed bitcode to not be injected manually by recipe itself, and it's weird to see a dedicated recipe option for a compiler flag. It should come from |
|
@SpaceIm I would agree, but it would appear that the recipe is ignoring Although I suppose I could make it check that configuration for this particular flag? |
|
boost is still written in conan v1 style, maybe it listens CC, CXX and LD env vars instead. But yes a good migration of this recipe to conan v2 would ensure that the new config tools.build.cflags etc are propagated to the build |
|
Looks like it does look at the CXXFLAGS in It would be great if the Boost recipe were migrated to v2, but that's really outside of the scope of time I actually have to dedicate to this right now. I've already got a list of around 15 recipes we use internally that I have to update! |
Conan v1 pipelineAll green in build 2 (
|
Hooks produced the following warnings for commit 6e92162boost/1.77.0boost/1.72.0boost/1.71.0boost/1.75.0boost/1.74.0boost/1.73.0boost/1.70.0boost/1.79.0boost/1.78.0boost/1.76.0boost/1.80.0 |
ericLemanissier
approved these changes
Nov 10, 2022
franramirez688
approved these changes
Nov 10, 2022
| @@ -1109,7 +1116,8 @@ def add_defines(library): | |||
| if self.options.multithreading: | |||
| cxx_flags.append("-DBOOST_SP_USE_SPINLOCK") | |||
|
|
|||
| cxx_flags.append("-fembed-bitcode") | |||
| if "embed_bitcode" in self.options and self.options.embed_bitcode: | |||
There was a problem hiding this comment.
Could it be simplified by:
Suggested change
| if "embed_bitcode" in self.options and self.options.embed_bitcode: | |
| if self.options.get_safe("embed_bitcode"): |
?
|
I don't understand why it's accepted... we should not add this kind of specific recipe option doing babysitting of cflags or cxxflags (rtti, bitcode, lto etc), but instead ensure that tools.build.cflags, tools.build.cxxflags & tools.build.ldflags config are honored, and that default builds are consistent across all conan-center recipes (rtti by default, no bitcode by default, no lto by default etc) |
4 tasks
@SpaceIm You were right. I thought that, as boost is not already migrated to Conan v2, it didn't belong to this PR to add those configuration flags but I just saw your recently opened PR #14139, and, yes, I agree with that. We can start adding those mechanisms and honor the new variables. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As of Xcode 14, Bitcode is deprecated and no longer accepted for AppStore submissions.
(see: https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes under the "Apple Clang Compiler: Deprecations" heading.)
This commit adds an
embed_bitcodeoption with a default value ofTrueto match the current behavior.When the compiler version is 14.0+ the option is deleted and the
-fembed-bitcodeflag is not added to the cxx_flags.Specify library name and version: lib/1.0
This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!