`--feature-powerset --at-least-one-of` silently drops unknown features

[rukai]

If I run cargo hack --feature-powerset --at-least-one-of foo,bar where foo is not a feature of the project, foo will be dropped and the command will complete successfully. This is surprising, I would expect the command to fail and report to the user that one of the specified features does not exist in the project.

[taiki-e]

cc @kornelski who added this option in https://github.com/taiki-e/cargo-hack/pull/193

[taiki-e]

Perhaps just need to add the check like the following? https://github.com/taiki-e/cargo-hack/blob/8fa7f67166823d74e41099ed3be320b723771c83/src/main.rs#L203-L207

[kornelski]

cargo hack has a different handling of features in a workspace than Cargo. In Cargo, commands like cargo check --all --features=foo require that at least one crate has the specified feature. But then it still checks all crates in the workspace, and only sets the feature for packages that have such feature. Cargo never fails due to trying to set wrong feature on a wrong package.

cargo hack has much less useful behaviors:

• without --ignore-unknown-features, it will always want to use all features specified on all packages, and end up setting wrong features on packages, and fail. To me this behavior is frustrating and not useful, because if features differ between workspace packages (which in my projects they always do), cargo hack just becomes unusable.

• with --ignore-unknown-features, or in --at-least-one-of, it will ignore features that aren't applicable, without causing errors. This fails to catch invalid feature flags specified, but otherwise works like Cargo does natively, and easily deals with diverse packages in a workspace.

[kornelski]

One way to address it, would be to make --at-least-one-of trigger an error instead of skipping features that aren't applicable to a given package, unless --ignore-unknown-features is specified. However, this wouldn't be backwards compatible — existing users would have to add --ignore-unknown-features to get the current behavior.

I think a better solution would be to change cargo hack to check features the same way Cargo does. That would make most commands just work sensibly out of the box, and --ignore-unknown-features would be unnecessary.

[taiki-e]

In Cargo, commands like cargo check --all --features=foo require that at least one crate has the specified feature.

To be clear: This is the behavior of v2 resolver. (The behavior of the v1 resolver is very odd compared to the v2 resolver and to cargo-hack.) https://doc.rust-lang.org/nightly/cargo/reference/features.html#resolver-version-2-command-line-flags

The current behavior of cargo-hack is the result of a commitment to “being explicit” in order to handle its footguns at a time when only v1 resolver existed.

I think a better solution would be to change cargo hack to check features the same way Cargo does. That would make most commands just work sensibly out of the box, and --ignore-unknown-features would be unnecessary.

Agreed. I think it would be reasonable to make the behavior here similar to a v2 resolver and only raise an error if the features specified are not included in any of the crates that are built, and allow the rest.

[xStrom]

Having --ignore-unknown-features would still have value. I copy-paste the same CI script around for over 20 repos. The no_std step uses --ignore-unknown-features --features libm. Most no_std packages have the libm feature, but there are simpler no_std packages which don't have the libm feature.

I have no problem with the proposed solution of only raising an error if the specified features are not included in any of the packages, if the --ignore-unknown-features flag remains to allow me to continue without an error even in workspaces where no package has the specified feature.

[rukai]

Agreed. I think it would be reasonable to make the behavior here similar to a v2 resolver and only raise an error if the features specified are not included in any of the crates that are built, and allow the rest.

Yeah, that's what I'm after

[taiki-e]

@xStrom

Having --ignore-unknown-features would still have value. I copy-paste the same CI script around for over 20 repos. The no_std step uses --ignore-unknown-features --features libm. Most no_std packages have the libm feature, but there are simpler no_std packages which don't have the libm feature.

This seems a reasonable use case. (Anyway I intended to keep --ignore-unknown-features for compatibility, even if there were no use cases.)