Add a second "DO NOT USE" feature to gix-packetline-blocking
#1939
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
Byron
approved these changes
Apr 8, 2025
|
In that case, I'll merge this, then propose the alternative separately. |
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Apr 8, 2025
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.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Apr 8, 2025
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 was referenced Apr 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a
futures-litefeature togix-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-blockingthrough an alias namedgix-packetline.This new feature should never be used except to keep
cargo checkcommands with--workspacethat listgix-packetline/futures-litefrom failing due to the absent feature. Suchcargo checkcommands are rare andcargo checkshould 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 #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 (#1123) of the
async-iofeature ofgix-packetline-blocking, which likewise shouldn't be used. However,gix-packetline-blocking/futures-liteis even less elegant thangix-packetline-blocking/async-io, sincegix-packetlinedoesn't explicitly delcare afutures-litefeature. Instead, we currrently don't usedep:forfutures-litebecause it breakscargo-auditable(#1929).This is not ready because there is at least one alternative that I am going to at least describe here and possibly change this to use. See #1929 (comment) for context, especially the part under "Edit 3".