Skip to content

Unify manual git operations WriteCommit, WriteTree, and WriteBlob to localrepo package

John Cai requested to merge jc-unify-git-operations into master

Currently when we want to execute a plumbing Git command to write a blob, commit, or tree manually there is the internal/git/gittest package. This presents some issues.

Firstly, there is some level of code duplication between internal/git/localrepo and internal/git/gittest.

Secondly, internal/git/gittest suggests that it should not be used in production. Indeed the functions that currently take care of writing blobs, commits, and trees are only for tests--but even if we were to make a non-test version of the function, the package name is still problematic. This means that we would likely add the same functionality to internal/git/localrepo, which will increase the amount of code duplication.

To solve this, we can unify everything into the internal/git/localrepo package and refactor all the call sites to use localrepo. Unfortunately there are still a few places that need to use the original internal/git/gittest functions as they take a full repository path. Namely, Praefect tests cannot use the localrepo version because the using a locator won't give the right repository path.

Also, internal/git/catfile cannot depend on internal/git/localrepo since internal/git/localrepo already depends on internal/git/catfile this would introduce a circular dependency. To get around this, ideally tests in internal/git/catfile would be in package catfile_test and be blackbox tests. However, they are currently written as whitebox tests and would be hard to re-write as blackbox tests. So, it's easier to just continue calling the gittest functions.

Merge request reports