Skip to content

Commit

Permalink
Lazy load object format with command line and don't do it in OpenRepo…
Browse files Browse the repository at this point in the history
…sitory (#29712)

Most time, when invoking `git.OpenRepository`, `objectFormat` will not
be used, so it's a waste to invoke commandline to get the object format.
This PR make it a lazy operation, only invoke that when necessary.
  • Loading branch information
lunny authored Mar 12, 2024
1 parent 7f856d5 commit e84e5db
Show file tree
Hide file tree
Showing 15 changed files with 72 additions and 31 deletions.
5 changes: 3 additions & 2 deletions modules/git/blame_sha256_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,12 @@ func TestReadingBlameOutputSha256(t *testing.T) {
},
}

objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)
for _, c := range cases {
commit, err := repo.GetCommit(c.CommitID)
assert.NoError(t, err)

blameReader, err := CreateBlameReader(ctx, repo.objectFormat, "./tests/repos/repo6_blame_sha256", commit, "blame.txt", c.Bypass)
blameReader, err := CreateBlameReader(ctx, objectFormat, "./tests/repos/repo6_blame_sha256", commit, "blame.txt", c.Bypass)
assert.NoError(t, err)
assert.NotNil(t, blameReader)
defer blameReader.Close()
Expand Down
4 changes: 3 additions & 1 deletion modules/git/blame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,13 @@ func TestReadingBlameOutput(t *testing.T) {
},
}

objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)
for _, c := range cases {
commit, err := repo.GetCommit(c.CommitID)
assert.NoError(t, err)

blameReader, err := CreateBlameReader(ctx, repo.objectFormat, "./tests/repos/repo6_blame", commit, "blame.txt", c.Bypass)
blameReader, err := CreateBlameReader(ctx, objectFormat, "./tests/repos/repo6_blame", commit, "blame.txt", c.Bypass)
assert.NoError(t, err)
assert.NotNil(t, blameReader)
defer blameReader.Close()
Expand Down
7 changes: 5 additions & 2 deletions modules/git/commit_sha256_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,13 @@ func TestHasPreviousCommitSha256(t *testing.T) {
commit, err := repo.GetCommit("f004f41359117d319dedd0eaab8c5259ee2263da839dcba33637997458627fdc")
assert.NoError(t, err)

objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)

parentSHA := MustIDFromString("b0ec7af4547047f12d5093e37ef8f1b3b5415ed8ee17894d43a34d7d34212e9c")
notParentSHA := MustIDFromString("42e334efd04cd36eea6da0599913333c26116e1a537ca76e5b6e4af4dda00236")
assert.Equal(t, repo.objectFormat, parentSHA.Type())
assert.Equal(t, repo.objectFormat.Name(), "sha256")
assert.Equal(t, objectFormat, parentSHA.Type())
assert.Equal(t, objectFormat.Name(), "sha256")

haz, err := commit.HasPreviousCommit(parentSHA)
assert.NoError(t, err)
Expand Down
5 changes: 0 additions & 5 deletions modules/git/repo_base_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,6 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) {
repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(ctx, repoPath)
repo.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(ctx, repoPath)

repo.objectFormat, err = repo.GetObjectFormat()
if err != nil {
return nil, err
}

return repo, nil
}

Expand Down
7 changes: 6 additions & 1 deletion modules/git/repo_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,12 @@ func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions)
}
}()

len := repo.objectFormat.FullLength()
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}

len := objectFormat.FullLength()
commits := []*Commit{}
shaline := make([]byte, len+1)
for {
Expand Down
5 changes: 4 additions & 1 deletion modules/git/repo_commit_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ func (repo *Repository) RemoveReference(name string) error {

// ConvertToHash returns a Hash object from a potential ID string
func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
objectFormat := repo.objectFormat
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}
if len(commitID) == hash.HexSize && objectFormat.IsValid(commitID) {
ID, err := NewIDFromString(commitID)
if err == nil {
Expand Down
9 changes: 6 additions & 3 deletions modules/git/repo_commit_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,11 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id ObjectID)

// ConvertToGitID returns a GitHash object from a potential ID string
func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
IDType := repo.objectFormat
if len(commitID) == IDType.FullLength() && IDType.IsValid(commitID) {
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}
if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) {
ID, err := NewIDFromString(commitID)
if err == nil {
return ID, nil
Expand All @@ -142,7 +145,7 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {

wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx)
defer cancel()
_, err := wr.Write([]byte(commitID + "\n"))
_, err = wr.Write([]byte(commitID + "\n"))
if err != nil {
return nil, err
}
Expand Down
6 changes: 5 additions & 1 deletion modules/git/repo_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,12 @@ func (repo *Repository) GetPatch(base, head string, w io.Writer) error {
// If base is undefined empty SHA (zeros), it only returns the files changed in the head commit
// If base is the SHA of an empty tree (EmptyTreeSHA), it returns the files changes from the initial commit to the head commit
func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, error) {
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}
cmd := NewCommand(repo.Ctx, "diff-tree", "--name-only", "--root", "--no-commit-id", "-r", "-z")
if base == repo.objectFormat.EmptyObjectID().String() {
if base == objectFormat.EmptyObjectID().String() {
cmd.AddDynamicArguments(head)
} else {
cmd.AddDynamicArguments(base, head)
Expand Down
9 changes: 6 additions & 3 deletions modules/git/repo_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,20 @@ func TestGetCommitFilesChanged(t *testing.T) {
assert.NoError(t, err)
defer repo.Close()

objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)

testCases := []struct {
base, head string
files []string
}{
{
repo.objectFormat.EmptyObjectID().String(),
objectFormat.EmptyObjectID().String(),
"95bb4d39648ee7e325106df01a621c530863a653",
[]string{"file1.txt"},
},
{
repo.objectFormat.EmptyObjectID().String(),
objectFormat.EmptyObjectID().String(),
"8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2",
[]string{"file2.txt"},
},
Expand All @@ -146,7 +149,7 @@ func TestGetCommitFilesChanged(t *testing.T) {
[]string{"file2.txt"},
},
{
repo.objectFormat.EmptyTree().String(),
objectFormat.EmptyTree().String(),
"8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2",
[]string{"file1.txt", "file2.txt"},
},
Expand Down
6 changes: 5 additions & 1 deletion modules/git/repo_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,18 @@ func (repo *Repository) LsFiles(filenames ...string) ([]string, error) {

// RemoveFilesFromIndex removes given filenames from the index - it does not check whether they are present.
func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error {
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return err
}
cmd := NewCommand(repo.Ctx, "update-index", "--remove", "-z", "--index-info")
stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer)
buffer := new(bytes.Buffer)
for _, file := range filenames {
if file != "" {
buffer.WriteString("0 ")
buffer.WriteString(repo.objectFormat.EmptyObjectID().String())
buffer.WriteString(objectFormat.EmptyObjectID().String())
buffer.WriteByte('\t')
buffer.WriteString(file)
buffer.WriteByte('\000')
Expand Down
4 changes: 2 additions & 2 deletions modules/git/repo_tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) {
break
}

tag, err := parseTagRef(repo.objectFormat, ref)
tag, err := parseTagRef(ref)
if err != nil {
return nil, 0, fmt.Errorf("GetTagInfos: parse tag: %w", err)
}
Expand All @@ -161,7 +161,7 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) {
}

// parseTagRef parses a tag from a 'git for-each-ref'-produced reference.
func parseTagRef(objectFormat ObjectFormat, ref map[string]string) (tag *Tag, err error) {
func parseTagRef(ref map[string]string) (tag *Tag, err error) {
tag = &Tag{
Type: ref["objecttype"],
Name: ref["refname:lstrip=2"],
Expand Down
3 changes: 1 addition & 2 deletions modules/git/repo_tag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ func TestRepository_GetAnnotatedTag(t *testing.T) {
}

func TestRepository_parseTagRef(t *testing.T) {
sha1 := Sha1ObjectFormat
tests := []struct {
name string

Expand Down Expand Up @@ -351,7 +350,7 @@ Add changelog of v1.9.1 (#7859)
for _, test := range tests {
tc := test // don't close over loop variable
t.Run(tc.name, func(t *testing.T) {
got, err := parseTagRef(sha1, tc.givenRef)
got, err := parseTagRef(tc.givenRef)

if tc.wantErr {
require.Error(t, err)
Expand Down
7 changes: 6 additions & 1 deletion modules/git/repo_tree_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ func (repo *Repository) getTree(id ObjectID) (*Tree, error) {

// GetTree find the tree object in the repository.
func (repo *Repository) GetTree(idStr string) (*Tree, error) {
if len(idStr) != repo.objectFormat.FullLength() {
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}

if len(idStr) != objectFormat.FullLength() {
res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(idStr).RunStdString(&RunOpts{Dir: repo.Path})
if err != nil {
return nil, err
Expand Down
12 changes: 10 additions & 2 deletions modules/git/repo_tree_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ func (repo *Repository) getTree(id ObjectID) (*Tree, error) {
case "tree":
tree := NewTree(repo, id)
tree.ResolvedID = id
tree.entries, err = catBatchParseTreeEntries(repo.objectFormat, tree, rd, size)
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}
tree.entries, err = catBatchParseTreeEntries(objectFormat, tree, rd, size)
if err != nil {
return nil, err
}
Expand All @@ -69,7 +73,11 @@ func (repo *Repository) getTree(id ObjectID) (*Tree, error) {

// GetTree find the tree object in the repository.
func (repo *Repository) GetTree(idStr string) (*Tree, error) {
if len(idStr) != repo.objectFormat.FullLength() {
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}
if len(idStr) != objectFormat.FullLength() {
res, err := repo.GetRefCommitID(idStr)
if err != nil {
return nil, err
Expand Down
14 changes: 10 additions & 4 deletions modules/git/tree_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ func (t *Tree) ListEntries() (Entries, error) {
return nil, runErr
}

var err error
t.entries, err = parseTreeEntries(t.repo.objectFormat, stdout, t)
objectFormat, err := t.repo.GetObjectFormat()
if err != nil {
return nil, err
}
t.entries, err = parseTreeEntries(objectFormat, stdout, t)
if err == nil {
t.entriesParsed = true
}
Expand All @@ -101,8 +104,11 @@ func (t *Tree) listEntriesRecursive(extraArgs TrustedCmdArgs) (Entries, error) {
return nil, runErr
}

var err error
t.entriesRecursive, err = parseTreeEntries(t.repo.objectFormat, stdout, t)
objectFormat, err := t.repo.GetObjectFormat()
if err != nil {
return nil, err
}
t.entriesRecursive, err = parseTreeEntries(objectFormat, stdout, t)
if err == nil {
t.entriesRecursiveParsed = true
}
Expand Down

0 comments on commit e84e5db

Please sign in to comment.