Skip to content
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

Print name of env var in --print=deployment-target #133041

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Nov 14, 2024

The deployment target environment variable is OS-specific, and if you're in a place where you're asking rustc for the deployment target, you're likely to also wanna know the name of the environment variable. I myself wanted this for some code I'm working on in bootstrap, for example.

Behaviour before this PR:

$ rustc --print=deployment-target --target=aarch64-apple-darwin
deployment_target=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
deployment_target=1.0

Behaviour after this PR:

$ rustc --print=deployment-target --target=aarch64-apple-darwin
MACOSX_DEPLOYMENT_TARGET=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
XROS_DEPLOYMENT_TARGET=1.0

My belief is that this option is extremely rarely used in general, and a GitHub search for "rustc print deployment-target" seems to confirm this, it revealed only the following actual pieces of code using this:

maturin does .split('=').last(), so it will continue to work after this change, but cc v1.0.84 did .strip_prefix("deployment_target=") since this PR, so it would break. That's probably fine though, it was broken in a lot of scenarios anyway, and got reverted in v1.0.85.

So while this is technically a breaking change, I really doubt that anyone is going to observe it, so it's probably fine.

@BlackHoleFox wdyt?

@rustbot label O-apple
r? compiler

@madsmtm madsmtm changed the title Print env var in --print=deployment-target Print name of env var in --print=deployment-target Nov 14, 2024
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 14, 2024
@rustbot

This comment was marked as off-topic.

@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 14, 2024

This also makes it easier to use in shell scripts, you can now do for example:

export $(rustc --print=deployment-target --target=$MY_APPLE_TARGET)
# Or
env $(rustc --print=deployment-target --target=$MY_APPLE_TARGET) clang --target $MY_APPLE_TARGET foo.c

@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 15, 2024

Rustbot didn't seem to pick it up the first time, so:

@rustbot label O-apple

@rustbot rustbot added the O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) label Nov 15, 2024
@fee1-dead
Copy link
Member

r? compiler

@rustbot rustbot assigned davidtwco and unassigned fee1-dead Nov 15, 2024
@BlackHoleFox
Copy link
Contributor

iirc one of the reasons I made it print without the variable name is so it has a higher chance of working with non-Apple OSes whenever RFCs/interested parties get along to making cfg()s for configuring what version of Windows, Android, etc a Rust build should target. I don't think its set in stone those would have env variables to go along with?

@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 16, 2024

iirc one of the reasons I made it print without the variable name is so it has a higher chance of working with non-Apple OSes whenever RFCs/interested parties get along to making cfg()s for configuring what version of Windows, Android, etc a Rust build should target. I don't think its set in stone those would have env variables to go along with?

That's fair reasoning, although then IMO it should've been called rustc --print=target-os-version, and honestly just returned it directly without the deployment_target= in front.

Given that we now have some cruft in the front anyhow, I think we might as well use it for something useful. Besides, if this is expanded to other targets, they can just use deployment_target= like done today.

@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 16, 2024

#133092 is what I wanted it for, btw

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense, "deployment target" seems to be very Apple-specific terminology, so I think if we wanted to introduce a "target OS version" alternative that supported more platforms (and just returned the deployment target for Apple platforms) then we could still do that and keep this as Apple-specific.

@davidtwco
Copy link
Member

Based on your description, it doesn't seem like this is likely to break anything, but we can always revert it if we get reports of issues.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 21, 2024

📌 Commit 44933b5 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 21, 2024
…v-var, r=davidtwco

Print name of env var in `--print=deployment-target`

The deployment target environment variable is OS-specific, and if you're in a place where you're asking `rustc` for the deployment target, you're likely to also wanna know the name of the environment variable. I myself wanted this for some code I'm working on in bootstrap, for example.

Behaviour before this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
deployment_target=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
deployment_target=1.0
```

Behaviour after this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
MACOSX_DEPLOYMENT_TARGET=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
XROS_DEPLOYMENT_TARGET=1.0
```

My _belief_ is that this option is extremely rarely used in general, and a GitHub search for "rustc print deployment-target" seems to confirm this, it revealed only the following actual pieces of code using this:
- https://github.com/PyO3/maturin/blob/b292ef69349f2a56cb8ab1b59fda0be3d3b9f138/src/build_context.rs#L1199-L1220
- https://github.com/rust-lang/cc-rs/blob/daab9244b03e244c4f2511944870d719c443f61f/src/lib.rs#L3422-L3426

`maturin` does `.split('=').last()`, so it will continue to work after this change, but `cc v1.0.84` did `.strip_prefix("deployment_target=")` since [this PR](rust-lang/cc-rs#848), so it would break. That's _probably_ fine though, it was broken in a lot of scenarios anyway, and [got](rust-lang/cc-rs#901) [reverted](rust-lang/cc-rs#943) in `v1.0.85`.

So while this is _technically_ a breaking change, I really doubt that anyone is going to observe it, so it's probably fine.

`@BlackHoleFox` wdyt?

`@rustbot` label O-apple
r? compiler
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 22, 2024
…v-var, r=davidtwco

Print name of env var in `--print=deployment-target`

The deployment target environment variable is OS-specific, and if you're in a place where you're asking `rustc` for the deployment target, you're likely to also wanna know the name of the environment variable. I myself wanted this for some code I'm working on in bootstrap, for example.

Behaviour before this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
deployment_target=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
deployment_target=1.0
```

Behaviour after this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
MACOSX_DEPLOYMENT_TARGET=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
XROS_DEPLOYMENT_TARGET=1.0
```

My _belief_ is that this option is extremely rarely used in general, and a GitHub search for "rustc print deployment-target" seems to confirm this, it revealed only the following actual pieces of code using this:
- https://github.com/PyO3/maturin/blob/b292ef69349f2a56cb8ab1b59fda0be3d3b9f138/src/build_context.rs#L1199-L1220
- https://github.com/rust-lang/cc-rs/blob/daab9244b03e244c4f2511944870d719c443f61f/src/lib.rs#L3422-L3426

`maturin` does `.split('=').last()`, so it will continue to work after this change, but `cc v1.0.84` did `.strip_prefix("deployment_target=")` since [this PR](rust-lang/cc-rs#848), so it would break. That's _probably_ fine though, it was broken in a lot of scenarios anyway, and [got](rust-lang/cc-rs#901) [reverted](rust-lang/cc-rs#943) in `v1.0.85`.

So while this is _technically_ a breaking change, I really doubt that anyone is going to observe it, so it's probably fine.

``@BlackHoleFox`` wdyt?

``@rustbot`` label O-apple
r? compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2024
Rollup of 13 pull requests

Successful merges:

 - rust-lang#130867 (distinguish overflow and unimplemented in Step::steps_between)
 - rust-lang#131859 (Update TRPL to add new Chapter 17: Async and Await)
 - rust-lang#132090 (Stop being so bail-y in candidate assembly)
 - rust-lang#132658 (Detect const in pattern with typo)
 - rust-lang#133041 (Print name of env var in `--print=deployment-target`)
 - rust-lang#133102 (aarch64 softfloat target: always pass floats in int registers)
 - rust-lang#133159 (Don't allow `-Zunstable-options` to take a value )
 - rust-lang#133217 ([AIX] Add option -X32_64 to the "strip" command)
 - rust-lang#133237 (Minimally constify `Add`)
 - rust-lang#133238 (re-export `is_loongarch_feature_detected`)
 - rust-lang#133286 (Re-delay a resolve `bug` related to `Self`-ctor in patterns)
 - rust-lang#133301 (Add code example for `wrapping_neg` method for signed integers)
 - rust-lang#133313 (Use arc4random of libc for RTEMS target)

Failed merges:

 - rust-lang#133215 (Fix missing submodule in `./x vendor`)

r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu
Copy link
Member

Failed in rollup:

---- [run-make] tests/run-make/apple-sdk-version stdout ----
[trimmed]
--- stderr -------------------------------
thread 'main' panicked at /Users/runner/work/rust/rust/tests/run-make/apple-sdk-version/rmake.rs:30:70:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 22, 2024
The deployment target environment variable is OS-specific, and if you're
in a place where you're asking `rustc` for the deployment target, you're
likely to also wanna know the environment variable.
@madsmtm madsmtm force-pushed the print-deployment-target-env-var branch from 44933b5 to 431c500 Compare November 22, 2024 19:50
@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 22, 2024

Oops, fixed now.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 22, 2024
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 2, 2024

📌 Commit 431c500 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2024
jhpratt added a commit to jhpratt/rust that referenced this pull request Dec 3, 2024
…v-var, r=davidtwco

Print name of env var in `--print=deployment-target`

The deployment target environment variable is OS-specific, and if you're in a place where you're asking `rustc` for the deployment target, you're likely to also wanna know the name of the environment variable. I myself wanted this for some code I'm working on in bootstrap, for example.

Behaviour before this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
deployment_target=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
deployment_target=1.0
```

Behaviour after this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
MACOSX_DEPLOYMENT_TARGET=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
XROS_DEPLOYMENT_TARGET=1.0
```

My _belief_ is that this option is extremely rarely used in general, and a GitHub search for "rustc print deployment-target" seems to confirm this, it revealed only the following actual pieces of code using this:
- https://github.com/PyO3/maturin/blob/b292ef69349f2a56cb8ab1b59fda0be3d3b9f138/src/build_context.rs#L1199-L1220
- https://github.com/rust-lang/cc-rs/blob/daab9244b03e244c4f2511944870d719c443f61f/src/lib.rs#L3422-L3426

`maturin` does `.split('=').last()`, so it will continue to work after this change, but `cc v1.0.84` did `.strip_prefix("deployment_target=")` since [this PR](rust-lang/cc-rs#848), so it would break. That's _probably_ fine though, it was broken in a lot of scenarios anyway, and [got](rust-lang/cc-rs#901) [reverted](rust-lang/cc-rs#943) in `v1.0.85`.

So while this is _technically_ a breaking change, I really doubt that anyone is going to observe it, so it's probably fine.

`@BlackHoleFox` wdyt?

`@rustbot` label O-apple
r? compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#132723 (Unify `sysroot_target_{bin,lib}dir` handling)
 - rust-lang#133041 (Print name of env var in `--print=deployment-target`)
 - rust-lang#133325 (Reimplement `~const` trait specialization)
 - rust-lang#133395 (Add simd_relaxed_fma intrinsic)
 - rust-lang#133517 (Deeply normalize when computing implied outlives bounds)
 - rust-lang#133785 (Add const evaluation error UI test.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3ec2107 into rust-lang:master Dec 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
Rollup merge of rust-lang#133041 - madsmtm:print-deployment-target-env-var, r=davidtwco

Print name of env var in `--print=deployment-target`

The deployment target environment variable is OS-specific, and if you're in a place where you're asking `rustc` for the deployment target, you're likely to also wanna know the name of the environment variable. I myself wanted this for some code I'm working on in bootstrap, for example.

Behaviour before this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
deployment_target=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
deployment_target=1.0
```

Behaviour after this PR:
```console
$ rustc --print=deployment-target --target=aarch64-apple-darwin
MACOSX_DEPLOYMENT_TARGET=11.0
$ rustc --print=deployment-target --target=aarch64-apple-visionos
XROS_DEPLOYMENT_TARGET=1.0
```

My _belief_ is that this option is extremely rarely used in general, and a GitHub search for "rustc print deployment-target" seems to confirm this, it revealed only the following actual pieces of code using this:
- https://github.com/PyO3/maturin/blob/b292ef69349f2a56cb8ab1b59fda0be3d3b9f138/src/build_context.rs#L1199-L1220
- https://github.com/rust-lang/cc-rs/blob/daab9244b03e244c4f2511944870d719c443f61f/src/lib.rs#L3422-L3426

`maturin` does `.split('=').last()`, so it will continue to work after this change, but `cc v1.0.84` did `.strip_prefix("deployment_target=")` since [this PR](rust-lang/cc-rs#848), so it would break. That's _probably_ fine though, it was broken in a lot of scenarios anyway, and [got](rust-lang/cc-rs#901) [reverted](rust-lang/cc-rs#943) in `v1.0.85`.

So while this is _technically_ a breaking change, I really doubt that anyone is going to observe it, so it's probably fine.

``@BlackHoleFox`` wdyt?

``@rustbot`` label O-apple
r? compiler
@madsmtm madsmtm deleted the print-deployment-target-env-var branch December 3, 2024 12:56
@fmease
Copy link
Member

fmease commented Dec 4, 2024

sync @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants