Skip to content

Write .cargo/config.toml in x vendor#144124

Open
bjorn3 wants to merge 1 commit into
rust-lang:mainfrom
bjorn3:better_vendor
Open

Write .cargo/config.toml in x vendor#144124
bjorn3 wants to merge 1 commit into
rust-lang:mainfrom
bjorn3:better_vendor

Conversation

@bjorn3

@bjorn3 bjorn3 commented Jul 18, 2025

Copy link
Copy Markdown
Member

x vendor hides the cargo vendor telling you what to write to .cargo/config.toml. to use the vendored sources, so without this PR it isn't obvious how to use the vendored sources.

@rustbot

rustbot commented Jul 18, 2025

Copy link
Copy Markdown
Collaborator

r? @clubby789

rustbot has assigned @clubby789.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 18, 2025
@clubby789

Copy link
Copy Markdown
Contributor

Why not just print out the config info captured? That avoids the trailing

To use vendored sources, add this to your .cargo/config.toml for this project:

and avoids automatically overwriting a potentially existing config.toml.

@bjorn3

bjorn3 commented Jul 18, 2025

Copy link
Copy Markdown
Member Author

I found this will working on vendoring support for -Zbuild-std. That will require two separate .cargo/config.toml files to be written. One in the root and one in library/.

@clubby789

Copy link
Copy Markdown
Contributor

Sounds good - the broken output is a bit unfortunate but not the end of the world. r=me when green

@Kobzol

Kobzol commented Jul 18, 2025

Copy link
Copy Markdown
Member

I don't think people will appreciate when running x vendor might overwrite their .cargo/config.toml file. Bootstrap should just print what to do, or this behavior should be opt-in.


impl Step for Vendor {
type Output = VendorOutput;
type Output = ();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why remove this? GenerateCopyright can still make use of the final path, instead of having to know about VENDOR_DIR, which is an implementation detail of the vendor step.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

VendorOutput only ever contained the data that should be written to .cargo/config.toml.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't like overwriting the config.toml file on disk (as per my other comment), but if we did that, we can at least return the path to the vendor directory from the step, to avoid other steps reaching for VENDOR_DIR.

@rustbot

This comment has been minimized.

@bors

bors commented Nov 27, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #148987) made this pull request unmergeable. Please resolve the merge conflicts.

@bjorn3

bjorn3 commented Nov 27, 2025

Copy link
Copy Markdown
Member Author

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2025
retr0b0y pushed a commit to retr0b0y/rust that referenced this pull request Jun 30, 2026
helce pushed a commit to helce/rust that referenced this pull request Jun 30, 2026
Initially, this patch was proposed in rust-lang#144124.

Co-authored-by: Yuriy A. Dyagterev <dyagter_y@mcst.ru>
@rustbot

rustbot commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@bjorn3

bjorn3 commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

I found this will working on vendoring support for -Zbuild-std. That will require two separate .cargo/config.toml files to be written. One in the root and one in library/.

This change has recently landed.

@Kobzol

Kobzol commented Jul 1, 2026

Copy link
Copy Markdown
Member

This now produces a change where x vendor will overwrite .cargo/config.toml files in the source directory. I don't think that we should be doing that, if possible.

I'd suggest adding a flag to Vendor which tells it whether to write the config files or not. If the flag is unset (which should happen in x vendor), then bootstrap should instead print out the contents of the corresponding config.toml files and where should they be created by the user.

@bjorn3

bjorn3 commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

The only change in this PR is to also write the config files with ./x.py vendor. In all other cases they were already written. I guess I can change it to leave a message.

@Kobzol

Kobzol commented Jul 1, 2026

Copy link
Copy Markdown
Member

Yeah, that is the only situation that changed, but it is still a change, and I don't think it's great if bootstrap goes and modifies the source directories (which I consider anything under library and .cargo to be).

@bjorn3

bjorn3 commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

I mean it already modifies the source directory by adding vendor directories to the root and to library.

@Kobzol

Kobzol commented Jul 1, 2026

Copy link
Copy Markdown
Member

Yeah, strictly speaking that is also source directory modification, but then we could say that creating the build directory is also modifying the "sources" :) I meant modifying things that could be reasonably customized locally by the developer and are not considered to be "build outputs", which I'd say both build and vendor falls under.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants