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

Consider publishing clippy_utils as a crates.io crate for others to reuse #13556

Closed
alice-i-cecile opened this issue Oct 16, 2024 · 12 comments · Fixed by #13700
Closed

Consider publishing clippy_utils as a crates.io crate for others to reuse #13556

alice-i-cecile opened this issue Oct 16, 2024 · 12 comments · Fixed by #13700

Comments

@alice-i-cecile
Copy link

alice-i-cecile commented Oct 16, 2024

Description

Over at the bevy_cli project, we're implementing custom lints for Bevy, and bootstrapping off of Clippy's existing tools :) We'd love to publish this to crates.io so folks can cargo install bevy_cli, but can't do so easily due to clippy_utils only existing as a git dependency.

We don't particularly care about stability or the like: simply publishing whatever you have periodically would be enough.

Prompted by TheBevyFlock/bevy_cli#150

@BD103
Copy link
Contributor

BD103 commented Oct 16, 2024

For the period, you could definitely match what Cargo does and publish for every stable release (as long as each release provides what nightly version they were pinned to at that time).

Publishing clippy_utils will also require clippy_config, but that's also reserved on https://crates.io.

Thanks for your consideration!

@Alexendoo Alexendoo added the I-nominated Issue: Nominated to be discussed at the next Clippy meeting label Oct 17, 2024
@flip1995
Copy link
Member

flip1995 commented Oct 18, 2024

as long as each release provides what nightly version they were pinned to at that time

How would you recommend to best provide this information?

Publishing clippy_utils will also require clippy_config, but that's also reserved on https://crates.io.

Yeah, I made sure that those crates are squatted for us (with me and rust-lang-owner as owners) recently, in anticipation that such a request would come up at some point. I just thought it would be from the dylint maintainers. cc @smoelius

@BD103
Copy link
Contributor

BD103 commented Oct 18, 2024

as long as each release provides what nightly version they were pinned to at that time

How would you recommend to best provide this information?

The channel field within rust-toolchain is enough! :)

If releases are to be automated, you can automatically extract this value using Taplo. I did something very similar in bevy_lint's CI.

@flip1995
Copy link
Member

flip1995 commented Oct 18, 2024

Offtopic: shouldn't this be

toolchain: ${{ needs.extract-rust-version.outputs.toolchain }}

in your workflow file?


Extracting this in an automated way from the rust-toolchain file is easy. I'd probably just use a grep command though. My question is more: where should I put this information? Just in the README of clippy_utils/clippy_config?

@BD103
Copy link
Contributor

BD103 commented Oct 18, 2024

Offtopic: shouldn't this be

toolchain: ${{ needs.extract-rust-version.outputs.toolchain }}

in your workflow file?

For a second I thought so too, but I rename it in the job outputs:

    outputs:
      channel: ${{ steps.toolchain.outputs.toolchain }}
      components: ${{ steps.toolchain.outputs.components }}

This is confusing, though, so I'll probably change it.

Extracting this in an automated way from the rust-toolchain file is easy. I'd probably just use a grep command though. My question is more: where should I put this information? Just in the README of clippy_utils/clippy_config?

If you use Github Releases, you could add it there. (Maybe also the CHANGELOG.md?)

@flip1995
Copy link
Member

CHANGELOG.md is just for stable and external releases for Clippy itself. Not for something like this. And I'd like to keep it that way.

If we would start using Github releases, I think we would need to copy the whole changelog there every time, so that it is a proper release. I'd rather avoid that.

So I guess, I will add it to the README of the crates, so that it also shows up on the crates.io page when publishing.

@BD103
Copy link
Contributor

BD103 commented Oct 18, 2024

So I guess, I will add it to the README of the crates, so that it also shows up on the crates.io page when publishing.

If that's too much of a hassle, each release is tagged, so I can figure it out using the tagged commit.

@flip1995
Copy link
Member

Let's wait for the clippy meeting where we talk about this. And if we decide that we want to publish it, I'll think about what would work for me and other people involved in releases and would make it easy for you to find the toolchain.

@smoelius
Copy link
Contributor

We'd love to publish this to crates.io so folks can cargo install bevy_cli, but can't do so easily due to clippy_utils only existing as a git dependency.

Won't the user have to specify the toolchain on the command line, e.g.:

cargo +nightly-2024-10-03 install bevy_cli

Or am I missing something?

@BD103
Copy link
Contributor

BD103 commented Oct 30, 2024

Won't the user have to specify the toolchain on the command line, e.g.:

cargo +nightly-2024-10-03 install bevy_cli

Or am I missing something?

You're right, but I still prefer it to --git https://github.com/ TheBevyFlock/bevy_cli --tag lint-v0.1.0 bevy_lint! It's shorter and I trust it more, since unlike tags versions published to crates.io cannot be modified.

@flip1995 flip1995 removed the I-nominated Issue: Nominated to be discussed at the next Clippy meeting label Nov 12, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 15, 2024
In preparation of #13556, I want to remove the dependency on
`clippy_config`, as I don't think that we want to publish that for
outside consumers. To do this the 2 dependecies on `clippy_config` had
to be removed:

1. The MSRV implementation was in `clippy_config`, but was required in
`qualify_min_const`. I think exposing the MSRV infrastructure and the
MSRVs we defined might also be helpful for `clippy_utils` users. I don't
see why it should not be able to live in `clippy_utils` from a technical
point of few.
2. The `create_disallowed_map` function that took in a
`clippy_utils::types::DisallowedPath` is moved to the `DisallowedPath`
implementation. This also fits there and is only useful for Clippy and
not in `clippy_utils` for external consumers.

`clippy_config` now depends in `clippy_utils`, so the dependecy just got
reversed. But having the `clippy_utils` crate as the base of the
dependency tree in Clippy makes sense.

changelog: none
github-merge-queue bot pushed a commit that referenced this issue Nov 17, 2024
Follow up to #13691

Adds metadata to the `clippy_utils/Cargo.toml`, which is mostly copied
from the root `Cargo.toml`.
Adds a `README.md` file listing the nightly version `clippy_utils` can
be used with, mentions that there are no stability guarantees and the
license.

The next PR will add automation to update the nightly toolchains in
those files and the versions in the `Cargo.toml`s.

cc #13556

changelog: none
github-merge-queue bot pushed a commit that referenced this issue Nov 19, 2024
Based on #13693

Adds 2 subcommands to `cargo dev`:

- `cargo dev sync update_nightly`: Which updates the nightly versions in
`rust-toolchain` and `clippy_utils/README.md`
- `cargo dev release bump_version`: Bumps the version in all relevant
`Cargo.toml` files

Those are pulled out of
#12759, which I'll rebase
on this.

Next step is to update the documentation, which I'll partly pull out of
#12762

r? @blyxyas (as you reviewed the first PR in the chain and were assigned
to the second one)

cc #13556

changelog: none
@flip1995
Copy link
Member

https://crates.io/crates/clippy_utils

#13700 will close this issue.

github-merge-queue bot pushed a commit that referenced this issue Nov 29, 2024
This updates the documentation after #13694. It is not based on that PR
chain and can be merged independently, but should be merged after that
PR.

This is partly pulled from #12762, but removing the Josh parts.

This includes instructions on how to publish `clippy_utils`.

Closes #13556 (yes, this
is the final PR 🙂)

r? @blyxyas

changelog: `clippy_utils` is now published to crates.io
@BD103
Copy link
Contributor

BD103 commented Nov 29, 2024

Thank you so much! :D

BD103 added a commit to TheBevyFlock/bevy_cli that referenced this issue Nov 29, 2024
`clippy_utils` used to be a Git dependency, but thanks to
rust-lang/rust-clippy#13556 it is now being
published on <https://crates.io>! This PR switches to the crates.io
version, and bumps the pinned nightly toolchain to match it (as
specified [here](https://lib.rs/crates/clippy_utils)).

We still want to pin a specific version of `clippy_utils`, since it does
not follow semantic versioning, but now we have the ability to publish
ourselves to crates.io!

Closes #150.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants