From 605326f9f6d7aca4891870083da78ae155867611 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Dec 2023 17:15:34 +0800 Subject: [PATCH 01/12] Adjust object format interface --- models/repo/repo.go | 2 +- modules/git/blame_test.go | 2 +- modules/git/object_format.go | 51 ++++--------------- modules/git/object_id.go | 2 +- modules/git/object_id_gogit.go | 2 +- modules/git/parse_gogit_test.go | 14 ++--- modules/git/parse_nogogit_test.go | 6 +-- modules/git/ref.go | 2 +- modules/git/repo_tag_test.go | 2 +- modules/git/tag_test.go | 2 +- modules/repository/commits_test.go | 2 +- modules/repository/generate.go | 4 +- routers/api/v1/repo/repo.go | 2 +- routers/web/repo/githttp.go | 2 +- services/convert/git_commit_test.go | 2 +- services/migrations/common.go | 2 +- services/repository/create.go | 2 +- services/wiki/wiki.go | 2 +- services/wiki/wiki_test.go | 2 +- .../git_helper_for_declarative_test.go | 2 +- 20 files changed, 39 insertions(+), 68 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 59f68df996a46..af49bed65f0fe 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -277,7 +277,7 @@ func (repo *Repository) AfterLoad() { repo.NumOpenProjects = repo.NumProjects - repo.NumClosedProjects repo.NumOpenActionRuns = repo.NumActionRuns - repo.NumClosedActionRuns - repo.ObjectFormat = git.ObjectFormatFromID(git.Sha1) + repo.ObjectFormat = git.Sha1ObjectFormat{} } // LoadAttributes loads attributes of the repository. diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index 0afc6d2a1f0f7..c163f3be5d651 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -39,7 +39,7 @@ func TestReadingBlameOutput(t *testing.T) { } for _, bypass := range []bool{false, true} { - blameReader, err := CreateBlameReader(ctx, &Sha1ObjectFormat{}, "./tests/repos/repo5_pulls", commit, "README.md", bypass) + blameReader, err := CreateBlameReader(ctx, Sha1ObjectFormat{}, "./tests/repos/repo5_pulls", commit, "README.md", bypass) assert.NoError(t, err) assert.NotNil(t, blameReader) defer blameReader.Close() diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 7f5d09170c6fb..e392141223eb1 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -5,22 +5,13 @@ package git import ( "crypto/sha1" - "fmt" "regexp" - "strings" -) - -type ObjectFormatID int - -const ( - Sha1 ObjectFormatID = iota ) // sha1Pattern can be used to determine if a string is an valid sha var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) type ObjectFormat interface { - ID() ObjectFormatID String() string // Empty is the hash of empty git @@ -43,61 +34,41 @@ type ObjectFormat interface { /* SHA1 Type */ type Sha1ObjectFormat struct{} -func (*Sha1ObjectFormat) ID() ObjectFormatID { return Sha1 } -func (*Sha1ObjectFormat) String() string { return "sha1" } -func (*Sha1ObjectFormat) Empty() ObjectID { return &Sha1Hash{} } -func (*Sha1ObjectFormat) EmptyTree() ObjectID { +func (Sha1ObjectFormat) String() string { return "sha1" } +func (Sha1ObjectFormat) Empty() ObjectID { return &Sha1Hash{} } +func (Sha1ObjectFormat) EmptyTree() ObjectID { return &Sha1Hash{ 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04, } } -func (*Sha1ObjectFormat) FullLength() int { return 40 } -func (*Sha1ObjectFormat) IsValid(input string) bool { +func (Sha1ObjectFormat) FullLength() int { return 40 } +func (Sha1ObjectFormat) IsValid(input string) bool { return sha1Pattern.MatchString(input) } -func (*Sha1ObjectFormat) MustID(b []byte) ObjectID { +func (Sha1ObjectFormat) MustID(b []byte) ObjectID { var id Sha1Hash copy(id[0:20], b) return &id } -func (h *Sha1ObjectFormat) MustIDFromString(s string) ObjectID { +func (h Sha1ObjectFormat) MustIDFromString(s string) ObjectID { return MustIDFromString(h, s) } -func (h *Sha1ObjectFormat) NewID(b []byte) (ObjectID, error) { +func (h Sha1ObjectFormat) NewID(b []byte) (ObjectID, error) { return IDFromRaw(h, b) } -func (h *Sha1ObjectFormat) NewIDFromString(s string) (ObjectID, error) { +func (h Sha1ObjectFormat) NewIDFromString(s string) (ObjectID, error) { return genericIDFromString(h, s) } -func (*Sha1ObjectFormat) NewEmptyID() ObjectID { +func (Sha1ObjectFormat) NewEmptyID() ObjectID { return NewSha1() } -func (h *Sha1ObjectFormat) NewHasher() HasherInterface { +func (h Sha1ObjectFormat) NewHasher() HasherInterface { return &Sha1Hasher{sha1.New()} } - -// utils -func ObjectFormatFromID(id ObjectFormatID) ObjectFormat { - switch id { - case Sha1: - return &Sha1ObjectFormat{} - } - - return nil -} - -func ObjectFormatFromString(hash string) (ObjectFormat, error) { - switch strings.ToLower(hash) { - case "sha1": - return &Sha1ObjectFormat{}, nil - } - - return nil, fmt.Errorf("unknown hash type: %s", hash) -} diff --git a/modules/git/object_id.go b/modules/git/object_id.go index 3cba6d4f724db..a72c164a5ea3f 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -32,7 +32,7 @@ func (h *Sha1Hash) IsZero() bool { return bytes.Equal(empty[:], h[:]) } func (h *Sha1Hash) RawValue() []byte { return h[:] } -func (*Sha1Hash) Type() ObjectFormat { return &Sha1ObjectFormat{} } +func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat{} } func NewSha1() *Sha1Hash { return &Sha1Hash{} diff --git a/modules/git/object_id_gogit.go b/modules/git/object_id_gogit.go index 50917f0552d54..8e4d6ea8c2608 100644 --- a/modules/git/object_id_gogit.go +++ b/modules/git/object_id_gogit.go @@ -12,7 +12,7 @@ import ( func ParseGogitHash(h plumbing.Hash) ObjectID { switch hash.Size { case 20: - return ObjectFormatFromID(Sha1).MustID(h[:]) + return Sha1ObjectFormat{}.MustID(h[:]) } return nil diff --git a/modules/git/parse_gogit_test.go b/modules/git/parse_gogit_test.go index 7ba50cbff90d4..100c55c06b37a 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: ObjectFormatFromID(Sha1).MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), + ID: Sha1ObjectFormat{}.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(ObjectFormatFromID(Sha1).MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), + Hash: plumbing.Hash(Sha1ObjectFormat{}.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: ObjectFormatFromID(Sha1).MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), + ID: Sha1ObjectFormat{}.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(ObjectFormatFromID(Sha1).MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), + Hash: plumbing.Hash(Sha1ObjectFormat{}.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), Name: "example/\n.txt", Mode: filemode.Symlink, }, @@ -54,10 +54,10 @@ func TestParseTreeEntries(t *testing.T) { sized: true, }, { - ID: ObjectFormatFromID(Sha1).MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"), + ID: Sha1ObjectFormat{}.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"), sized: true, gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(ObjectFormatFromID(Sha1).MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()), + Hash: plumbing.Hash(Sha1ObjectFormat{}.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()), Name: "example", Mode: filemode.Dir, }, @@ -67,7 +67,7 @@ func TestParseTreeEntries(t *testing.T) { } for _, testCase := range testCases { - entries, err := ParseTreeEntries(ObjectFormatFromID(Sha1), []byte(testCase.Input)) + entries, err := ParseTreeEntries(Sha1ObjectFormat{}, []byte(testCase.Input)) assert.NoError(t, err) if len(entries) > 1 { fmt.Println(testCase.Expected[0].ID) diff --git a/modules/git/parse_nogogit_test.go b/modules/git/parse_nogogit_test.go index 0b78c081cd6f7..a06d89f982584 100644 --- a/modules/git/parse_nogogit_test.go +++ b/modules/git/parse_nogogit_test.go @@ -12,7 +12,7 @@ import ( ) func TestParseTreeEntriesLong(t *testing.T) { - objectFormat := ObjectFormatFromID(Sha1) + objectFormat := Sha1ObjectFormat{} testCases := []struct { Input string @@ -66,7 +66,7 @@ func TestParseTreeEntriesLong(t *testing.T) { } func TestParseTreeEntriesShort(t *testing.T) { - objectFormat := ObjectFormatFromID(Sha1) + objectFormat := Sha1ObjectFormat{} testCases := []struct { Input string @@ -102,7 +102,7 @@ func TestParseTreeEntriesShort(t *testing.T) { func TestParseTreeEntriesInvalid(t *testing.T) { // there was a panic: "runtime error: slice bounds out of range" when the input was invalid: #20315 - entries, err := ParseTreeEntries(ObjectFormatFromID(Sha1), []byte("100644 blob ea0d83c9081af9500ac9f804101b3fd0a5c293af")) + entries, err := ParseTreeEntries(Sha1ObjectFormat{}, []byte("100644 blob ea0d83c9081af9500ac9f804101b3fd0a5c293af")) assert.Error(t, err) assert.Len(t, entries, 0) } diff --git a/modules/git/ref.go b/modules/git/ref.go index b96b4ababb412..fce2a201d1a59 100644 --- a/modules/git/ref.go +++ b/modules/git/ref.go @@ -205,7 +205,7 @@ func RefURL(repoURL, ref string) string { return repoURL + "/src/branch/" + refName case refFullName.IsTag(): return repoURL + "/src/tag/" + refName - case !ObjectFormatFromID(Sha1).IsValid(ref): + case !Sha1ObjectFormat{}.IsValid(ref): // assume they mean a branch return repoURL + "/src/branch/" + refName default: diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index c7699f4a7d2c6..98676a5fc8feb 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -194,7 +194,7 @@ func TestRepository_GetAnnotatedTag(t *testing.T) { } func TestRepository_parseTagRef(t *testing.T) { - sha1 := ObjectFormatFromID(Sha1) + sha1 := Sha1ObjectFormat{} tests := []struct { name string diff --git a/modules/git/tag_test.go b/modules/git/tag_test.go index 129c1e3a02303..3350262d6d320 100644 --- a/modules/git/tag_test.go +++ b/modules/git/tag_test.go @@ -49,7 +49,7 @@ ono`), tag: Tag{ } for _, test := range testData { - tag, err := parseTagData(ObjectFormatFromID(Sha1), test.data) + tag, err := parseTagData(Sha1ObjectFormat{}, test.data) assert.NoError(t, err) assert.EqualValues(t, test.tag.ID, tag.ID) assert.EqualValues(t, test.tag.Object, tag.Object) diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 57f0c90fc6c6e..69132f7eb2368 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -169,7 +169,7 @@ func TestListToPushCommits(t *testing.T) { When: now, } - hashType := git.ObjectFormatFromID(git.Sha1) + hashType := git.Sha1ObjectFormat{} const hexString1 = "0123456789abcdef0123456789abcdef01234567" hash1, err := hashType.NewIDFromString(hexString1) assert.NoError(t, err) diff --git a/modules/repository/generate.go b/modules/repository/generate.go index c143431b7c6a8..ed11d53fff953 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -224,7 +224,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r } // FIXME: fix the hash - if err := git.InitRepository(ctx, tmpDir, false, git.ObjectFormatFromID(git.Sha1)); err != nil { + if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat{}); err != nil { return err } @@ -358,7 +358,7 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ } // FIXME - fix the hash - if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.ObjectFormatFromID(git.Sha1)); err != nil { + if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat{}); err != nil { return generateRepo, err } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 1767a7fa674d5..21e0552901c0c 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -253,7 +253,7 @@ func CreateUserRepo(ctx *context.APIContext, owner *user_model.User, opt api.Cre DefaultBranch: opt.DefaultBranch, TrustModel: repo_model.ToTrustModel(opt.TrustModel), IsTemplate: opt.Template, - ObjectFormat: git.ObjectFormatFromID(git.Sha1), + ObjectFormat: git.Sha1ObjectFormat{}, }) if err != nil { if repo_model.IsErrRepoAlreadyExist(err) { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index dd47bd79d94e1..b3e54cd7fdac1 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -329,7 +329,7 @@ func dummyInfoRefs(ctx *context.Context) { } }() - if err := git.InitRepository(ctx, tmpDir, true, git.ObjectFormatFromID(git.Sha1)); err != nil { + if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormat{}); err != nil { log.Error("Failed to init bare repo for git-receive-pack cache: %v", err) return } diff --git a/services/convert/git_commit_test.go b/services/convert/git_commit_test.go index d8c1fdfed78a7..6e37ce4eaeba7 100644 --- a/services/convert/git_commit_test.go +++ b/services/convert/git_commit_test.go @@ -19,7 +19,7 @@ import ( func TestToCommitMeta(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) headRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - sha1 := git.ObjectFormatFromID(git.Sha1) + sha1 := git.Sha1ObjectFormat{} signature := &git.Signature{Name: "Test Signature", Email: "test@email.com", When: time.Unix(0, 0)} tag := &git.Tag{ Name: "Test Tag", diff --git a/services/migrations/common.go b/services/migrations/common.go index 278c156b036cb..488d96c00821d 100644 --- a/services/migrations/common.go +++ b/services/migrations/common.go @@ -49,7 +49,7 @@ func CheckAndEnsureSafePR(pr *base.PullRequest, commonCloneBaseURL string, g bas // SECURITY: SHAs Must be a SHA // FIXME: hash only a SHA1 - CommitType := git.ObjectFormatFromID(git.Sha1) + CommitType := git.Sha1ObjectFormat{} if pr.MergeCommitSHA != "" && !CommitType.IsValid(pr.MergeCommitSHA) { WarnAndNotice("PR #%d in %s has invalid MergeCommitSHA: %s", pr.Number, g, pr.MergeCommitSHA) pr.MergeCommitSHA = "" diff --git a/services/repository/create.go b/services/repository/create.go index a41904eb7cdf0..9af85b9f2cdf7 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -211,7 +211,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt } if opts.ObjectFormat == nil { - opts.ObjectFormat = git.ObjectFormatFromID(git.Sha1) + opts.ObjectFormat = git.Sha1ObjectFormat{} } // Check if label template exist diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index ecda926ec1e2d..fd2bc8b1dd027 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -36,7 +36,7 @@ func InitWiki(ctx context.Context, repo *repo_model.Repository) error { return nil } - if err := git.InitRepository(ctx, repo.WikiPath(), true, git.ObjectFormatFromID(git.Sha1)); err != nil { + if err := git.InitRepository(ctx, repo.WikiPath(), true, git.Sha1ObjectFormat{}); err != nil { return fmt.Errorf("InitRepository: %w", err) } else if err = repo_module.CreateDelegateHooks(repo.WikiPath()); err != nil { return fmt.Errorf("createDelegateHooks: %w", err) diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go index 9981fb42583f3..de79910c888d3 100644 --- a/services/wiki/wiki_test.go +++ b/services/wiki/wiki_test.go @@ -302,7 +302,7 @@ func TestPrepareWikiFileName_FirstPage(t *testing.T) { // Now create a temporaryDirectory tmpDir := t.TempDir() - err := git.InitRepository(git.DefaultContext, tmpDir, true, git.ObjectFormatFromID(git.Sha1)) + err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormat{}) assert.NoError(t, err) gitRepo, err := git.OpenRepository(git.DefaultContext, tmpDir) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index de671dec19e98..bea948691c29e 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -120,7 +120,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) { func doGitInitTestRepository(dstPath string) func(*testing.T) { return func(t *testing.T) { // Init repository in dstPath - assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.ObjectFormatFromID(git.Sha1))) + assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormat{})) // forcibly set default branch to master _, _, err := git.NewCommand(git.DefaultContext, "symbolic-ref", "HEAD", git.BranchPrefix+"master").RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) From 619483aeaa2330b2d64eec851feee20fbb69643f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Dec 2023 18:05:05 +0800 Subject: [PATCH 02/12] Use ObjectFormatName in repository struct --- models/git/branch_test.go | 3 +- models/repo/repo.go | 4 +- modules/git/object_format.go | 13 +++++- modules/git/repo.go | 7 +++- modules/repository/generate.go | 4 +- modules/repository/init.go | 4 +- routers/api/v1/repo/repo.go | 24 +++++------ routers/web/repo/githttp.go | 2 +- routers/web/repo/repo.go | 24 +++++------ services/forms/repo_form.go | 3 +- services/migrations/gitea_uploader_test.go | 2 +- services/packages/cargo/index.go | 2 +- services/pull/temp_repo.go | 9 +--- services/repository/check.go | 2 +- services/repository/create.go | 42 +++++++++---------- services/repository/files/cherry_pick.go | 3 +- services/repository/files/temp_repo.go | 4 +- services/repository/files/update.go | 3 +- services/repository/files/upload.go | 2 +- services/wiki/wiki.go | 2 +- services/wiki/wiki_test.go | 2 +- .../git_helper_for_declarative_test.go | 2 +- 22 files changed, 84 insertions(+), 79 deletions(-) diff --git a/models/git/branch_test.go b/models/git/branch_test.go index adcf9fd305ae8..153a1aa977971 100644 --- a/models/git/branch_test.go +++ b/models/git/branch_test.go @@ -29,8 +29,9 @@ 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: repo.ObjectFormat.MustIDFromString(secondBranch.CommitID), + ID: objectFormat.MustIDFromString(secondBranch.CommitID), CommitMessage: secondBranch.CommitMessage, Committer: &git.Signature{ When: secondBranch.CommitTime.AsLocalTime(), diff --git a/models/repo/repo.go b/models/repo/repo.go index af49bed65f0fe..50e4af779b22f 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -180,7 +180,7 @@ type Repository struct { IsFsckEnabled bool `xorm:"NOT NULL DEFAULT true"` CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"` Topics []string `xorm:"TEXT JSON"` - ObjectFormat git.ObjectFormat `xorm:"-"` + ObjectFormatName string `xorm:"-"` TrustModel TrustModelType @@ -277,7 +277,7 @@ func (repo *Repository) AfterLoad() { repo.NumOpenProjects = repo.NumProjects - repo.NumClosedProjects repo.NumOpenActionRuns = repo.NumActionRuns - repo.NumClosedActionRuns - repo.ObjectFormat = git.Sha1ObjectFormat{} + repo.ObjectFormatName = "sha1" } // LoadAttributes loads attributes of the repository. diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 7b4ac196cb237..76e1effa2a293 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -11,6 +11,8 @@ import ( // sha1Pattern can be used to determine if a string is an valid sha var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) +const Sha1ObjectFormatName = "sha1" + type ObjectFormat interface { String() string @@ -33,7 +35,7 @@ type ObjectFormat interface { type Sha1ObjectFormat struct{} -func (Sha1ObjectFormat) String() string { return "sha1" } +func (Sha1ObjectFormat) String() string { return Sha1ObjectFormatName } func (Sha1ObjectFormat) Empty() ObjectID { return &Sha1Hash{} } func (Sha1ObjectFormat) EmptyTree() ObjectID { return &Sha1Hash{ @@ -71,3 +73,12 @@ func (Sha1ObjectFormat) NewEmptyID() ObjectID { func (h Sha1ObjectFormat) NewHasher() HasherInterface { return &Sha1Hasher{sha1.New()} } + +func ObjectFormatFromName(name string) ObjectFormat { + switch name { + case "sha1": + return Sha1ObjectFormat{} + default: + return nil + } +} diff --git a/modules/git/repo.go b/modules/git/repo.go index c036a217eb25c..c566e31cf024a 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -90,7 +90,7 @@ func GetObjectFormatOfRepo(ctx context.Context, repoPath string) (ObjectFormat, } // InitRepository initializes a new Git repository. -func InitRepository(ctx context.Context, repoPath string, bare bool, objectFormat ObjectFormat) error { +func InitRepository(ctx context.Context, repoPath string, bare bool, objectFormatName string) error { err := os.MkdirAll(repoPath, os.ModePerm) if err != nil { return err @@ -98,7 +98,10 @@ func InitRepository(ctx context.Context, repoPath string, bare bool, objectForma cmd := NewCommand(ctx, "init") if SupportHashSha256 { - cmd.AddOptionValues("--object-format", objectFormat.String()) + if objectFormatName == "" { + objectFormatName = "sha1" + } + cmd.AddOptionValues("--object-format", objectFormatName) } if bare { cmd.AddArguments("--bare") diff --git a/modules/repository/generate.go b/modules/repository/generate.go index ed11d53fff953..f7eedbadebf13 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -224,7 +224,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r } // FIXME: fix the hash - if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat{}); err != nil { + if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat{}.String()); err != nil { return err } @@ -358,7 +358,7 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ } // FIXME - fix the hash - if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat{}); err != nil { + if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat{}.String()); err != nil { return generateRepo, err } diff --git a/modules/repository/init.go b/modules/repository/init.go index a9b5aab16aab5..b90b234a73f80 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -188,7 +188,7 @@ func InitRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi return nil } -func CheckInitRepository(ctx context.Context, owner, name string, objectFormat git.ObjectFormat) (err error) { +func CheckInitRepository(ctx context.Context, owner, name, objectFormatName string) (err error) { // Somehow the directory could exist. repoPath := repo_model.RepoPath(owner, name) isExist, err := util.IsExist(repoPath) @@ -204,7 +204,7 @@ func CheckInitRepository(ctx context.Context, owner, name string, objectFormat g } // Init git bare new repository. - if err = git.InitRepository(ctx, repoPath, true, objectFormat); err != nil { + if err = git.InitRepository(ctx, repoPath, true, objectFormatName); err != nil { return fmt.Errorf("git.InitRepository: %w", err) } else if err = CreateDelegateHooks(repoPath); err != nil { return fmt.Errorf("createDelegateHooks: %w", err) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 21e0552901c0c..920ada2954c11 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -242,18 +242,18 @@ func CreateUserRepo(ctx *context.APIContext, owner *user_model.User, opt api.Cre } repo, err := repo_service.CreateRepository(ctx, ctx.Doer, owner, repo_service.CreateRepoOptions{ - Name: opt.Name, - Description: opt.Description, - IssueLabels: opt.IssueLabels, - Gitignores: opt.Gitignores, - License: opt.License, - Readme: opt.Readme, - IsPrivate: opt.Private, - AutoInit: opt.AutoInit, - DefaultBranch: opt.DefaultBranch, - TrustModel: repo_model.ToTrustModel(opt.TrustModel), - IsTemplate: opt.Template, - ObjectFormat: git.Sha1ObjectFormat{}, + Name: opt.Name, + Description: opt.Description, + IssueLabels: opt.IssueLabels, + Gitignores: opt.Gitignores, + License: opt.License, + Readme: opt.Readme, + IsPrivate: opt.Private, + AutoInit: opt.AutoInit, + DefaultBranch: opt.DefaultBranch, + TrustModel: repo_model.ToTrustModel(opt.TrustModel), + IsTemplate: opt.Template, + ObjectFormatName: git.Sha1ObjectFormatName, }) if err != nil { if repo_model.IsErrRepoAlreadyExist(err) { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index b3e54cd7fdac1..c3a61b2bbe55c 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -329,7 +329,7 @@ func dummyInfoRefs(ctx *context.Context) { } }() - if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormat{}); err != nil { + if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormatName); err != nil { log.Error("Failed to init bare repo for git-receive-pack cache: %v", err) return } diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 7a2976f8dcaac..b16e28383629e 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -278,18 +278,18 @@ func CreatePost(ctx *context.Context) { } } else { repo, err = repo_service.CreateRepository(ctx, ctx.Doer, ctxUser, repo_service.CreateRepoOptions{ - Name: form.RepoName, - Description: form.Description, - Gitignores: form.Gitignores, - IssueLabels: form.IssueLabels, - License: form.License, - Readme: form.Readme, - IsPrivate: form.Private || setting.Repository.ForcePrivate, - DefaultBranch: form.DefaultBranch, - AutoInit: form.AutoInit, - IsTemplate: form.Template, - TrustModel: repo_model.ToTrustModel(form.TrustModel), - ObjectFormat: form.ObjectFormat, + Name: form.RepoName, + Description: form.Description, + Gitignores: form.Gitignores, + IssueLabels: form.IssueLabels, + License: form.License, + Readme: form.Readme, + IsPrivate: form.Private || setting.Repository.ForcePrivate, + DefaultBranch: form.DefaultBranch, + AutoInit: form.AutoInit, + IsTemplate: form.Template, + TrustModel: repo_model.ToTrustModel(form.TrustModel), + ObjectFormatName: form.ObjectFormatName, }) if err == nil { log.Trace("Repository created [%d]: %s/%s", repo.ID, ctxUser.Name, repo.Name) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index f6ef97dfdc3b6..86599000a5882 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -13,7 +13,6 @@ import ( issues_model "code.gitea.io/gitea/models/issues" project_model "code.gitea.io/gitea/models/project" "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web/middleware" @@ -54,7 +53,7 @@ type CreateRepoForm struct { TrustModel string ForkSingleBranch string - ObjectFormat git.ObjectFormat + ObjectFormatName string } // Validate validates the fields diff --git a/services/migrations/gitea_uploader_test.go b/services/migrations/gitea_uploader_test.go index b6c9b814772a5..3dec3a26fc4a9 100644 --- a/services/migrations/gitea_uploader_test.go +++ b/services/migrations/gitea_uploader_test.go @@ -232,7 +232,7 @@ func TestGiteaUploadUpdateGitForPullRequest(t *testing.T) { // fromRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) baseRef := "master" - assert.NoError(t, git.InitRepository(git.DefaultContext, fromRepo.RepoPath(), false, fromRepo.ObjectFormat)) + assert.NoError(t, git.InitRepository(git.DefaultContext, fromRepo.RepoPath(), false, fromRepo.ObjectFormatName)) err := git.NewCommand(git.DefaultContext, "symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseRef).Run(&git.RunOpts{Dir: fromRepo.RepoPath()}) assert.NoError(t, err) assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte(fmt.Sprintf("# Testing Repository\n\nOriginally created in: %s", fromRepo.RepoPath())), 0o644)) diff --git a/services/packages/cargo/index.go b/services/packages/cargo/index.go index 48bd0a4d80a25..9514e35bedd9a 100644 --- a/services/packages/cargo/index.go +++ b/services/packages/cargo/index.go @@ -271,7 +271,7 @@ func alterRepositoryContent(ctx context.Context, doer *user_model.User, repo *re if !git.IsErrBranchNotExist(err) || !repo.IsEmpty { return err } - if err := t.Init(repo.ObjectFormat); err != nil { + if err := t.Init(repo.ObjectFormatName); err != nil { return err } } else { diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index fde8673a2403f..36bdbde55c7a3 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -93,14 +93,8 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) baseRepoPath := pr.BaseRepo.RepoPath() headRepoPath := pr.HeadRepo.RepoPath() - objectFormat, err := git.GetObjectFormatOfRepo(ctx, baseRepoPath) - if err != nil { - log.Error("Unable to fetch ObjectFormat of repository %s: %v", baseRepoPath, err) - cancel() - return nil, nil, err - } - if err := git.InitRepository(ctx, tmpBasePath, false, objectFormat); err != nil { + if err := git.InitRepository(ctx, tmpBasePath, false, pr.BaseRepo.ObjectFormatName); err != nil { log.Error("Unable to init tmpBasePath for %-v: %v", pr, err) cancel() return nil, nil, err @@ -174,6 +168,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) } trackingBranch := "tracking" + objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName) // Fetch head branch var headBranch string if pr.Flow == issues_model.PullRequestFlowGithub { diff --git a/services/repository/check.go b/services/repository/check.go index 23c4f79bf2045..b874ede51fdbc 100644 --- a/services/repository/check.go +++ b/services/repository/check.go @@ -192,7 +192,7 @@ func ReinitMissingRepositories(ctx context.Context) error { default: } log.Trace("Initializing %d/%d...", repo.OwnerID, repo.ID) - if err := git.InitRepository(ctx, repo.RepoPath(), true, repo.ObjectFormat); err != nil { + if err := git.InitRepository(ctx, repo.RepoPath(), true, repo.ObjectFormatName); err != nil { log.Error("Unable (re)initialize repository %d at %s. Error: %v", repo.ID, repo.RepoPath(), err) if err2 := system_model.CreateRepositoryNotice("InitRepository [%d]: %v", repo.ID, err); err2 != nil { log.Error("CreateRepositoryNotice: %v", err2) diff --git a/services/repository/create.go b/services/repository/create.go index 9af85b9f2cdf7..bcf2c85c21817 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -27,23 +27,23 @@ import ( // CreateRepoOptions contains the create repository options type CreateRepoOptions struct { - Name string - Description string - OriginalURL string - GitServiceType api.GitServiceType - Gitignores string - IssueLabels string - License string - Readme string - DefaultBranch string - IsPrivate bool - IsMirror bool - IsTemplate bool - AutoInit bool - Status repo_model.RepositoryStatus - TrustModel repo_model.TrustModelType - MirrorInterval string - ObjectFormat git.ObjectFormat + Name string + Description string + OriginalURL string + GitServiceType api.GitServiceType + Gitignores string + IssueLabels string + License string + Readme string + DefaultBranch string + IsPrivate bool + IsMirror bool + IsTemplate bool + AutoInit bool + Status repo_model.RepositoryStatus + TrustModel repo_model.TrustModelType + MirrorInterval string + ObjectFormatName string } func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, repoPath string, opts CreateRepoOptions) error { @@ -135,7 +135,7 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, // InitRepository initializes README and .gitignore if needed. func initRepository(ctx context.Context, repoPath string, u *user_model.User, repo *repo_model.Repository, opts CreateRepoOptions) (err error) { - if err = repo_module.CheckInitRepository(ctx, repo.OwnerName, repo.Name, opts.ObjectFormat); err != nil { + if err = repo_module.CheckInitRepository(ctx, repo.OwnerName, repo.Name, opts.ObjectFormatName); err != nil { return err } @@ -210,10 +210,6 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt opts.DefaultBranch = setting.Repository.DefaultBranch } - if opts.ObjectFormat == nil { - opts.ObjectFormat = git.Sha1ObjectFormat{} - } - // Check if label template exist if len(opts.IssueLabels) > 0 { if _, err := repo_module.LoadTemplateLabelsByDisplayName(opts.IssueLabels); err != nil { @@ -239,7 +235,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt TrustModel: opts.TrustModel, IsMirror: opts.IsMirror, DefaultBranch: opts.DefaultBranch, - ObjectFormat: opts.ObjectFormat, + ObjectFormatName: opts.ObjectFormatName, } var rollbackRepo *repo_model.Repository diff --git a/services/repository/files/cherry_pick.go b/services/repository/files/cherry_pick.go index 0085e88d55274..e88ea16119799 100644 --- a/services/repository/files/cherry_pick.go +++ b/services/repository/files/cherry_pick.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/services/pull" @@ -66,7 +67,7 @@ func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_mod } parent, err := commit.ParentID(0) if err != nil { - parent = repo.ObjectFormat.EmptyTree() + parent = git.ObjectFormatFromName(repo.ObjectFormatName).EmptyTree() } base, right := parent.String(), commit.ID.String() diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 0b5aaba1544ad..6a0b7b675c81e 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -77,8 +77,8 @@ func (t *TemporaryUploadRepository) Clone(branch string) error { } // Init the repository -func (t *TemporaryUploadRepository) Init(objectFormat git.ObjectFormat) error { - if err := git.InitRepository(t.ctx, t.basePath, false, objectFormat); err != nil { +func (t *TemporaryUploadRepository) Init(objectFormatName string) error { + if err := git.InitRepository(t.ctx, t.basePath, false, objectFormatName); err != nil { return err } gitRepo, err := git.OpenRepository(t.ctx, t.basePath) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index d202717ef5706..dd8d9ee42563d 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -155,8 +155,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use if !git.IsErrBranchNotExist(err) || !repo.IsEmpty { return nil, err } - objectFormat, _ := gitRepo.GetObjectFormat() - if err := t.Init(objectFormat); err != nil { + if err := t.Init(repo.ObjectFormatName); err != nil { return nil, err } hasOldBranch = false diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index 8be8773544779..61e38b55a3705 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -91,7 +91,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use if !git.IsErrBranchNotExist(err) || !repo.IsEmpty { return err } - if err = t.Init(repo.ObjectFormat); err != nil { + if err = t.Init(repo.ObjectFormatName); err != nil { return err } hasOldBranch = false diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index fd2bc8b1dd027..f98854c8dd5f6 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -36,7 +36,7 @@ func InitWiki(ctx context.Context, repo *repo_model.Repository) error { return nil } - if err := git.InitRepository(ctx, repo.WikiPath(), true, git.Sha1ObjectFormat{}); err != nil { + if err := git.InitRepository(ctx, repo.WikiPath(), true, repo.ObjectFormatName); err != nil { return fmt.Errorf("InitRepository: %w", err) } else if err = repo_module.CreateDelegateHooks(repo.WikiPath()); err != nil { return fmt.Errorf("createDelegateHooks: %w", err) diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go index de79910c888d3..fe0d2a68e0c70 100644 --- a/services/wiki/wiki_test.go +++ b/services/wiki/wiki_test.go @@ -302,7 +302,7 @@ func TestPrepareWikiFileName_FirstPage(t *testing.T) { // Now create a temporaryDirectory tmpDir := t.TempDir() - err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormat{}) + err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormatName) assert.NoError(t, err) gitRepo, err := git.OpenRepository(git.DefaultContext, tmpDir) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index bea948691c29e..3e4084a341f20 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -120,7 +120,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) { func doGitInitTestRepository(dstPath string) func(*testing.T) { return func(t *testing.T) { // Init repository in dstPath - assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormat{})) + assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormatName)) // forcibly set default branch to master _, _, err := git.NewCommand(git.DefaultContext, "symbolic-ref", "HEAD", git.BranchPrefix+"master").RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) From 20b098a592b8d6d646ce018b10aac65531b3009b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Dec 2023 18:14:51 +0800 Subject: [PATCH 03/12] use const variable --- modules/git/object_format.go | 2 +- modules/git/object_id.go | 2 +- modules/git/repo.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 76e1effa2a293..8a14522aaa9ff 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -76,7 +76,7 @@ func (h Sha1ObjectFormat) NewHasher() HasherInterface { func ObjectFormatFromName(name string) ObjectFormat { switch name { - case "sha1": + case Sha1ObjectFormatName: return Sha1ObjectFormat{} default: return nil diff --git a/modules/git/object_id.go b/modules/git/object_id.go index b6041f3c089f0..1e5659aec98a2 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -41,7 +41,7 @@ func NewSha1() *Sha1Hash { func NewHash(hash string) (ObjectID, error) { hash = strings.ToLower(hash) switch hash { - case "sha1": + case Sha1ObjectFormatName: return &Sha1Hash{}, nil } diff --git a/modules/git/repo.go b/modules/git/repo.go index c566e31cf024a..f7e32bebaa12d 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -99,7 +99,7 @@ func InitRepository(ctx context.Context, repoPath string, bare bool, objectForma cmd := NewCommand(ctx, "init") if SupportHashSha256 { if objectFormatName == "" { - objectFormatName = "sha1" + objectFormatName = Sha1ObjectFormatName } cmd.AddOptionValues("--object-format", objectFormatName) } From 1294c37ebee3df84e2a0803591e9fd7a0b451cd5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Dec 2023 19:45:01 +0800 Subject: [PATCH 04/12] Add validate for objectformat --- modules/git/object_format.go | 8 ++++++++ modules/git/repo.go | 3 +++ 2 files changed, 11 insertions(+) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 8a14522aaa9ff..569e0ffadb858 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -13,6 +13,14 @@ var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) const Sha1ObjectFormatName = "sha1" +func IsValidObjectFormat(name string) bool { + switch name { + case Sha1ObjectFormatName: + return true + } + return false +} + type ObjectFormat interface { String() string diff --git a/modules/git/repo.go b/modules/git/repo.go index f7e32bebaa12d..be272bc4be58d 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -101,6 +101,9 @@ func InitRepository(ctx context.Context, repoPath string, bare bool, objectForma if objectFormatName == "" { objectFormatName = Sha1ObjectFormatName } + if !IsValidObjectFormat(objectFormatName) { + return fmt.Errorf("invalid object format: %s", objectFormatName) + } cmd.AddOptionValues("--object-format", objectFormatName) } if bare { From 3db18697bbab039097287c6f73045073a1983c8f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Dec 2023 19:55:39 +0800 Subject: [PATCH 05/12] merge the switch --- modules/git/object_format.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 569e0ffadb858..d96c1d5437b90 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -13,14 +13,6 @@ var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) const Sha1ObjectFormatName = "sha1" -func IsValidObjectFormat(name string) bool { - switch name { - case Sha1ObjectFormatName: - return true - } - return false -} - type ObjectFormat interface { String() string @@ -90,3 +82,7 @@ func ObjectFormatFromName(name string) ObjectFormat { return nil } } + +func IsValidObjectFormat(name string) bool { + return ObjectFormatFromName(name) != nil +} From 6042d1bee6321f897f11d81bce9d48fa5f6117e2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Dec 2023 23:32:41 +0800 Subject: [PATCH 06/12] Follow wxiaoguang's suggestion --- modules/git/blame_test.go | 2 +- modules/git/object_format.go | 32 +++++++++---------- modules/git/object_id.go | 6 ++-- modules/git/object_id_gogit.go | 2 +- modules/git/parse_gogit_test.go | 14 ++++---- modules/git/parse_nogogit_test.go | 6 ++-- modules/git/ref.go | 2 +- modules/git/repo.go | 2 +- modules/git/repo_tag_test.go | 2 +- modules/git/tag_test.go | 2 +- modules/repository/commits_test.go | 2 +- modules/repository/generate.go | 4 +-- routers/api/v1/repo/repo.go | 2 +- routers/web/repo/githttp.go | 2 +- services/convert/git_commit_test.go | 2 +- services/migrations/common.go | 2 +- services/wiki/wiki_test.go | 2 +- .../git_helper_for_declarative_test.go | 2 +- 18 files changed, 44 insertions(+), 44 deletions(-) diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index c163f3be5d651..327edab767089 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -39,7 +39,7 @@ func TestReadingBlameOutput(t *testing.T) { } for _, bypass := range []bool{false, true} { - blameReader, err := CreateBlameReader(ctx, Sha1ObjectFormat{}, "./tests/repos/repo5_pulls", commit, "README.md", bypass) + blameReader, err := CreateBlameReader(ctx, Sha1ObjectFormat, "./tests/repos/repo5_pulls", commit, "README.md", bypass) assert.NoError(t, err) assert.NotNil(t, blameReader) defer blameReader.Close() diff --git a/modules/git/object_format.go b/modules/git/object_format.go index d96c1d5437b90..ebb1604c2e6c2 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -11,8 +11,6 @@ import ( // sha1Pattern can be used to determine if a string is an valid sha var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) -const Sha1ObjectFormatName = "sha1" - type ObjectFormat interface { String() string @@ -33,51 +31,53 @@ type ObjectFormat interface { NewHasher() HasherInterface } -type Sha1ObjectFormat struct{} +type Sha1ObjectFormatImpl struct{} -func (Sha1ObjectFormat) String() string { return Sha1ObjectFormatName } -func (Sha1ObjectFormat) Empty() ObjectID { return &Sha1Hash{} } -func (Sha1ObjectFormat) EmptyTree() ObjectID { +func (Sha1ObjectFormatImpl) String() string { return "sha1" } +func (Sha1ObjectFormatImpl) Empty() ObjectID { return &Sha1Hash{} } +func (Sha1ObjectFormatImpl) EmptyTree() ObjectID { return &Sha1Hash{ 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04, } } -func (Sha1ObjectFormat) FullLength() int { return 40 } -func (Sha1ObjectFormat) IsValid(input string) bool { +func (Sha1ObjectFormatImpl) FullLength() int { return 40 } +func (Sha1ObjectFormatImpl) IsValid(input string) bool { return sha1Pattern.MatchString(input) } -func (Sha1ObjectFormat) MustID(b []byte) ObjectID { +func (Sha1ObjectFormatImpl) MustID(b []byte) ObjectID { var id Sha1Hash copy(id[0:20], b) return &id } -func (h Sha1ObjectFormat) MustIDFromString(s string) ObjectID { +func (h Sha1ObjectFormatImpl) MustIDFromString(s string) ObjectID { return MustIDFromString(h, s) } -func (h Sha1ObjectFormat) NewID(b []byte) (ObjectID, error) { +func (h Sha1ObjectFormatImpl) NewID(b []byte) (ObjectID, error) { return IDFromRaw(h, b) } -func (h Sha1ObjectFormat) NewIDFromString(s string) (ObjectID, error) { +func (h Sha1ObjectFormatImpl) NewIDFromString(s string) (ObjectID, error) { return genericIDFromString(h, s) } -func (Sha1ObjectFormat) NewEmptyID() ObjectID { +func (Sha1ObjectFormatImpl) NewEmptyID() ObjectID { return NewSha1() } -func (h Sha1ObjectFormat) NewHasher() HasherInterface { +func (h Sha1ObjectFormatImpl) NewHasher() HasherInterface { return &Sha1Hasher{sha1.New()} } +var Sha1ObjectFormat ObjectFormat = Sha1ObjectFormatImpl{} + func ObjectFormatFromName(name string) ObjectFormat { switch name { - case Sha1ObjectFormatName: - return Sha1ObjectFormat{} + case Sha1ObjectFormat.String(): + return Sha1ObjectFormat default: return nil } diff --git a/modules/git/object_id.go b/modules/git/object_id.go index 1e5659aec98a2..af70e22ac4387 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -31,7 +31,7 @@ func (h *Sha1Hash) IsZero() bool { return bytes.Equal(empty[:], h[:]) } func (h *Sha1Hash) RawValue() []byte { return h[:] } -func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat{} } +func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat } func NewSha1() *Sha1Hash { return &Sha1Hash{} @@ -41,7 +41,7 @@ func NewSha1() *Sha1Hash { func NewHash(hash string) (ObjectID, error) { hash = strings.ToLower(hash) switch hash { - case Sha1ObjectFormatName: + case Sha1ObjectFormat.String(): return &Sha1Hash{}, nil } @@ -75,7 +75,7 @@ func genericIDFromString(h ObjectFormat, s string) (ObjectID, error) { func IDFromString(hexHash string) (ObjectID, error) { switch len(hexHash) { case 40: - hashType := Sha1ObjectFormat{} + hashType := Sha1ObjectFormat h, err := hashType.NewIDFromString(hexHash) if err != nil { return nil, err diff --git a/modules/git/object_id_gogit.go b/modules/git/object_id_gogit.go index 8e4d6ea8c2608..0cebb0d50b5ff 100644 --- a/modules/git/object_id_gogit.go +++ b/modules/git/object_id_gogit.go @@ -12,7 +12,7 @@ import ( func ParseGogitHash(h plumbing.Hash) ObjectID { switch hash.Size { case 20: - return Sha1ObjectFormat{}.MustID(h[:]) + return Sha1ObjectFormat.MustID(h[:]) } return nil diff --git a/modules/git/parse_gogit_test.go b/modules/git/parse_gogit_test.go index 100c55c06b37a..9755f81cce617 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: Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(Sha1ObjectFormat{}.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), + Hash: plumbing.Hash(Sha1ObjectFormat.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: Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"), gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(Sha1ObjectFormat{}.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()), + Hash: plumbing.Hash(Sha1ObjectFormat.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: Sha1ObjectFormat.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"), sized: true, gogitTreeEntry: &object.TreeEntry{ - Hash: plumbing.Hash(Sha1ObjectFormat{}.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()), + Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()), Name: "example", Mode: filemode.Dir, }, @@ -67,7 +67,7 @@ func TestParseTreeEntries(t *testing.T) { } for _, testCase := range testCases { - entries, err := ParseTreeEntries(Sha1ObjectFormat{}, []byte(testCase.Input)) + entries, err := ParseTreeEntries(Sha1ObjectFormat, []byte(testCase.Input)) assert.NoError(t, err) if len(entries) > 1 { fmt.Println(testCase.Expected[0].ID) diff --git a/modules/git/parse_nogogit_test.go b/modules/git/parse_nogogit_test.go index a06d89f982584..36313e00f331c 100644 --- a/modules/git/parse_nogogit_test.go +++ b/modules/git/parse_nogogit_test.go @@ -12,7 +12,7 @@ import ( ) func TestParseTreeEntriesLong(t *testing.T) { - objectFormat := Sha1ObjectFormat{} + objectFormat := Sha1ObjectFormat testCases := []struct { Input string @@ -66,7 +66,7 @@ func TestParseTreeEntriesLong(t *testing.T) { } func TestParseTreeEntriesShort(t *testing.T) { - objectFormat := Sha1ObjectFormat{} + objectFormat := Sha1ObjectFormat testCases := []struct { Input string @@ -102,7 +102,7 @@ func TestParseTreeEntriesShort(t *testing.T) { func TestParseTreeEntriesInvalid(t *testing.T) { // there was a panic: "runtime error: slice bounds out of range" when the input was invalid: #20315 - entries, err := ParseTreeEntries(Sha1ObjectFormat{}, []byte("100644 blob ea0d83c9081af9500ac9f804101b3fd0a5c293af")) + entries, err := ParseTreeEntries(Sha1ObjectFormat, []byte("100644 blob ea0d83c9081af9500ac9f804101b3fd0a5c293af")) assert.Error(t, err) assert.Len(t, entries, 0) } diff --git a/modules/git/ref.go b/modules/git/ref.go index fce2a201d1a59..ed801f20d5c34 100644 --- a/modules/git/ref.go +++ b/modules/git/ref.go @@ -205,7 +205,7 @@ func RefURL(repoURL, ref string) string { return repoURL + "/src/branch/" + refName case refFullName.IsTag(): return repoURL + "/src/tag/" + refName - case !Sha1ObjectFormat{}.IsValid(ref): + case !Sha1ObjectFormat.IsValid(ref): // assume they mean a branch return repoURL + "/src/branch/" + refName default: diff --git a/modules/git/repo.go b/modules/git/repo.go index be272bc4be58d..9b83683432be3 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -99,7 +99,7 @@ func InitRepository(ctx context.Context, repoPath string, bare bool, objectForma cmd := NewCommand(ctx, "init") if SupportHashSha256 { if objectFormatName == "" { - objectFormatName = Sha1ObjectFormatName + objectFormatName = Sha1ObjectFormat.String() } if !IsValidObjectFormat(objectFormatName) { return fmt.Errorf("invalid object format: %s", objectFormatName) diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index 98676a5fc8feb..48c1bc41c2375 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -194,7 +194,7 @@ func TestRepository_GetAnnotatedTag(t *testing.T) { } func TestRepository_parseTagRef(t *testing.T) { - sha1 := Sha1ObjectFormat{} + sha1 := Sha1ObjectFormat tests := []struct { name string diff --git a/modules/git/tag_test.go b/modules/git/tag_test.go index 3350262d6d320..e5a702273b40a 100644 --- a/modules/git/tag_test.go +++ b/modules/git/tag_test.go @@ -49,7 +49,7 @@ ono`), tag: Tag{ } for _, test := range testData { - tag, err := parseTagData(Sha1ObjectFormat{}, test.data) + tag, err := parseTagData(Sha1ObjectFormat, test.data) assert.NoError(t, err) assert.EqualValues(t, test.tag.ID, tag.ID) assert.EqualValues(t, test.tag.Object, tag.Object) diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 69132f7eb2368..afcb183d7237a 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -169,7 +169,7 @@ func TestListToPushCommits(t *testing.T) { When: now, } - hashType := git.Sha1ObjectFormat{} + hashType := git.Sha1ObjectFormat const hexString1 = "0123456789abcdef0123456789abcdef01234567" hash1, err := hashType.NewIDFromString(hexString1) assert.NoError(t, err) diff --git a/modules/repository/generate.go b/modules/repository/generate.go index f7eedbadebf13..f2e63982ed462 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -224,7 +224,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r } // FIXME: fix the hash - if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat{}.String()); err != nil { + if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat.String()); err != nil { return err } @@ -358,7 +358,7 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ } // FIXME - fix the hash - if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat{}.String()); err != nil { + if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat.String()); err != nil { return generateRepo, err } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 920ada2954c11..307c1c53322d9 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -253,7 +253,7 @@ func CreateUserRepo(ctx *context.APIContext, owner *user_model.User, opt api.Cre DefaultBranch: opt.DefaultBranch, TrustModel: repo_model.ToTrustModel(opt.TrustModel), IsTemplate: opt.Template, - ObjectFormatName: git.Sha1ObjectFormatName, + ObjectFormatName: git.Sha1ObjectFormat.String(), }) if err != nil { if repo_model.IsErrRepoAlreadyExist(err) { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index c3a61b2bbe55c..4a4f9863392dc 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -329,7 +329,7 @@ func dummyInfoRefs(ctx *context.Context) { } }() - if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormatName); err != nil { + if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormat.String()); err != nil { log.Error("Failed to init bare repo for git-receive-pack cache: %v", err) return } diff --git a/services/convert/git_commit_test.go b/services/convert/git_commit_test.go index 6e37ce4eaeba7..ade6e948d2622 100644 --- a/services/convert/git_commit_test.go +++ b/services/convert/git_commit_test.go @@ -19,7 +19,7 @@ import ( func TestToCommitMeta(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) headRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - sha1 := git.Sha1ObjectFormat{} + sha1 := git.Sha1ObjectFormat signature := &git.Signature{Name: "Test Signature", Email: "test@email.com", When: time.Unix(0, 0)} tag := &git.Tag{ Name: "Test Tag", diff --git a/services/migrations/common.go b/services/migrations/common.go index 488d96c00821d..d88518899d052 100644 --- a/services/migrations/common.go +++ b/services/migrations/common.go @@ -49,7 +49,7 @@ func CheckAndEnsureSafePR(pr *base.PullRequest, commonCloneBaseURL string, g bas // SECURITY: SHAs Must be a SHA // FIXME: hash only a SHA1 - CommitType := git.Sha1ObjectFormat{} + CommitType := git.Sha1ObjectFormat if pr.MergeCommitSHA != "" && !CommitType.IsValid(pr.MergeCommitSHA) { WarnAndNotice("PR #%d in %s has invalid MergeCommitSHA: %s", pr.Number, g, pr.MergeCommitSHA) pr.MergeCommitSHA = "" diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go index fe0d2a68e0c70..ff1b272f09258 100644 --- a/services/wiki/wiki_test.go +++ b/services/wiki/wiki_test.go @@ -302,7 +302,7 @@ func TestPrepareWikiFileName_FirstPage(t *testing.T) { // Now create a temporaryDirectory tmpDir := t.TempDir() - err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormatName) + err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormat.String()) assert.NoError(t, err) gitRepo, err := git.OpenRepository(git.DefaultContext, tmpDir) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 3e4084a341f20..59c8e3f8f35bd 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -120,7 +120,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) { func doGitInitTestRepository(dstPath string) func(*testing.T) { return func(t *testing.T) { // Init repository in dstPath - assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormatName)) + assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormat.String())) // forcibly set default branch to master _, _, err := git.NewCommand(git.DefaultContext, "symbolic-ref", "HEAD", git.BranchPrefix+"master").RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) From bc54beccb8441ab09adb8571364e8513f4e4cd0a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Dec 2023 00:00:38 +0800 Subject: [PATCH 07/12] More refactoring --- modules/git/commit.go | 2 +- modules/git/object_format.go | 34 ++++++++++--------- modules/git/object_id.go | 31 +++++++---------- modules/git/object_id_test.go | 2 +- modules/git/repo.go | 2 +- modules/git/repo_commit_gogit.go | 4 +-- modules/git/repo_compare.go | 2 +- modules/git/repo_compare_test.go | 4 +-- modules/git/repo_index.go | 2 +- modules/git/tag.go | 4 +-- modules/git/tag_test.go | 4 +-- modules/repository/generate.go | 4 +-- routers/api/v1/repo/repo.go | 2 +- routers/api/v1/utils/git.go | 2 +- routers/private/hook_pre_receive.go | 6 ++-- routers/private/hook_verification.go | 2 +- routers/private/hook_verification_test.go | 4 +-- routers/web/repo/branch.go | 2 +- routers/web/repo/compare.go | 2 +- routers/web/repo/githttp.go | 2 +- routers/web/repo/setting/webhook.go | 2 +- services/agit/agit.go | 4 +-- services/convert/git_commit_test.go | 8 ++--- services/gitdiff/gitdiff.go | 6 ++-- services/mirror/mirror_pull.go | 2 +- services/pull/pull.go | 2 +- services/release/release.go | 6 ++-- services/repository/branch.go | 2 +- services/repository/push.go | 14 ++++---- services/wiki/wiki_test.go | 2 +- .../git_helper_for_declarative_test.go | 2 +- 31 files changed, 82 insertions(+), 85 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index a8b6c0e8f793e..5d960e92f3384 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -236,7 +236,7 @@ func (c *Commit) IsForcePush(oldCommitID string) (bool, error) { if err != nil { return false, err } - if oldCommitID == objectFormat.Empty().String() { + if oldCommitID == objectFormat.EmptyObjectID().String() { return false, nil } diff --git a/modules/git/object_format.go b/modules/git/object_format.go index ebb1604c2e6c2..7a595946c3c84 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -12,10 +12,10 @@ import ( var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) type ObjectFormat interface { - String() string - - // Empty is the hash of empty git - Empty() ObjectID + // Name returns the name of the object format + Name() string + // EmptyObjectID creates a new empty ObjectID from an object format hash name + EmptyObjectID() ObjectID // EmptyTree is the hash of an empty tree EmptyTree() ObjectID // FullLength is the length of the hash's hex string @@ -26,15 +26,17 @@ type ObjectFormat interface { MustIDFromString(s string) ObjectID NewID(b []byte) (ObjectID, error) NewIDFromString(s string) (ObjectID, error) - NewEmptyID() ObjectID NewHasher() HasherInterface } type Sha1ObjectFormatImpl struct{} -func (Sha1ObjectFormatImpl) String() string { return "sha1" } -func (Sha1ObjectFormatImpl) Empty() ObjectID { return &Sha1Hash{} } +func (Sha1ObjectFormatImpl) Name() string { return "sha1" } +func (Sha1ObjectFormatImpl) EmptyObjectID() ObjectID { + return &Sha1Hash{} +} + func (Sha1ObjectFormatImpl) EmptyTree() ObjectID { return &Sha1Hash{ 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, @@ -64,23 +66,23 @@ func (h Sha1ObjectFormatImpl) NewIDFromString(s string) (ObjectID, error) { return genericIDFromString(h, s) } -func (Sha1ObjectFormatImpl) NewEmptyID() ObjectID { - return NewSha1() -} - func (h Sha1ObjectFormatImpl) NewHasher() HasherInterface { return &Sha1Hasher{sha1.New()} } var Sha1ObjectFormat ObjectFormat = Sha1ObjectFormatImpl{} +var SupportedObjectFormats = []ObjectFormat{ + Sha1ObjectFormat, +} + func ObjectFormatFromName(name string) ObjectFormat { - switch name { - case Sha1ObjectFormat.String(): - return Sha1ObjectFormat - default: - return nil + for _, objectFormat := range SupportedObjectFormats { + if name == objectFormat.Name() { + return objectFormat + } } + return nil } func IsValidObjectFormat(name string) bool { diff --git a/modules/git/object_id.go b/modules/git/object_id.go index af70e22ac4387..5af32092576e5 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -33,16 +33,15 @@ func (h *Sha1Hash) IsZero() bool { func (h *Sha1Hash) RawValue() []byte { return h[:] } func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat } -func NewSha1() *Sha1Hash { - return &Sha1Hash{} -} +var _ ObjectID = &Sha1Hash{} -// NewHash is for generic implementations -func NewHash(hash string) (ObjectID, error) { +// EmptyObjectID creates a new ObjectID from an object format hash name +func EmptyObjectID(hash string) (ObjectID, error) { hash = strings.ToLower(hash) - switch hash { - case Sha1ObjectFormat.String(): - return &Sha1Hash{}, nil + for _, objectFormat := range SupportedObjectFormats { + if objectFormat.Name() == hash { + return objectFormat.EmptyObjectID(), nil + } } return nil, errors.New("unsupported hash type") @@ -50,7 +49,7 @@ func NewHash(hash string) (ObjectID, error) { func IDFromRaw(h ObjectFormat, b []byte) (ObjectID, error) { if len(b) != h.FullLength()/2 { - return h.Empty(), fmt.Errorf("length must be %d: %v", h.FullLength(), b) + return h.EmptyObjectID(), fmt.Errorf("length must be %d: %v", h.FullLength(), b) } return h.MustID(b), nil } @@ -63,24 +62,20 @@ func MustIDFromString(h ObjectFormat, s string) ObjectID { func genericIDFromString(h ObjectFormat, s string) (ObjectID, error) { s = strings.TrimSpace(s) if len(s) != h.FullLength() { - return h.Empty(), fmt.Errorf("length must be %d: %s", h.FullLength(), s) + return h.EmptyObjectID(), fmt.Errorf("length must be %d: %s", h.FullLength(), s) } b, err := hex.DecodeString(s) if err != nil { - return h.Empty(), err + return h.EmptyObjectID(), err } return h.NewID(b) } func IDFromString(hexHash string) (ObjectID, error) { - switch len(hexHash) { - case 40: - hashType := Sha1ObjectFormat - h, err := hashType.NewIDFromString(hexHash) - if err != nil { - return nil, err + for _, objectFormat := range SupportedObjectFormats { + if len(hexHash) == objectFormat.FullLength() { + return objectFormat.NewIDFromString(hexHash) } - return h, nil } return nil, fmt.Errorf("invalid hash hex string: '%s' len: %d", hexHash, len(hexHash)) diff --git a/modules/git/object_id_test.go b/modules/git/object_id_test.go index c78a215755396..1ad40096a07b1 100644 --- a/modules/git/object_id_test.go +++ b/modules/git/object_id_test.go @@ -10,7 +10,7 @@ import ( ) func TestIsValidSHAPattern(t *testing.T) { - h := NewSha1().Type() + h := Sha1ObjectFormat assert.True(t, h.IsValid("fee1")) assert.True(t, h.IsValid("abc000")) assert.True(t, h.IsValid("9023902390239023902390239023902390239023")) diff --git a/modules/git/repo.go b/modules/git/repo.go index 9b83683432be3..52e54715d6532 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -99,7 +99,7 @@ func InitRepository(ctx context.Context, repoPath string, bare bool, objectForma cmd := NewCommand(ctx, "init") if SupportHashSha256 { if objectFormatName == "" { - objectFormatName = Sha1ObjectFormat.String() + objectFormatName = Sha1ObjectFormat.Name() } if !IsValidObjectFormat(objectFormatName) { return fmt.Errorf("invalid object format: %s", objectFormatName) diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index 893055bccdf6b..d0992fd385ae0 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -54,9 +54,9 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { if err != nil { if strings.Contains(err.Error(), "unknown revision or path") || strings.Contains(err.Error(), "fatal: Needed a single revision") { - return objectFormat.Empty(), ErrNotExist{commitID, ""} + return objectFormat.EmptyObjectID(), ErrNotExist{commitID, ""} } - return objectFormat.Empty(), err + return objectFormat.EmptyObjectID(), err } return objectFormat.NewIDFromString(actualCommitID) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 8885df4f7088e..0e9a0c70d791b 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -284,7 +284,7 @@ func (repo *Repository) GetPatch(base, head string, w io.Writer) error { // 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) { cmd := NewCommand(repo.Ctx, "diff-tree", "--name-only", "--root", "--no-commit-id", "-r", "-z") - if base == repo.objectFormat.Empty().String() { + if base == repo.objectFormat.EmptyObjectID().String() { cmd.AddDynamicArguments(head) } else { cmd.AddDynamicArguments(base, head) diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index 9bfaa5c02a564..526b21355075a 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -131,12 +131,12 @@ func TestGetCommitFilesChanged(t *testing.T) { files []string }{ { - repo.objectFormat.Empty().String(), + repo.objectFormat.EmptyObjectID().String(), "95bb4d39648ee7e325106df01a621c530863a653", []string{"file1.txt"}, }, { - repo.objectFormat.Empty().String(), + repo.objectFormat.EmptyObjectID().String(), "8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2", []string{"file2.txt"}, }, diff --git a/modules/git/repo_index.go b/modules/git/repo_index.go index 6f43734655f35..e3b19bf036463 100644 --- a/modules/git/repo_index.go +++ b/modules/git/repo_index.go @@ -101,7 +101,7 @@ func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error { for _, file := range filenames { if file != "" { buffer.WriteString("0 ") - buffer.WriteString(repo.objectFormat.Empty().String()) + buffer.WriteString(repo.objectFormat.EmptyObjectID().String()) buffer.WriteByte('\t') buffer.WriteString(file) buffer.WriteByte('\000') diff --git a/modules/git/tag.go b/modules/git/tag.go index 27358d74f8db8..c7d0d8aef92b2 100644 --- a/modules/git/tag.go +++ b/modules/git/tag.go @@ -35,8 +35,8 @@ func (tag *Tag) Commit(gitRepo *Repository) (*Commit, error) { // \n\n separate headers from message func parseTagData(objectFormat ObjectFormat, data []byte) (*Tag, error) { tag := new(Tag) - tag.ID = objectFormat.NewEmptyID() - tag.Object = objectFormat.NewEmptyID() + tag.ID = objectFormat.EmptyObjectID() + tag.Object = objectFormat.EmptyObjectID() tag.Tagger = &Signature{} // we now have the contents of the commit object. Let's investigate... nextline := 0 diff --git a/modules/git/tag_test.go b/modules/git/tag_test.go index e5a702273b40a..f980b0c560c4f 100644 --- a/modules/git/tag_test.go +++ b/modules/git/tag_test.go @@ -22,7 +22,7 @@ tagger Lucas Michot 1484491741 +0100 `), tag: Tag{ Name: "", - ID: NewSha1(), + ID: Sha1ObjectFormat.EmptyObjectID(), Object: &Sha1Hash{0x3b, 0x11, 0x4a, 0xb8, 0x0, 0xc6, 0x43, 0x2a, 0xd4, 0x23, 0x87, 0xcc, 0xf6, 0xbc, 0x8d, 0x43, 0x88, 0xa2, 0x88, 0x5a}, Type: "commit", Tagger: &Signature{Name: "Lucas Michot", Email: "lucas@semalead.com", When: time.Unix(1484491741, 0)}, @@ -39,7 +39,7 @@ o ono`), tag: Tag{ Name: "", - ID: NewSha1(), + ID: Sha1ObjectFormat.EmptyObjectID(), Object: &Sha1Hash{0x7c, 0xdf, 0x42, 0xc0, 0xb1, 0xcc, 0x76, 0x3a, 0xb7, 0xe4, 0xc3, 0x3c, 0x47, 0xa2, 0x4e, 0x27, 0xc6, 0x6b, 0xfc, 0xcc}, Type: "commit", Tagger: &Signature{Name: "Lucas Michot", Email: "lucas@semalead.com", When: time.Unix(1484553735, 0)}, diff --git a/modules/repository/generate.go b/modules/repository/generate.go index f2e63982ed462..f8478b8c1852a 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -224,7 +224,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r } // FIXME: fix the hash - if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat.String()); err != nil { + if err := git.InitRepository(ctx, tmpDir, false, git.Sha1ObjectFormat.Name()); err != nil { return err } @@ -358,7 +358,7 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ } // FIXME - fix the hash - if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat.String()); err != nil { + if err = CheckInitRepository(ctx, owner.Name, generateRepo.Name, git.Sha1ObjectFormat.Name()); err != nil { return generateRepo, err } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 307c1c53322d9..6eb2cc4227429 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -253,7 +253,7 @@ func CreateUserRepo(ctx *context.APIContext, owner *user_model.User, opt api.Cre DefaultBranch: opt.DefaultBranch, TrustModel: repo_model.ToTrustModel(opt.TrustModel), IsTemplate: opt.Template, - ObjectFormatName: git.Sha1ObjectFormat.String(), + ObjectFormatName: git.Sha1ObjectFormat.Name(), }) if err != nil { if repo_model.IsErrRepoAlreadyExist(err) { diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index eb82c505440fe..dfb1a130c3711 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -81,7 +81,7 @@ func ConvertToObjectID(ctx gocontext.Context, repo *context.Repository, commitID gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, repo.Repository.RepoPath()) if err != nil { - return objectFormat.Empty(), fmt.Errorf("RepositoryFromContextOrOpen: %w", err) + return objectFormat.EmptyObjectID(), fmt.Errorf("RepositoryFromContextOrOpen: %w", err) } defer closer.Close() diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 8811809710116..90d8287f06fd2 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -147,7 +147,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r gitRepo := ctx.Repo.GitRepo objectFormat, _ := gitRepo.GetObjectFormat() - if branchName == repo.DefaultBranch && newCommitID == objectFormat.Empty().String() { + if branchName == repo.DefaultBranch && newCommitID == objectFormat.EmptyObjectID().String() { log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ UserMsg: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName), @@ -175,7 +175,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // First of all we need to enforce absolutely: // // 1. Detect and prevent deletion of the branch - if newCommitID == objectFormat.Empty().String() { + if newCommitID == objectFormat.EmptyObjectID().String() { log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo) ctx.JSON(http.StatusForbidden, private.Response{ UserMsg: fmt.Sprintf("branch %s is protected from deletion", branchName), @@ -184,7 +184,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r } // 2. Disallow force pushes to protected branches - if oldCommitID != objectFormat.Empty().String() { + if oldCommitID != objectFormat.EmptyObjectID().String() { output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1").AddDynamicArguments(oldCommitID, "^"+newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ctx.env}) if err != nil { log.Error("Unable to detect force push between: %s and %s in %-v Error: %v", oldCommitID, newCommitID, repo, err) diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 6725205cc6d2c..8b2d0dd848a9d 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -30,7 +30,7 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env [] var command *git.Command objectFormat, _ := repo.GetObjectFormat() - if oldCommitID == objectFormat.Empty().String() { + if oldCommitID == objectFormat.EmptyObjectID().String() { // When creating a new branch, the oldCommitID is empty, by using "newCommitID --not --all": // List commits that are reachable by following the newCommitID, exclude "all" existing heads/tags commits // So, it only lists the new commits received, doesn't list the commits already present in the receiving repository diff --git a/routers/private/hook_verification_test.go b/routers/private/hook_verification_test.go index 7263ebc4235d3..04445b8eaf36c 100644 --- a/routers/private/hook_verification_test.go +++ b/routers/private/hook_verification_test.go @@ -30,9 +30,9 @@ func TestVerifyCommits(t *testing.T) { verified bool }{ {"72920278f2f999e3005801e5d5b8ab8139d3641c", "d766f2917716d45be24bfa968b8409544941be32", true}, - {objectFormat.Empty().String(), "93eac826f6188f34646cea81bf426aa5ba7d3bfe", true}, // New branch with verified commit + {objectFormat.EmptyObjectID().String(), "93eac826f6188f34646cea81bf426aa5ba7d3bfe", true}, // New branch with verified commit {"9779d17a04f1e2640583d35703c62460b2d86e0a", "72920278f2f999e3005801e5d5b8ab8139d3641c", false}, - {objectFormat.Empty().String(), "9ce3f779ae33f31fce17fac3c512047b75d7498b", false}, // New branch with unverified commit + {objectFormat.EmptyObjectID().String(), "9ce3f779ae33f31fce17fac3c512047b75d7498b", false}, // New branch with unverified commit } for _, tc := range testCases { diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index b6de5bf800e2d..c543160f42040 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -158,7 +158,7 @@ func RestoreBranchPost(ctx *context.Context) { if err := repo_service.PushUpdate( &repo_module.PushUpdateOptions{ RefFullName: git.RefNameFromBranch(deletedBranch.Name), - OldCommitID: objectFormat.Empty().String(), + OldCommitID: objectFormat.EmptyObjectID().String(), NewCommitID: deletedBranch.CommitID, PusherID: ctx.Doer.ID, PusherName: ctx.Doer.Name, diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 5b1638d3f5f53..f3b95b68fe5a2 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -317,7 +317,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo { ci.BaseBranch = baseCommit.ID.String() ctx.Data["BaseBranch"] = ci.BaseBranch baseIsCommit = true - } else if ci.BaseBranch == objectFormat.Empty().String() { + } else if ci.BaseBranch == objectFormat.EmptyObjectID().String() { if isSameRepo { ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(ci.HeadBranch)) } else { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 4a4f9863392dc..6d3dd5a3fe629 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -329,7 +329,7 @@ func dummyInfoRefs(ctx *context.Context) { } }() - if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormat.String()); err != nil { + if err := git.InitRepository(ctx, tmpDir, true, git.Sha1ObjectFormat.Name()); err != nil { log.Error("Failed to init bare repo for git-receive-pack cache: %v", err) return } diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index 8c232a4cb8d43..ab3c70006f795 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -662,7 +662,7 @@ func TestWebhook(ctx *context.Context) { return } commit = &git.Commit{ - ID: objectFormat.NewEmptyID(), + ID: objectFormat.EmptyObjectID(), Author: ghost.NewGitSig(), Committer: ghost.NewGitSig(), CommitMessage: "This is a fake commit", diff --git a/services/agit/agit.go b/services/agit/agit.go index e354b9169a201..bc68372570402 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -39,7 +39,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. objectFormat, _ := gitRepo.GetObjectFormat() for i := range opts.OldCommitIDs { - if opts.NewCommitIDs[i] == objectFormat.Empty().String() { + if opts.NewCommitIDs[i] == objectFormat.EmptyObjectID().String() { results = append(results, private.HookProcReceiveRefResult{ OriginalRef: opts.RefFullNames[i], OldOID: opts.OldCommitIDs[i], @@ -153,7 +153,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. results = append(results, private.HookProcReceiveRefResult{ Ref: pr.GetGitRefName(), OriginalRef: opts.RefFullNames[i], - OldOID: objectFormat.Empty().String(), + OldOID: objectFormat.EmptyObjectID().String(), NewOID: opts.NewCommitIDs[i], }) continue diff --git a/services/convert/git_commit_test.go b/services/convert/git_commit_test.go index ade6e948d2622..73cb5e8c7170b 100644 --- a/services/convert/git_commit_test.go +++ b/services/convert/git_commit_test.go @@ -23,8 +23,8 @@ func TestToCommitMeta(t *testing.T) { signature := &git.Signature{Name: "Test Signature", Email: "test@email.com", When: time.Unix(0, 0)} tag := &git.Tag{ Name: "Test Tag", - ID: sha1.Empty(), - Object: sha1.Empty(), + ID: sha1.EmptyObjectID(), + Object: sha1.EmptyObjectID(), Type: "Test Type", Tagger: signature, Message: "Test Message", @@ -34,8 +34,8 @@ func TestToCommitMeta(t *testing.T) { assert.NotNil(t, commitMeta) assert.EqualValues(t, &api.CommitMeta{ - SHA: sha1.Empty().String(), - URL: util.URLJoin(headRepo.APIURL(), "git/commits", sha1.Empty().String()), + SHA: sha1.EmptyObjectID().String(), + URL: util.URLJoin(headRepo.APIURL(), "git/commits", sha1.EmptyObjectID().String()), Created: time.Unix(0, 0), }, commitMeta) } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 05d4a0555fc6c..75fc687c86a52 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1120,7 +1120,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return nil, err } - if (len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.Empty().String()) && commit.ParentCount() == 0 { + if (len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.EmptyObjectID().String()) && commit.ParentCount() == 0 { cmdDiff.AddArguments("diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"). AddArguments(opts.WhitespaceBehavior...). AddDynamicArguments(objectFormat.EmptyTree().String()). @@ -1229,7 +1229,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi } diffPaths := []string{opts.BeforeCommitID + separator + opts.AfterCommitID} - if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.Empty().String() { + if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.EmptyObjectID().String() { diffPaths = []string{objectFormat.EmptyTree().String(), opts.AfterCommitID} } diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) @@ -1267,7 +1267,7 @@ func GetPullDiffStats(gitRepo *git.Repository, opts *DiffOptions) (*PullDiffStat } diffPaths := []string{opts.BeforeCommitID + separator + opts.AfterCommitID} - if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.Empty().String() { + if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.EmptyObjectID().String() { diffPaths = []string{objectFormat.EmptyTree().String(), opts.AfterCommitID} } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index b3ecf2fc54e1a..6f03e14ab08bd 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -484,7 +484,7 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { } notify_service.SyncPushCommits(ctx, m.Repo.MustOwner(ctx), m.Repo, &repo_module.PushUpdateOptions{ RefFullName: result.refName, - OldCommitID: objectFormat.Empty().String(), + OldCommitID: objectFormat.EmptyObjectID().String(), NewCommitID: commitID, }, repo_module.NewPushCommits()) notify_service.SyncCreateRef(ctx, m.Repo.MustOwner(ctx), m.Repo, result.refName, commitID) diff --git a/services/pull/pull.go b/services/pull/pull.go index a16d1be1c1cbd..6094a4ed31b9e 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -328,7 +328,7 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, if err == nil { for _, pr := range prs { objectFormat, _ := git.GetObjectFormatOfRepo(ctx, pr.BaseRepo.RepoPath()) - if newCommitID != "" && newCommitID != objectFormat.Empty().String() { + if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() { changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID) if err != nil { log.Error("checkIfPRContentChanged: %v", err) diff --git a/services/release/release.go b/services/release/release.go index 4cd520e82f965..fc91171fba6ae 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -89,14 +89,14 @@ func createTag(ctx context.Context, gitRepo *git.Repository, rel *repo_model.Rel objectFormat, _ := gitRepo.GetObjectFormat() commits := repository.NewPushCommits() commits.HeadCommit = repository.CommitToPushCommit(commit) - commits.CompareURL = rel.Repo.ComposeCompareURL(objectFormat.Empty().String(), commit.ID.String()) + commits.CompareURL = rel.Repo.ComposeCompareURL(objectFormat.EmptyObjectID().String(), commit.ID.String()) refFullName := git.RefNameFromTag(rel.TagName) notify_service.PushCommits( ctx, rel.Publisher, rel.Repo, &repository.PushUpdateOptions{ RefFullName: refFullName, - OldCommitID: objectFormat.Empty().String(), + OldCommitID: objectFormat.EmptyObjectID().String(), NewCommitID: commit.ID.String(), }, commits) notify_service.CreateRef(ctx, rel.Publisher, rel.Repo, refFullName, commit.ID.String()) @@ -335,7 +335,7 @@ func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *re &repository.PushUpdateOptions{ RefFullName: refName, OldCommitID: rel.Sha1, - NewCommitID: objectFormat.Empty().String(), + NewCommitID: objectFormat.EmptyObjectID().String(), }, repository.NewPushCommits()) notify_service.DeleteRef(ctx, doer, repo, refName) diff --git a/services/repository/branch.go b/services/repository/branch.go index b797917757ede..dca938444aa35 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -408,7 +408,7 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R &repo_module.PushUpdateOptions{ RefFullName: git.RefNameFromBranch(branchName), OldCommitID: commit.ID.String(), - NewCommitID: objectFormat.Empty().String(), + NewCommitID: objectFormat.EmptyObjectID().String(), PusherID: doer.ID, PusherName: doer.Name, RepoUserName: repo.OwnerName, diff --git a/services/repository/push.go b/services/repository/push.go index 3003933c34bcf..3bc7a78cb955d 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -111,7 +111,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { log.Trace("pushUpdates: %-v %s %s %s", repo, opts.OldCommitID, opts.NewCommitID, opts.RefFullName) if opts.IsNewRef() && opts.IsDelRef() { - return fmt.Errorf("old and new revisions are both %s", objectFormat.Empty()) + return fmt.Errorf("old and new revisions are both %s", objectFormat.EmptyObjectID()) } if opts.RefFullName.IsTag() { if pusher == nil || pusher.ID != opts.PusherID { @@ -131,7 +131,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { &repo_module.PushUpdateOptions{ RefFullName: git.RefNameFromTag(tagName), OldCommitID: opts.OldCommitID, - NewCommitID: objectFormat.Empty().String(), + NewCommitID: objectFormat.EmptyObjectID().String(), }, repo_module.NewPushCommits()) delTags = append(delTags, tagName) @@ -144,13 +144,13 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { commits := repo_module.NewPushCommits() commits.HeadCommit = repo_module.CommitToPushCommit(newCommit) - commits.CompareURL = repo.ComposeCompareURL(objectFormat.Empty().String(), opts.NewCommitID) + commits.CompareURL = repo.ComposeCompareURL(objectFormat.EmptyObjectID().String(), opts.NewCommitID) notify_service.PushCommits( ctx, pusher, repo, &repo_module.PushUpdateOptions{ RefFullName: opts.RefFullName, - OldCommitID: objectFormat.Empty().String(), + OldCommitID: objectFormat.EmptyObjectID().String(), NewCommitID: opts.NewCommitID, }, commits) @@ -234,7 +234,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } oldCommitID := opts.OldCommitID - if oldCommitID == objectFormat.Empty().String() && len(commits.Commits) > 0 { + if oldCommitID == objectFormat.EmptyObjectID().String() && len(commits.Commits) > 0 { oldCommit, err := gitRepo.GetCommit(commits.Commits[len(commits.Commits)-1].Sha1) if err != nil && !git.IsErrNotExist(err) { log.Error("unable to GetCommit %s from %-v: %v", oldCommitID, repo, err) @@ -250,11 +250,11 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } } - if oldCommitID == objectFormat.Empty().String() && repo.DefaultBranch != branch { + if oldCommitID == objectFormat.EmptyObjectID().String() && repo.DefaultBranch != branch { oldCommitID = repo.DefaultBranch } - if oldCommitID != objectFormat.Empty().String() { + if oldCommitID != objectFormat.EmptyObjectID().String() { commits.CompareURL = repo.ComposeCompareURL(oldCommitID, opts.NewCommitID) } else { commits.CompareURL = "" diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go index ff1b272f09258..277fa086ac13c 100644 --- a/services/wiki/wiki_test.go +++ b/services/wiki/wiki_test.go @@ -302,7 +302,7 @@ func TestPrepareWikiFileName_FirstPage(t *testing.T) { // Now create a temporaryDirectory tmpDir := t.TempDir() - err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormat.String()) + err := git.InitRepository(git.DefaultContext, tmpDir, true, git.Sha1ObjectFormat.Name()) assert.NoError(t, err) gitRepo, err := git.OpenRepository(git.DefaultContext, tmpDir) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 59c8e3f8f35bd..77fe07128e75a 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -120,7 +120,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) { func doGitInitTestRepository(dstPath string) func(*testing.T) { return func(t *testing.T) { // Init repository in dstPath - assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormat.String())) + assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false, git.Sha1ObjectFormat.Name())) // forcibly set default branch to master _, _, err := git.NewCommand(git.DefaultContext, "symbolic-ref", "HEAD", git.BranchPrefix+"master").RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) From 7a5da47c54ed83bbf42bb00080fb46e0cbab1006 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Dec 2023 00:05:02 +0800 Subject: [PATCH 08/12] optimaztion --- modules/git/object_format.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 7a595946c3c84..ee7e659ed04dd 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -32,16 +32,21 @@ type ObjectFormat interface { type Sha1ObjectFormatImpl struct{} +var ( + emptyObjectID = &Sha1Hash{} + emptyTree = &Sha1Hash{ + 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, + 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04, + } +) + func (Sha1ObjectFormatImpl) Name() string { return "sha1" } func (Sha1ObjectFormatImpl) EmptyObjectID() ObjectID { - return &Sha1Hash{} + return emptyObjectID } func (Sha1ObjectFormatImpl) EmptyTree() ObjectID { - return &Sha1Hash{ - 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, - 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04, - } + return emptyTree } func (Sha1ObjectFormatImpl) FullLength() int { return 40 } func (Sha1ObjectFormatImpl) IsValid(input string) bool { @@ -74,6 +79,7 @@ var Sha1ObjectFormat ObjectFormat = Sha1ObjectFormatImpl{} var SupportedObjectFormats = []ObjectFormat{ Sha1ObjectFormat, + // TODO: add sha256 } func ObjectFormatFromName(name string) ObjectFormat { From b98f37bfc8bd21b15a0fa98959f21e3efa7a9d3d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Dec 2023 00:46:42 +0800 Subject: [PATCH 09/12] Follow wxiaoguang's suggestion --- modules/git/object_format.go | 3 +++ modules/git/object_id.go | 9 ++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index ee7e659ed04dd..243c9412d2af7 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -83,6 +83,9 @@ var SupportedObjectFormats = []ObjectFormat{ } func ObjectFormatFromName(name string) ObjectFormat { + if name == "" { + return Sha1ObjectFormat + } for _, objectFormat := range SupportedObjectFormats { if name == objectFormat.Name() { return objectFormat diff --git a/modules/git/object_id.go b/modules/git/object_id.go index 5af32092576e5..e8a43aa40a74c 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -36,10 +36,13 @@ func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat } var _ ObjectID = &Sha1Hash{} // EmptyObjectID creates a new ObjectID from an object format hash name -func EmptyObjectID(hash string) (ObjectID, error) { - hash = strings.ToLower(hash) +func EmptyObjectID(objectFormatName string) (ObjectID, error) { + if objectFormatName == "" { + return Sha1ObjectFormat.EmptyObjectID(), nil + } + objectFormatName = strings.ToLower(objectFormatName) for _, objectFormat := range SupportedObjectFormats { - if objectFormat.Name() == hash { + if objectFormat.Name() == objectFormatName { return objectFormat.EmptyObjectID(), nil } } From a6433aa5169d6f93fded70804ea56a6ae802e81a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Dec 2023 00:54:43 +0800 Subject: [PATCH 10/12] Add comment --- models/repo/repo.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/repo/repo.go b/models/repo/repo.go index 50e4af779b22f..29eb7501a812f 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -277,6 +277,8 @@ func (repo *Repository) AfterLoad() { repo.NumOpenProjects = repo.NumProjects - repo.NumClosedProjects repo.NumOpenActionRuns = repo.NumActionRuns - repo.NumClosedActionRuns + // this is a temporary behaviour to support old repos, next step is to store the object format in the database + // and read from database so this line could be removed. To not depend on git module, we use a constant variable here repo.ObjectFormatName = "sha1" } From 47fedfa7188066f091e2a2ba89b266ae4137eaf9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Dec 2023 00:58:29 +0800 Subject: [PATCH 11/12] Simple the code --- modules/git/object_id.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/modules/git/object_id.go b/modules/git/object_id.go index e8a43aa40a74c..a90683678a817 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -37,14 +37,9 @@ var _ ObjectID = &Sha1Hash{} // EmptyObjectID creates a new ObjectID from an object format hash name func EmptyObjectID(objectFormatName string) (ObjectID, error) { - if objectFormatName == "" { - return Sha1ObjectFormat.EmptyObjectID(), nil - } - objectFormatName = strings.ToLower(objectFormatName) - for _, objectFormat := range SupportedObjectFormats { - if objectFormat.Name() == objectFormatName { - return objectFormat.EmptyObjectID(), nil - } + objectFormat := ObjectFormatFromName(objectFormatName) + if objectFormat != nil { + return objectFormat.EmptyObjectID(), nil } return nil, errors.New("unsupported hash type") From d044663b759c537fb8ba5c7c79c30e460a84436e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Dec 2023 19:19:17 +0800 Subject: [PATCH 12/12] Remove the empty check on ObjectFormatFromName --- models/git/branch_test.go | 1 + modules/git/object_format.go | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/models/git/branch_test.go b/models/git/branch_test.go index 153a1aa977971..8febc80f14747 100644 --- a/models/git/branch_test.go +++ b/models/git/branch_test.go @@ -20,6 +20,7 @@ import ( func TestAddDeletedBranch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.EqualValues(t, git.Sha1ObjectFormat.Name(), repo.ObjectFormatName) firstBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{ID: 1}) assert.True(t, firstBranch.IsDeleted) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 243c9412d2af7..ee7e659ed04dd 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -83,9 +83,6 @@ var SupportedObjectFormats = []ObjectFormat{ } func ObjectFormatFromName(name string) ObjectFormat { - if name == "" { - return Sha1ObjectFormat - } for _, objectFormat := range SupportedObjectFormats { if name == objectFormat.Name() { return objectFormat