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

chore(xtask): Add cargo xtask unpublished #12039

Merged
merged 4 commits into from
Apr 27, 2023
Merged

chore(xtask): Add cargo xtask unpublished #12039

merged 4 commits into from
Apr 27, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Apr 25, 2023

What does this PR try to resolve?

This tries to make it easy to see what existing versions have not been published. A future version of this could post to a PR what the current delta in version numbers for touched crates so reviewer have more context when deciding if they should ask for a crate version to be bumped

$ cargo unpublished
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
     Running `/home/epage/src/personal/cargo/target/debug/xtask unpublished`
    Updating crates.io index
name published current
==== ========= =======
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.0 0.71.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0

Room for improvement

  • Aligning the start of each column
  • Filtering the list by a commit range
  • Adding this to an action to post to a review
  • Maybe sorting the output
  • Marking some our crates as package.publish = false, like benchsuite and resolver-tests

How should we test and review this PR?

This is broken down commit by commit for easier seeing of the building blocks for our first xtask

@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2023

r? @weihanglo

(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 Apr 25, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

It's both promising and unfortunate. I mean it's nice to have this by just reusing aargo-the-library. On the other hand we need to pull the whole library, increasing compile time a lot.

Anyhow, thank you for making this prototype so fast!

@@ -0,0 +1,2 @@
[alias]
xtask = "run --package xtask --"
Copy link
Member

Choose a reason for hiding this comment

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

I thought you leaned toward multiple binaries (matklad/cargo-xtask#2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its been a while since I've thought of xtasks all that much (that issue being almost 4 years old!). There are trade offs, performance in some cases vs performance in others, discoverability (run cargo xtask to discover all xtasks), etc.

No strong preference for what works with cargo

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could build each binary on demand. xtask binary acts as a thin layer of parent command delegates other subcommands.

That is, when running cargo xtask unpublished it spawns a process running cargo run -p xtask --bin unpublished. With that we can probably avoid polling too many dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

I am thinking of making src/doc/build.sh an xtask, which shouldn't compile the whole Cargo when running.

Copy link
Member

Choose a reason for hiding this comment

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

I was wrong. That mean we need unpublished to be a separate bin. However, I feel like we should go multi-binaries way to make build time less suck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've gone ahead and created a dedicated binary

@epage
Copy link
Contributor Author

epage commented Apr 25, 2023

It's both promising and unfortunate. I mean it's nice to have this by just reusing aargo-the-library. On the other hand we need to pull the whole library, increasing compile time a lot.

I had considered writing this with cargo metadata but then I felt this would be a good oppotunity to dogfood.

@epage
Copy link
Contributor Author

epage commented Apr 25, 2023

(in particular, using crates-index to walk the index only supports git and didn't want to enshrine that in our workflow)

This will allow running an xtask without requiring building the world.
In most cases, a user will already have been building cargo but not in
CI.

The packages keep an `xtask-` prefix to help raise awareness of them but
exposed as `cargo <suffix>` to avoid having a direction proxy to wrap
`cargo run -p xtask-<suffix>` as `cargo xtask <suffix>`.
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

This looks good to me. I don't have any particular view of what it would evolve over time at this moment. Keep in mind that xtask discoverability is not good enough in multi-binaries layout.

Given it is not really hard to tweak later, I'll merge as-is.

Thank you for kicking off this!

use cargo::util::command_prelude::*;

pub fn cli() -> clap::Command {
clap::Command::new("xtask-unpublished")
Copy link
Member

Choose a reason for hiding this comment

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

We should document it. Let's do it after figuring out a better look of xtask for cargo.

@weihanglo
Copy link
Member

@bors r+

Note that the output format is not really suitable for CI job. Whoever going to get that done will need to make some changes.

@bors
Copy link
Contributor

bors commented Apr 26, 2023

📌 Commit 5b13963 has been approved by weihanglo

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 Apr 26, 2023
@bors
Copy link
Contributor

bors commented Apr 26, 2023

⌛ Testing commit 5b13963 with merge 8bcf8fe...

@weihanglo weihanglo added the A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. label Apr 26, 2023
@bors
Copy link
Contributor

bors commented Apr 27, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 8bcf8fe to master...

@bors bors merged commit 8bcf8fe into rust-lang:master Apr 27, 2023
@epage epage deleted the xtask branch April 27, 2023 00:44
epage added a commit to epage/cargo that referenced this pull request Apr 27, 2023
This is a follow up to rust-lang#12039.
This makes it easier for tools to report less irrelevant information.

I did both `publish = false` and `version = "0.0.0"` to help draw
attention to the fact that these crates are internal (inspired by a
matklad post).

I left `cargo-test-macro` and `cargo-test-support` in for my own
personal bias of one day wanting to see those crates published...

The only one removed that had previously been published was `mdman` but
seeing as that was a `0.0.0`, I'm assuming that was a mistake or just
reserving the name.

Before:
```console
$ cargo unpublished
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.1 0.72.0
semver-check - 0.1.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0
```

After:
```console
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
cargo 0.70.1 0.72.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
```
bors added a commit that referenced this pull request Apr 27, 2023
chore: Mark unpublished crates as such

This is a follow up to #12039.
This makes it easier for tools to report less irrelevant information.

I did both `publish = false` and `version = "0.0.0"` to help draw attention to the fact that these crates are internal (inspired by a matklad post).

I left `cargo-test-macro` and `cargo-test-support` in for my own personal bias of one day wanting to see those crates published...

The only one removed that had previously been published was `mdman` but seeing as that was a `0.0.0`, I'm assuming that was a mistake or just reserving the name.

Before:
```console
$ cargo unpublished
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.1 0.72.0
semver-check - 0.1.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0
```

After:
```console
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
cargo 0.70.1 0.72.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
```
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2023
Update cargo

16 commits in 9e586fbd8b931494067144623b76c37d213b1ab6..ac84010322a31f4a581dafe26258aa4ac8dea9cd
2023-04-25 22:09:11 +0000 to 2023-05-02 13:41:16 +0000
- docs(registry): Further specify owner-remove response (rust-lang/cargo#12056) (rust-lang/cargo#12068)
- Remove repeated definite articles (rust-lang/cargo#12067)
- Document that adding `#[non_exhaustive]` on existing items is breaking. (rust-lang/cargo#10877)
- docs(commands): add missed preposition (rust-lang/cargo#12073)
- Fix warning with unused mut (rust-lang/cargo#12065)
- chore: move build-man workflow away from shell (rust-lang/cargo#12048)
- feat: Add `-Zmsrv-policy` feature flag (rust-lang/cargo#12043)
- chore: new xtask to check stale paths in autolabel defintions (rust-lang/cargo#12051)
- cargo-tree: Handle -e no-proc-macro when building the graph (rust-lang/cargo#12044)
- chore: update trigger_files in autolabel (rust-lang/cargo#12052)
- fix broken markdown in docs (rust-lang/cargo#12049)
- home: fix & enhance documentation (rust-lang/cargo#12047)
- chore: Mark unpublished crates as such (rust-lang/cargo#12045)
- Include rust-version in publish request (rust-lang/cargo#12041)
- chore(xtask): Add `cargo xtask unpublished` (rust-lang/cargo#12039)
- docs(ref): Specify 'rust_version' in Index format (rust-lang/cargo#12040)

r? `@ghost`
charles-r-earp pushed a commit to charles-r-earp/cargo that referenced this pull request May 3, 2023
This is a follow up to rust-lang#12039.
This makes it easier for tools to report less irrelevant information.

I did both `publish = false` and `version = "0.0.0"` to help draw
attention to the fact that these crates are internal (inspired by a
matklad post).

I left `cargo-test-macro` and `cargo-test-support` in for my own
personal bias of one day wanting to see those crates published...

The only one removed that had previously been published was `mdman` but
seeing as that was a `0.0.0`, I'm assuming that was a mistake or just
reserving the name.

Before:
```console
$ cargo unpublished
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.1 0.72.0
semver-check - 0.1.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0
```

After:
```console
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
cargo 0.70.1 0.72.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
```
RalfJung pushed a commit to RalfJung/miri that referenced this pull request May 3, 2023
Update cargo

16 commits in 9e586fbd8b931494067144623b76c37d213b1ab6..ac84010322a31f4a581dafe26258aa4ac8dea9cd
2023-04-25 22:09:11 +0000 to 2023-05-02 13:41:16 +0000
- docs(registry): Further specify owner-remove response (rust-lang/cargo#12056) (rust-lang/cargo#12068)
- Remove repeated definite articles (rust-lang/cargo#12067)
- Document that adding `#[non_exhaustive]` on existing items is breaking. (rust-lang/cargo#10877)
- docs(commands): add missed preposition (rust-lang/cargo#12073)
- Fix warning with unused mut (rust-lang/cargo#12065)
- chore: move build-man workflow away from shell (rust-lang/cargo#12048)
- feat: Add `-Zmsrv-policy` feature flag (rust-lang/cargo#12043)
- chore: new xtask to check stale paths in autolabel defintions (rust-lang/cargo#12051)
- cargo-tree: Handle -e no-proc-macro when building the graph (rust-lang/cargo#12044)
- chore: update trigger_files in autolabel (rust-lang/cargo#12052)
- fix broken markdown in docs (rust-lang/cargo#12049)
- home: fix & enhance documentation (rust-lang/cargo#12047)
- chore: Mark unpublished crates as such (rust-lang/cargo#12045)
- Include rust-version in publish request (rust-lang/cargo#12041)
- chore(xtask): Add `cargo xtask unpublished` (rust-lang/cargo#12039)
- docs(ref): Specify 'rust_version' in Index format (rust-lang/cargo#12040)

r? `@ghost`
@ehuss ehuss added this to the 1.71.0 milestone May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants