Monorepo for Tangled โ€” https://tangled.org

RFC: outside knotserver itself, define custom types instead of relying on go-git #311

open
opened by runxiyu.tngl.sh edited

Rationale#

I am not familiar with XRPC, but it appears that our XRPC responses use types from types/, which are defined in terms of the types exposed by external Go libraries such as go-git.

This makes it difficult to experiment with switching individual knot servers to different Git libraries, as any hashes/refs must be converted into types specific to go-git, thereby causing the entire protocol to be go-git-centered.

Current status#

  • hook/setup.go
    SetupRepo uses git.PlainOpen to try to open the repo.
  • appview/pages/pages.go
    RepoTagsParams uses a map keyed by plumbing.Hashs and a slice of object.Commits.
  • appview/repo/tags.go
    Repo.Tags uses a map keyed by plumbing.Hashs.
  • appview/repo/log.go
    Repo.Commit uses plumbing.IsHash.
  • appview/repo/artifact.go
    Repo.DeleteArtifact uses plumbing.NewHash.
  • appview/repo/tree.go
    Repo.Tree uses plumbing.NewHash.
  • appview/repo/archive.go
    Repo.DownloadArchive uses plumbing.ReferenceName.
  • appview/repo/repo_util.go
    uniqueEmails uses object.Commit.
  • appview/repo/index.go
    Repo.buildIndexResponse uses plumbing.NewHash.
  • appview/ingester.go
    Ingester.ingestArtifact uses plumbing.Hash.
  • appview/commitverify/verify.go
    GetVerifiedObjectCommits and ObjectCommitToNiceDiff use object.Commit.
  • appview/models/pipeline.go
    Trigger.TargetRef uses plumbing.ReferenceName.
  • appview/models/artifact.go
    Artifact uses plumbing.Hash.
  • appview/db/artifact.go
    GetArtifact uses plumbing.Hash.
  • appview/state/knotstream.go
    updateRepoLanguages uses plumbing.ReferenceName.
  • workflow/def.go
    Constraint.MatchRef uses plumbing.ReferenceName.
  • types/diff.go
    NiceDiff uses object.Signature.
  • types/repo.go
    Many types here use object.Commit.
  • types/tree.go
    LastCommitInfo uses plumbing.Hash.

These are largely trivial and could be defined ourselves. However, it may be difficult to remain compatible with the existing protocol, unless we closely match go-git's types.

Note that, in particular, go-git's plumbing.Hash is a [20]byte, which means that protocols that use it as the hash could only really support SHA-1 hashes. This may be undesirable but perhaps necessary for compatibility with existing knot servers?

Some alternative libraries use schemes such as this:

type Hash struct {
	data [maxHashSize]byte
	size int
}

Thank you for your summary! I generally agree with that we should remove the go-git dependency on xrpc types. Some notes about the list:

  • hook/ is part of knot, so we can ignore it.
  • methods like plumbing.IsHash or plumbing.NewHash seems fine as they don't receive go-git types.

I think defining our own Hash, Commit, Signature, and ReferenceName types for types/ package is good place to start.

My opinion with this topic is that we shouldn't use xrpc to get git information at first place. Appview can fetch the entire repository on refUpdate events and directly gather informations from it. But this is long term goal and considering we would need types to represent git objects on appview anyway, I think defining custom types is definitely valuable.

methods like plumbing.IsHash or plumbing.NewHash seems fine as they don't receive go-git types.

I think IsHash is probably fine (other than its lack of support for SHA-256), but NewHash produces go-git types so... not sure?

@runxiyu.tngl.sh I mean they only generates a hash which is just [20]byte as you said, so it isn't hard to interchange. let's just ignore the sha-256 for now and gradually migrate over new git interfaces.

Then we only have to focus on removing go-git dependency from types package. Once it's done, we can start refactoring appview.

in a similar vein, i don't see the issue with the following usages as well:

  • plumbing.ReferenceName is just a string alias, and is serialized to a string
  • not sure what the issue is with using git.PlainOpen either

the more concerning usages are object.Commit (i may have a WIP branch that fixes this gradually!) and maybe object.Signature. thanks for the comprehensive list @runxiyu.tngl.sh.

sign up or login to add to the discussion
Labels

None yet.

area

None yet.

assignee

None yet.

Participants 3
AT URI
at://did:plc:oy6bo3cgc6erpohuxwscacbp/sh.tangled.repo.issue/3m6jalc3qfe22