-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
docs(cargo-fix): troubleshooting failing fixes #15148
base: master
Are you sure you want to change the base?
Conversation
I believe it is a leftover when cargo-fix was an external command.
src/doc/man/cargo-fix.md
Outdated
While `cargo fix` strives to automatically apply fixes, it can still fail for | ||
various reasons. Here are some suggestions for troubleshooting fix failures: | ||
|
||
* Unlike other commands, `cargo fix` implies `--all-targets` by default. | ||
Try fixing one Cargo target at a time instead of everything in one run. | ||
For instance, narrow the scope to a single binary target: | ||
|
||
```sh | ||
cargo fix --bin mybin | ||
``` | ||
|
||
* If you have a Cargo workspace containing multiple packages, | ||
fix them one by one to avoid unwanted feature unification between packages. | ||
For instance, fix only the `bar` package : | ||
|
||
```sh | ||
cargo fix --package bar | ||
``` |
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.
imo these give suggestions without context around them or why other failures may happen which
This also feels like its heavily written for editions but is at the same heading level as editions
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.
Yes it is missing contexts. I am not sure how to put one. The biggest one in my mind is feature unifications. There might be some other I am not aware of.
feels like its heavily written for edition
Not really? One can run plain cargo fix
and hitting it hard due to having mutually exclusive features.
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.
Hmm, maybe I can reframe as edition troubleshooting, as we do give users specific steps for edition migration
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.
Moved it to under migration and polished it a bit.
There are a couple of reasons taht `cargo fix` fails. This doc update lists some approaches to narrow the scope to help users deal with those failures one by one.
If you encounter failing fixes during migration, instead of fixing everything | ||
in a single `cargo fix` invocation, try fixing only a subset of packages for | ||
specific features and Cargo targets. Fixing each combination separately can | ||
help identify the root cause of failures more easily and avoid unintended | ||
feature unification between packages. For example: |
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.
This is less about "failing fixes" and more about "unrelated compilation errors", right?
For the conntext, if I'm understanding correctly, this is coming down to either helping the user are working around #2980 (a workflow that runs counter to Cargo's design) or dealing with #14415 (a bug). This is not something unique to cargo fix
except that --all-targets
is the default which can get unexpected unification from dev-dependencies when combined with migrating multiple packages at once. I would assume someone knows that they can't run cargo check --workspace --all-targets
so I wonder why cargo fix
failing would be so surprising and without knowing how to workaround it already. Note that we give a Migrating <build-target-path>
message for each build target, which is an indirect way of telling users that --all-targets
is being used.
A small part of me also wonders about the balance in meeting a user where they are at vs documenting (and somewhat blessing) unsupported workflows.
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.
Yes I agree the troubleshooting can be generalized to cargo check
. To me the distinction is that unlike failures in cargo check
, cargo fix --all-features
is a suggested workflow here. People are exposed to potential failure immediately.
It would be great as well if we can find a place for general troubleshooting
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 would assume someone knows that they can't run
cargo check --workspace --all-targets
so I wonder whycargo fix
failing would be so surprising and without knowing how to workaround it already.
I would also say from my observation at work, average Rust developers don't really know how each command actually works. What they do is fiddling around different flags, not to mention the indirection from Migrating <path>
message. We are able to tell it's feature unification issue because we have read the cargo doc a hundred times.
A small part of me also wonders about the balance in meeting a user where they are at vs documenting (and somewhat blessing) unsupported workflows.
Mutually exclusive features isn't really officially unsupported. It is indeed unrecommended and Cargo features can't handle it well now though. The officially suggested alternative cfg-if
may still cause issues like one wants is to test only a specific feature but it got overridden and test failed. So we also want to take care of those alternative workflows.
Had some discussion during today's office hours. Some recaps:
|
### What does this PR try to resolve? `\h` controls horizontal spacing. This changes unordered lists from moving 2 units (`\h'+02'`) to 3 units (`\h'+03'`). This looks more nature. Most other popular software programs follow the same style. For example, git-merge, tar, and bash. ### How should we test and review this PR? ``` cargo build target/debug/cargo help package ``` ### Additional information This was found during #15148.
What does this PR try to resolve?
There are a couple of reasons making
cargo fix
failed.This doc update lists some approaches to narrow the scope
to help users deal with those failures one by one.
How should we test and review this PR?
I am not satisfied with this writing. Suggestions are welcome :)
Additional information
See rust-lang/rust#136345.