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

docs(ref): Add section on Apple deployment target environment variables #13781

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Apr 19, 2024

These aren't actually read by Cargo (yet), but since rustc doesn't read any other environment variables by itself, this seemed like the most obvious place to document them.

To get into the habit of using it:
@rustbot ping macos
@rustbot label O-macos

These aren't actually read by Cargo (yet), but since `rustc` doesn't read any other environment variables by itself, this seemed like the most obvious place to document them.
@rustbot
Copy link
Collaborator

rustbot commented Apr 19, 2024

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 19, 2024

Error: Only Rust team members can ping teams.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@rustbot rustbot added the O-macos OS: macOS label Apr 19, 2024
@@ -430,3 +430,33 @@ Cargo exposes this environment variable to 3rd party subcommands
* `CARGO` --- Path to the `cargo` binary performing the build.

For extended information about your environment you may run `cargo metadata`.

## Apple deployment targets environment variables
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR!

I am a bit unsure about it for some reasons:

  • This is a reference-like documentation for environment variables Cargo cares about. The new paragraph added is a “best practice” like guide, which seems to be a misplacement here.
  • There is no a single paragraph just for a specific platform in this reference. Not sure why prefer putting Apple specific things here. They might change over time so should avoid mentioning any single value.
  • People have different ways to set up their workflows. Haven't fully got the idea of emphasizing the [env] section here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the PR!

I am a bit unsure about it for some reasons:

* This is a reference-like documentation for environment variables Cargo cares about. The new paragraph added is a “best practice” like guide, which seems to be a misplacement here.

Do you mean that I should write it more tersely, or what do you mean by "best practice"?

* There is no a single paragraph just for a specific platform in this reference. Not sure why prefer putting Apple specific things here. They might change over time so should avoid mentioning any single value.

I'm not really aware of other platforms, but I think that these (along with SDKROOT) is the only environment variables that rustc and clang ever reads? And then I thought it'd make sense to document them in the same place as where other environment variables in the Rust ecosystem (i.e. Cargo's) is documented.

* People have different ways to set up their workflows. Haven't fully got the idea of emphasizing the `[env]` section here.

Emphazising [env] is not really important, mostly wanted to say that it was possible, i.e. that the parsing of the variables happens after [env].

Copy link
Member

Choose a reason for hiding this comment

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

By best practice I mean this addition isn't going to be use as a reference book you will check very often. It's more like a how-to guide, which requires a different kind of reading flow. For example, the Cargo Guide shares a more similar reading flow with best practices like docs.

We couuld possibly tweak it into a reference-like doc, but at that point we might probably just refer to Apple's official documentation instead.

the only environment variables that rustc and clang ever reads

Then in my opinion this might be better being in rustc doc or elsewhere. Cargo is not the only tool calling rustc. Cargo also doesn't read those env vars, so prefer not adding them here to avoid any false impression that Cargo handles them for users.


I understand discoverability matters. Putting it here seems like a good place to expose to a wider audience. Cargo already have some cookbook like content for some CI best practices. That style of content is a better fit for this I feel like. Rust By Examples might also be a good place for it.

Copy link
Member

Choose a reason for hiding this comment

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

Like, we can combine some practices from #13115 and write something somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've submitted rust-lang/rust#124772 to update the documentation in the platform support docs, let's see where that one goes first.

Copy link
Member

Choose a reason for hiding this comment

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

rust-lang/rust#124772 just got merged. Should we close this then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'd still prefer this to be here in some way, maybe just as a quick one or two-liner that links to the platform support docs?

I'd also be fine with writing a fuller guide-level documentation, if you think that'd be useful? Though it might also be a bit too platform-specific to really fit in the Cargo guide?

Copy link
Member

Choose a reason for hiding this comment

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

To me, still feel a bit odd to have a paragraph explaining something that Cargo doesn't really read. Even some variables like RUST_BACKTRACE, and RUSTUP_TOOLCHAIN, CLIPPY_CONF_DIR, which could also potentially affect how Cargo works, are left off because they are either an implementation detail, or not really directly read/written/controlled by Cargo. I just don't know how to fit them in.

@weihanglo weihanglo added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation O-macos OS: macOS S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants