add Build::apple_deployment_target function #938
Conversation
flag_if_supported doesn't work for this because try_get_compiler adds the Apple deployment target argument before flags_supported is processed, so users can only append a second -mmacosx-version-min flag via flag_if_supported. To work around this, add a function specifically for this purpose.
| @@ -3321,7 +3333,7 @@ impl Build { | |||
| Ok(ret) | |||
| } | |||
|
|
|||
| fn apple_deployment_version( | |||
| fn get_apple_deployment_target( | |||
There was a problem hiding this comment.
Can we change the return type to Cow<'_, str> to avoid cloning str?
|
This seems like related to #902 |
|
cc @Be-ing if you don't mind it, I can update the PR for you. |
|
I did some more testing, and the existing |
|
IMO it's better to add this API, not just due to the order of cmdline arg, but cc-rs seems to have different behavior based on apple deployment target version and build target. |
Yes, but that can be addressed using the existing |
|
If that can be reliably solved by But I'm not sure if it would work on gcc or msvc. |
| .or_else(|| rustc_provided_target(rustc, target)) | ||
| .map(maybe_cpp_version_baseline) | ||
| .unwrap_or_else(|| OLD_IOS_MINIMUM_VERSION.into()), | ||
|
|
||
| AppleOs::WatchOs => deployment_from_env("WATCHOS_DEPLOYMENT_TARGET") | ||
| AppleOs::WatchOs => deployment_from_env("_DEPLOYMENT_TARGET") |
There was a problem hiding this comment.
This looks like an error maybe?
There was a problem hiding this comment.
whoops, yeah that was an accident
|
I don't think this is needed anymore with #943. If someone wants a lower deployment target than the SDK default, they can set the |
|
@Be-ing On the contrary I think this should still be added. That last option is the only programmatic way to do it and manipulating the process' environment variables is an uh, dicey area. Having a proper library API to control this for each target would be nice imo. |
|
Now that the old behavior of defaulting to the SDK's default deployment target has been restored, what is the need for this API? The default is a recent deployment target, so old code builds fine. A crate specifying a lower minimum than the default wouldn't really change anything. |
What do you mean? |
|
If a crate specifies a lower deployment target than the default, it's effectively superfluous because it would have built anyway. Only if every crate in the dependency graph that uses cc specifies the same lower deployment target will the resulting binary actually be usable on the lower deployment target. That's best achieved via the environment variable because that effects every crate that uses cc. |
|
Seems like a reasonable take, the well-stated counter-argument is appreciated :) |
flag_if_supported doesn't work for this because try_get_compiler adds the Apple deployment target argument before flags_supported is processed, so users can only append a second -mmacosx-version-min flag via flag_if_supported. To work around this, add a function specifically for this purpose.