Skip to content

Change config paths to only check CARGO_HOME for cargo-script #14749

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 4, 2024
Merged

Change config paths to only check CARGO_HOME for cargo-script #14749

merged 1 commit into from
Nov 4, 2024

Conversation

@0xPoe
Copy link
Member

@0xPoe 0xPoe commented Oct 30, 2024 •
edited
Loading

What does this PR try to resolve?

ref #12207 (comment)
After this change, we will only check the configs from the CARGO_HOME.

How should we test and review this PR?

The test has been updated.

Additional information

r? @epage

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-run S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2024
@0xPoe
Copy link
Member Author

0xPoe commented Oct 30, 2024

@epage Even I found the code and changed it. But I am a little confused about your comments.

When we talked about this as a Cargo team, we decided this need to reload to cargo home, like cargo install does, to avoid downloading a script and being subject to spurious files in your ~/Downloads, at the cost of losing the config from your repo.

What do you mean by downloading a script? And what kind of spurious files might end up being created? I thought it only read the config files.

@epage
Copy link
Contributor

epage commented Oct 30, 2024

What do you mean by downloading a script? And what kind of spurious files might end up being created? I thought it only read the config files.

If I download a script off the internet, my browser will put it in ~/Downloads. That is a grab bag directory with who knows what in it. A user might have extracted a project into that directory to look at that could have dropped a .cargo/config.toml into it. The concern is with that being used when it wasn't intended. That unfortunately means we'll not load any config for cargo scripts.

@0xPoe 0xPoe force-pushed the rustin-patch-cargo-script branch from c00c27a to cbb9dae Compare October 31, 2024 14:29
@0xPoe 0xPoe marked this pull request as ready for review October 31, 2024 14:32
0xPoe
0xPoe commented Oct 31, 2024
View reviewed changes
Copy link
Member Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

🔢 Self-check (PR reviewed by myself and ready for feedback.)

epage
epage reviewed Oct 31, 2024
View reviewed changes
@epage epage mentioned this pull request Oct 31, 2024
Open
39 tasks
@0xPoe 0xPoe force-pushed the rustin-patch-cargo-script branch from cbb9dae to a61d845 Compare November 3, 2024 10:04
0xPoe

This comment was marked as outdated.

Sign in to view

@0xPoe 0xPoe force-pushed the rustin-patch-cargo-script branch 2 times, most recently from a2d6143 to 09faa3e Compare November 3, 2024 10:09
@0xPoe 0xPoe force-pushed the rustin-patch-cargo-script branch from 09faa3e to bd47da1 Compare November 3, 2024 10:12
0xPoe
0xPoe commented Nov 3, 2024
View reviewed changes
Copy link
Member Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

🔢 Self-check (PR reviewed by myself and ready for feedback.)

@0xPoe 0xPoe requested a review from epage November 3, 2024 10:14
@epage
Copy link
Contributor

epage commented Nov 4, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Nov 4, 2024

📌 Commit bd47da1 has been approved by epage

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 4, 2024
@bors
Copy link
Contributor

bors commented Nov 4, 2024

⌛ Testing commit bd47da1 with merge ad3cdb4...

@bors
Copy link
Contributor

bors commented Nov 4, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing ad3cdb4 to master...

@bors bors merged commit ad3cdb4 into rust-lang:master Nov 4, 2024
22 checks passed
@0xPoe
Copy link
Member Author

0xPoe commented Nov 5, 2024

Thanks for your review! 💚 💙 💜 💛 ❤️

@weihanglo weihanglo mentioned this pull request Nov 9, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 10, 2024
@bors
Auto merge of rust-lang#132829 - weihanglo:update-cargo, r=weihanglo
303fc05 
Update cargo

16 commits in 0310497822a7a673a330a5dd068b7aaa579a265e..4a2d8dc636445b276288543882e076f254b3ae95
2024-11-01 19:27:56 +0000 to 2024-11-09 19:10:33 +0000
- test: adjust `cargo_test_env` to unblock rust submodule update (rust-lang/cargo#14803)
- feat(warnings): add build.warnings option (rust-lang/cargo#14388)
- Revert "feat: Add `CARGO_RUSTC_CURRENT_DIR`" (rust-lang/cargo#14799)
- CI: make the `lint-docs` job required (rust-lang/cargo#14797)
- Switch CI from bors to merge queue (rust-lang/cargo#14718)
- docs(test):  Document Execs assertions based on port effort (rust-lang/cargo#14793)
- fix(test): Make redactions consistent with snapbox (rust-lang/cargo#14790)
- test(gc): Update remaining unordered tests to snapbox (rust-lang/cargo#14785)
- Normalize the `target` paths (rust-lang/cargo#14497)
- rustfix: replace special-case duplicate handling with error (rust-lang/cargo#14782)
- test: Update some emaining unordered tests to snapbox (rust-lang/cargo#14781)
- Change config paths to only check CARGO_HOME for cargo-script (rust-lang/cargo#14749)
- Enable transfer feature in triagebot (rust-lang/cargo#14777)
- Add transactional semantics to `rustfix` (rust-lang/cargo#14747)
- doc: fix `GlobalContext` reference (rust-lang/cargo#14773)
- chore: update handlebars to v6, fix build error (rust-lang/cargo#14772)
@rustbot rustbot added this to the 1.84.0 milestone Nov 10, 2024
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
@bors @mati865
Auto merge of rust-lang#132829 - weihanglo:update-cargo, r=weihanglo
b5e0764 
Update cargo

16 commits in 0310497822a7a673a330a5dd068b7aaa579a265e..4a2d8dc636445b276288543882e076f254b3ae95
2024-11-01 19:27:56 +0000 to 2024-11-09 19:10:33 +0000
- test: adjust `cargo_test_env` to unblock rust submodule update (rust-lang/cargo#14803)
- feat(warnings): add build.warnings option (rust-lang/cargo#14388)
- Revert "feat: Add `CARGO_RUSTC_CURRENT_DIR`" (rust-lang/cargo#14799)
- CI: make the `lint-docs` job required (rust-lang/cargo#14797)
- Switch CI from bors to merge queue (rust-lang/cargo#14718)
- docs(test):  Document Execs assertions based on port effort (rust-lang/cargo#14793)
- fix(test): Make redactions consistent with snapbox (rust-lang/cargo#14790)
- test(gc): Update remaining unordered tests to snapbox (rust-lang/cargo#14785)
- Normalize the `target` paths (rust-lang/cargo#14497)
- rustfix: replace special-case duplicate handling with error (rust-lang/cargo#14782)
- test: Update some emaining unordered tests to snapbox (rust-lang/cargo#14781)
- Change config paths to only check CARGO_HOME for cargo-script (rust-lang/cargo#14749)
- Enable transfer feature in triagebot (rust-lang/cargo#14777)
- Add transactional semantics to `rustfix` (rust-lang/cargo#14747)
- doc: fix `GlobalContext` reference (rust-lang/cargo#14773)
- chore: update handlebars to v6, fix build error (rust-lang/cargo#14772)
@epage epage mentioned this pull request Nov 15, 2024
Open
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@epage epage Awaiting requested review from epage

Assignees

@epage epage

Labels

A-cli Area: Command-line interface, option parsing, etc. Command-run S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.84.0

Development

Successfully merging this pull request may close these issues.

4 participants

@0xPoe @epage @bors @rustbot