Refactoring git domain to use async generators, remove readable-stream, simplify utilities

[tegefaulkes]

Description

This PR addresses refactoring the git domain to be more clear and more in line with our latest practices.

Issues Fixed

• Fixes Review and Refactor of git/utils.ts for Serving Vault Git Repositories #298
• Ref ENG-68
• Fixes Replace readable-stream usage with Web Streams in the Vaults and Git Domains #523

Tasks

• 1. Factor out http stream construction to a new http.ts file.
• 2. Keep git and FS interaction utility in utils.ts.
• 3. Refactor existing code that we are keeping to be more in line with current practices.
• 4. Refactor the HTTP response to be streaming focused. Using generators for most of the back end logic and convert to a webstream for the rpc stage.
• 5. All types should be formalised where appropriate.
• 6. Inline documentation should be updated and all relevant spec details should be included along side the code.
• 7. Tests need to updated and refactored to reflect the new refactoring changes.
• 8. Performance is an issue here, so benchmarks should be added to test for future regressions. - Differed to a new PR. It will test the current changes but nothing here will depend on benchmarks, so I'm just going to make it separate.
• Check if Replace readable-stream usage with Web Streams in the Vaults and Git Domains #523 has been addressed fully

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[linear]

ENG-68 Review and Refactor of `git/utils.ts` for Serving Vault Git Repositories

[tegefaulkes]

The reference discovery stage has been completed. I implemented it as AsyncGenerators which made it stream-able but also very clean logic wise. I opted for generators over webstreams for this stage since composing streams together is a lot cleaner with generators than streams.

[CMCDragonkai]

Generators are a higher level construct indeed. I think streams represent a lower level construct, and also streams are actually synchronous. Whereas generators represent coroutines in the javascript runtime. So if you're dealing at the object-level then async generator abstraction is better abstraction.

[CMCDragonkai]

In the future, you should write up your own branch name.

[CMCDragonkai]

New types and new classes need to be documented into the spec. Like this:

Module

type X = ...

type Y = ...

Give a reason as why they exist now.

[CMCDragonkai]

Expected to take the whole cycle.

[CMCDragonkai]

This is a bug with the vault log - pull system. Needs to be fast forward merge.

[tegefaulkes]

Progress update: I've converted to using generators and swapped over to using them in the code. The test I've been using as a sanity check is passing as well. I'm going to go back through the code and clean it up a little now.

[tegefaulkes]

isometric-git actually provides a lot of the functionality we needed in the git domain. I was actually able to replace most of the utils.ts functions with it. I did some cursory checks and the things run just as fast, the whole git domain has been simplified immensely.

It seems odd that isometic-git wasn't used to handle interacting with the git structure in the first place. Was the git domain created before we started using isometric-git?

[CMCDragonkai]

Originally isogit did not have server-side functionality. Only client side. You were able to replace it all? I thought there was git server functionality that was missing. Remember in the future we may want to expose an http API.

[tegefaulkes]

It is missing the server side functionality, but it does provide a lot of plumbing functionality for interacting with the git structure that the utils.ts was replicating.

For example, Listing all the existing refs, resolving them to objectIds, then finding all of the dependent objectIds, and finally creating the packfile format that gets sent over can all be done with isogit. The only thing I had to do was generate the HTTP responses in the proper format.

[CMCDragonkai]

Then yes it was a quick and dirty job.

[CMCDragonkai]

Originally we even wanted to use libgit2 but it was easily adapted to work on a in-memory virtual filesystem. Too tied into posix fs semantics.

[tegefaulkes]

Mostly done now. Just need to fix some tests in the vaults domain now. Along with doing the benchmarks.

I think the benchmarks can be moved to a 2nd stage PR. I also think the whole vaults domain needs a general review and touch up, also in a separate PR.

[tegefaulkes]

Tests are fixed, I added a end-to-end integration tests of of cloning and pulling in the http.test.ts tests. This does a git clone/pull using just the new http code directly and the normal file system. So no efs or networking. It should be a good basis of comparison between adding efs and networking separately in the benchmarks.

I'm going to look into the clone/pull bug mentioned from before and then move on the merge prep. I'll start a new PR for adding the benchmarks.

[tegefaulkes]

1 test relating to discovery and 2 tests relating to MDNS are consistently failing. I'm not sure why and they're wait out of scope of this work so it shouldn't be affected.

[tegefaulkes]

This is ready for review. I'm going to start the benchmarks in a new PR for now.

[CMCDragonkai]

I added some additional review comments with a first glance. Also this review comment previously should be addressed: #709 (review).

Otherwise is there specific things you want reviewed?

[CMCDragonkai]

The final checklist should be ticked off.

[tegefaulkes]

Nothing specific needs to be reviewed, just need the extra pair of eyes for the due diligence.

I'm leaving the checklist along for when the review changes have been addressed.

[tegefaulkes]

Done and ready to merge. I'm going to rebase the related PRs before merging this.