Make gix crates ready for automatic cfg checking #1123
Conversation
|
The CI failures are related to some new clippy lints and the nightly feature activation being now tied to an actual feature making them fail with Let me know how you want to proceed? |
|
Thanks a lot for contributing the preparation for the upcoming
This whole crate is a hack, unfortunately, so that there is a blocking version of the crate that can be used internally, without having the overhead of a real copy. Thus, I don't think one should add anything else to it - it fulfils its purpose and ideally won't be used. I hope that once there are new async-related features, it can be removed entirely. Or in other words,
It's a very nice gesture, but I think if it should be useful it needs to be made so it can't actually fail a build. Ideally, it provides a preview of what's to come, but I can't have failures on a daily basis because new lint was added. Right now, I see this every 6 weeks or so when a new stable release trickles down, and that seems good enough.
Everything you think would work also works for me. There is just this one requirement that |
Otherwise there would be many unexpected_cfgs warnings for async-io coming from the gix-packetline sources.
|
Regarding the changes to
Completely agree, it's unreasonable to expect someone to block on nightly. My idea is to run the job as normal but to mark it as not failling, that's why I added Changed to
Seems to work and it-fact it was even used before, so. |
Let's remove nightly, it's clean and will definitely work. It's also only tangentially related to this PR, so I think it's the right thing to do. I will probably just do that myself before merging, so nothing left to be done here. Thanks again for your help, it's much appreciated! |
And even if it was advisory only, what good does it do if nobody looks at it? And I know myself :D.
| @@ -12,8 +12,13 @@ include = ["src/**/*", "LICENSE-*"] | |||
| [lib] | |||
| doctest = false | |||
|
|
|||
| [features] | |||
According to whom is it an anti-pattern? I've not seen any talk of that and found it much better than using features. |
I meant that with check-cfg+Cargo it's a bit of "anti-pattern" since enabling the |
|
Its not sufficient with I wouldn't classify that as an anti-pattern but a relevant use case for us to consider. |
This adds a `futures-lite` feature to `gix-packetline-blocking`. The new feature is undocumented except for the warning not to use it. It does nothing, and its purpose is to support an existing internal use of `gix-packetline-blocking` through an alias named `gix-packetline`. This new feature should never be used except to keep `cargo check` commands with `--workspace` that list `gix-packetline/futures-lite` from failing due to the absent feature. Such `cargo check` commands are rare and `cargo check` should not typically be used this way. But RustRover automatically composes and runs such a command. This fixes a RustRover project discovery breakage commented on in GitoxideLabs#1929. This "ghost feature" may be removed without warning. Nothing should rely on it in production or otherwise in a significant way. It is a bug for any software to break or change behavior if it is removed. This addition is similar to the addition in be4de0d (GitoxideLabs#1123) of the `async-io` feature of `gix-packetline-blocking`, which likewise shouldn't be used. However, `gix-packetline-blocking/futures-lite` is even less elegant than `gix-packetline-blocking/async-io`, since `gix-packetline` doesn't explicitly delcare a `futures-lite` feature. Instead, we currrently don't use `dep:` for `futures-lite` because it breaks `cargo-auditable` (GitoxideLabs#1929).
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in the previous commit, and instead changes `gix-filter` so that it depends on `gix-packetline-blocking` without aliasing it `gix-packetline`, instead replacing each use of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. The long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed. This is because it is referenced in attributes in the code of `gix-packetline-blocking` (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). So removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely causing various other reasonable operations to fail.
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in the previous commit, and instead changes `gix-filter` so that it depends on `gix-packetline-blocking` without aliasing it `gix-packetline`, instead replacing each use of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. The long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed. This is because it is referenced in attributes in the code of `gix-packetline-blocking` (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). So removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely causing various other reasonable operations to fail.
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in d5dd239 (GitoxideLabs#1939), and instead: - Changes `gix-filter` to depends on `gix-packetline-blocking` without aliasing it `gix-packetline`. - Replaces uses of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. Thus, this replaces the solution in GitoxideLabs#1939 for the problem discussed in GitoxideLabs#1929 (comment) with a different solution that avoids carrying a "ghost feature." In contrast, the long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed, because: - It is referenced in attributes in `gix-packetline-blocking` code (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). So removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely making various other reasonable operations fail.
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in d5dd239 (GitoxideLabs#1939), and instead: - Changes `gix-filter` to depends on `gix-packetline-blocking` without aliasing it `gix-packetline`. - Replaces uses of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. Thus, this replaces the solution in GitoxideLabs#1939 for the problem discussed in GitoxideLabs#1929 (comment) with a different solution that avoids carrying a "ghost feature." In contrast, the long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed, because: - It is referenced in attributes in `gix-packetline-blocking` code (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). - For that reason, removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely making various other reasonable operations fail.
This PR makes the necessary changes to make gitoxide crates ready for
-Zcheck-cfg(automatic cfgs checking) being enabled by default.--cfg docsrsand#[cfg_attr(docsrs, ...)]which are an anti-pattern and simply use the already existing featuredocument-features.serdefeature ingix-fsand remove a dead code from the now non-existinglean-clifeature.async-iofeature togix-packetline-blockingsince being a symlink togix-packetlineit needs to have all the feature of the former.Maybe instead of adding the
async-iofeature we could allow the lint forgix-packetline-blocking?lint-nightlyto run nightly tools, in particularcargo check -Zcheck-cfg.