Lazy load object format with command line and don't do it in OpenRepository (#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.
This commit is contained in:
Lunny Xiao 2024-03-12 12:21:27 +08:00 committed by GitHub
parent 7f856d5d74
commit e84e5db6de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 72 additions and 31 deletions

View File

@ -118,11 +118,12 @@ func TestReadingBlameOutputSha256(t *testing.T) {
}, },
} }
objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)
for _, c := range cases { for _, c := range cases {
commit, err := repo.GetCommit(c.CommitID) commit, err := repo.GetCommit(c.CommitID)
assert.NoError(t, err) assert.NoError(t, err)
blameReader, err := CreateBlameReader(ctx, objectFormat, "./tests/repos/repo6_blame_sha256", commit, "blame.txt", c.Bypass)
blameReader, err := CreateBlameReader(ctx, repo.objectFormat, "./tests/repos/repo6_blame_sha256", commit, "blame.txt", c.Bypass)
assert.NoError(t, err) assert.NoError(t, err)
assert.NotNil(t, blameReader) assert.NotNil(t, blameReader)
defer blameReader.Close() defer blameReader.Close()

View File

@ -118,11 +118,13 @@ func TestReadingBlameOutput(t *testing.T) {
}, },
} }
objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)
for _, c := range cases { for _, c := range cases {
commit, err := repo.GetCommit(c.CommitID) commit, err := repo.GetCommit(c.CommitID)
assert.NoError(t, err) 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.NoError(t, err)
assert.NotNil(t, blameReader) assert.NotNil(t, blameReader)
defer blameReader.Close() defer blameReader.Close()

View File

@ -140,10 +140,13 @@ func TestHasPreviousCommitSha256(t *testing.T) {
commit, err := repo.GetCommit("f004f41359117d319dedd0eaab8c5259ee2263da839dcba33637997458627fdc") commit, err := repo.GetCommit("f004f41359117d319dedd0eaab8c5259ee2263da839dcba33637997458627fdc")
assert.NoError(t, err) assert.NoError(t, err)
objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)
parentSHA := MustIDFromString("b0ec7af4547047f12d5093e37ef8f1b3b5415ed8ee17894d43a34d7d34212e9c") parentSHA := MustIDFromString("b0ec7af4547047f12d5093e37ef8f1b3b5415ed8ee17894d43a34d7d34212e9c")
notParentSHA := MustIDFromString("42e334efd04cd36eea6da0599913333c26116e1a537ca76e5b6e4af4dda00236") notParentSHA := MustIDFromString("42e334efd04cd36eea6da0599913333c26116e1a537ca76e5b6e4af4dda00236")
assert.Equal(t, repo.objectFormat, parentSHA.Type()) assert.Equal(t, objectFormat, parentSHA.Type())
assert.Equal(t, repo.objectFormat.Name(), "sha256") assert.Equal(t, objectFormat.Name(), "sha256")
haz, err := commit.HasPreviousCommit(parentSHA) haz, err := commit.HasPreviousCommit(parentSHA)
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -71,11 +71,6 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) {
repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(ctx, repoPath) repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(ctx, repoPath)
repo.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(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 return repo, nil
} }

View File

@ -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{} commits := []*Commit{}
shaline := make([]byte, len+1) shaline := make([]byte, len+1)
for { for {

View File

@ -41,7 +41,10 @@ func (repo *Repository) RemoveReference(name string) error {
// ConvertToHash returns a Hash object from a potential ID string // ConvertToHash returns a Hash object from a potential ID string
func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { 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) { if len(commitID) == hash.HexSize && objectFormat.IsValid(commitID) {
ID, err := NewIDFromString(commitID) ID, err := NewIDFromString(commitID)
if err == nil { if err == nil {

View File

@ -132,8 +132,11 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id ObjectID)
// ConvertToGitID returns a GitHash object from a potential ID string // ConvertToGitID returns a GitHash object from a potential ID string
func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
IDType := repo.objectFormat objectFormat, err := repo.GetObjectFormat()
if len(commitID) == IDType.FullLength() && IDType.IsValid(commitID) { if err != nil {
return nil, err
}
if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) {
ID, err := NewIDFromString(commitID) ID, err := NewIDFromString(commitID)
if err == nil { if err == nil {
return ID, nil return ID, nil
@ -142,7 +145,7 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx)
defer cancel() defer cancel()
_, err := wr.Write([]byte(commitID + "\n")) _, err = wr.Write([]byte(commitID + "\n"))
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -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 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 // 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) { 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") 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) cmd.AddDynamicArguments(head)
} else { } else {
cmd.AddDynamicArguments(base, head) cmd.AddDynamicArguments(base, head)

View File

@ -126,17 +126,20 @@ func TestGetCommitFilesChanged(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
defer repo.Close() defer repo.Close()
objectFormat, err := repo.GetObjectFormat()
assert.NoError(t, err)
testCases := []struct { testCases := []struct {
base, head string base, head string
files []string files []string
}{ }{
{ {
repo.objectFormat.EmptyObjectID().String(), objectFormat.EmptyObjectID().String(),
"95bb4d39648ee7e325106df01a621c530863a653", "95bb4d39648ee7e325106df01a621c530863a653",
[]string{"file1.txt"}, []string{"file1.txt"},
}, },
{ {
repo.objectFormat.EmptyObjectID().String(), objectFormat.EmptyObjectID().String(),
"8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2", "8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2",
[]string{"file2.txt"}, []string{"file2.txt"},
}, },
@ -146,7 +149,7 @@ func TestGetCommitFilesChanged(t *testing.T) {
[]string{"file2.txt"}, []string{"file2.txt"},
}, },
{ {
repo.objectFormat.EmptyTree().String(), objectFormat.EmptyTree().String(),
"8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2", "8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2",
[]string{"file1.txt", "file2.txt"}, []string{"file1.txt", "file2.txt"},
}, },

View File

@ -94,6 +94,10 @@ func (repo *Repository) LsFiles(filenames ...string) ([]string, error) {
// RemoveFilesFromIndex removes given filenames from the index - it does not check whether they are present. // RemoveFilesFromIndex removes given filenames from the index - it does not check whether they are present.
func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error { 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") cmd := NewCommand(repo.Ctx, "update-index", "--remove", "-z", "--index-info")
stdout := new(bytes.Buffer) stdout := new(bytes.Buffer)
stderr := new(bytes.Buffer) stderr := new(bytes.Buffer)
@ -101,7 +105,7 @@ func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error {
for _, file := range filenames { for _, file := range filenames {
if file != "" { if file != "" {
buffer.WriteString("0 ") buffer.WriteString("0 ")
buffer.WriteString(repo.objectFormat.EmptyObjectID().String()) buffer.WriteString(objectFormat.EmptyObjectID().String())
buffer.WriteByte('\t') buffer.WriteByte('\t')
buffer.WriteString(file) buffer.WriteString(file)
buffer.WriteByte('\000') buffer.WriteByte('\000')

View File

@ -141,7 +141,7 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) {
break break
} }
tag, err := parseTagRef(repo.objectFormat, ref) tag, err := parseTagRef(ref)
if err != nil { if err != nil {
return nil, 0, fmt.Errorf("GetTagInfos: parse tag: %w", err) return nil, 0, fmt.Errorf("GetTagInfos: parse tag: %w", err)
} }
@ -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. // 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{ tag = &Tag{
Type: ref["objecttype"], Type: ref["objecttype"],
Name: ref["refname:lstrip=2"], Name: ref["refname:lstrip=2"],

View File

@ -194,7 +194,6 @@ func TestRepository_GetAnnotatedTag(t *testing.T) {
} }
func TestRepository_parseTagRef(t *testing.T) { func TestRepository_parseTagRef(t *testing.T) {
sha1 := Sha1ObjectFormat
tests := []struct { tests := []struct {
name string name string
@ -351,7 +350,7 @@ Add changelog of v1.9.1 (#7859)
for _, test := range tests { for _, test := range tests {
tc := test // don't close over loop variable tc := test // don't close over loop variable
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
got, err := parseTagRef(sha1, tc.givenRef) got, err := parseTagRef(tc.givenRef)
if tc.wantErr { if tc.wantErr {
require.Error(t, err) require.Error(t, err)

View File

@ -21,7 +21,12 @@ func (repo *Repository) getTree(id ObjectID) (*Tree, error) {
// GetTree find the tree object in the repository. // GetTree find the tree object in the repository.
func (repo *Repository) GetTree(idStr string) (*Tree, error) { 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}) res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(idStr).RunStdString(&RunOpts{Dir: repo.Path})
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -51,7 +51,11 @@ func (repo *Repository) getTree(id ObjectID) (*Tree, error) {
case "tree": case "tree":
tree := NewTree(repo, id) tree := NewTree(repo, id)
tree.ResolvedID = 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 { if err != nil {
return nil, err return nil, err
} }
@ -69,7 +73,11 @@ func (repo *Repository) getTree(id ObjectID) (*Tree, error) {
// GetTree find the tree object in the repository. // GetTree find the tree object in the repository.
func (repo *Repository) GetTree(idStr string) (*Tree, error) { 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) res, err := repo.GetRefCommitID(idStr)
if err != nil { if err != nil {
return nil, err return nil, err

View File

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