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.

(cherry picked from commit e84e5db6de0306d514b1f1a9657931fb7197a188)
This commit is contained in:
Lunny Xiao 2024-03-12 12:21:27 +08:00 committed by Earl Warren
parent e825d007b1
commit c9d9255244
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
15 changed files with 72 additions and 31 deletions

View file

@ -120,11 +120,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()

View file

@ -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()

View file

@ -152,10 +152,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)

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.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(ctx, repoPath)
repo.objectFormat, err = repo.GetObjectFormat()
if err != nil {
return nil, err
}
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{}
shaline := make([]byte, len+1)
for {

View file

@ -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 {

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
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
@ -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
}

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 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)

View file

@ -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"},
},
@ -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"},
},

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.
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)
@ -101,7 +105,7 @@ func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error {
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')

View file

@ -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)
}
@ -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"],

View file

@ -194,7 +194,6 @@ func TestRepository_GetAnnotatedTag(t *testing.T) {
}
func TestRepository_parseTagRef(t *testing.T) {
sha1 := Sha1ObjectFormat
tests := []struct {
name string
@ -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)

View file

@ -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

View file

@ -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
}
@ -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

View file

@ -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
}
@ -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
}