some kind of #[cfg] story for rustfix

[nikomatsakis]

I think we need to be able to handle multiple #[cfg] settings for rustfix — at minimum, using #[cfg(test)] code is super common. We obviously can't handle all cfg's at once, but perhaps we should include some way to run rustfix more than once, with distinct cfg settings? (And the default could run for both no cfg and #[cfg(test)]?)

cc @alexcrichton @killercup @zackmdavis

[alexcrichton]

I think the suggestion here to make is:

cargo fix --prepare-for 2018 -- --all-targets

That'll catch tests and then you'd also need to pass --target and --features for other common cfg situations.

Unfortunately I'm not sure we can realistically and automatically do better than that

[nikomatsakis]

I think having people manually specify the cfg flags or other features they want is just fine. I have no idea what --all-targets means though; what is a "target"?

[alexcrichton]

Ah sorry that's a relatively new feature in Cargo where "target" means "test" or "benchark" or "binary" or "library". So instead of cargo build --tests --lib --bins --examples --benches you can do cargo build --all-targets`

[Mark-Simulacrum]

Marking as part of the milestone, but solely to verify that migrating in presence of features and/or targets can be done manually (so --all-targets, maybe --features ...); we don't need automation for alpha.

[nikomatsakis]

It seems to me that to mark this bug as closed we need the following:

• Some documented way to migrate tests
• Some documented way to migrate code that is gated on a feature (or the absence of a feature)

@alexcrichton's comment above suggests that the first item can be achieved by executing the following command:

cargo fix --prepare-for 2018 -- --all-targets

If so, I think that's fine, but we have to update the migration instructions. It is not clear to me if there is also a way to handle features, but if there is, then it should also be documented and explained.

Specifically, it seems fine to me if users have to manually re-run cargo fix with various combinations of features to ensure complete coverage. But it should be possible.

@killercup — the reason we pinged you is to confirm the presence/absence of those capabilities in rustfix.

[Mark-Simulacrum]

So --all-targets seems to work great. Feature handling is harder, but since rustfix supports incremental migration that's fine, for crates with non-mutually exclusive features, cargo +nightly fix --prepare-for 2018 -- --all-targets --no-default-features and cargo +nightly fix --prepare-for 2018 --allow-dirty -- --all-targets --all-features should be sufficient to migrate.

If exclusive features are present users need to run with --feature blah and then --features baz. For the alpha release that seems sufficient; we might want rustfix to automatically do this with some flag, but I don't think that's very feasible due to exclusionary feature groups.

This is now largely an issue of updating the edition guide, filing an issue there as well which I'll link from to here.

[nikomatsakis]

If exclusive features are present...

I don't see how we can possibly do better than this. It seems fine to me that if people have a complex web of features, they can run the migration tool more than once.

Features that truly cannot be used together are truly problematic for cargo anyway, since cargo takes the union of the features that appear in the dependency graph (and hence one crate might demand feature X and another unrelated crate might demand feature Y).

The other scenario that could happen though is that there is code which is dead unless feature X xor feature Y is used. Not sure how common that would be, but it would not be handled by running with "zero" and "all" features.

Regardless, it seems to me that for alpha at least we are done here. I could imagine adding other convenience feature(s) (e.g., perhaps some way to run repeatedly for each feature individually) but it would come later if at all (imo), and would probably want to be informed by the patterns that users need.

Maybe we should ask people to give feedback on precisely this point? We could ask them what feature "patterns" they might have found useful.

[Mark-Simulacrum]

I like the idea of collecting feedback on how features are used and what patterns they'd fine useful.

I think one place where we potentially could do better is add an allow-by-default lint that has a span of every byte that it "sees" -- essentially all code. However, since cfg'd code would not be seen by this lint it could work as a code coverage like tool for rustfix. I'm not sure if that's feasible, but seems plausible.

[alexcrichton]

I've opened rust-lang/rustfix#128 to help this along and try to officially give this "the same story as all other cargo commands"

[nrc]

I think we really need to make --all-targets the default here, it's pretty hard to discover even with help messages - e.g., rust-lang/rls#943 (comment)

[alexcrichton]

@nrc perhaps yeah, but I'm sort of coming around to the idea of having cargo fix behave like the rest of Cargo. It seems odd to me if cargo fix is the odd-one-out. Because naively you'd expect cargo check to check everything as well, right?

I think we'll probably want to update the edition guide though for cargo fix --all-targets --prepare-for 2018 to make it more visible!

[alexcrichton]

Ok cargo fix is now merged into Cargo which boasts a suite of new top-level flags like --test, --all-targets, etc. I've additionally submitted a PR to update the edition guide to suggest --all-targets by default (for now).

Otherwise I'm now going to close this in favor of two issues:

• cargo fix: Pass --all-targets by default cargo#5739 - passing --all-targets by default (which I think is likely to happen)
• cargo fix: Print targets/features/cfg that will be fixed cargo#5740 - print what may be left out of cargo fix

I think those should be sufficient to close this, so closing!