-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Implement host
-target substitution
#15838
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
Conversation
r? @weihanglo rustbot has assigned @weihanglo. Use |
In writing up this PR's explanation, I was unsure how to properly outline the following:
... due to my unfamiliarity with how testing the program that tests programs would exactly work. Perhaps, simply clone the PR and try to run Thanks. |
I have additionally tested locally that the following
Should I somehow mirror this within the PR, rather than just ensuring locally that builds with the above conditions do not compile? I think I could write an always-fail test that does this, but I wanted to make sure that's the way we want it done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I added more writing style suggestions then.
Also, feel free to rebase and organize your git history to make it more clearer to read and trace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I believe this is the last review round!
BTW, would you mind cleanup your commit history. There are some intermediate commits that may be good if we squash them for easy git diff tracing in the future.
src/doc/src/reference/config.md
Outdated
Possible values: | ||
- Any supported target in `rustc --print target-list`. | ||
- `"host"`, which will internally be substituted by the host's target. This can be particularly useful if you're cross-compiling some crates, and don't want to specify your host's machine as a target (for instance, an `xtask` in a shared project that may be worked on by many hosts). | ||
- A path to a custom target specification. See [Custom Target Lookup Path](https://doc.rust-lang.org/rustc/targets/custom.html#custom-target-lookup-path) for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually use relative paths to link to docs under the same toolchain versions.
- A path to a custom target specification. See [Custom Target Lookup Path](https://doc.rust-lang.org/rustc/targets/custom.html#custom-target-lookup-path) for more information. | |
- A path to a custom target specification. See [Custom Target Lookup Path](../../rustc/targets/custom.html#custom-target-lookup-path) for more information. |
(Doesn't matter for man pages as they don't support relative paths beyond cargo book)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered about that! Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized manpage also support it. Nice. However, we forgot to update config.md (which is this file).
@weihanglo Yes, I plan to clean up the history some time today. It's been a while since I've done a rebase & squash, so I'll need to do some research to make sure I don't permanently damage something with Git. |
See some resources Rust Compiler Development Guide: https://rustc-dev-guide.rust-lang.org/git.html#rebasing-and-conflicts. And I just explained why/how we maintain good Git history in Cargo in another PR 😆 |
I can also do the rebase for you, if you don't mind no longer being the committer of those commits. You will still be the author of those and GitHub gives contribution credits to authors AFAIK. |
That's alright, I can do it. It's valuable knowledge, this git-fu. Thank you for the resources. I'm working on it now. |
31c33a7
to
6993a86
Compare
This commit history looks good to me. Any concerns? |
src/doc/src/reference/config.md
Outdated
Possible values: | ||
- Any supported target in `rustc --print target-list`. | ||
- `"host"`, which will internally be substituted by the host's target. This can be particularly useful if you're cross-compiling some crates, and don't want to specify your host's machine as a target (for instance, an `xtask` in a shared project that may be worked on by many hosts). | ||
- A path to a custom target specification. See [Custom Target Lookup Path](https://doc.rust-lang.org/rustc/targets/custom.html#custom-target-lookup-path) for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized manpage also support it. Nice. However, we forgot to update config.md (which is this file).
We also rebuilt the manual pages, to ensure they're up-to-date with the changes made in the `host` target specifier PR.
Thank you for catching the URL usage in the It may be worth considering expanding the contribution guide to include more information on how documentation is written and contributed. There were many elements in this PR that were directly related to things that aren't covered in the contribution guide: syntax (e.g. that a special documentation preprocessor is used), relative pathing (confusion above), the need to run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working with us for this feature!
I noticed and understood the frictions when contributing to doc. To me, writing docs is never an easy task. Natural langauges, unlike programming languages, is too ambiguous and people tend to be opinioned on writing. Even among maintainers we often disagree each other's writing. You're right that there should be a clearer writing guideline. We do have one but is not really useful.
Based on your comment #15838 (comment), I've created a doc PR #15854 to improve this situation a bit. Hope it will help others. And again sorry for the frustration 😞.
@weihanglo No frustration at all; just thinking of how to make the process smoother for contributors in the future. Thanks for your help in getting this PR merged so quickly—it was great to receive such quick feedback. |
What does this PR try to resolve?
This PR resolves: #13051
Namely, it allows users to invoke cargo subcommands that accept a
--target
directive to specify thehost
target, which is later substituted in the command processing layer by the host's real target triple (for instance, on most Linux distributions,cargo build --target host
would effectively runcargo build --target x86_64-unknown-linux-gnu
).This additionally applies to usage within
config.toml
, like so: