Skip to content

Add error message when cargo fix on an empty repo#11400

Merged
bors merged 5 commits intorust-lang:masterfrom
0xPoe:rustin-patch-fix-error
Nov 28, 2022
Merged

Add error message when cargo fix on an empty repo#11400
bors merged 5 commits intorust-lang:masterfrom
0xPoe:rustin-patch-fix-error

Conversation

@0xPoe
Copy link
Member

@0xPoe 0xPoe commented Nov 22, 2022 •
edited
Loading

What does this PR try to resolve?

close #11380

Add error message when cargo fix on an empty repo.

How should we test and review this PR?

  • unit test
set -eux

cargo +nightly new repro
cd repro
echo "fn main() { let _ = 0.clone(); }" > src/main.rs
cargo fix

@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2022

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 22, 2022
epage
epage reviewed Nov 22, 2022
View reviewed changes
@0xPoe 0xPoe requested a review from epage November 26, 2022 11:38
0xPoe added 5 commits November 26, 2022 20:15
@0xPoe
Add error message when cargo fix on an empty repo
a1f2f5a 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe
Suppress error with --allow-dirty and add a test
89b8a8b 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe
Remove new_repo_without_add_and_commit
25f7d4b 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe
Include untracked files
3189dc3 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe
Fix fix_in_existing_repo_weird_ignore broken test
21b2fe9 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe 0xPoe force-pushed the rustin-patch-fix-error branch from 2abaace to 21b2fe9 Compare November 26, 2022 12:15
@epage
Copy link
Contributor

epage commented Nov 28, 2022

@bors r+

We are expanding our check but I do not think this would break a workflow to be considered a breaking change.

I am somewhat concerned about this complaining about editor temp files and such when a user hasn't configured their personal gitignore. If we push people to use --allow-dirty too much, then they will likely miss important things. However, the only good idea I had here was limiting the check to cargo package --list but I assume that most implicit or explicitly configured lists of files will still include these untracked files .

We do have other issues that could improve the dirty workflow overall though,

So I figure we can go ahead with this.

@bors
Copy link
Contributor

bors commented Nov 28, 2022

📌 Commit 21b2fe9 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 28, 2022
@bors
Copy link
Contributor

bors commented Nov 28, 2022

⌛ Testing commit 21b2fe9 with merge a2ea66b...

@bors
Copy link
Contributor

bors commented Nov 28, 2022

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

@bors bors merged commit a2ea66b into rust-lang:master Nov 28, 2022
@weihanglo weihanglo mentioned this pull request Nov 29, 2022
Merged
weihanglo added a commit to weihanglo/rust that referenced this pull request Dec 3, 2022
@weihanglo
Update cargo
9bd6062 
9 commits in e027c4b5d25af2119b1956fac42863b9b3242744..f6e737b1e3386adb89333bf06a01f68a91ac5306
2022-11-25 19:44:46 +0000 to 2022-12-02 20:21:24 +0000
- Refactor generate_targets into separate module (rust-lang/cargo#11445)
- Improve file found in multiple build targets warning (rust-lang/cargo#11299)
- Error when precise without -p flag (rust-lang/cargo#11349)
- Improve strategy for selecting targets to be scraped for examples (rust-lang/cargo#11430)
- Aware of compression ratio for unpack size limit (rust-lang/cargo#11337)
- Add test for rustdoc-map generation when using sparse registries (rust-lang/cargo#11403)
- Add error message when `cargo fix` on an empty repo (rust-lang/cargo#11400)
- Store the sparse+ prefix in the SourceId for sparse registries (rust-lang/cargo#11387)
- Update documentation for -Zrustdoc-scrape-examples in the Cargo Book (rust-lang/cargo#11425)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2022
@bors
Auto merge of rust-lang#105073 - weihanglo:update-cargo, r=weihanglo
dd7a125 
Update cargo

9 commits in e027c4b5d25af2119b1956fac42863b9b3242744..f6e737b1e3386adb89333bf06a01f68a91ac5306
2022-11-25 19:44:46 +0000 to 2022-12-02 20:21:24 +0000
- Refactor generate_targets into separate module (rust-lang/cargo#11445)
- Improve file found in multiple build targets warning (rust-lang/cargo#11299)
- Error when precise without -p flag (rust-lang/cargo#11349)
- Improve strategy for selecting targets to be scraped for examples (rust-lang/cargo#11430)
- Aware of compression ratio for unpack size limit (rust-lang/cargo#11337)
- Add test for rustdoc-map generation when using sparse registries (rust-lang/cargo#11403)
- Add error message when `cargo fix` on an empty repo (rust-lang/cargo#11400)
- Store the sparse+ prefix in the SourceId for sparse registries (rust-lang/cargo#11387)
- Update documentation for -Zrustdoc-scrape-examples in the Cargo Book (rust-lang/cargo#11425)
@ehuss ehuss added this to the 1.67.0 milestone Dec 14, 2022
@Alexendoo Alexendoo mentioned this pull request Jan 31, 2023
Open
@bryanlarsen
Copy link

bryanlarsen commented Jan 31, 2023

@bors r+

We are expanding our check but I do not think this would break a workflow to be considered a breaking change.

I am somewhat concerned about this complaining about editor temp files and such when a user hasn't configured their personal gitignore. If we push people to use --allow-dirty too much, then they will likely miss important things. However, the only good idea I had here was limiting the check to cargo package --list but I assume that most implicit or explicitly configured lists of files will still include these untracked files .

We do have other issues that could improve the dirty workflow overall though,

So I figure we can go ahead with this.

I'm one with a broken workflow. I should probably clean up my untracked files, but the broken workflow was jarring...

Perhaps an --allow-untracked flag? That would be much less dangerous than training people to use --allow-dirty

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

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

Development

Successfully merging this pull request may close these issues.

cargo fix does not warn about modifying untracked files

6 participants

@0xPoe @rustbot @epage @bors @bryanlarsen @ehuss