Deduplicate packetline crates

[djc]

Trying to make a little progress on #2152 (and this copied crate stuff seems extremely ugly anyway).

Why does cargo check pass while cargo check -p gix break? Something to do with feature unification? This means I'm not getting feedback from VS Code on errors, which makes it much harder to get stuff done.

[Byron]

Thanks for giving it a shot, it's much appreciated!

Feature-unification is the reason for requiring a differently named copy. It's very ugly, but there was no other way.
It all goes down to wanting to be able to 'switch' async on or off with minimal code duplication and shared tests. This is how one atrocity led to another.
Like mentioned before, I'd live better without async entirely (in this codebase).

[djc]

Feature-unification is the reason for requiring a differently named copy. It's very ugly, but there was no other way.
It all goes down to wanting to be able to 'switch' async on or off with minimal code duplication and shared tests. This is how one atrocity led to another.

Do you have reason to think the approach in this PR would not work? It seems like an existence proof contradicting that "there was no other way" (and I don't think it introduces substantial code duplication, although the tests might be a bit harder).

[Byron]

Apologies, I didn't even look at the code yet and wanted to wait until you think it's ready for review.
My statement was about how this came to be, it's not the wish to torture users, but to solve a specific problem that led down this path.
Sometimes other solutions aren't seen in time leading to suboptimal results, and I genuinely hope that this PR can show the way.

[djc]

Apologies, I didn't even look at the code yet and wanted to wait until you think it's ready for review.

It obviously doesn't pass CI yet, but I'd like some early feedback before I try to tackle the remaining issues. I think the idea is conceptually simple: the gix-packetline crate offers both async and sync APIs, separately (both are toggled by a Cargo feature flag), and consumers explicitly use the API for the mode they want to use. I don't think there's substantially more library-level code duplication. For now, this leaves mutually exclusive features in higher-level crates alone, at some point the I/O mode will bubble up to the top-level API and then we have to decide how to best present that.

[Byron]

I think I understand now what this PR is going for and have a feeling that it could work. Maybe gix-packetline never needed have offer one unified codepath that can be 'toggled' async due to its primitive nature.

Now that I think about it, maybe the tests for gix-packetline then will be more duplicated, but that should be alright as they are stable.

And yes, I agree that from here more opportunities for similar updates may show up.

[Byron]

And this is what can't be happening. Setting this to blocking sets it to blocking for everyone in the dependency tree, even though there maybe others who want it async.

This PR solves this by making gix-packetline features additive, which might be OK to adjust here assuming that the code that uses it already is clearly split into sync and async.

If that's the case, I think it makes total sense to continue this effort.

[lorenzleutgeb]

In principle, if there are two features "gix-packetline/blocking-io" and "gix-packetline/async-io" (which seems to be the case IIUC), and these are additive, this could be fine.

However, it'd be good to understand why this can't be:

[features]
blocking-io = ["gix-packetline/blocking-io"]
[dependencies]
gix-packetline = { version = "...", path = "...", features = [] } # for emphasis

That is, why can't gix-filter also have a feature toggle on its I/O, and propagate down to gix-packetline?
If this is impossible, then this must imply that gix-filter somehow intrinsically requires blocking I/O. And then this would be the next best frontier to push, i.e. to work on getting gix-filter to support both blocking and async. Or you decide that this is not worth doing and just declare gix-filter to require blocking I/O, full stop. You could future-proof by requiring the feature flag "blocking-io" for the crate to compile at all (like gix-hash now requires "sha1"), just to make room for a potential "async-io" down the line.

So long as this is not mutually exclusive with enabling async I/O, this might just be a minor annoyance...

[djc]

On main, gix-filter unconditionally depends on gix-packetline-blocking, which I take to mean that gix-filter indeed depends on the blocking I/O implementation in gix-packetline. Solving that problem is not my goal for now -- allowing the features not to be mutually exclusive is.

[djc]

This feels like it's pretty close. Somehow some builds in the full tests end up enabling both async-client and blocking-client on gix-transport which are supposed to be mutually exclusive on this branch as before -- I'm not sure what's triggering that? It doesn't feel like any of the changes I made should be capable of that, they're all supposed to be upstream of gix-transport.

djc-2021 dedup-packetline gitoxide $ cargo check --features lean-async
    Checking gix-packetline v0.19.1 (/Users/djc/src/gitoxide/gix-packetline)
    Checking gix-filter v0.20.0 (/Users/djc/src/gitoxide/gix-filter)
    Checking gix-transport v0.48.0 (/Users/djc/src/gitoxide/gix-transport)
error: Cannot set both 'blocking-client' and 'async-client' features as they are mutually exclusive
  --> gix-transport/src/lib.rs:92:1
   |
92 | compile_error!("Cannot set both 'blocking-client' and 'async-client' features as they are mutually exclusive");
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^