Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy load object format with command line and don't do it in OpenRepository #29712

Merged
merged 2 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading