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.

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

Allow using --exclude without also specifying --workspace. #258

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

xStrom
Copy link
Contributor

@xStrom xStrom commented Nov 1, 2024

We're using cargo-hack at Linebender, in ~15 repositories and growing. Thanks for your continued maintenance of this project!

In order for us to maintain CI scripts across that many repositories, we try to write them in a super generic way. So that we can mostly copy-paste the script and only have minimal configuration variables per repository.

For example, one set of variables we currently have for Xilem is:

# List of packages that will be checked with the minimum supported Rust version.
RUST_MIN_VER_PKGS: "-p xilem -p xilem_core -p masonry"
# List of packages that can not target Wasm.
NO_WASM_PKGS: "--exclude masonry --exclude xilem"

So for the MSRV job we reference RUST_MIN_VER_PKGS instead of --workspace and for the Wasm job we add NO_WASM_PKGS. Works well. However a complication arises with the MSRV Wasm job! Because the combination of -p xilem -p xilem_core -p masonry --exclude masonry --exclude xilem does not currently work with cargo-hack.

Our current solution is to just have a third variable:

# RUST_MIN_VER_PKGS - NO_WASM_PKGS, evaluated.
RUST_MIN_VER_WASM_PKGS: "-p xilem_core"

It works, of course. However it would be nice to not have to keep this additional variable in sync, especially across all our repositories.

This PR here addresses the problem at the cargo-hack level by removing the requirement to also specify --workspace when using --exclude. This works nicely and allows us to remove that extra variable and skip the work of maintaining it for each repository.

Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @xStrom!

@taiki-e taiki-e merged commit 9a8bafd into taiki-e:main Nov 2, 2024
28 checks passed
@taiki-e
Copy link
Owner

taiki-e commented Nov 2, 2024

Published in 0.6.33.

@taiki-e taiki-e added the A-workspace Area: cargo workspace (--workspace, --exclude, --package, etc.) label Nov 2, 2024
@xStrom xStrom deleted the exclude branch November 2, 2024 09:12
github-merge-queue bot pushed a commit to linebender/vello that referenced this pull request Nov 3, 2024
`cargo-hack` has gained the ability to use `--exclude` along with `-p`
in [cargo-hack#258](taiki-e/cargo-hack#258).
This allows us to simplify our CI configuration.
github-merge-queue bot pushed a commit to linebender/xilem that referenced this pull request Nov 3, 2024
`cargo-hack` has gained the ability to use `--exclude` along with `-p`
in [cargo-hack#258](taiki-e/cargo-hack#258).
This allows us to simplify our CI configuration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-workspace Area: cargo workspace (--workspace, --exclude, --package, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants