From 4eb2a29910779ac6005a5d67f31067a1132c5297 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 19 Dec 2023 15:20:47 +0800 Subject: [PATCH] Improve ObjectFormat interface (#28496) The 4 functions are duplicated, especially as interface methods. I think we just need to keep `MustID` the only one and remove other 3. ``` MustID(b []byte) ObjectID MustIDFromString(s string) ObjectID NewID(b []byte) (ObjectID, error) NewIDFromString(s string) (ObjectID, error) ``` Introduced the new interfrace method `ComputeHash` which will replace the interface `HasherInterface`. Now we don't need to keep two interfaces. Reintroduced `git.NewIDFromString` and `git.MustIDFromString`. The new function will detect the hash length to decide which objectformat of it. If it's 40, then it's SHA1. If it's 64, then it's SHA256. This will be right if the commitID is a full one. So the parameter should be always a full commit id. @AdamMajer Please review. --- cmd/hook.go | 4 +- models/git/branch_test.go | 3 +- models/git/commit_status.go | 2 +- modules/git/commit_info_nogogit.go | 2 +- modules/git/commit_reader.go | 4 +- modules/git/commit_test.go | 4 +- modules/git/last_commit_cache.go | 6 +- modules/git/object_format.go | 35 ++++----- modules/git/object_id.go | 90 ++++++---------------- modules/git/parse_gogit.go | 2 +- modules/git/parse_gogit_test.go | 12 +-- modules/git/parse_nogogit.go | 2 +- modules/git/parse_nogogit_test.go | 12 +-- modules/git/pipeline/lfs_nogogit.go | 6 +- modules/git/repo.go | 2 +- modules/git/repo_blob.go | 2 +- modules/git/repo_blob_test.go | 2 +- modules/git/repo_commit.go | 4 +- modules/git/repo_commit_gogit.go | 4 +- modules/git/repo_commit_nogogit.go | 4 +- modules/git/repo_index.go | 4 +- modules/git/repo_language_stats_nogogit.go | 2 +- modules/git/repo_object.go | 4 +- modules/git/repo_ref_nogogit.go | 2 +- modules/git/repo_tag.go | 10 +-- modules/git/repo_tag_gogit.go | 2 +- modules/git/repo_tag_nogogit.go | 2 +- modules/git/repo_tag_test.go | 12 +-- modules/git/repo_tree.go | 2 +- modules/git/repo_tree_gogit.go | 2 +- modules/git/repo_tree_nogogit.go | 2 +- modules/git/tag.go | 2 +- modules/repository/commits_test.go | 7 +- modules/repository/push.go | 4 +- routers/api/v1/utils/git.go | 2 +- routers/private/hook_verification.go | 4 +- routers/web/repo/setting/lfs.go | 2 +- services/actions/commit_status.go | 2 +- services/pull/patch.go | 8 +- 39 files changed, 109 insertions(+), 168 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index 6f31c326fd06..6a3358853ddc 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -377,7 +377,7 @@ Gitea or set your environment appropriately.`, "") newCommitIDs[count] = string(fields[1]) refFullNames[count] = git.RefName(fields[2]) - commitID, _ := git.IDFromString(newCommitIDs[count]) + commitID, _ := git.NewIDFromString(newCommitIDs[count]) if refFullNames[count] == git.BranchPrefix+"master" && !commitID.IsZero() && count == total { masterPushed = true } @@ -671,7 +671,7 @@ Gitea or set your environment appropriately.`, "") if err != nil { return err } - commitID, _ := git.IDFromString(rs.OldOID) + commitID, _ := git.NewIDFromString(rs.OldOID) if !commitID.IsZero() { err = writeDataPktLine(ctx, os.Stdout, []byte("option old-oid "+rs.OldOID)) if err != nil { diff --git a/models/git/branch_test.go b/models/git/branch_test.go index 8febc80f1474..d480e2ec30c2 100644 --- a/models/git/branch_test.go +++ b/models/git/branch_test.go @@ -30,9 +30,8 @@ func TestAddDeletedBranch(t *testing.T) { secondBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: "branch2"}) assert.True(t, secondBranch.IsDeleted) - objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) commit := &git.Commit{ - ID: objectFormat.MustIDFromString(secondBranch.CommitID), + ID: git.MustIDFromString(secondBranch.CommitID), CommitMessage: secondBranch.CommitMessage, Committer: &git.Signature{ When: secondBranch.CommitTime.AsLocalTime(), diff --git a/models/git/commit_status.go b/models/git/commit_status.go index a22fd2304362..488e45de26f3 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -114,7 +114,7 @@ WHEN NOT MATCHED // GetNextCommitStatusIndex retried 3 times to generate a resource index func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { - _, err := git.IDFromString(sha) + _, err := git.NewIDFromString(sha) if err != nil { return 0, git.ErrInvalidSHA{SHA: sha} } diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go index 8cf8200c3f74..e469d2cab637 100644 --- a/modules/git/commit_info_nogogit.go +++ b/modules/git/commit_info_nogogit.go @@ -153,7 +153,7 @@ func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string, if typ != "commit" { return nil, fmt.Errorf("unexpected type: %s for commit id: %s", typ, commitID) } - c, err = CommitFromReader(commit.repo, commit.ID.Type().MustIDFromString(commitID), io.LimitReader(batchReader, size)) + c, err = CommitFromReader(commit.repo, MustIDFromString(commitID), io.LimitReader(batchReader, size)) if err != nil { return nil, err } diff --git a/modules/git/commit_reader.go b/modules/git/commit_reader.go index 08a529a13219..d74bcffed8a6 100644 --- a/modules/git/commit_reader.go +++ b/modules/git/commit_reader.go @@ -71,10 +71,10 @@ readLoop: switch string(split[0]) { case "tree": - commit.Tree = *NewTree(gitRepo, objectID.Type().MustIDFromString(string(data))) + commit.Tree = *NewTree(gitRepo, MustIDFromString(string(data))) _, _ = payloadSB.Write(line) case "parent": - commit.Parents = append(commit.Parents, objectID.Type().MustIDFromString(string(data))) + commit.Parents = append(commit.Parents, MustIDFromString(string(data))) _, _ = payloadSB.Write(line) case "author": commit.Author = &Signature{} diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index dec67f662860..e512eecc563b 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -135,8 +135,8 @@ func TestHasPreviousCommit(t *testing.T) { commit, err := repo.GetCommit("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0") assert.NoError(t, err) - parentSHA := repo.objectFormat.MustIDFromString("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2") - notParentSHA := repo.objectFormat.MustIDFromString("2839944139e0de9737a044f78b0e4b40d989a9e3") + parentSHA := MustIDFromString("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2") + notParentSHA := MustIDFromString("2839944139e0de9737a044f78b0e4b40d989a9e3") haz, err := commit.HasPreviousCommit(parentSHA) assert.NoError(t, err) diff --git a/modules/git/last_commit_cache.go b/modules/git/last_commit_cache.go index 55585ac4ac9e..44a4f4372861 100644 --- a/modules/git/last_commit_cache.go +++ b/modules/git/last_commit_cache.go @@ -92,11 +92,7 @@ func (c *LastCommitCache) Get(ref, entryPath string) (*Commit, error) { // GetCommitByPath gets the last commit for the entry in the provided commit func (c *LastCommitCache) GetCommitByPath(commitID, entryPath string) (*Commit, error) { - objectFormat, err := c.repo.GetObjectFormat() - if err != nil { - return nil, err - } - sha, err := objectFormat.NewIDFromString(commitID) + sha, err := NewIDFromString(commitID) if err != nil { return nil, err } diff --git a/modules/git/object_format.go b/modules/git/object_format.go index ee7e659ed04d..27771e74598d 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -6,6 +6,7 @@ package git import ( "crypto/sha1" "regexp" + "strconv" ) // sha1Pattern can be used to determine if a string is an valid sha @@ -20,14 +21,12 @@ type ObjectFormat interface { EmptyTree() ObjectID // FullLength is the length of the hash's hex string FullLength() int - + // IsValid returns true if the input is a valid hash IsValid(input string) bool + // MustID creates a new ObjectID from a byte slice MustID(b []byte) ObjectID - MustIDFromString(s string) ObjectID - NewID(b []byte) (ObjectID, error) - NewIDFromString(s string) (ObjectID, error) - - NewHasher() HasherInterface + // ComputeHash compute the hash for a given ObjectType and content + ComputeHash(t ObjectType, content []byte) ObjectID } type Sha1ObjectFormatImpl struct{} @@ -59,20 +58,18 @@ func (Sha1ObjectFormatImpl) MustID(b []byte) ObjectID { return &id } -func (h Sha1ObjectFormatImpl) MustIDFromString(s string) ObjectID { - return MustIDFromString(h, s) -} +// ComputeHash compute the hash for a given ObjectType and content +func (h Sha1ObjectFormatImpl) ComputeHash(t ObjectType, content []byte) ObjectID { + hasher := sha1.New() + _, _ = hasher.Write(t.Bytes()) + _, _ = hasher.Write([]byte(" ")) + _, _ = hasher.Write([]byte(strconv.FormatInt(int64(len(content)), 10))) + _, _ = hasher.Write([]byte{0}) -func (h Sha1ObjectFormatImpl) NewID(b []byte) (ObjectID, error) { - return IDFromRaw(h, b) -} - -func (h Sha1ObjectFormatImpl) NewIDFromString(s string) (ObjectID, error) { - return genericIDFromString(h, s) -} - -func (h Sha1ObjectFormatImpl) NewHasher() HasherInterface { - return &Sha1Hasher{sha1.New()} + // HashSum generates a SHA1 for the provided hash + var sha1 Sha1Hash + copy(sha1[:], hasher.Sum(nil)) + return &sha1 } var Sha1ObjectFormat ObjectFormat = Sha1ObjectFormatImpl{} diff --git a/modules/git/object_id.go b/modules/git/object_id.go index a90683678a81..01c23ed3dae8 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -6,11 +6,7 @@ package git import ( "bytes" "encoding/hex" - "errors" "fmt" - "hash" - "strconv" - "strings" ) type ObjectID interface { @@ -35,48 +31,36 @@ func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat } var _ ObjectID = &Sha1Hash{} -// EmptyObjectID creates a new ObjectID from an object format hash name -func EmptyObjectID(objectFormatName string) (ObjectID, error) { - objectFormat := ObjectFormatFromName(objectFormatName) - if objectFormat != nil { - return objectFormat.EmptyObjectID(), nil - } - - return nil, errors.New("unsupported hash type") -} - -func IDFromRaw(h ObjectFormat, b []byte) (ObjectID, error) { - if len(b) != h.FullLength()/2 { - return h.EmptyObjectID(), fmt.Errorf("length must be %d: %v", h.FullLength(), b) - } - return h.MustID(b), nil -} - -func MustIDFromString(h ObjectFormat, s string) ObjectID { - b, _ := hex.DecodeString(s) - return h.MustID(b) -} - -func genericIDFromString(h ObjectFormat, s string) (ObjectID, error) { - s = strings.TrimSpace(s) - if len(s) != h.FullLength() { - return h.EmptyObjectID(), fmt.Errorf("length must be %d: %s", h.FullLength(), s) - } - b, err := hex.DecodeString(s) +func MustIDFromString(hexHash string) ObjectID { + id, err := NewIDFromString(hexHash) if err != nil { - return h.EmptyObjectID(), err + panic(err) } - return h.NewID(b) + return id } -func IDFromString(hexHash string) (ObjectID, error) { +func NewIDFromString(hexHash string) (ObjectID, error) { + var theObjectFormat ObjectFormat for _, objectFormat := range SupportedObjectFormats { if len(hexHash) == objectFormat.FullLength() { - return objectFormat.NewIDFromString(hexHash) + theObjectFormat = objectFormat + break } } - return nil, fmt.Errorf("invalid hash hex string: '%s' len: %d", hexHash, len(hexHash)) + if theObjectFormat == nil { + return nil, fmt.Errorf("length %d has no matched object format: %s", len(hexHash), hexHash) + } + + b, err := hex.DecodeString(hexHash) + if err != nil { + return nil, err + } + + if len(b) != theObjectFormat.FullLength()/2 { + return theObjectFormat.EmptyObjectID(), fmt.Errorf("length must be %d: %v", theObjectFormat.FullLength(), b) + } + return theObjectFormat.MustID(b), nil } func IsEmptyCommitID(commitID string) bool { @@ -84,7 +68,7 @@ func IsEmptyCommitID(commitID string) bool { return true } - id, err := IDFromString(commitID) + id, err := NewIDFromString(commitID) if err != nil { return false } @@ -92,37 +76,9 @@ func IsEmptyCommitID(commitID string) bool { return id.IsZero() } -// HasherInterface is a struct that will generate a Hash -type HasherInterface interface { - hash.Hash - - HashSum() ObjectID -} - -type Sha1Hasher struct { - hash.Hash -} - // ComputeBlobHash compute the hash for a given blob content func ComputeBlobHash(hashType ObjectFormat, content []byte) ObjectID { - return ComputeHash(hashType, ObjectBlob, content) -} - -// ComputeHash compute the hash for a given ObjectType and content -func ComputeHash(hashType ObjectFormat, t ObjectType, content []byte) ObjectID { - h := hashType.NewHasher() - _, _ = h.Write(t.Bytes()) - _, _ = h.Write([]byte(" ")) - _, _ = h.Write([]byte(strconv.FormatInt(int64(len(content)), 10))) - _, _ = h.Write([]byte{0}) - return h.HashSum() -} - -// HashSum generates a SHA1 for the provided hash -func (h *Sha1Hasher) HashSum() ObjectID { - var sha1 Sha1Hash - copy(sha1[:], h.Hash.Sum(nil)) - return &sha1 + return hashType.ComputeHash(ObjectBlob, content) } type ErrInvalidSHA struct { diff --git a/modules/git/parse_gogit.go b/modules/git/parse_gogit.go index 6c22ea8da7bf..d1fdd346e415 100644 --- a/modules/git/parse_gogit.go +++ b/modules/git/parse_gogit.go @@ -57,7 +57,7 @@ func parseTreeEntries(data []byte, ptree *Tree) ([]*TreeEntry, error) { return nil, fmt.Errorf("Invalid ls-tree output: %s", string(data)) } var err error - entry.ID, err = IDFromString(string(data[pos : pos+hash.Size*2])) + entry.ID, err = NewIDFromString(string(data[pos : pos+hash.Size*2])) if err != nil { return nil, fmt.Errorf("invalid ls-tree output: %w", err) } diff --git a/modules/git/parse_gogit_test.go b/modules/git/parse_gogit_test.go index 9755f81cce61..d9e5b4441fff 100644 --- a/modules/git/parse_gogit_test.go +++ b/modules/git/parse_gogit_test.go @@ -28,9 +28,9 @@ func TestParseTreeEntries(t *testing.T) { Input: "100644 blob 61ab7345a1a3bbc590068ccae37b8515cfc5843c 1022\texample/file2.txt\n", Expected: []*TreeEntry{ { - ID: Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), + ID: MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), + Hash: plumbing.Hash(MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), Name: "example/file2.txt", Mode: filemode.Regular, }, @@ -44,9 +44,9 @@ func TestParseTreeEntries(t *testing.T) { "040000 tree 1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8 -\texample\n", Expected: []*TreeEntry{ { - ID: Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), + ID: MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), + Hash: plumbing.Hash(MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), Name: "example/\n.txt", Mode: filemode.Symlink, }, @@ -54,10 +54,10 @@ func TestParseTreeEntries(t *testing.T) { sized: true, }, { - ID: Sha1ObjectFormat.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"), + ID: MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"), sized: true, gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()), + Hash: plumbing.Hash(MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()), Name: "example", Mode: filemode.Dir, }, diff --git a/modules/git/parse_nogogit.go b/modules/git/parse_nogogit.go index e35704eb34e9..225342cc5a10 100644 --- a/modules/git/parse_nogogit.go +++ b/modules/git/parse_nogogit.go @@ -72,7 +72,7 @@ func parseTreeEntries(objectFormat ObjectFormat, data []byte, ptree *Tree) ([]*T return nil, fmt.Errorf("unknown type: %v", string(entryMode)) } - entry.ID, err = objectFormat.NewIDFromString(string(entryObjectID)) + entry.ID, err = NewIDFromString(string(entryObjectID)) if err != nil { return nil, fmt.Errorf("invalid ls-tree output (invalid object id): %q, err: %w", line, err) } diff --git a/modules/git/parse_nogogit_test.go b/modules/git/parse_nogogit_test.go index 36313e00f331..f037fd7a2e75 100644 --- a/modules/git/parse_nogogit_test.go +++ b/modules/git/parse_nogogit_test.go @@ -26,28 +26,28 @@ func TestParseTreeEntriesLong(t *testing.T) { `, Expected: []*TreeEntry{ { - ID: objectFormat.MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"), + ID: MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"), name: "README.md", entryMode: EntryModeBlob, size: 8218, sized: true, }, { - ID: objectFormat.MustIDFromString("037f27dc9d353ae4fd50f0474b2194c593914e35"), + ID: MustIDFromString("037f27dc9d353ae4fd50f0474b2194c593914e35"), name: "README_ZH.md", entryMode: EntryModeBlob, size: 4681, sized: true, }, { - ID: objectFormat.MustIDFromString("9846a94f7e8350a916632929d0fda38c90dd2ca8"), + ID: MustIDFromString("9846a94f7e8350a916632929d0fda38c90dd2ca8"), name: "SECURITY.md", entryMode: EntryModeBlob, size: 429, sized: true, }, { - ID: objectFormat.MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"), + ID: MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"), name: "assets", entryMode: EntryModeTree, sized: true, @@ -78,12 +78,12 @@ func TestParseTreeEntriesShort(t *testing.T) { `, Expected: []*TreeEntry{ { - ID: objectFormat.MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"), + ID: MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"), name: "README.md", entryMode: EntryModeBlob, }, { - ID: objectFormat.MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"), + ID: MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"), name: "assets", entryMode: EntryModeTree, }, diff --git a/modules/git/pipeline/lfs_nogogit.go b/modules/git/pipeline/lfs_nogogit.go index 89cbf740ffa9..a725f4799db8 100644 --- a/modules/git/pipeline/lfs_nogogit.go +++ b/modules/git/pipeline/lfs_nogogit.go @@ -115,11 +115,7 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err continue case "commit": // Read in the commit to get its tree and in case this is one of the last used commits - objectFormat, err := repo.GetObjectFormat() - if err != nil { - return nil, err - } - curCommit, err = git.CommitFromReader(repo, objectFormat.MustIDFromString(string(commitID)), io.LimitReader(batchReader, size)) + curCommit, err = git.CommitFromReader(repo, git.MustIDFromString(string(commitID)), io.LimitReader(batchReader, size)) if err != nil { return nil, err } diff --git a/modules/git/repo.go b/modules/git/repo.go index 52e54715d653..7ccce0ba20e6 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -81,7 +81,7 @@ func GetObjectFormatOfRepo(ctx context.Context, repoPath string) (ObjectFormat, return nil, errors.New(stderr.String()) } - h, err := IDFromString(strings.TrimRight(stdout.String(), "\n")) + h, err := NewIDFromString(strings.TrimRight(stdout.String(), "\n")) if err != nil { return nil, err } diff --git a/modules/git/repo_blob.go b/modules/git/repo_blob.go index b5447b2bb131..698b6c7074a6 100644 --- a/modules/git/repo_blob.go +++ b/modules/git/repo_blob.go @@ -5,7 +5,7 @@ package git // GetBlob finds the blob object in the repository. func (repo *Repository) GetBlob(idStr string) (*Blob, error) { - id, err := repo.objectFormat.NewIDFromString(idStr) + id, err := NewIDFromString(idStr) if err != nil { return nil, err } diff --git a/modules/git/repo_blob_test.go b/modules/git/repo_blob_test.go index e12257395418..8a5f5fcd5b05 100644 --- a/modules/git/repo_blob_test.go +++ b/modules/git/repo_blob_test.go @@ -61,7 +61,7 @@ func TestRepository_GetBlob_NoId(t *testing.T) { defer r.Close() testCase := "" - testError := fmt.Errorf("length must be 40: %s", testCase) + testError := fmt.Errorf("length %d has no matched object format: %s", len(testCase), testCase) blob, err := r.GetBlob(testCase) assert.Nil(t, blob) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 58bbcf9303f2..ccb3eb4adef0 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -63,7 +63,7 @@ func (repo *Repository) getCommitByPathWithID(id ObjectID, relpath string) (*Com return nil, runErr } - id, err := repo.objectFormat.NewIDFromString(stdout) + id, err := NewIDFromString(stdout) if err != nil { return nil, err } @@ -254,7 +254,7 @@ func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions) } return commits, err } - objectID, err := repo.objectFormat.NewIDFromString(string(shaline[0:len])) + objectID, err := NewIDFromString(string(shaline[0:len])) if err != nil { return nil, err } diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index d0992fd385ae..4cab9575644c 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -43,7 +43,7 @@ func (repo *Repository) RemoveReference(name string) error { func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { objectFormat := repo.objectFormat if len(commitID) == hash.HexSize && objectFormat.IsValid(commitID) { - ID, err := objectFormat.NewIDFromString(commitID) + ID, err := NewIDFromString(commitID) if err == nil { return ID, nil } @@ -59,7 +59,7 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { return objectFormat.EmptyObjectID(), err } - return objectFormat.NewIDFromString(actualCommitID) + return NewIDFromString(actualCommitID) } // IsCommitExist returns true if given commit exists in current repository. diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index 018e271c394e..f0214e1ff858 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/modules/git/repo_commit_nogogit.go @@ -135,7 +135,7 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id ObjectID) func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { IDType := repo.objectFormat if len(commitID) == IDType.FullLength() && IDType.IsValid(commitID) { - ID, err := repo.objectFormat.NewIDFromString(commitID) + ID, err := NewIDFromString(commitID) if err == nil { return ID, nil } @@ -155,5 +155,5 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { return nil, err } - return repo.objectFormat.MustIDFromString(string(sha)), nil + return MustIDFromString(string(sha)), nil } diff --git a/modules/git/repo_index.go b/modules/git/repo_index.go index e3b19bf03646..47705a92af6f 100644 --- a/modules/git/repo_index.go +++ b/modules/git/repo_index.go @@ -30,7 +30,7 @@ func (repo *Repository) ReadTreeToIndex(treeish string, indexFilename ...string) treeish = res[:len(res)-1] } } - id, err := objectFormat.NewIDFromString(treeish) + id, err := NewIDFromString(treeish) if err != nil { return err } @@ -128,7 +128,7 @@ func (repo *Repository) WriteTree() (*Tree, error) { if runErr != nil { return nil, runErr } - id, err := repo.objectFormat.NewIDFromString(strings.TrimSpace(stdout)) + id, err := NewIDFromString(strings.TrimSpace(stdout)) if err != nil { return nil, err } diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index b733c119f9e7..1d94ad6c00f4 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -39,7 +39,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err return nil, ErrNotExist{commitID, ""} } - sha, err := repo.objectFormat.NewIDFromString(string(shaBytes)) + sha, err := NewIDFromString(string(shaBytes)) if err != nil { log.Debug("Unable to get commit for: %s. Err: %v", commitID, err) return nil, ErrNotExist{commitID, ""} diff --git a/modules/git/repo_object.go b/modules/git/repo_object.go index 220fdb38074d..3d48b91c6d96 100644 --- a/modules/git/repo_object.go +++ b/modules/git/repo_object.go @@ -46,7 +46,7 @@ func (repo *Repository) GetObjectFormat() (ObjectFormat, error) { if err != nil { return nil, err } - hash, err := IDFromString(str) + hash, err := NewIDFromString(str) if err != nil { return nil, err } @@ -62,7 +62,7 @@ func (repo *Repository) HashObject(reader io.Reader) (ObjectID, error) { if err != nil { return nil, err } - return repo.objectFormat.NewIDFromString(idStr) + return NewIDFromString(idStr) } func (repo *Repository) hashObject(reader io.Reader, save bool) (string, error) { diff --git a/modules/git/repo_ref_nogogit.go b/modules/git/repo_ref_nogogit.go index c1be60871cfe..ac53d661b517 100644 --- a/modules/git/repo_ref_nogogit.go +++ b/modules/git/repo_ref_nogogit.go @@ -75,7 +75,7 @@ func (repo *Repository) GetRefsFiltered(pattern string) ([]*Reference, error) { if pattern == "" || strings.HasPrefix(refName, pattern) { r := &Reference{ Name: refName, - Object: repo.objectFormat.MustIDFromString(sha), + Object: MustIDFromString(sha), Type: typ, repo: repo, } diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 0b08e457cbe9..698b9b41f3ce 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -84,7 +84,7 @@ func (repo *Repository) GetTag(name string) (*Tag, error) { return nil, err } - id, err := repo.objectFormat.NewIDFromString(idStr) + id, err := NewIDFromString(idStr) if err != nil { return nil, err } @@ -98,7 +98,7 @@ func (repo *Repository) GetTag(name string) (*Tag, error) { // GetTagWithID returns a Git tag by given name and ID func (repo *Repository) GetTagWithID(idStr, name string) (*Tag, error) { - id, err := repo.objectFormat.NewIDFromString(idStr) + id, err := NewIDFromString(idStr) if err != nil { return nil, err } @@ -165,7 +165,7 @@ func parseTagRef(objectFormat ObjectFormat, ref map[string]string) (tag *Tag, er Name: ref["refname:short"], } - tag.ID, err = objectFormat.NewIDFromString(ref["objectname"]) + tag.ID, err = NewIDFromString(ref["objectname"]) if err != nil { return nil, fmt.Errorf("parse objectname '%s': %w", ref["objectname"], err) } @@ -175,7 +175,7 @@ func parseTagRef(objectFormat ObjectFormat, ref map[string]string) (tag *Tag, er tag.Object = tag.ID } else { // annotated tag - tag.Object, err = objectFormat.NewIDFromString(ref["object"]) + tag.Object, err = NewIDFromString(ref["object"]) if err != nil { return nil, fmt.Errorf("parse object '%s': %w", ref["object"], err) } @@ -208,7 +208,7 @@ func parseTagRef(objectFormat ObjectFormat, ref map[string]string) (tag *Tag, er // GetAnnotatedTag returns a Git tag by its SHA, must be an annotated tag func (repo *Repository) GetAnnotatedTag(sha string) (*Tag, error) { - id, err := repo.objectFormat.NewIDFromString(sha) + id, err := NewIDFromString(sha) if err != nil { return nil, err } diff --git a/modules/git/repo_tag_gogit.go b/modules/git/repo_tag_gogit.go index c3711ba5a07d..4a7a06e9bdaf 100644 --- a/modules/git/repo_tag_gogit.go +++ b/modules/git/repo_tag_gogit.go @@ -88,7 +88,7 @@ func (repo *Repository) getTag(tagID ObjectID, name string) (*Tag, error) { // every tag should have a commit ID so return all errors return nil, err } - commitID, err := IDFromString(commitIDStr) + commitID, err := NewIDFromString(commitIDStr) if err != nil { return nil, err } diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go index 3cea4894f17a..5d98fadd5436 100644 --- a/modules/git/repo_tag_nogogit.go +++ b/modules/git/repo_tag_nogogit.go @@ -64,7 +64,7 @@ func (repo *Repository) getTag(tagID ObjectID, name string) (*Tag, error) { // every tag should have a commit ID so return all errors return nil, err } - commitID, err := repo.objectFormat.NewIDFromString(commitIDStr) + commitID, err := NewIDFromString(commitIDStr) if err != nil { return nil, err } diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index 48c1bc41c237..6b9df1746f7d 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -224,8 +224,8 @@ func TestRepository_parseTagRef(t *testing.T) { want: &Tag{ Name: "v1.9.1", - ID: sha1.MustIDFromString("ab23e4b7f4cd0caafe0174c0e7ef6d651ba72889"), - Object: sha1.MustIDFromString("ab23e4b7f4cd0caafe0174c0e7ef6d651ba72889"), + ID: MustIDFromString("ab23e4b7f4cd0caafe0174c0e7ef6d651ba72889"), + Object: MustIDFromString("ab23e4b7f4cd0caafe0174c0e7ef6d651ba72889"), Type: "commit", Tagger: parseAuthorLine(t, "Foo Bar 1565789218 +0300"), Message: "Add changelog of v1.9.1 (#7859)\n\n* add changelog of v1.9.1\n* Update CHANGELOG.md\n", @@ -253,8 +253,8 @@ func TestRepository_parseTagRef(t *testing.T) { want: &Tag{ Name: "v0.0.1", - ID: sha1.MustIDFromString("8c68a1f06fc59c655b7e3905b159d761e91c53c9"), - Object: sha1.MustIDFromString("3325fd8a973321fd59455492976c042dde3fd1ca"), + ID: MustIDFromString("8c68a1f06fc59c655b7e3905b159d761e91c53c9"), + Object: MustIDFromString("3325fd8a973321fd59455492976c042dde3fd1ca"), Type: "tag", Tagger: parseAuthorLine(t, "Foo Bar 1565789218 +0300"), Message: "Add changelog of v1.9.1 (#7859)\n\n* add changelog of v1.9.1\n* Update CHANGELOG.md\n", @@ -311,8 +311,8 @@ qbHDASXl want: &Tag{ Name: "v0.0.1", - ID: sha1.MustIDFromString("8c68a1f06fc59c655b7e3905b159d761e91c53c9"), - Object: sha1.MustIDFromString("3325fd8a973321fd59455492976c042dde3fd1ca"), + ID: MustIDFromString("8c68a1f06fc59c655b7e3905b159d761e91c53c9"), + Object: MustIDFromString("3325fd8a973321fd59455492976c042dde3fd1ca"), Type: "tag", Tagger: parseAuthorLine(t, "Foo Bar 1565789218 +0300"), Message: "Add changelog of v1.9.1 (#7859)\n\n* add changelog of v1.9.1\n* Update CHANGELOG.md", diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 9ee80351f0f6..ab48d47d13c0 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -63,5 +63,5 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt if err != nil { return nil, ConcatenateError(err, stderr.String()) } - return repo.objectFormat.NewIDFromString(strings.TrimSpace(stdout.String())) + return NewIDFromString(strings.TrimSpace(stdout.String())) } diff --git a/modules/git/repo_tree_gogit.go b/modules/git/repo_tree_gogit.go index 415572e65a13..6391959e6acd 100644 --- a/modules/git/repo_tree_gogit.go +++ b/modules/git/repo_tree_gogit.go @@ -30,7 +30,7 @@ func (repo *Repository) GetTree(idStr string) (*Tree, error) { idStr = res[:len(res)-1] } } - id, err := repo.objectFormat.NewIDFromString(idStr) + id, err := NewIDFromString(idStr) if err != nil { return nil, err } diff --git a/modules/git/repo_tree_nogogit.go b/modules/git/repo_tree_nogogit.go index f502cc140b7f..20c92a79ed9b 100644 --- a/modules/git/repo_tree_nogogit.go +++ b/modules/git/repo_tree_nogogit.go @@ -75,7 +75,7 @@ func (repo *Repository) GetTree(idStr string) (*Tree, error) { idStr = res } } - id, err := repo.objectFormat.NewIDFromString(idStr) + id, err := NewIDFromString(idStr) if err != nil { return nil, err } diff --git a/modules/git/tag.go b/modules/git/tag.go index c7d0d8aef92b..01a8d6f6a552 100644 --- a/modules/git/tag.go +++ b/modules/git/tag.go @@ -50,7 +50,7 @@ l: reftype := line[:spacepos] switch string(reftype) { case "object": - id, err := objectFormat.NewIDFromString(string(line[spacepos+1:])) + id, err := NewIDFromString(string(line[spacepos+1:])) if err != nil { return nil, err } diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index afcb183d7237..827b2a98493b 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -144,7 +144,7 @@ func TestCommitToPushCommit(t *testing.T) { When: now, } const hexString = "0123456789abcdef0123456789abcdef01234567" - sha1, err := git.IDFromString(hexString) + sha1, err := git.NewIDFromString(hexString) assert.NoError(t, err) pushCommit := CommitToPushCommit(&git.Commit{ ID: sha1, @@ -169,12 +169,11 @@ func TestListToPushCommits(t *testing.T) { When: now, } - hashType := git.Sha1ObjectFormat const hexString1 = "0123456789abcdef0123456789abcdef01234567" - hash1, err := hashType.NewIDFromString(hexString1) + hash1, err := git.NewIDFromString(hexString1) assert.NoError(t, err) const hexString2 = "fedcba9876543210fedcba9876543210fedcba98" - hash2, err := hashType.NewIDFromString(hexString2) + hash2, err := git.NewIDFromString(hexString2) assert.NoError(t, err) l := []*git.Commit{ diff --git a/modules/repository/push.go b/modules/repository/push.go index 25695336a52d..cf047847b6ca 100644 --- a/modules/repository/push.go +++ b/modules/repository/push.go @@ -20,13 +20,13 @@ type PushUpdateOptions struct { // IsNewRef return true if it's a first-time push to a branch, tag or etc. func (opts *PushUpdateOptions) IsNewRef() bool { - commitID, err := git.IDFromString(opts.OldCommitID) + commitID, err := git.NewIDFromString(opts.OldCommitID) return err == nil && commitID.IsZero() } // IsDelRef return true if it's a deletion to a branch or tag func (opts *PushUpdateOptions) IsDelRef() bool { - commitID, err := git.IDFromString(opts.NewCommitID) + commitID, err := git.NewIDFromString(opts.NewCommitID) return err == nil && commitID.IsZero() } diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index dfb1a130c371..39714e343f01 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -73,7 +73,7 @@ func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (str func ConvertToObjectID(ctx gocontext.Context, repo *context.Repository, commitID string) (git.ObjectID, error) { objectFormat, _ := repo.GitRepo.GetObjectFormat() if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) { - sha, err := objectFormat.NewIDFromString(commitID) + sha, err := git.NewIDFromString(commitID) if err == nil { return sha, nil } diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 8b2d0dd848a9..42b8e5abed81 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -83,8 +83,8 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { _ = stdoutReader.Close() _ = stdoutWriter.Close() }() - objectFormat, _ := repo.GetObjectFormat() - commitID := objectFormat.MustIDFromString(sha) + + commitID := git.MustIDFromString(sha) return git.NewCommand(repo.Ctx, "cat-file", "commit").AddDynamicArguments(sha). Run(&git.RunOpts{ diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index 230f1a2a608a..edf1298c2073 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -395,7 +395,7 @@ func LFSFileFind(ctx *context.Context) { objectID = git.ComputeBlobHash(objectFormat, []byte(pointer.StringContent())) sha = objectID.String() } else { - objectID = objectFormat.MustIDFromString(sha) + objectID = git.MustIDFromString(sha) } ctx.Data["LFSFilesLink"] = ctx.Repo.RepoLink + "/settings/lfs" ctx.Data["Oid"] = oid diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index c1fc1eda92aa..72a3ab7ac60e 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -115,7 +115,7 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } creator := user_model.NewActionsUser() - commitID, err := git.IDFromString(sha) + commitID, err := git.NewIDFromString(sha) if err != nil { return fmt.Errorf("HashTypeInterfaceFromHashString: %w", err) } diff --git a/services/pull/patch.go b/services/pull/patch.go index 1dbbec373d16..acaff04bda47 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -130,8 +130,6 @@ func (e *errMergeConflict) Error() string { func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, gitRepo *git.Repository) error { log.Trace("Attempt to merge:\n%v", file) - objectFormat, _ := gitRepo.GetObjectFormat() - switch { case file.stage1 != nil && (file.stage2 == nil || file.stage3 == nil): // 1. Deleted in one or both: @@ -148,7 +146,7 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g // 2. Added in ours but not in theirs or identical in both // // Not a genuine conflict just add to the index - if err := gitRepo.AddObjectToIndex(file.stage2.mode, objectFormat.MustIDFromString(file.stage2.sha), file.stage2.path); err != nil { + if err := gitRepo.AddObjectToIndex(file.stage2.mode, git.MustIDFromString(file.stage2.sha), file.stage2.path); err != nil { return err } return nil @@ -161,7 +159,7 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g // 4. Added in theirs but not ours: // // Not a genuine conflict just add to the index - return gitRepo.AddObjectToIndex(file.stage3.mode, objectFormat.MustIDFromString(file.stage3.sha), file.stage3.path) + return gitRepo.AddObjectToIndex(file.stage3.mode, git.MustIDFromString(file.stage3.sha), file.stage3.path) case file.stage1 == nil: // 5. Created by new in both // @@ -222,7 +220,7 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g return err } hash = strings.TrimSpace(hash) - return gitRepo.AddObjectToIndex(file.stage2.mode, objectFormat.MustIDFromString(hash), file.stage2.path) + return gitRepo.AddObjectToIndex(file.stage2.mode, git.MustIDFromString(hash), file.stage2.path) default: if file.stage1 != nil { return &errMergeConflict{file.stage1.path}