What is the purpose of gix-packetline-blocking?

[blinxen]

I am at, what I think, the end stage of packaging gix for Fedora and came across gix-packetline-blocking. The source code seems to be a symlink to gix-packetline. The only difference is the Cargo.toml file. Is there a special reason why you decided to duplicate this crate? From crates.io is can see that only gix-filters depends on this duplicate crate.

[Byron]

gix-filters implements a fitler driver which can communicate with filter programs using a certain protocol. All 'messages' are encoded using packetlines.
However, gix-packetline requires one to choose a sync or async implementation - both are mutually exclusive within the same dependency tree. If gix-filter would use gix-packetline it would have to chose the sync implementation, which effectively sets it for everyone using gix-filter, which would make the async implementation unavailable entirely.

Symlinking the code and adding a new manifest seemed like a decent-enough fix. Unfortunately, another side-effect is that one can't easily run cargo anymore on a Windows VM that shares the drive with the host as symlinks don't come across properly.

Probably it would be better to just copy-paste the code (instead of symlinking), or have both implementations available in gix-packetline side-by-side.

[Byron]

While thinking about it I realized that the crate is lacking one critical feature: abstract over test-code just like maybe_async does. The problem is that tests now would have to be duplicated, whereas previously one test-suite could test both versions of the code.

With keyword-generics this will of course be possible, and maybe it's even possible with a combination of async-generic and maybe-async, but I wonder if I will keep my sanity getting there 😅.

Maybe I should settle for just testing the blocking version then even though…the way the crate works it's easy to introduce legitimate bugs due to the if async {} else {} conditionals that would be in many, if not all the places.

I hope I am missing something.

[NobodyXu]

I think the only way for testing to work for async and sync is to duplicate it, but since the macro does the job for you I don't think that's a huge problem?

Even with keywords generics, the compiler would still duplicate the functions for you anyway, sync and async cannot be abstracted into one function easily unless you use an effect system, which will require generator.

An effect system yields when doing an I/O or any other effect, and then let the caller to decide how to execute the I/O (async, sync, etc) and pass back the return to callee.

I don't see Rust going down this path, it might be easier to make everything async with a context defining how I/O is done and then expose a sync interface implemented based on that, with a sync implementation of I/O drivers, simply call Future::poll once and asserts it never returns Poll::Pending

[Byron]

I think the only way for testing to work for async and sync is to duplicate it, but since the macro does the job for you I don't think that's a huge problem?

Yes - if anything can't be duplicated then it's the tests. If the code under test is duplicated that's drift would be caught by the tests, but if tests drift then… it's like having two different codebases disguised as a single crate.

Even with keywords generics, the compiler would still duplicate the functions for you anyway, sync and async cannot be abstracted into one function easily unless you use an effect system, which will require generator.

I was probably under the false assumption that with keyword generics the compiler can chose the correct 'effect' based on what's needed, so a single function has various versions internally and the right one is picked. foo() could be sync or async depending on context. For tests this would mean that the maybe_async trick should still work, as it relies on sync and async not changing the names of functions. Right now this has the unfortunate side-effect that a cargo feature is used to toggle, so sync and async can't live side-by-side in the same dependency tree even. In future, I'd hope they can as no feature toggle will be needed anymore. Instead, the tests need a feature toggle simply to tell maybe_async which version of the code to 'write' - with await, or without.

Is this anticipated future anything I can hope for?

If not, maybe I can hack maybe_async to have another mode that changes the function names as well - how hard could it be 😅. Because then I don't care anymore about the _async suffix - a single test-suite would still be possible then.

What do you think?

[NobodyXu]

Is this anticipated future anything I can hope for?

Yeah, though I think keyword effect won't be stablised in 1-2 years...It isn't even in nightly yet and it's still in pre-RFC stage.

If not, maybe I can hack maybe_async to have another mode that changes the function names as well - how hard could it be 😅. Because then I don't care anymore about the _async suffix - a single test-suite would still be possible then.

I think having feature switching between async and sync as in maybe-async just doesn't work, Rust feature is meant to be addictive.

So changing function names or putting stuff in a different module is really the only solution.

Yes - if anything can't be duplicated then it's the tests. If the code under test is duplicated that's drift would be caught by the tests, but if tests drift then… it's like having two different codebases disguised as a single crate.

Aha sorry I didn't realize the problem.
Since the async and sync has different function name, it will create a new problem.

The solution could be to write another proc-macro in gix-macros that automatically duplicates test functions and renames functions invoked.

[Byron]

The solution could be to write another proc-macro in gix-macros that automatically duplicates test functions and renames functions invoked.

Indeed, that would be optimal as it could then run the whole test-suite at once, both in sync and async mode, exercising both code-paths. Somehow I feel that adjusting maybe-async to deal with differently named functions would be easier, even if it means to run the tests twice with a feature toggle.

Even though I have only been talking about tests being deduplicated thus far, I may also add that the way this is currently implemented, all custom traits that touch IO also can be used with maybe-async, making the consumer of them agnostic as long as maybe-async is used (and gix does it like that).

So for this to work, something like maybe-async is truly required. All the rest, I think I can make work with async-generic as effectively the lower-level plumbing is already split between async and sync implementations, even though the traits they implement are 'agnostic' right now.

Oh, thinking about it, maybe in order to also fix the non-additive nature of sync/async feature toggles, one could also have _async suffixes in the exposed traits (even though async will still only be available if a feature is toggled, but that's additive now). That way, a macro like maybe async can be used to do the duplication so we'd get fetch() and fetch_async() in the consumers as well.

All this sounds quite complicated, but it's doable I suppose.
And it's kind of sad as well that all this isn't naturally supported by Rust yet, async has never been anything I truly liked.