Autotools adds apple deployement target flag - #7862
Conversation
|
|
| if tools.is_apple_os(self._os): | ||
| concat = " ".join(tmp_compilation_flags) | ||
| if self._os_version and "-version-min" not in concat and "-target" not in concat: | ||
| tmp_compilation_flags.append(tools.apple_deployment_target_flag(self._os, |
There was a problem hiding this comment.
I would like to understand a bit better (not a OSX user) what is the effect of not adding this flag. Is it a compilation error? What have been the users doing so far for this?
Also, is there a chance that this could be breaking for other users that do not expect this flag to be added? Should this flag be added in other generators as well (cmake, xcode)?
There was a problem hiding this comment.
If this flag is not set and someone is trying to link a library with no minimal os version against an application explicitely specifying it then warning will span like mentioned here: bincrafters/community#1264 (comment)
Regarding the users that do not expect this flag to be added I wondered why they would have specify the os.version setting that (if I'm understanding well) states that the dependencies should support such OS.
Cmake generators allready handle the os.version flags by setting the CMAKE_OSX_DEPLOYMENT_TARGET definition: https://github.com/conan-io/conan/blob/develop/conans/client/build/cmake_flags.py#L216-L219
memsharded
left a comment
There was a problem hiding this comment.
It is looking good, please have a look to the CXXFLAGS issue and tell me.
| tmp_compilation_flags.append(pic_flag(self._conanfile.settings)) | ||
| if tools.is_apple_os(self._os): | ||
| concat = " ".join(tmp_compilation_flags) | ||
| if os.environ.get("CFLAGS", None): |
There was a problem hiding this comment.
Couldn't the version-min come defined also in CXXFLAGS? I think it would make sense check that var too, just in case, as C++ users are probably defining it in that var, what do you think?
|
Merged! Will be in Conan 1.31, thanks for your contribution @MartinDelille When it is released, maybe it is worth to revisit those recipes in conan-center-index to simplify them if the flags are no longer necessary in the recipes. |
|
Ok I leave this in my inbox and will look at it when 1.31 will be out! |
Changelog: Fix: Automatically add OSX deployment flags in
AutootoolsBuildEnvironmentwith the value ofos_version, unless the values are already defined in environment variablesCFLAGSorCXXFLAGS.Docs: Omit
developbranch, documenting this one.After submitting this pull request on CCI I realized that maybe that was something that
AutoToolsBuildEnvironmentcould do automatically since 4 recipes are using the same kind of code: zlib, icu, libcurl, openssl