Repository navigation
Review and Refactor of git/utils.ts for Serving Vault Git Repositories #298
Description
Activity
Something else I forgot to mention was this: https://github.com/MatrixAI/js-polykey/pull/266/files#r743402477
As a general comment from what I have seen, when cloning and pulling isomorphic-git searched for the
packed-refsfile inside the.gitdirectory. It should be auto generated when you initialise a git repository. I haven't been able to find any issues related to it on isomorphic git so it could be something to do with our implementation. I haven't looked too much into itIn another PR I might have a look at the need to write this file here. I remember that cloning and pulling with iso-git fails without (and for some reason it isnt automatically written). This may be because we are looking for it in the git utility functions rather than it being a requirement in the iso-git library. See the packedrefs function in git/utils.ts
Comment about how isomorphic-git library is only intended for client-side usage. We need to address this comment #266 (comment) in order to refactor and good documentation and understanding of how our git server actually works.
@tegefaulkes based on our discussions and review of the git utilities in #266, can you link all those discussions and reviews in this issue?
I'm going to copy relevant information here rather than link to every comment. But the original thread is here. #266 (comment)
Summary
There are 3 main functions here.
requesthandleInfoRequesthandlePackRequest
requestreturns a function that is used as the custom HTTP handler that we give to the git clone and pull functions. It's job is to convert the HTTP requests the git functions make into GRPC calls and then return them in the proper HTTP response format. It calls the GRPC functionsagentService.ts:vaultsGitInfoGetandagentService.ts:vaultsGitPackGet.
agentService.ts:vaultsGitInfoGetcallsvaultManager.handleInfoRequestandagentService.ts:vaultsGitPackGetcallsvaultManager.handlePackRequest. Overall this is just streaming the data and the request function is returning that as the required HTTP response.vaultManager.handleInfoRequestjob is pretty simple. It's getting a list of all refs. resolving them to their sha hash and then formatting it into a stream. I think the idea here is just to get a full list of refs+hashes.vaultManager.handlePackRequestIs a lot more complex. There is a lot going on here but it boils down to..- Getting a list of refs that need to be retrieved.
- getting a list of commits related to the refs.
- getting all of the git objects related to the commits.
- packing and formatting all of the git objects.
- sending all of the packed objects back through a stream.
This is all handled by the top level functions
log,listObjectsandpack. Everything under that is for working with the git data structure.┌──────────────────┐ ┌────────────────┐ │ │ │ │ │ Vault Manager ├──────► IsoGit │ │ ---------------- │ │ │ HTTP ┌──────────┐ │ │ │ Clone()───────┼───────► GRPC │ │ │ │ Pull() │ GET &│ Client │ │ Vault map ◄────┼──────┼──┐ ┌───────┐ │ POST │ │ │ Add vault │ └──┼───┼───────┼─┘ │ │ │ │ │ │ │ └┬─┬───────┘ └──────────────────┘ │ │ │ │ │ GET │ │ ▼ │ │Server stream ┌─┴───▼────┐ ┌────────┐ │ └──────► │ vault │ │ │ │ │ internal─┼──┤► EFS │ └────────► │ │ │ │ Duplex stream └──────────┘ └────────┘ POSTUseful technical documentation
https://www.git-scm.com/docs/http-protocol
https://github.com/git/git/tree/master/Documentation/technical
Documentation of note:- https://github.com/git/git/blob/master/Documentation/technical/pack-format.txt
- https://github.com/git/git/blob/master/Documentation/technical/pack-protocol.txt
- https://github.com/git/git/blob/master/Documentation/technical/protocol-capabilities.txt
- https://github.com/git/git/blob/master/Documentation/technical/http-protocol.txt
- https://github.com/MatrixAI/js-polykey/wiki/git-api
17 remaining items
There's alot we can do.
The whole clone and pull which uses HTTP streams - because we are now using JS-rpc, and we have our own muxing and demuxing protocol, I'm thinking that you would want to preserve the HTTP protocol and properly write a wrapping and unwrapping functions. That way in the future we could maintain compatibility with real git protocols, while wrapping them into our js-rpc system. It should be made as simple and as fast as possible - benchmarks should be created for these in our
benches.From a haskell perspective, always try to think in terms of symmetry and reflexivity.
Almost every single object - whether packs, refs or all other primitive concepts that we're using should all be given their own type in
types.tsand a corresponding constructor and deconstructor.An OOP style would create things like
class GitTag,class GitRef,class GitObject... etc.Any usage of magic strings, magic numbers should be removed/abstracted from the git utilities and if they need to exit, be made into documented constants. All functions that are part of some overall protocol of either a data format or communication format should also be bundled together. This means the utils.ts can be turned into a utils directory and all the functions that relate a single format can be bundled together. For example https://user-images.githubusercontent.com/22425593/147191011-faf83025-2865-4118-a91d-7a44d40e556d.png this shows that the git utilities is very imperative API, just random functions that are called together to construct some sort of data structure. The functions could be "object-orientised" to fit our overall Polykey style. It looks like the existing git utilities is based on a C-like programming style, very imperative, very procedural. We should have objects/classes instead that construct the domains we need for handling the git request.
Moving forward I'm going to do the following.
- Create a
http.tsfile that has factories for creating webstreams that provide the HTTP protocol responses. The high level aspect of streaming these responses will be handled here. - The
utils.tsfile will contain smaller utilities for generating the smaller aspects of the HTTP response. This includes any magic values as constants and utilities such as generating the line length header and multiplexing. - All aspects of the domain will be reviewed for proper typing. Typing will be updated as required.
- Optimisations will be applied as needed. Already I can see that every other line of the pack section is just
0005�repeated. It's the equivalent of sending and empty string between each actual packet of data.
- Create a
This is a very old spec. I won't modify it too much besides changing the task list and adding a footnote.
Why reopen this? IS there another PRs targeting this?
I didn't? Might be some linear weirdness but I haven't done anything besides merge this and then rebase some other PRs.
Linear had the PR as non-closing since I only referenced it with the
Refkeyword. It seems then it merged, this was closed but the github automation moved the linear issue back toinProgress? Likely reopening this.I'm closing it again.
I'm not sure the linear sync is working properly. The linear issue didn't auto close after I closed this. This is a little frustrating.
I didn't reference any PRs in my issues. Not sure if that is thing that is causing problems.
Specification
The underlying workings of
git/utils.tsis largely a mystery to our team. We want to perform a deep dive into the workings of this code, and remove/refactor any of our own implementation, potentially also replacing their usage in favour of theisomorphic-git/internal-apis. There seems to be some overlap with what our git utils perform, and whatiso-gitprovides.To clarify... we use git in 2 ways: as a client and as a server.
When using isogit, it only provides the primitives to use git as a client. It does not provide the primitives to use git as a server.
Why do we use git as a client and server? Well our entire secrets management system is built as a stack, from bottom to top:
pk secrets *Polykey-CLI#32 (this is what creates "vault operations")As you can see, the "Virtual Git plus Filesystem" layer makes use of isogit. A virtual git repository is created on the virtual filesystem. This is how we are able to then stack unix operations with roughly similar semantics later, in order to maintain API consistency with what developers are used to.
Now the issue is that isogit by itself can act like a git client. That means it can clone/pull repositories, it can create repositories, it can create commits, it can manage the git objects... etc.
However isogit cannot "serve" the repositories. And this is essential to be able to share our vaults.
To serve our repositories we need to know how to act like a "git server" when we use isogit or something were to use git to try and clone/pull our vaults.
So a long time ago, we developed the git server primitives and put them all into
git/utils.ts. However this code has not been heavily tested, nor has it gone through any refactoring since then. And since then we have forgotten how it all works, and it's not in the programming style that we have developed over the last few years working on Polykey.So the
git/utils.tsneeds to go under review, and most likely some heavy refactoring with the addition of tests, to ensure that acting like a git server makes sense here.Now there is definitely cross over between isogit and the
git/utils.ts. However we aren't aware of what these are. And there probably should be a bunch of "shared types" and similar constructs. We just need to get into it, and get it done.How this was addressed
After reviewing the code as it was, we ended up just cleaning it up by refactoring the
httpresponse to be streamed using an async generator. Most of the utility functions were removed in favour of using theisomorphic-gitplumbing functions for interacting with the git data structure.Ultimately this didn't speed things up very much, however, it did significantly reduce the complexity of the git domain. So it is much easier to follow along with what is happening. There is also a slight improvement with pulling a repo. We use the
havesprovided by the client to only grab the minimum objects required for the pull.Additional context
Tasks