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.

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

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

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

Rustin170506
Copy link
Member

@Rustin170506 Rustin170506 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
@Rustin170506
Copy link
Member Author

@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.

@Rustin170506 Rustin170506 marked this pull request as ready for review October 31, 2024 14:32
Rustin170506
Rustin170506 commented Oct 31, 2024
View reviewed changes
Copy link
Member Author

@Rustin170506 Rustin170506 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
tests/testsuite/script.rs Outdated Show resolved Hide resolved
@epage epage mentioned this pull request Oct 31, 2024
Open
33 tasks
Rustin170506

This comment was marked as outdated.

Sign in to view

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

@Rustin170506 Rustin170506 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
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
@Rustin170506
Copy link
Member Author

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)
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
@Rustin170506 @epage @bors @rustbot