From cfe6779d4eb2f3869357768fe58863642f79c5a9 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Mon, 6 May 2024 23:06:45 +0800 Subject: [PATCH 01/58] Get repo list with OrderBy alpha should respect owner too (#30784) (#30875) Backport #30784 by @6543 instead of: - zowner/gcode - awesome/nul - zowner/nul - zowner/zzz we will get: - awesome/nul - zowner/gcode - zowner/nul - zowner/zzz Co-authored-by: 6543 <6543@obermui.de> --- models/repo/search.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/repo/search.go b/models/repo/search.go index 4d64acf8cfe3..54d6dcfb440c 100644 --- a/models/repo/search.go +++ b/models/repo/search.go @@ -8,14 +8,14 @@ import "code.gitea.io/gitea/models/db" // SearchOrderByMap represents all possible search order var SearchOrderByMap = map[string]map[string]db.SearchOrderBy{ "asc": { - "alpha": db.SearchOrderByAlphabetically, + "alpha": "owner_name ASC, name ASC", "created": db.SearchOrderByOldest, "updated": db.SearchOrderByLeastUpdated, "size": db.SearchOrderBySize, "id": db.SearchOrderByID, }, "desc": { - "alpha": db.SearchOrderByAlphabeticallyReverse, + "alpha": "owner_name DESC, name DESC", "created": db.SearchOrderByNewest, "updated": db.SearchOrderByRecentUpdated, "size": db.SearchOrderBySizeReverse, From ad5a8d043c6818c0c496ebae2f5ea9373219bcd6 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Tue, 7 May 2024 08:33:28 +0800 Subject: [PATCH 02/58] Make "sync branch" also sync object format and add tests (#30878) (#30880) Backport #30878 by wxiaoguang Co-authored-by: wxiaoguang --- modules/git/repo.go | 27 --------------------------- modules/repository/branch.go | 10 ++++++++++ modules/repository/branch_test.go | 31 +++++++++++++++++++++++++++++++ services/repository/adopt.go | 4 ++++ 4 files changed, 45 insertions(+), 27 deletions(-) create mode 100644 modules/repository/branch_test.go diff --git a/modules/git/repo.go b/modules/git/repo.go index 4511e900e08a..c5ba5117a786 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -7,7 +7,6 @@ package git import ( "bytes" "context" - "errors" "fmt" "io" "net/url" @@ -63,32 +62,6 @@ func IsRepoURLAccessible(ctx context.Context, url string) bool { return err == nil } -// GetObjectFormatOfRepo returns the hash type of repository at a given path -func GetObjectFormatOfRepo(ctx context.Context, repoPath string) (ObjectFormat, error) { - var stdout, stderr strings.Builder - - err := NewCommand(ctx, "hash-object", "--stdin").Run(&RunOpts{ - Dir: repoPath, - Stdout: &stdout, - Stderr: &stderr, - Stdin: &strings.Reader{}, - }) - if err != nil { - return nil, err - } - - if stderr.Len() > 0 { - return nil, errors.New(stderr.String()) - } - - h, err := NewIDFromString(strings.TrimRight(stdout.String(), "\n")) - if err != nil { - return nil, err - } - - return h.Type(), nil -} - // InitRepository initializes a new Git repository. func InitRepository(ctx context.Context, repoPath string, bare bool, objectFormatName string) error { err := os.MkdirAll(repoPath, os.ModePerm) diff --git a/modules/repository/branch.go b/modules/repository/branch.go index e448490f4ac1..a3fca7c7ce40 100644 --- a/modules/repository/branch.go +++ b/modules/repository/branch.go @@ -5,6 +5,7 @@ package repository import ( "context" + "fmt" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -36,6 +37,15 @@ func SyncRepoBranches(ctx context.Context, repoID, doerID int64) (int64, error) } func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, doerID int64) (int64, error) { + objFmt, err := gitRepo.GetObjectFormat() + if err != nil { + return 0, fmt.Errorf("GetObjectFormat: %w", err) + } + _, err = db.GetEngine(ctx).ID(repo.ID).Update(&repo_model.Repository{ObjectFormatName: objFmt.Name()}) + if err != nil { + return 0, fmt.Errorf("UpdateRepository: %w", err) + } + allBranches := container.Set[string]{} { branches, _, err := gitRepo.GetBranchNames(0, 0) diff --git a/modules/repository/branch_test.go b/modules/repository/branch_test.go new file mode 100644 index 000000000000..acf75a1ac0cd --- /dev/null +++ b/modules/repository/branch_test.go @@ -0,0 +1,31 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestSyncRepoBranches(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + _, err := db.GetEngine(db.DefaultContext).ID(1).Update(&repo_model.Repository{ObjectFormatName: "bad-fmt"}) + assert.NoError(t, db.TruncateBeans(db.DefaultContext, &git_model.Branch{})) + assert.NoError(t, err) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.Equal(t, "bad-fmt", repo.ObjectFormatName) + _, err = SyncRepoBranches(db.DefaultContext, 1, 0) + assert.NoError(t, err) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.Equal(t, "sha1", repo.ObjectFormatName) + branch, err := git_model.GetBranch(db.DefaultContext, 1, "master") + assert.NoError(t, err) + assert.EqualValues(t, "master", branch.Name) +} diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 31e3e581b313..1ad1ef5b2cf0 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -195,6 +195,10 @@ func adoptRepository(ctx context.Context, repoPath string, u *user_model.User, r } defer gitRepo.Close() + if _, err = repo_module.SyncRepoBranchesWithRepo(ctx, repo, gitRepo, 0); err != nil { + return fmt.Errorf("SyncRepoBranches: %w", err) + } + if err = repo_module.SyncReleasesWithTags(ctx, repo, gitRepo); err != nil { return fmt.Errorf("SyncReleasesWithTags: %w", err) } From d5563be0eeba8e46d8d0e9974c55fa20519f968f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 7 May 2024 10:07:33 +0800 Subject: [PATCH 03/58] Make sure git version&feature are always prepared (#30877) (#30879) Backport #30877 --- cmd/hook.go | 7 +- cmd/serv.go | 2 +- modules/git/blame.go | 2 +- modules/git/commit.go | 2 +- modules/git/git.go | 183 +++++++++++-------------- modules/git/object_format.go | 6 +- modules/git/object_id.go | 2 +- modules/git/remote.go | 2 +- modules/git/repo.go | 2 +- modules/git/repo_base.go | 6 - modules/git/repo_base_gogit.go | 4 +- modules/git/repo_base_nogogit.go | 4 +- modules/git/repo_commit.go | 2 +- modules/git/repo_commitgraph.go | 2 +- modules/lfs/pointer_scanner_nogogit.go | 2 +- routers/init.go | 6 +- routers/private/hook_pre_receive.go | 2 +- routers/private/hook_proc_receive.go | 2 +- routers/private/serv.go | 2 +- routers/web/admin/config.go | 2 +- routers/web/misc/misc.go | 2 +- routers/web/repo/githttp.go | 2 +- routers/web/repo/repo.go | 2 +- services/gitdiff/gitdiff.go | 2 +- services/pull/patch.go | 2 +- services/pull/temp_repo.go | 2 +- services/repository/files/patch.go | 2 +- tests/integration/git_test.go | 2 +- 28 files changed, 114 insertions(+), 144 deletions(-) delete mode 100644 modules/git/repo_base.go diff --git a/cmd/hook.go b/cmd/hook.go index 2a9c25add523..9c1cb66f2a4d 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -220,10 +220,7 @@ Gitea or set your environment appropriately.`, "") } } - supportProcReceive := false - if git.CheckGitVersionAtLeast("2.29") == nil { - supportProcReceive = true - } + supportProcReceive := git.DefaultFeatures().SupportProcReceive for scanner.Scan() { // TODO: support news feeds for wiki @@ -497,7 +494,7 @@ Gitea or set your environment appropriately.`, "") return nil } - if git.CheckGitVersionAtLeast("2.29") != nil { + if !git.DefaultFeatures().SupportProcReceive { return fail(ctx, "No proc-receive support", "current git version doesn't support proc-receive.") } diff --git a/cmd/serv.go b/cmd/serv.go index 90190a19db64..2bfd1110617e 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -178,7 +178,7 @@ func runServ(c *cli.Context) error { } if len(words) < 2 { - if git.CheckGitVersionAtLeast("2.29") == nil { + if git.DefaultFeatures().SupportProcReceive { // for AGit Flow if cmd == "ssh_info" { fmt.Print(`{"type":"gitea","version":1}`) diff --git a/modules/git/blame.go b/modules/git/blame.go index 69e1b08f93b4..a9b2706f2173 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -132,7 +132,7 @@ func (r *BlameReader) Close() error { // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (*BlameReader, error) { var ignoreRevsFile *string - if CheckGitVersionAtLeast("2.23") == nil && !bypassBlameIgnore { + if DefaultFeatures().CheckVersionAtLeast("2.23") && !bypassBlameIgnore { ignoreRevsFile = tryCreateBlameIgnoreRevsFile(commit) } diff --git a/modules/git/commit.go b/modules/git/commit.go index d96cef37c8d2..86adaa79a667 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -423,7 +423,7 @@ func (c *Commit) GetSubModule(entryname string) (*SubModule, error) { // GetBranchName gets the closest branch name (as returned by 'git name-rev --name-only') func (c *Commit) GetBranchName() (string, error) { cmd := NewCommand(c.repo.Ctx, "name-rev") - if CheckGitVersionAtLeast("2.13.0") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.13.0") { cmd.AddArguments("--exclude", "refs/tags/*") } cmd.AddArguments("--name-only", "--no-undefined").AddDynamicArguments(c.ID.String()) diff --git a/modules/git/git.go b/modules/git/git.go index e411269f7c50..05ca26085553 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -22,42 +22,63 @@ import ( "github.com/hashicorp/go-version" ) -// RequiredVersion is the minimum Git version required -const RequiredVersion = "2.0.0" +const RequiredVersion = "2.0.0" // the minimum Git version required -var ( - // GitExecutable is the command name of git - // Could be updated to an absolute path while initialization - GitExecutable = "git" - - // DefaultContext is the default context to run git commands in, must be initialized by git.InitXxx - DefaultContext context.Context +type Features struct { + gitVersion *version.Version - DefaultFeatures struct { - GitVersion *version.Version + UsingGogit bool + SupportProcReceive bool // >= 2.29 + SupportHashSha256 bool // >= 2.42, SHA-256 repositories no longer an ‘experimental curiosity’ + SupportedObjectFormats []ObjectFormat // sha1, sha256 +} - SupportProcReceive bool // >= 2.29 - SupportHashSha256 bool // >= 2.42, SHA-256 repositories no longer an ‘experimental curiosity’ - } +var ( + GitExecutable = "git" // the command name of git, will be updated to an absolute path during initialization + DefaultContext context.Context // the default context to run git commands in, must be initialized by git.InitXxx + defaultFeatures *Features ) -// loadGitVersion tries to get the current git version and stores it into a global variable -func loadGitVersion() error { - // doesn't need RWMutex because it's executed by Init() - if DefaultFeatures.GitVersion != nil { - return nil +func (f *Features) CheckVersionAtLeast(atLeast string) bool { + return f.gitVersion.Compare(version.Must(version.NewVersion(atLeast))) >= 0 +} + +// VersionInfo returns git version information +func (f *Features) VersionInfo() string { + return f.gitVersion.Original() +} + +func DefaultFeatures() *Features { + if defaultFeatures == nil { + if !setting.IsProd || setting.IsInTesting { + log.Warn("git.DefaultFeatures is called before git.InitXxx, initializing with default values") + } + if err := InitSimple(context.Background()); err != nil { + log.Fatal("git.InitSimple failed: %v", err) + } } + return defaultFeatures +} +func loadGitVersionFeatures() (*Features, error) { stdout, _, runErr := NewCommand(DefaultContext, "version").RunStdString(nil) if runErr != nil { - return runErr + return nil, runErr } ver, err := parseGitVersionLine(strings.TrimSpace(stdout)) - if err == nil { - DefaultFeatures.GitVersion = ver + if err != nil { + return nil, err } - return err + + features := &Features{gitVersion: ver, UsingGogit: isGogit} + features.SupportProcReceive = features.CheckVersionAtLeast("2.29") + features.SupportHashSha256 = features.CheckVersionAtLeast("2.42") && !isGogit + features.SupportedObjectFormats = []ObjectFormat{Sha1ObjectFormat} + if features.SupportHashSha256 { + features.SupportedObjectFormats = append(features.SupportedObjectFormats, Sha256ObjectFormat) + } + return features, nil } func parseGitVersionLine(s string) (*version.Version, error) { @@ -85,56 +106,24 @@ func SetExecutablePath(path string) error { return fmt.Errorf("git not found: %w", err) } GitExecutable = absPath + return nil +} - if err = loadGitVersion(); err != nil { - return fmt.Errorf("unable to load git version: %w", err) - } - - versionRequired, err := version.NewVersion(RequiredVersion) - if err != nil { - return err - } - - if DefaultFeatures.GitVersion.LessThan(versionRequired) { +func ensureGitVersion() error { + if !DefaultFeatures().CheckVersionAtLeast(RequiredVersion) { moreHint := "get git: https://git-scm.com/download/" if runtime.GOOS == "linux" { // there are a lot of CentOS/RHEL users using old git, so we add a special hint for them - if _, err = os.Stat("/etc/redhat-release"); err == nil { + if _, err := os.Stat("/etc/redhat-release"); err == nil { // ius.io is the recommended official(git-scm.com) method to install git moreHint = "get git: https://git-scm.com/download/linux and https://ius.io" } } - return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", DefaultFeatures.GitVersion.Original(), RequiredVersion, moreHint) + return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", DefaultFeatures().gitVersion.Original(), RequiredVersion, moreHint) } - if err = checkGitVersionCompatibility(DefaultFeatures.GitVersion); err != nil { - return fmt.Errorf("installed git version %s has a known compatibility issue with Gitea: %w, please upgrade (or downgrade) git", DefaultFeatures.GitVersion.String(), err) - } - return nil -} - -// VersionInfo returns git version information -func VersionInfo() string { - if DefaultFeatures.GitVersion == nil { - return "(git not found)" - } - format := "%s" - args := []any{DefaultFeatures.GitVersion.Original()} - // Since git wire protocol has been released from git v2.18 - if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { - format += ", Wire Protocol %s Enabled" - args = append(args, "Version 2") // for focus color - } - - return fmt.Sprintf(format, args...) -} - -func checkInit() error { - if setting.Git.HomePath == "" { - return errors.New("unable to init Git's HomeDir, incorrect initialization of the setting and git modules") - } - if DefaultContext != nil { - log.Warn("git module has been initialized already, duplicate init may work but it's better to fix it") + if err := checkGitVersionCompatibility(DefaultFeatures().gitVersion); err != nil { + return fmt.Errorf("installed git version %s has a known compatibility issue with Gitea: %w, please upgrade (or downgrade) git", DefaultFeatures().gitVersion.String(), err) } return nil } @@ -154,8 +143,12 @@ func HomeDir() string { // InitSimple initializes git module with a very simple step, no config changes, no global command arguments. // This method doesn't change anything to filesystem. At the moment, it is only used by some Gitea sub-commands. func InitSimple(ctx context.Context) error { - if err := checkInit(); err != nil { - return err + if setting.Git.HomePath == "" { + return errors.New("unable to init Git's HomeDir, incorrect initialization of the setting and git modules") + } + + if DefaultContext != nil && (!setting.IsProd || setting.IsInTesting) { + log.Warn("git module has been initialized already, duplicate init may work but it's better to fix it") } DefaultContext = ctx @@ -165,13 +158,16 @@ func InitSimple(ctx context.Context) error { defaultCommandExecutionTimeout = time.Duration(setting.Git.Timeout.Default) * time.Second } - return SetExecutablePath(setting.Git.Path) -} + if err := SetExecutablePath(setting.Git.Path); err != nil { + return err + } -// InitFull initializes git module with version check and change global variables, sync gitconfig. -// It should only be called once at the beginning of the program initialization (TestMain/GlobalInitInstalled) as this code makes unsynchronized changes to variables. -func InitFull(ctx context.Context) (err error) { - if err = InitSimple(ctx); err != nil { + var err error + defaultFeatures, err = loadGitVersionFeatures() + if err != nil { + return err + } + if err = ensureGitVersion(); err != nil { return err } @@ -179,26 +175,28 @@ func InitFull(ctx context.Context) (err error) { if _, ok := os.LookupEnv("GNUPGHOME"); !ok { _ = os.Setenv("GNUPGHOME", filepath.Join(HomeDir(), ".gnupg")) } + return nil +} + +// InitFull initializes git module with version check and change global variables, sync gitconfig. +// It should only be called once at the beginning of the program initialization (TestMain/GlobalInitInstalled) as this code makes unsynchronized changes to variables. +func InitFull(ctx context.Context) (err error) { + if err = InitSimple(ctx); err != nil { + return err + } // Since git wire protocol has been released from git v2.18 - if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { + if setting.Git.EnableAutoGitWireProtocol && DefaultFeatures().CheckVersionAtLeast("2.18") { globalCommandArgs = append(globalCommandArgs, "-c", "protocol.version=2") } // Explicitly disable credential helper, otherwise Git credentials might leak - if CheckGitVersionAtLeast("2.9") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.9") { globalCommandArgs = append(globalCommandArgs, "-c", "credential.helper=") } - DefaultFeatures.SupportProcReceive = CheckGitVersionAtLeast("2.29") == nil - DefaultFeatures.SupportHashSha256 = CheckGitVersionAtLeast("2.42") == nil && !isGogit - if DefaultFeatures.SupportHashSha256 { - SupportedObjectFormats = append(SupportedObjectFormats, Sha256ObjectFormat) - } else { - log.Warn("sha256 hash support is disabled - requires Git >= 2.42. Gogit is currently unsupported") - } if setting.LFS.StartServer { - if CheckGitVersionAtLeast("2.1.2") != nil { + if !DefaultFeatures().CheckVersionAtLeast("2.1.2") { return errors.New("LFS server support requires Git >= 2.1.2") } globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") @@ -238,13 +236,13 @@ func syncGitConfig() (err error) { return err } - if CheckGitVersionAtLeast("2.10") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.10") { if err := configSet("receive.advertisePushOptions", "true"); err != nil { return err } } - if CheckGitVersionAtLeast("2.18") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.18") { if err := configSet("core.commitGraph", "true"); err != nil { return err } @@ -256,7 +254,7 @@ func syncGitConfig() (err error) { } } - if DefaultFeatures.SupportProcReceive { + if DefaultFeatures().SupportProcReceive { // set support for AGit flow if err := configAddNonExist("receive.procReceiveRefs", "refs/for"); err != nil { return err @@ -294,7 +292,7 @@ func syncGitConfig() (err error) { } // By default partial clones are disabled, enable them from git v2.22 - if !setting.Git.DisablePartialClone && CheckGitVersionAtLeast("2.22") == nil { + if !setting.Git.DisablePartialClone && DefaultFeatures().CheckVersionAtLeast("2.22") { if err = configSet("uploadpack.allowfilter", "true"); err != nil { return err } @@ -309,21 +307,6 @@ func syncGitConfig() (err error) { return err } -// CheckGitVersionAtLeast check git version is at least the constraint version -func CheckGitVersionAtLeast(atLeast string) error { - if DefaultFeatures.GitVersion == nil { - panic("git module is not initialized") // it shouldn't happen - } - atLeastVersion, err := version.NewVersion(atLeast) - if err != nil { - return err - } - if DefaultFeatures.GitVersion.Compare(atLeastVersion) < 0 { - return fmt.Errorf("installed git binary version %s is not at least %s", DefaultFeatures.GitVersion.Original(), atLeast) - } - return nil -} - func checkGitVersionCompatibility(gitVer *version.Version) error { badVersions := []struct { Version *version.Version diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 3de9ff8cf456..242d782e1778 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -120,12 +120,8 @@ var ( Sha256ObjectFormat ObjectFormat = Sha256ObjectFormatImpl{} ) -var SupportedObjectFormats = []ObjectFormat{ - Sha1ObjectFormat, -} - func ObjectFormatFromName(name string) ObjectFormat { - for _, objectFormat := range SupportedObjectFormats { + for _, objectFormat := range DefaultFeatures().SupportedObjectFormats { if name == objectFormat.Name() { return objectFormat } diff --git a/modules/git/object_id.go b/modules/git/object_id.go index 33e5085005d2..82d30184dff7 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -54,7 +54,7 @@ func (*Sha256Hash) Type() ObjectFormat { return Sha256ObjectFormat } func NewIDFromString(hexHash string) (ObjectID, error) { var theObjectFormat ObjectFormat - for _, objectFormat := range SupportedObjectFormats { + for _, objectFormat := range DefaultFeatures().SupportedObjectFormats { if len(hexHash) == objectFormat.FullLength() { theObjectFormat = objectFormat break diff --git a/modules/git/remote.go b/modules/git/remote.go index 3585313f6ae3..7b10e6b66315 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -12,7 +12,7 @@ import ( // GetRemoteAddress returns remote url of git repository in the repoPath with special remote name func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (string, error) { var cmd *Command - if CheckGitVersionAtLeast("2.7") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.7") { cmd = NewCommand(ctx, "remote", "get-url").AddDynamicArguments(remoteName) } else { cmd = NewCommand(ctx, "config", "--get").AddDynamicArguments("remote." + remoteName + ".url") diff --git a/modules/git/repo.go b/modules/git/repo.go index c5ba5117a786..1c223018addd 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -74,7 +74,7 @@ func InitRepository(ctx context.Context, repoPath string, bare bool, objectForma if !IsValidObjectFormat(objectFormatName) { return fmt.Errorf("invalid object format: %s", objectFormatName) } - if DefaultFeatures.SupportHashSha256 { + if DefaultFeatures().SupportHashSha256 { cmd.AddOptionValues("--object-format", objectFormatName) } diff --git a/modules/git/repo_base.go b/modules/git/repo_base.go deleted file mode 100644 index 6c148d9af516..000000000000 --- a/modules/git/repo_base.go +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright 2021 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package git - -var isGogit bool diff --git a/modules/git/repo_base_gogit.go b/modules/git/repo_base_gogit.go index 0cd07dcdc897..a1127f4e6c5b 100644 --- a/modules/git/repo_base_gogit.go +++ b/modules/git/repo_base_gogit.go @@ -22,9 +22,7 @@ import ( "github.com/go-git/go-git/v5/storage/filesystem" ) -func init() { - isGogit = true -} +const isGogit = true // Repository represents a Git repository. type Repository struct { diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go index 5511526e7821..bc241cdd7950 100644 --- a/modules/git/repo_base_nogogit.go +++ b/modules/git/repo_base_nogogit.go @@ -15,9 +15,7 @@ import ( "code.gitea.io/gitea/modules/util" ) -func init() { - isGogit = false -} +const isGogit = false // Repository represents a Git repository. type Repository struct { diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index f9168bef7eec..8c3285769e4f 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -438,7 +438,7 @@ func (repo *Repository) getCommitsBeforeLimit(id ObjectID, num int) ([]*Commit, } func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) { - if CheckGitVersionAtLeast("2.7.0") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.7.0") { stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", "--format=%(refname:strip=2)"). AddOptionFormat("--count=%d", limit). AddOptionValues("--contains", commit.ID.String(), BranchPrefix). diff --git a/modules/git/repo_commitgraph.go b/modules/git/repo_commitgraph.go index 492438be3791..087d5bcec4c2 100644 --- a/modules/git/repo_commitgraph.go +++ b/modules/git/repo_commitgraph.go @@ -11,7 +11,7 @@ import ( // WriteCommitGraph write commit graph to speed up repo access // this requires git v2.18 to be installed func WriteCommitGraph(ctx context.Context, repoPath string) error { - if CheckGitVersionAtLeast("2.18") == nil { + if DefaultFeatures().CheckVersionAtLeast("2.18") { if _, _, err := NewCommand(ctx, "commit-graph", "write").RunStdString(&RunOpts{Dir: repoPath}); err != nil { return fmt.Errorf("unable to write commit-graph for '%s' : %w", repoPath, err) } diff --git a/modules/lfs/pointer_scanner_nogogit.go b/modules/lfs/pointer_scanner_nogogit.go index 658b98feabd8..c37a93e73bfd 100644 --- a/modules/lfs/pointer_scanner_nogogit.go +++ b/modules/lfs/pointer_scanner_nogogit.go @@ -41,7 +41,7 @@ func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan c go pipeline.BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) // 1. Run batch-check on all objects in the repository - if git.CheckGitVersionAtLeast("2.6.0") != nil { + if !git.DefaultFeatures().CheckVersionAtLeast("2.6.0") { revListReader, revListWriter := io.Pipe() shasToCheckReader, shasToCheckWriter := io.Pipe() wg.Add(2) diff --git a/routers/init.go b/routers/init.go index 030ef3c740d8..56c95cd1cab7 100644 --- a/routers/init.go +++ b/routers/init.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/system" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web/routing" actions_router "code.gitea.io/gitea/routers/api/actions" @@ -112,7 +113,10 @@ func InitWebInstallPage(ctx context.Context) { // InitWebInstalled is for global installed configuration. func InitWebInstalled(ctx context.Context) { mustInitCtx(ctx, git.InitFull) - log.Info("Git version: %s (home: %s)", git.VersionInfo(), git.HomeDir()) + log.Info("Git version: %s (home: %s)", git.DefaultFeatures().VersionInfo(), git.HomeDir()) + if !git.DefaultFeatures().SupportHashSha256 { + log.Warn("sha256 hash support is disabled - requires Git >= 2.42." + util.Iif(git.DefaultFeatures().UsingGogit, " Gogit is currently unsupported.", "")) + } // Setup i18n translation.InitLocales(ctx) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index f35eb77d42e1..efdbc114ef4b 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -122,7 +122,7 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { preReceiveBranch(ourCtx, oldCommitID, newCommitID, refFullName) case refFullName.IsTag(): preReceiveTag(ourCtx, oldCommitID, newCommitID, refFullName) - case git.DefaultFeatures.SupportProcReceive && refFullName.IsFor(): + case git.DefaultFeatures().SupportProcReceive && refFullName.IsFor(): preReceiveFor(ourCtx, oldCommitID, newCommitID, refFullName) default: ourCtx.AssertCanWriteCode() diff --git a/routers/private/hook_proc_receive.go b/routers/private/hook_proc_receive.go index cee3bbdd1212..efb3f5831e42 100644 --- a/routers/private/hook_proc_receive.go +++ b/routers/private/hook_proc_receive.go @@ -18,7 +18,7 @@ import ( // HookProcReceive proc-receive hook - only handles agit Proc-Receive requests at present func HookProcReceive(ctx *gitea_context.PrivateContext) { opts := web.GetForm(ctx).(*private.HookOptions) - if !git.DefaultFeatures.SupportProcReceive { + if !git.DefaultFeatures().SupportProcReceive { ctx.Status(http.StatusNotFound) return } diff --git a/routers/private/serv.go b/routers/private/serv.go index 85368a0aed4c..1c309865d7df 100644 --- a/routers/private/serv.go +++ b/routers/private/serv.go @@ -297,7 +297,7 @@ func ServCommand(ctx *context.PrivateContext) { } } else { // Because of the special ref "refs/for" we will need to delay write permission check - if git.DefaultFeatures.SupportProcReceive && unitType == unit.TypeCode { + if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode { mode = perm.AccessModeRead } diff --git a/routers/web/admin/config.go b/routers/web/admin/config.go index 48f80dbbf1ca..fd8c73b62d8d 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -112,7 +112,7 @@ func Config(ctx *context.Context) { ctx.Data["OfflineMode"] = setting.OfflineMode ctx.Data["RunUser"] = setting.RunUser ctx.Data["RunMode"] = util.ToTitleCase(setting.RunMode) - ctx.Data["GitVersion"] = git.VersionInfo() + ctx.Data["GitVersion"] = git.DefaultFeatures().VersionInfo() ctx.Data["AppDataPath"] = setting.AppDataPath ctx.Data["RepoRootPath"] = setting.RepoRootPath diff --git a/routers/web/misc/misc.go b/routers/web/misc/misc.go index ac5496ce91ae..caaca7f5211c 100644 --- a/routers/web/misc/misc.go +++ b/routers/web/misc/misc.go @@ -15,7 +15,7 @@ import ( ) func SSHInfo(rw http.ResponseWriter, req *http.Request) { - if !git.DefaultFeatures.SupportProcReceive { + if !git.DefaultFeatures().SupportProcReceive { rw.WriteHeader(http.StatusNotFound) return } diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 8fb6d930688f..f0579b56ea13 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -183,7 +183,7 @@ func httpBase(ctx *context.Context) *serviceHandler { if repoExist { // Because of special ref "refs/for" .. , need delay write permission check - if git.DefaultFeatures.SupportProcReceive { + if git.DefaultFeatures().SupportProcReceive { accessMode = perm.AccessModeRead } diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 4e448933c794..48be1c22968f 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -180,7 +180,7 @@ func Create(ctx *context.Context) { ctx.Data["CanCreateRepo"] = ctx.Doer.CanCreateRepo() ctx.Data["MaxCreationLimit"] = ctx.Doer.MaxCreationLimit() - ctx.Data["SupportedObjectFormats"] = git.SupportedObjectFormats + ctx.Data["SupportedObjectFormats"] = git.DefaultFeatures().SupportedObjectFormats ctx.Data["DefaultObjectFormat"] = git.Sha1ObjectFormat ctx.HTML(http.StatusOK, tplCreate) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index d115686491a4..3a35d24dfffd 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1143,7 +1143,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi // so if we are using at least this version of git we don't have to tell ParsePatch to do // the skipping for us parsePatchSkipToFile := opts.SkipTo - if opts.SkipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil { + if opts.SkipTo != "" && git.DefaultFeatures().CheckVersionAtLeast("2.31") { cmdDiff.AddOptionFormat("--skip-to=%s", opts.SkipTo) parsePatchSkipToFile = "" } diff --git a/services/pull/patch.go b/services/pull/patch.go index 12b79a06253d..981bc989fca4 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -383,7 +383,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * cmdApply.AddArguments("--ignore-whitespace") } is3way := false - if git.CheckGitVersionAtLeast("2.32.0") == nil { + if git.DefaultFeatures().CheckVersionAtLeast("2.32.0") { cmdApply.AddArguments("--3way") is3way = true } diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index 36bdbde55c7a..e5753178b898 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -104,7 +104,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) baseBranch := "base" fetchArgs := git.TrustedCmdArgs{"--no-tags"} - if git.CheckGitVersionAtLeast("2.25.0") == nil { + if git.DefaultFeatures().CheckVersionAtLeast("2.25.0") { // Writing the commit graph can be slow and is not needed here fetchArgs = append(fetchArgs, "--no-write-commit-graph") } diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index e5f7e2af965d..ab0e7ffd36fd 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -148,7 +148,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user stderr := &strings.Builder{} cmdApply := git.NewCommand(ctx, "apply", "--index", "--recount", "--cached", "--ignore-whitespace", "--whitespace=fix", "--binary") - if git.CheckGitVersionAtLeast("2.32") == nil { + if git.DefaultFeatures().CheckVersionAtLeast("2.32") { cmdApply.AddArguments("-3") } diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 818e1fa65350..7f87aeda4bf4 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -698,7 +698,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB defer tests.PrintCurrentTest(t)() // skip this test if git version is low - if git.CheckGitVersionAtLeast("2.29") != nil { + if !git.DefaultFeatures().SupportProcReceive { return } From 216c8eada3c0727288dc5565ae9fdd798b17c463 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Tue, 7 May 2024 21:59:00 +0800 Subject: [PATCH 04/58] Fix missing migrate actions artifacts (#30874) (#30886) Backport #30874 by @lunny The actions artifacts should be able to be migrate to the new storage place. Co-authored-by: Lunny Xiao --- cmd/migrate_storage.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go index aa49445a89e8..357416fc339f 100644 --- a/cmd/migrate_storage.go +++ b/cmd/migrate_storage.go @@ -34,7 +34,7 @@ var CmdMigrateStorage = &cli.Command{ Name: "type", Aliases: []string{"t"}, Value: "", - Usage: "Type of stored files to copy. Allowed types: 'attachments', 'lfs', 'avatars', 'repo-avatars', 'repo-archivers', 'packages', 'actions-log'", + Usage: "Type of stored files to copy. Allowed types: 'attachments', 'lfs', 'avatars', 'repo-avatars', 'repo-archivers', 'packages', 'actions-log', 'actions-artifacts", }, &cli.StringFlag{ Name: "storage", @@ -160,6 +160,13 @@ func migrateActionsLog(ctx context.Context, dstStorage storage.ObjectStorage) er }) } +func migrateActionsArtifacts(ctx context.Context, dstStorage storage.ObjectStorage) error { + return db.Iterate(ctx, nil, func(ctx context.Context, artifact *actions_model.ActionArtifact) error { + _, err := storage.Copy(dstStorage, artifact.ArtifactPath, storage.ActionsArtifacts, artifact.ArtifactPath) + return err + }) +} + func runMigrateStorage(ctx *cli.Context) error { stdCtx, cancel := installSignals() defer cancel() @@ -223,13 +230,14 @@ func runMigrateStorage(ctx *cli.Context) error { } migratedMethods := map[string]func(context.Context, storage.ObjectStorage) error{ - "attachments": migrateAttachments, - "lfs": migrateLFS, - "avatars": migrateAvatars, - "repo-avatars": migrateRepoAvatars, - "repo-archivers": migrateRepoArchivers, - "packages": migratePackages, - "actions-log": migrateActionsLog, + "attachments": migrateAttachments, + "lfs": migrateLFS, + "avatars": migrateAvatars, + "repo-avatars": migrateRepoAvatars, + "repo-archivers": migrateRepoArchivers, + "packages": migratePackages, + "actions-log": migrateActionsLog, + "actions-artifacts": migrateActionsArtifacts, } tp := strings.ToLower(ctx.String("type")) From d410e2acce22e5b3518a9bf64a9152b32a91fe18 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 7 May 2024 18:35:02 +0200 Subject: [PATCH 05/58] Repository explore alphabetically order respect owner name (#30882) similar to #30784 but only for the repo explore page is covered by #30876 for the main branch --- routers/web/explore/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/web/explore/repo.go b/routers/web/explore/repo.go index 66477a255ccc..764c226d40ba 100644 --- a/routers/web/explore/repo.go +++ b/routers/web/explore/repo.go @@ -71,9 +71,9 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { case "leastupdate": orderBy = db.SearchOrderByLeastUpdated case "reversealphabetically": - orderBy = db.SearchOrderByAlphabeticallyReverse + orderBy = "owner_name DESC, name DESC" case "alphabetically": - orderBy = db.SearchOrderByAlphabetically + orderBy = "owner_name ASC, name ASC" case "reversesize": orderBy = db.SearchOrderBySizeReverse case "size": From d4c2db39bf239d071c6ac96815efa8fd8b03ad94 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 May 2024 21:34:43 +0800 Subject: [PATCH 06/58] Refactor AppURL usage (#30885) (#30891) Backport #30885 Fix #30883 Fix #29591 Co-authored-by: KN4CK3R --- models/repo/avatar.go | 12 ++--- models/user/avatar.go | 10 ++-- modules/httplib/url.go | 60 ++++++++++++++++++++- modules/httplib/url_test.go | 59 +++++++++++++++++--- modules/markup/html_codepreview.go | 2 +- routers/api/actions/artifacts.go | 11 ++-- routers/api/actions/artifactsv4.go | 9 ++-- routers/api/packages/container/container.go | 3 +- routers/common/middleware.go | 3 ++ routers/common/redirect.go | 2 +- routers/web/auth/auth.go | 2 +- services/context/base.go | 2 +- services/context/context_response.go | 2 +- 13 files changed, 138 insertions(+), 39 deletions(-) diff --git a/models/repo/avatar.go b/models/repo/avatar.go index 72ee938adae4..8395b8c2b741 100644 --- a/models/repo/avatar.go +++ b/models/repo/avatar.go @@ -9,10 +9,10 @@ import ( "image/png" "io" "net/url" - "strings" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/avatar" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" @@ -84,13 +84,7 @@ func (repo *Repository) relAvatarLink(ctx context.Context) string { return setting.AppSubURL + "/repo-avatars/" + url.PathEscape(repo.Avatar) } -// AvatarLink returns a link to the repository's avatar. +// AvatarLink returns the full avatar url with http host. TODO: refactor it to a relative URL, but it is still used in API response at the moment func (repo *Repository) AvatarLink(ctx context.Context) string { - link := repo.relAvatarLink(ctx) - // we only prepend our AppURL to our known (relative, internal) avatar link to get an absolute URL - if strings.HasPrefix(link, "/") && !strings.HasPrefix(link, "//") { - return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL)[1:] - } - // otherwise, return the link as it is - return link + return httplib.MakeAbsoluteURL(ctx, repo.relAvatarLink(ctx)) } diff --git a/models/user/avatar.go b/models/user/avatar.go index c6937d7b51f7..921bc1b1a1ce 100644 --- a/models/user/avatar.go +++ b/models/user/avatar.go @@ -9,11 +9,11 @@ import ( "fmt" "image/png" "io" - "strings" "code.gitea.io/gitea/models/avatars" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/avatar" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" @@ -89,13 +89,9 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string { return avatars.GenerateEmailAvatarFastLink(ctx, u.AvatarEmail, size) } -// AvatarLink returns the full avatar link with http host +// AvatarLink returns the full avatar url with http host. TODO: refactor it to a relative URL, but it is still used in API response at the moment func (u *User) AvatarLink(ctx context.Context) string { - link := u.AvatarLinkWithSize(ctx, 0) - if !strings.HasPrefix(link, "//") && !strings.Contains(link, "://") { - return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL+"/") - } - return link + return httplib.MakeAbsoluteURL(ctx, u.AvatarLinkWithSize(ctx, 0)) } // IsUploadAvatarChanged returns true if the current user's avatar would be changed with the provided data diff --git a/modules/httplib/url.go b/modules/httplib/url.go index 903799cb6886..541c4f325bb6 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -4,6 +4,8 @@ package httplib import ( + "context" + "net/http" "net/url" "strings" @@ -11,6 +13,10 @@ import ( "code.gitea.io/gitea/modules/util" ) +type RequestContextKeyStruct struct{} + +var RequestContextKey = RequestContextKeyStruct{} + func urlIsRelative(s string, u *url.URL) bool { // Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH" // Therefore we should ignore these redirect locations to prevent open redirects @@ -26,7 +32,56 @@ func IsRelativeURL(s string) bool { return err == nil && urlIsRelative(s, u) } -func IsCurrentGiteaSiteURL(s string) bool { +func guessRequestScheme(req *http.Request, def string) string { + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto + if s := req.Header.Get("X-Forwarded-Proto"); s != "" { + return s + } + if s := req.Header.Get("X-Forwarded-Protocol"); s != "" { + return s + } + if s := req.Header.Get("X-Url-Scheme"); s != "" { + return s + } + if s := req.Header.Get("Front-End-Https"); s != "" { + return util.Iif(s == "on", "https", "http") + } + if s := req.Header.Get("X-Forwarded-Ssl"); s != "" { + return util.Iif(s == "on", "https", "http") + } + return def +} + +func guessForwardedHost(req *http.Request) string { + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host + return req.Header.Get("X-Forwarded-Host") +} + +// GuessCurrentAppURL tries to guess the current full URL by http headers. It always has a '/' suffix, exactly the same as setting.AppURL +func GuessCurrentAppURL(ctx context.Context) string { + req, ok := ctx.Value(RequestContextKey).(*http.Request) + if !ok { + return setting.AppURL + } + if host := guessForwardedHost(req); host != "" { + // if it is behind a reverse proxy, use "https" as default scheme in case the site admin forgets to set the correct forwarded-protocol headers + return guessRequestScheme(req, "https") + "://" + host + setting.AppSubURL + "/" + } else if req.Host != "" { + // if it is not behind a reverse proxy, use the scheme from config options, meanwhile use "https" as much as possible + defaultScheme := util.Iif(setting.Protocol == "http", "http", "https") + return guessRequestScheme(req, defaultScheme) + "://" + req.Host + setting.AppSubURL + "/" + } + return setting.AppURL +} + +func MakeAbsoluteURL(ctx context.Context, s string) string { + if IsRelativeURL(s) { + return GuessCurrentAppURL(ctx) + strings.TrimPrefix(s, "/") + } + return s +} + +func IsCurrentGiteaSiteURL(ctx context.Context, s string) bool { u, err := url.Parse(s) if err != nil { return false @@ -45,5 +100,6 @@ func IsCurrentGiteaSiteURL(s string) bool { if u.Path == "" { u.Path = "/" } - return strings.HasPrefix(strings.ToLower(u.String()), strings.ToLower(setting.AppURL)) + urlLower := strings.ToLower(u.String()) + return strings.HasPrefix(urlLower, strings.ToLower(setting.AppURL)) || strings.HasPrefix(urlLower, strings.ToLower(GuessCurrentAppURL(ctx))) } diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index 9bf09bcf2f3e..e021cd610d3f 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -4,6 +4,8 @@ package httplib import ( + "context" + "net/http" "testing" "code.gitea.io/gitea/modules/setting" @@ -37,9 +39,44 @@ func TestIsRelativeURL(t *testing.T) { } } +func TestMakeAbsoluteURL(t *testing.T) { + defer test.MockVariableValue(&setting.Protocol, "http")() + defer test.MockVariableValue(&setting.AppURL, "http://the-host/sub/")() + defer test.MockVariableValue(&setting.AppSubURL, "/sub")() + + ctx := context.Background() + assert.Equal(t, "http://the-host/sub/", MakeAbsoluteURL(ctx, "")) + assert.Equal(t, "http://the-host/sub/foo", MakeAbsoluteURL(ctx, "foo")) + assert.Equal(t, "http://the-host/sub/foo", MakeAbsoluteURL(ctx, "/foo")) + assert.Equal(t, "http://other/foo", MakeAbsoluteURL(ctx, "http://other/foo")) + + ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + Host: "user-host", + }) + assert.Equal(t, "http://user-host/sub/foo", MakeAbsoluteURL(ctx, "/foo")) + + ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + Host: "user-host", + Header: map[string][]string{ + "X-Forwarded-Host": {"forwarded-host"}, + }, + }) + assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo")) + + ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + Host: "user-host", + Header: map[string][]string{ + "X-Forwarded-Host": {"forwarded-host"}, + "X-Forwarded-Proto": {"https"}, + }, + }) + assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo")) +} + func TestIsCurrentGiteaSiteURL(t *testing.T) { defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")() defer test.MockVariableValue(&setting.AppSubURL, "/sub")() + ctx := context.Background() good := []string{ "?key=val", "/sub", @@ -50,7 +87,7 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) { "http://localhost:3000/sub/", } for _, s := range good { - assert.True(t, IsCurrentGiteaSiteURL(s), "good = %q", s) + assert.True(t, IsCurrentGiteaSiteURL(ctx, s), "good = %q", s) } bad := []string{ ".", @@ -64,13 +101,23 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) { "http://other/", } for _, s := range bad { - assert.False(t, IsCurrentGiteaSiteURL(s), "bad = %q", s) + assert.False(t, IsCurrentGiteaSiteURL(ctx, s), "bad = %q", s) } setting.AppURL = "http://localhost:3000/" setting.AppSubURL = "" - assert.False(t, IsCurrentGiteaSiteURL("//")) - assert.False(t, IsCurrentGiteaSiteURL("\\\\")) - assert.False(t, IsCurrentGiteaSiteURL("http://localhost")) - assert.True(t, IsCurrentGiteaSiteURL("http://localhost:3000?key=val")) + assert.False(t, IsCurrentGiteaSiteURL(ctx, "//")) + assert.False(t, IsCurrentGiteaSiteURL(ctx, "\\\\")) + assert.False(t, IsCurrentGiteaSiteURL(ctx, "http://localhost")) + assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000?key=val")) + + ctx = context.WithValue(ctx, RequestContextKey, &http.Request{ + Host: "user-host", + Header: map[string][]string{ + "X-Forwarded-Host": {"forwarded-host"}, + "X-Forwarded-Proto": {"https"}, + }, + }) + assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000")) + assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host")) } diff --git a/modules/markup/html_codepreview.go b/modules/markup/html_codepreview.go index 5ef2217e3d76..5ab9290b3e42 100644 --- a/modules/markup/html_codepreview.go +++ b/modules/markup/html_codepreview.go @@ -42,7 +42,7 @@ func renderCodeBlock(ctx *RenderContext, node *html.Node) (urlPosStart, urlPosSt CommitID: node.Data[m[6]:m[7]], FilePath: node.Data[m[8]:m[9]], } - if !httplib.IsCurrentGiteaSiteURL(opts.FullURL) { + if !httplib.IsCurrentGiteaSiteURL(ctx.Ctx, opts.FullURL) { return 0, 0, "", nil } u, err := url.Parse(opts.FilePath) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 5bd004bd374a..35e3ee690683 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -71,6 +71,7 @@ import ( "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -184,8 +185,8 @@ type artifactRoutes struct { fs storage.ObjectStorage } -func (ar artifactRoutes) buildArtifactURL(runID int64, artifactHash, suffix string) string { - uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") + +func (ar artifactRoutes) buildArtifactURL(ctx *ArtifactContext, runID int64, artifactHash, suffix string) string { + uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(ar.prefix, "/") + strings.ReplaceAll(artifactRouteBase, "{run_id}", strconv.FormatInt(runID, 10)) + "/" + artifactHash + "/" + suffix return uploadURL @@ -224,7 +225,7 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *ArtifactContext) { // use md5(artifact_name) to create upload url artifactHash := fmt.Sprintf("%x", md5.Sum([]byte(req.Name))) resp := getUploadArtifactResponse{ - FileContainerResourceURL: ar.buildArtifactURL(runID, artifactHash, "upload"+retentionQuery), + FileContainerResourceURL: ar.buildArtifactURL(ctx, runID, artifactHash, "upload"+retentionQuery), } log.Debug("[artifact] get upload url: %s", resp.FileContainerResourceURL) ctx.JSON(http.StatusOK, resp) @@ -365,7 +366,7 @@ func (ar artifactRoutes) listArtifacts(ctx *ArtifactContext) { artifactHash := fmt.Sprintf("%x", md5.Sum([]byte(art.ArtifactName))) item := listArtifactsResponseItem{ Name: art.ArtifactName, - FileContainerResourceURL: ar.buildArtifactURL(runID, artifactHash, "download_url"), + FileContainerResourceURL: ar.buildArtifactURL(ctx, runID, artifactHash, "download_url"), } items = append(items, item) values[art.ArtifactName] = true @@ -437,7 +438,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) { } } if downloadURL == "" { - downloadURL = ar.buildArtifactURL(runID, strconv.FormatInt(artifact.ID, 10), "download") + downloadURL = ar.buildArtifactURL(ctx, runID, strconv.FormatInt(artifact.ID, 10), "download") } item := downloadArtifactResponseItem{ Path: util.PathJoinRel(itemPath, artifact.ArtifactPath), diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 8300989c75ce..dde9caf4f26b 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -92,6 +92,7 @@ import ( "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" @@ -160,9 +161,9 @@ func (r artifactV4Routes) buildSignature(endp, expires, artifactName string, tas return mac.Sum(nil) } -func (r artifactV4Routes) buildArtifactURL(endp, artifactName string, taskID int64) string { +func (r artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endp, artifactName string, taskID int64) string { expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") - uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(r.prefix, "/") + + uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(r.prefix, "/") + "/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(r.buildSignature(endp, expires, artifactName, taskID)) + "&expires=" + url.QueryEscape(expires) + "&artifactName=" + url.QueryEscape(artifactName) + "&taskID=" + fmt.Sprint(taskID) return uploadURL } @@ -278,7 +279,7 @@ func (r *artifactV4Routes) createArtifact(ctx *ArtifactContext) { respData := CreateArtifactResponse{ Ok: true, - SignedUploadUrl: r.buildArtifactURL("UploadArtifact", artifactName, ctx.ActionTask.ID), + SignedUploadUrl: r.buildArtifactURL(ctx, "UploadArtifact", artifactName, ctx.ActionTask.ID), } r.sendProtbufBody(ctx, &respData) } @@ -454,7 +455,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { } } if respData.SignedUrl == "" { - respData.SignedUrl = r.buildArtifactURL("DownloadArtifact", artifactName, ctx.ActionTask.ID) + respData.SignedUrl = r.buildArtifactURL(ctx, "DownloadArtifact", artifactName, ctx.ActionTask.ID) } r.sendProtbufBody(ctx, &respData) } diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 2cb16daebc57..1efd166eb3e6 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -17,6 +17,7 @@ import ( packages_model "code.gitea.io/gitea/models/packages" container_model "code.gitea.io/gitea/models/packages/container" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" packages_module "code.gitea.io/gitea/modules/packages" @@ -115,7 +116,7 @@ func apiErrorDefined(ctx *context.Context, err *namedError) { } func apiUnauthorizedError(ctx *context.Context) { - ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+setting.AppURL+`v2/token",service="container_registry",scope="*"`) + ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+httplib.GuessCurrentAppURL(ctx)+`v2/token",service="container_registry",scope="*"`) apiErrorDefined(ctx, errUnauthorized) } diff --git a/routers/common/middleware.go b/routers/common/middleware.go index c7c75fb099b4..8b661993bb34 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -4,11 +4,13 @@ package common import ( + go_context "context" "fmt" "net/http" "strings" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" @@ -34,6 +36,7 @@ func ProtocolMiddlewares() (handlers []any) { } }() req = req.WithContext(middleware.WithContextData(req.Context())) + req = req.WithContext(go_context.WithValue(req.Context(), httplib.RequestContextKey, req)) next.ServeHTTP(resp, req) }) }) diff --git a/routers/common/redirect.go b/routers/common/redirect.go index 34044e814bc6..d64f74ec82af 100644 --- a/routers/common/redirect.go +++ b/routers/common/redirect.go @@ -17,7 +17,7 @@ func FetchRedirectDelegate(resp http.ResponseWriter, req *http.Request) { // The typical page is "issue comment" page. The backend responds "/owner/repo/issues/1#comment-2", // then frontend needs this delegate to redirect to the new location with hash correctly. redirect := req.PostFormValue("redirect") - if !httplib.IsCurrentGiteaSiteURL(redirect) { + if !httplib.IsCurrentGiteaSiteURL(req.Context(), redirect) { resp.WriteHeader(http.StatusBadRequest) return } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 7c873796fe40..4083d64226cf 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -368,7 +368,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe return setting.AppSubURL + "/" } - if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(redirectTo) { + if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(ctx, redirectTo) { middleware.DeleteRedirectToCookie(ctx.Resp) if obeyRedirect { ctx.RedirectToCurrentSite(redirectTo) diff --git a/services/context/base.go b/services/context/base.go index 05b8ab1b9be0..29e62ae38924 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -254,7 +254,7 @@ func (b *Base) Redirect(location string, status ...int) { code = status[0] } - if strings.HasPrefix(location, "http://") || strings.HasPrefix(location, "https://") || strings.HasPrefix(location, "//") { + if !httplib.IsRelativeURL(location) { // Some browsers (Safari) have buggy behavior for Cookie + Cache + External Redirection, eg: /my-path => https://other/path // 1. the first request to "/my-path" contains cookie // 2. some time later, the request to "/my-path" doesn't contain cookie (caused by Prevent web tracking) diff --git a/services/context/context_response.go b/services/context/context_response.go index 87c34c35edeb..c43a649b49e1 100644 --- a/services/context/context_response.go +++ b/services/context/context_response.go @@ -52,7 +52,7 @@ func (ctx *Context) RedirectToCurrentSite(location ...string) { continue } - if !httplib.IsCurrentGiteaSiteURL(loc) { + if !httplib.IsCurrentGiteaSiteURL(ctx, loc) { continue } From ec3f5f9992d7ff8250c044a4467524d53bd50210 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 8 May 2024 22:17:18 +0800 Subject: [PATCH 07/58] Move database operations of merging a pull request to post receive hook and add a transaction (#30805) (#30888) Backport #30805 by @lunny Merging PR may fail because of various problems. The pull request may have a dirty state because there is no transaction when merging a pull request. ref https://github.com/go-gitea/gitea/pull/25741#issuecomment-2074126393 This PR moves all database update operations to post-receive handler for merging a pull request and having a database transaction. That means if database operations fail, then the git merging will fail, the git client will get a fail result. There are already many tests for pull request merging, so we don't need to add a new one. Co-authored-by: Lunny Xiao Co-authored-by: wxiaoguang --- cmd/hook.go | 3 ++ modules/private/hook.go | 2 + modules/repository/env.go | 8 +++ routers/private/hook_post_receive.go | 64 ++++++++++++++++++++++- routers/private/hook_post_receive_test.go | 49 +++++++++++++++++ services/contexttest/context_tests.go | 13 +++++ services/pull/merge.go | 27 ++++------ services/pull/update.go | 3 +- 8 files changed, 150 insertions(+), 19 deletions(-) create mode 100644 routers/private/hook_post_receive_test.go diff --git a/cmd/hook.go b/cmd/hook.go index 9c1cb66f2a4d..6e31710caff2 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -338,6 +338,7 @@ Gitea or set your environment appropriately.`, "") isWiki, _ := strconv.ParseBool(os.Getenv(repo_module.EnvRepoIsWiki)) repoName := os.Getenv(repo_module.EnvRepoName) pusherID, _ := strconv.ParseInt(os.Getenv(repo_module.EnvPusherID), 10, 64) + prID, _ := strconv.ParseInt(os.Getenv(repo_module.EnvPRID), 10, 64) pusherName := os.Getenv(repo_module.EnvPusherName) hookOptions := private.HookOptions{ @@ -347,6 +348,8 @@ Gitea or set your environment appropriately.`, "") GitObjectDirectory: os.Getenv(private.GitObjectDirectory), GitQuarantinePath: os.Getenv(private.GitQuarantinePath), GitPushOptions: pushOptions(), + PullRequestID: prID, + PushTrigger: repo_module.PushTrigger(os.Getenv(repo_module.EnvPushTrigger)), } oldCommitIDs := make([]string, hookBatchSize) newCommitIDs := make([]string, hookBatchSize) diff --git a/modules/private/hook.go b/modules/private/hook.go index 79c3d482298c..49d92987441c 100644 --- a/modules/private/hook.go +++ b/modules/private/hook.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" ) @@ -54,6 +55,7 @@ type HookOptions struct { GitQuarantinePath string GitPushOptions GitPushOptions PullRequestID int64 + PushTrigger repository.PushTrigger DeployKeyID int64 // if the pusher is a DeployKey, then UserID is the repo's org user. IsWiki bool ActionPerm int diff --git a/modules/repository/env.go b/modules/repository/env.go index 30edd1c9e326..e4f32092fc79 100644 --- a/modules/repository/env.go +++ b/modules/repository/env.go @@ -25,11 +25,19 @@ const ( EnvKeyID = "GITEA_KEY_ID" // public key ID EnvDeployKeyID = "GITEA_DEPLOY_KEY_ID" EnvPRID = "GITEA_PR_ID" + EnvPushTrigger = "GITEA_PUSH_TRIGGER" EnvIsInternal = "GITEA_INTERNAL_PUSH" EnvAppURL = "GITEA_ROOT_URL" EnvActionPerm = "GITEA_ACTION_PERM" ) +type PushTrigger string + +const ( + PushTriggerPRMergeToBase PushTrigger = "pr-merge-to-base" + PushTriggerPRUpdateWithBase PushTrigger = "pr-update-with-base" +) + // InternalPushingEnvironment returns an os environment to switch off hooks on push // It is recommended to avoid using this unless you are pushing within a transaction // or if you absolutely are sure that post-receive and pre-receive will do nothing diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 769a68970d21..b6699e465fcb 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -4,20 +4,25 @@ package private import ( + "context" "fmt" "net/http" + "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" access_model "code.gitea.io/gitea/models/perm/access" + pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/private" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" + timeutil "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" gitea_context "code.gitea.io/gitea/services/context" @@ -160,6 +165,14 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { } } + // handle pull request merging, a pull request action should push at least 1 commit + if opts.PushTrigger == repo_module.PushTriggerPRMergeToBase { + handlePullRequestMerging(ctx, opts, ownerName, repoName, updates) + if ctx.Written() { + return + } + } + isPrivate := opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate) isTemplate := opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate) // Handle Push Options @@ -174,7 +187,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { wasEmpty = repo.IsEmpty } - pusher, err := user_model.GetUserByID(ctx, opts.UserID) + pusher, err := loadContextCacheUser(ctx, opts.UserID) if err != nil { log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ @@ -309,3 +322,52 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { RepoWasEmpty: wasEmpty, }) } + +func loadContextCacheUser(ctx context.Context, id int64) (*user_model.User, error) { + return cache.GetWithContextCache(ctx, "hook_post_receive_user", id, func() (*user_model.User, error) { + return user_model.GetUserByID(ctx, id) + }) +} + +// handlePullRequestMerging handle pull request merging, a pull request action should push at least 1 commit +func handlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.HookOptions, ownerName, repoName string, updates []*repo_module.PushUpdateOptions) { + if len(updates) == 0 { + ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ + Err: fmt.Sprintf("Pushing a merged PR (pr:%d) no commits pushed ", opts.PullRequestID), + }) + return + } + + pr, err := issues_model.GetPullRequestByID(ctx, opts.PullRequestID) + if err != nil { + log.Error("GetPullRequestByID[%d]: %v", opts.PullRequestID, err) + ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "GetPullRequestByID failed"}) + return + } + + pusher, err := loadContextCacheUser(ctx, opts.UserID) + if err != nil { + log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) + ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Load pusher user failed"}) + return + } + + pr.MergedCommitID = updates[len(updates)-1].NewCommitID + pr.MergedUnix = timeutil.TimeStampNow() + pr.Merger = pusher + pr.MergerID = pusher.ID + err = db.WithTx(ctx, func(ctx context.Context) error { + // Removing an auto merge pull and ignore if not exist + if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { + return fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", opts.PullRequestID, err) + } + if _, err := pr.SetMerged(ctx); err != nil { + return fmt.Errorf("SetMerged failed: %s/%s Error: %v", ownerName, repoName, err) + } + return nil + }) + if err != nil { + log.Error("Failed to update PR to merged: %v", err) + ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to update PR to merged"}) + } +} diff --git a/routers/private/hook_post_receive_test.go b/routers/private/hook_post_receive_test.go new file mode 100644 index 000000000000..658557d3cfc0 --- /dev/null +++ b/routers/private/hook_post_receive_test.go @@ -0,0 +1,49 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package private + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + pull_model "code.gitea.io/gitea/models/pull" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/private" + repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/services/contexttest" + + "github.com/stretchr/testify/assert" +) + +func TestHandlePullRequestMerging(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + pr, err := issues_model.GetUnmergedPullRequest(db.DefaultContext, 1, 1, "branch2", "master", issues_model.PullRequestFlowGithub) + assert.NoError(t, err) + assert.NoError(t, pr.LoadBaseRepo(db.DefaultContext)) + + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr") + assert.NoError(t, err) + + autoMerge := unittest.AssertExistsAndLoadBean(t, &pull_model.AutoMerge{PullID: pr.ID}) + + ctx, resp := contexttest.MockPrivateContext(t, "/") + handlePullRequestMerging(ctx, &private.HookOptions{ + PullRequestID: pr.ID, + UserID: 2, + }, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, []*repo_module.PushUpdateOptions{ + {NewCommitID: "01234567"}, + }) + assert.Equal(t, 0, len(resp.Body.String())) + pr, err = issues_model.GetPullRequestByID(db.DefaultContext, pr.ID) + assert.NoError(t, err) + assert.True(t, pr.HasMerged) + assert.EqualValues(t, "01234567", pr.MergedCommitID) + + unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{ID: autoMerge.ID}) +} diff --git a/services/contexttest/context_tests.go b/services/contexttest/context_tests.go index 0c1e5ee54fe2..5624d2405872 100644 --- a/services/contexttest/context_tests.go +++ b/services/contexttest/context_tests.go @@ -94,6 +94,19 @@ func MockAPIContext(t *testing.T, reqPath string) (*context.APIContext, *httptes return ctx, resp } +func MockPrivateContext(t *testing.T, reqPath string) (*context.PrivateContext, *httptest.ResponseRecorder) { + resp := httptest.NewRecorder() + req := mockRequest(t, reqPath) + base, baseCleanUp := context.NewBaseContext(resp, req) + base.Data = middleware.GetContextData(req.Context()) + base.Locale = &translation.MockLocale{} + ctx := &context.PrivateContext{Base: base} + _ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later + chiCtx := chi.NewRouteContext() + ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx) + return ctx, resp +} + // LoadRepo load a repo into a test context. func LoadRepo(t *testing.T, ctx gocontext.Context, repoID int64) { var doer *user_model.User diff --git a/services/pull/merge.go b/services/pull/merge.go index 00f23e1e3ae2..20be7c5b5a87 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -18,7 +18,6 @@ import ( git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" access_model "code.gitea.io/gitea/models/perm/access" - pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -162,12 +161,6 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U pullWorkingPool.CheckIn(fmt.Sprint(pr.ID)) defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) - // Removing an auto merge pull and ignore if not exist - // FIXME: is this the correct point to do this? Shouldn't this be after IsMergeStyleAllowed? - if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { - return err - } - prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests) if err != nil { log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err) @@ -184,17 +177,15 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "") }() - pr.MergedCommitID, err = doMergeAndPush(ctx, pr, doer, mergeStyle, expectedHeadCommitID, message) + _, err = doMergeAndPush(ctx, pr, doer, mergeStyle, expectedHeadCommitID, message, repo_module.PushTriggerPRMergeToBase) if err != nil { return err } - pr.MergedUnix = timeutil.TimeStampNow() - pr.Merger = doer - pr.MergerID = doer.ID - - if _, err := pr.SetMerged(ctx); err != nil { - log.Error("SetMerged %-v: %v", pr, err) + // reload pull request because it has been updated by post receive hook + pr, err = issues_model.GetPullRequestByID(ctx, pr.ID) + if err != nil { + return err } if err := pr.LoadIssue(ctx); err != nil { @@ -245,7 +236,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U } // doMergeAndPush performs the merge operation without changing any pull information in database and pushes it up to the base repository -func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (string, error) { +func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, pushTrigger repo_module.PushTrigger) (string, error) { // Clone base repo. mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, expectedHeadCommitID) if err != nil { @@ -318,11 +309,13 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use pr.BaseRepo.Name, pr.ID, ) + + mergeCtx.env = append(mergeCtx.env, repo_module.EnvPushTrigger+"="+string(pushTrigger)) pushCmd := git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch) // Push back to upstream. - // TODO: this cause an api call to "/api/internal/hook/post-receive/...", - // that prevents us from doint the whole merge in one db transaction + // This cause an api call to "/api/internal/hook/post-receive/...", + // If it's merge, all db transaction and operations should be there but not here to prevent deadlock. if err := pushCmd.Run(mergeCtx.RunOpts()); err != nil { if strings.Contains(mergeCtx.errbuf.String(), "non-fast-forward") { return "", &git.ErrPushOutOfDate{ diff --git a/services/pull/update.go b/services/pull/update.go index bc8c4a25e5f6..939449b7544e 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -15,6 +15,7 @@ import ( 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/repository" ) // Update updates pull request with base branch. @@ -72,7 +73,7 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. BaseBranch: pr.HeadBranch, } - _, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message) + _, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase) defer func() { go AddTestPullRequestTask(doer, reversePR.HeadRepo.ID, reversePR.HeadBranch, false, "", "") From 271e8748a2035ebc836cc2d1e03f4e68b063697e Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 8 May 2024 23:12:37 +0800 Subject: [PATCH 08/58] Fix wrong transfer hint (#30889) (#30900) Backport #30889 by @lunny Fix #30187 Co-authored-by: Lunny Xiao --- routers/web/repo/setting/setting.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 17a400e3607e..1e0349cdeec4 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -789,6 +789,7 @@ func SettingsPost(ctx *context.Context) { ctx.Repo.GitRepo = nil } + oldFullname := repo.FullName() if err := repo_service.StartRepositoryTransfer(ctx, ctx.Doer, newOwner, repo, nil); err != nil { if repo_model.IsErrRepoAlreadyExist(err) { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil) @@ -803,8 +804,13 @@ func SettingsPost(ctx *context.Context) { return } - log.Trace("Repository transfer process was started: %s/%s -> %s", ctx.Repo.Owner.Name, repo.Name, newOwner) - ctx.Flash.Success(ctx.Tr("repo.settings.transfer_started", newOwner.DisplayName())) + if ctx.Repo.Repository.Status == repo_model.RepositoryPendingTransfer { + log.Trace("Repository transfer process was started: %s/%s -> %s", ctx.Repo.Owner.Name, repo.Name, newOwner) + ctx.Flash.Success(ctx.Tr("repo.settings.transfer_started", newOwner.DisplayName())) + } else { + log.Trace("Repository transferred: %s -> %s", oldFullname, ctx.Repo.Repository.FullName()) + ctx.Flash.Success(ctx.Tr("repo.settings.transfer_succeed")) + } ctx.Redirect(repo.Link() + "/settings") case "cancel_transfer": From 084bec89ed7ae0816fc2d8db6784ad22523d1fc4 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 8 May 2024 23:46:21 +0800 Subject: [PATCH 09/58] Fix various problems around projects board view (#30696) (#30902) Backport #30696 by @lunny # The problem The previous implementation will start multiple POST requests from the frontend when moving a column and another bug is moving the default column will never be remembered in fact. # What's changed - [x] This PR will allow the default column to move to a non-first position - [x] And it also uses one request instead of multiple requests when moving the columns - [x] Use a star instead of a pin as the icon for setting the default column action - [x] Inserted new column will be append to the end - [x] Fix #30701 the newly added issue will be append to the end of the default column - [x] Fix when deleting a column, all issues in it will be displayed from UI but database records exist. - [x] Add a limitation for columns in a project to 20. So the sorting will not be overflow because it's int8. Co-authored-by: Lunny Xiao Co-authored-by: silverwind Co-authored-by: wxiaoguang --- models/db/engine.go | 1 + models/issues/issue_project.go | 105 +++++++++++++++----------- models/project/board.go | 95 ++++++++++++++++++++--- models/project/board_test.go | 87 ++++++++++++++++++++- models/project/issue.go | 51 +++++++++++-- models/project/project.go | 7 ++ routers/web/org/projects.go | 69 ----------------- routers/web/repo/projects.go | 17 +++-- routers/web/repo/pull.go | 14 ++-- routers/web/shared/project/column.go | 48 ++++++++++++ routers/web/web.go | 3 + services/issue/issue.go | 2 +- templates/projects/view.tmpl | 2 +- tests/integration/org_project_test.go | 6 +- tests/integration/project_test.go | 64 ++++++++++++++++ web_src/js/features/repo-projects.js | 26 ++++--- 16 files changed, 430 insertions(+), 167 deletions(-) create mode 100644 routers/web/shared/project/column.go diff --git a/models/db/engine.go b/models/db/engine.go index 25f4066ea138..847ba58c2675 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -57,6 +57,7 @@ type Engine interface { SumInt(bean any, columnName string) (res int64, err error) Sync(...any) error Select(string) *xorm.Session + SetExpr(string, any) *xorm.Session NotIn(string, ...any) *xorm.Session OrderBy(any, ...any) *xorm.Session Exist(...any) (bool, error) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 907a5a17b9f2..e31d2ef15162 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -5,11 +5,11 @@ package issues import ( "context" - "fmt" "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/util" ) // LoadProject load the project the issue was assigned to @@ -90,58 +90,73 @@ func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (m return issuesMap, nil } -// ChangeProjectAssign changes the project associated with an issue -func ChangeProjectAssign(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := addUpdateIssueProject(ctx, issue, doer, newProjectID); err != nil { - return err - } - - return committer.Commit() -} +// IssueAssignOrRemoveProject changes the project associated with an issue +// If newProjectID is 0, the issue is removed from the project +func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + oldProjectID := issue.projectID(ctx) -func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID int64) error { - oldProjectID := issue.projectID(ctx) + if err := issue.LoadRepo(ctx); err != nil { + return err + } - if err := issue.LoadRepo(ctx); err != nil { - return err - } + // Only check if we add a new project and not remove it. + if newProjectID > 0 { + newProject, err := project_model.GetProjectByID(ctx, newProjectID) + if err != nil { + return err + } + if !newProject.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) { + return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) + } + if newColumnID == 0 { + newDefaultColumn, err := newProject.GetDefaultBoard(ctx) + if err != nil { + return err + } + newColumnID = newDefaultColumn.ID + } + } - // Only check if we add a new project and not remove it. - if newProjectID > 0 { - newProject, err := project_model.GetProjectByID(ctx, newProjectID) - if err != nil { + if _, err := db.GetEngine(ctx).Where("project_issue.issue_id=?", issue.ID).Delete(&project_model.ProjectIssue{}); err != nil { return err } - if newProject.RepoID != issue.RepoID && newProject.OwnerID != issue.Repo.OwnerID { - return fmt.Errorf("issue's repository is not the same as project's repository") - } - } - if _, err := db.GetEngine(ctx).Where("project_issue.issue_id=?", issue.ID).Delete(&project_model.ProjectIssue{}); err != nil { - return err - } + if oldProjectID > 0 || newProjectID > 0 { + if _, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeProject, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + OldProjectID: oldProjectID, + ProjectID: newProjectID, + }); err != nil { + return err + } + } + if newProjectID == 0 { + return nil + } + if newColumnID == 0 { + panic("newColumnID must not be zero") // shouldn't happen + } - if oldProjectID > 0 || newProjectID > 0 { - if _, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeProject, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - OldProjectID: oldProjectID, - ProjectID: newProjectID, - }); err != nil { + res := struct { + MaxSorting int64 + IssueCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as issue_count").Table("project_issue"). + Where("project_id=?", newProjectID). + And("project_board_id=?", newColumnID). + Get(&res); err != nil { return err } - } - - return db.Insert(ctx, &project_model.ProjectIssue{ - IssueID: issue.ID, - ProjectID: newProjectID, + newSorting := util.Iif(res.IssueCount > 0, res.MaxSorting+1, 0) + return db.Insert(ctx, &project_model.ProjectIssue{ + IssueID: issue.ID, + ProjectID: newProjectID, + ProjectBoardID: newColumnID, + Sorting: newSorting, + }) }) } diff --git a/models/project/board.go b/models/project/board.go index 7faabc52c58b..a52baa0c185f 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -5,12 +5,14 @@ package project import ( "context" + "errors" "fmt" "regexp" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -82,6 +84,17 @@ func (b *Board) NumIssues(ctx context.Context) int { return int(c) } +func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { + issues := make([]*ProjectIssue, 0, 5) + if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID). + And("project_board_id=?", b.ID). + OrderBy("sorting, id"). + Find(&issues); err != nil { + return nil, err + } + return issues, nil +} + func init() { db.RegisterModel(new(Board)) } @@ -150,12 +163,27 @@ func createBoardsForProjectsType(ctx context.Context, project *Project) error { return db.Insert(ctx, boards) } +// maxProjectColumns max columns allowed in a project, this should not bigger than 127 +// because sorting is int8 in database +const maxProjectColumns = 20 + // NewBoard adds a new project board to a given project func NewBoard(ctx context.Context, board *Board) error { if len(board.Color) != 0 && !BoardColorPattern.MatchString(board.Color) { return fmt.Errorf("bad color code: %s", board.Color) } - + res := struct { + MaxSorting int64 + ColumnCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as column_count").Table("project_board"). + Where("project_id=?", board.ProjectID).Get(&res); err != nil { + return err + } + if res.ColumnCount >= maxProjectColumns { + return fmt.Errorf("NewBoard: maximum number of columns reached") + } + board.Sorting = int8(util.Iif(res.ColumnCount > 0, res.MaxSorting+1, 0)) _, err := db.GetEngine(ctx).Insert(board) return err } @@ -189,7 +217,17 @@ func deleteBoardByID(ctx context.Context, boardID int64) error { return fmt.Errorf("deleteBoardByID: cannot delete default board") } - if err = board.removeIssues(ctx); err != nil { + // move all issues to the default column + project, err := GetProjectByID(ctx, board.ProjectID) + if err != nil { + return err + } + defaultColumn, err := project.GetDefaultBoard(ctx) + if err != nil { + return err + } + + if err = board.moveIssuesToAnotherColumn(ctx, defaultColumn); err != nil { return err } @@ -242,21 +280,15 @@ func UpdateBoard(ctx context.Context, board *Board) error { // GetBoards fetches all boards related to a project func (p *Project) GetBoards(ctx context.Context) (BoardList, error) { boards := make([]*Board, 0, 5) - - if err := db.GetEngine(ctx).Where("project_id=? AND `default`=?", p.ID, false).OrderBy("sorting").Find(&boards); err != nil { + if err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Find(&boards); err != nil { return nil, err } - defaultB, err := p.getDefaultBoard(ctx) - if err != nil { - return nil, err - } - - return append([]*Board{defaultB}, boards...), nil + return boards, nil } -// getDefaultBoard return default board and ensure only one exists -func (p *Project) getDefaultBoard(ctx context.Context) (*Board, error) { +// GetDefaultBoard return default board and ensure only one exists +func (p *Project) GetDefaultBoard(ctx context.Context) (*Board, error) { var board Board has, err := db.GetEngine(ctx). Where("project_id=? AND `default` = ?", p.ID, true). @@ -316,3 +348,42 @@ func UpdateBoardSorting(ctx context.Context, bs BoardList) error { return nil }) } + +func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) (BoardList, error) { + columns := make([]*Board, 0, 5) + if err := db.GetEngine(ctx). + Where("project_id =?", projectID). + In("id", columnsIDs). + OrderBy("sorting").Find(&columns); err != nil { + return nil, err + } + return columns, nil +} + +// MoveColumnsOnProject sorts columns in a project +func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) + columnIDs := util.ValuesOfMap(sortedColumnIDs) + movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) + if err != nil { + return err + } + if len(movedColumns) != len(sortedColumnIDs) { + return errors.New("some columns do not exist") + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + return fmt.Errorf("column[%d]'s projectID is not equal to project's ID [%d]", column.ProjectID, project.ID) + } + } + + for sorting, columnID := range sortedColumnIDs { + if _, err := sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID); err != nil { + return err + } + } + return nil + }) +} diff --git a/models/project/board_test.go b/models/project/board_test.go index 71ba29a5896d..da922ff7adaa 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -4,6 +4,8 @@ package project import ( + "fmt" + "strings" "testing" "code.gitea.io/gitea/models/db" @@ -19,7 +21,7 @@ func TestGetDefaultBoard(t *testing.T) { assert.NoError(t, err) // check if default board was added - board, err := projectWithoutDefault.getDefaultBoard(db.DefaultContext) + board, err := projectWithoutDefault.GetDefaultBoard(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(5), board.ProjectID) assert.Equal(t, "Uncategorized", board.Title) @@ -28,7 +30,7 @@ func TestGetDefaultBoard(t *testing.T) { assert.NoError(t, err) // check if multiple defaults were removed - board, err = projectWithMultipleDefaults.getDefaultBoard(db.DefaultContext) + board, err = projectWithMultipleDefaults.GetDefaultBoard(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), board.ProjectID) assert.Equal(t, int64(9), board.ID) @@ -42,3 +44,84 @@ func TestGetDefaultBoard(t *testing.T) { assert.Equal(t, int64(6), board.ProjectID) assert.False(t, board.Default) } + +func Test_moveIssuesToAnotherColumn(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + column1 := unittest.AssertExistsAndLoadBean(t, &Board{ID: 1, ProjectID: 1}) + + issues, err := column1.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 1) + assert.EqualValues(t, 1, issues[0].ID) + + column2 := unittest.AssertExistsAndLoadBean(t, &Board{ID: 2, ProjectID: 1}) + issues, err = column2.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 1) + assert.EqualValues(t, 3, issues[0].ID) + + err = column1.moveIssuesToAnotherColumn(db.DefaultContext, column2) + assert.NoError(t, err) + + issues, err = column1.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 0) + + issues, err = column2.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 2) + assert.EqualValues(t, 3, issues[0].ID) + assert.EqualValues(t, 0, issues[0].Sorting) + assert.EqualValues(t, 1, issues[1].ID) + assert.EqualValues(t, 1, issues[1].Sorting) +} + +func Test_MoveColumnsOnProject(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + project1 := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + columns, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, 0, columns[0].Sorting) // even if there is no default sorting, the code should also work + assert.EqualValues(t, 0, columns[1].Sorting) + assert.EqualValues(t, 0, columns[2].Sorting) + + err = MoveColumnsOnProject(db.DefaultContext, project1, map[int64]int64{ + 0: columns[1].ID, + 1: columns[2].ID, + 2: columns[0].ID, + }) + assert.NoError(t, err) + + columnsAfter, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columnsAfter, 3) + assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) + assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) + assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) +} + +func Test_NewBoard(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + project1 := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + columns, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + + for i := 0; i < maxProjectColumns-3; i++ { + err := NewBoard(db.DefaultContext, &Board{ + Title: fmt.Sprintf("board-%d", i+4), + ProjectID: project1.ID, + }) + assert.NoError(t, err) + } + err = NewBoard(db.DefaultContext, &Board{ + Title: "board-21", + ProjectID: project1.ID, + }) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "maximum number of columns reached")) +} diff --git a/models/project/issue.go b/models/project/issue.go index ebc9719de55d..32e72e909d5c 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // ProjectIssue saves relation from issue to a project @@ -17,7 +18,7 @@ type ProjectIssue struct { //revive:disable-line:exported IssueID int64 `xorm:"INDEX"` ProjectID int64 `xorm:"INDEX"` - // If 0, then it has not been added to a specific board in the project + // ProjectBoardID should not be zero since 1.22. If it's zero, the issue will not be displayed on UI and it might result in errors. ProjectBoardID int64 `xorm:"INDEX"` // the sorting order on the board @@ -79,11 +80,8 @@ func (p *Project) NumOpenIssues(ctx context.Context) int { func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs map[int64]int64) error { return db.WithTx(ctx, func(ctx context.Context) error { sess := db.GetEngine(ctx) + issueIDs := util.ValuesOfMap(sortedIssueIDs) - issueIDs := make([]int64, 0, len(sortedIssueIDs)) - for _, issueID := range sortedIssueIDs { - issueIDs = append(issueIDs, issueID) - } count, err := sess.Table(new(ProjectIssue)).Where("project_id=?", board.ProjectID).In("issue_id", issueIDs).Count() if err != nil { return err @@ -102,7 +100,44 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs }) } -func (b *Board) removeIssues(ctx context.Context) error { - _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = 0 WHERE project_board_id = ? ", b.ID) - return err +func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) error { + if b.ProjectID != newColumn.ProjectID { + return fmt.Errorf("columns have to be in the same project") + } + + if b.ID == newColumn.ID { + return nil + } + + res := struct { + MaxSorting int64 + IssueCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as issue_count"). + Table("project_issue"). + Where("project_id=?", newColumn.ProjectID). + And("project_board_id=?", newColumn.ID). + Get(&res); err != nil { + return err + } + + issues, err := b.GetIssues(ctx) + if err != nil { + return err + } + if len(issues) == 0 { + return nil + } + + nextSorting := util.Iif(res.IssueCount > 0, res.MaxSorting+1, 0) + return db.WithTx(ctx, func(ctx context.Context) error { + for i, issue := range issues { + issue.ProjectBoardID = newColumn.ID + issue.Sorting = nextSorting + int64(i) + if _, err := db.GetEngine(ctx).ID(issue.ID).Cols("project_board_id", "sorting").Update(issue); err != nil { + return err + } + } + return nil + }) } diff --git a/models/project/project.go b/models/project/project.go index 8f9ee2a99e9c..8be38694c522 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -161,6 +161,13 @@ func (p *Project) IsRepositoryProject() bool { return p.Type == TypeRepository } +func (p *Project) CanBeAccessedByOwnerRepo(ownerID int64, repo *repo_model.Repository) bool { + if p.Type == TypeRepository { + return repo != nil && p.RepoID == repo.ID // if a project belongs to a repository, then its OwnerID is 0 and can be ignored + } + return p.OwnerID == ownerID && p.RepoID == 0 +} + func init() { db.RegisterModel(new(Project)) } diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 7f78d1c830b7..50effbe9633f 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "net/http" - "strconv" "strings" "code.gitea.io/gitea/models/db" @@ -390,74 +389,6 @@ func ViewProject(ctx *context.Context) { ctx.HTML(http.StatusOK, tplProjectsView) } -func getActionIssues(ctx *context.Context) issues_model.IssueList { - commaSeparatedIssueIDs := ctx.FormString("issue_ids") - if len(commaSeparatedIssueIDs) == 0 { - return nil - } - issueIDs := make([]int64, 0, 10) - for _, stringIssueID := range strings.Split(commaSeparatedIssueIDs, ",") { - issueID, err := strconv.ParseInt(stringIssueID, 10, 64) - if err != nil { - ctx.ServerError("ParseInt", err) - return nil - } - issueIDs = append(issueIDs, issueID) - } - issues, err := issues_model.GetIssuesByIDs(ctx, issueIDs) - if err != nil { - ctx.ServerError("GetIssuesByIDs", err) - return nil - } - // Check access rights for all issues - issueUnitEnabled := ctx.Repo.CanRead(unit.TypeIssues) - prUnitEnabled := ctx.Repo.CanRead(unit.TypePullRequests) - for _, issue := range issues { - if issue.RepoID != ctx.Repo.Repository.ID { - ctx.NotFound("some issue's RepoID is incorrect", errors.New("some issue's RepoID is incorrect")) - return nil - } - if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled { - ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) - return nil - } - if err = issue.LoadAttributes(ctx); err != nil { - ctx.ServerError("LoadAttributes", err) - return nil - } - } - return issues -} - -// UpdateIssueProject change an issue's project -func UpdateIssueProject(ctx *context.Context) { - issues := getActionIssues(ctx) - if ctx.Written() { - return - } - - if err := issues.LoadProjects(ctx); err != nil { - ctx.ServerError("LoadProjects", err) - return - } - - projectID := ctx.FormInt64("id") - for _, issue := range issues { - if issue.Project != nil { - if issue.Project.ID == projectID { - continue - } - } - - if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) - return - } - } - - ctx.JSONOK() -} - // DeleteProjectBoard allows for the deletion of a project board func DeleteProjectBoard(ctx *context.Context) { if ctx.Doer == nil { diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 9b765e89e877..6186ee150cbf 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" @@ -383,17 +384,21 @@ func UpdateIssueProject(ctx *context.Context) { ctx.ServerError("LoadProjects", err) return } + if _, err := issues.LoadRepositories(ctx); err != nil { + ctx.ServerError("LoadProjects", err) + return + } projectID := ctx.FormInt64("id") for _, issue := range issues { - if issue.Project != nil { - if issue.Project.ID == projectID { + if issue.Project != nil && issue.Project.ID == projectID { + continue + } + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, 0); err != nil { + if errors.Is(err, util.ErrPermissionDenied) { continue } - } - - if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) + ctx.ServerError("IssueAssignOrRemoveProject", err) return } } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 7f131f2e984b..7041175d9ae1 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1329,14 +1329,12 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - if projectID > 0 { - if !ctx.Repo.CanWrite(unit.TypeProjects) { - ctx.Error(http.StatusBadRequest, "user hasn't the permission to write to projects") - return - } - if err := issues_model.ChangeProjectAssign(ctx, pullIssue, ctx.Doer, projectID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) - return + if projectID > 0 && ctx.Repo.CanWrite(unit.TypeProjects) { + if err := issues_model.IssueAssignOrRemoveProject(ctx, pullIssue, ctx.Doer, projectID, 0); err != nil { + if !errors.Is(err, util.ErrPermissionDenied) { + ctx.ServerError("IssueAssignOrRemoveProject", err) + return + } } } diff --git a/routers/web/shared/project/column.go b/routers/web/shared/project/column.go new file mode 100644 index 000000000000..599842ea9e9b --- /dev/null +++ b/routers/web/shared/project/column.go @@ -0,0 +1,48 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package project + +import ( + project_model "code.gitea.io/gitea/models/project" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/services/context" +) + +// MoveColumns moves or keeps columns in a project and sorts them inside that project +func MoveColumns(ctx *context.Context) { + project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) + if err != nil { + ctx.NotFoundOrServerError("GetProjectByID", project_model.IsErrProjectNotExist, err) + return + } + if !project.CanBeAccessedByOwnerRepo(ctx.ContextUser.ID, ctx.Repo.Repository) { + ctx.NotFound("CanBeAccessedByOwnerRepo", nil) + return + } + + type movedColumnsForm struct { + Columns []struct { + ColumnID int64 `json:"columnID"` + Sorting int64 `json:"sorting"` + } `json:"columns"` + } + + form := &movedColumnsForm{} + if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { + ctx.ServerError("DecodeMovedColumnsForm", err) + return + } + + sortedColumnIDs := make(map[int64]int64) + for _, column := range form.Columns { + sortedColumnIDs[column.Sorting] = column.ColumnID + } + + if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { + ctx.ServerError("MoveColumnsOnProject", err) + return + } + + ctx.JSONOK() +} diff --git a/routers/web/web.go b/routers/web/web.go index 91ab378d97c5..e1482c1e4ada 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -37,6 +37,7 @@ import ( "code.gitea.io/gitea/routers/web/repo" "code.gitea.io/gitea/routers/web/repo/actions" repo_setting "code.gitea.io/gitea/routers/web/repo/setting" + "code.gitea.io/gitea/routers/web/shared/project" "code.gitea.io/gitea/routers/web/user" user_setting "code.gitea.io/gitea/routers/web/user/setting" "code.gitea.io/gitea/routers/web/user/setting/security" @@ -999,6 +1000,7 @@ func registerRoutes(m *web.Route) { m.Post("/new", web.Bind(forms.CreateProjectForm{}), org.NewProjectPost) m.Group("/{id}", func() { m.Post("", web.Bind(forms.EditProjectBoardForm{}), org.AddBoardToProjectPost) + m.Post("/move", project.MoveColumns) m.Post("/delete", org.DeleteProject) m.Get("/edit", org.RenderEditProject) @@ -1354,6 +1356,7 @@ func registerRoutes(m *web.Route) { m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) m.Group("/{id}", func() { m.Post("", web.Bind(forms.EditProjectBoardForm{}), repo.AddBoardToProjectPost) + m.Post("/move", project.MoveColumns) m.Post("/delete", repo.DeleteProject) m.Get("/edit", repo.RenderEditProject) diff --git a/services/issue/issue.go b/services/issue/issue.go index b0e50f2b89a1..72ea66c8d98c 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -42,7 +42,7 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo } } if projectID > 0 { - if err := issues_model.ChangeProjectAssign(ctx, issue, issue.Poster, projectID); err != nil { + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, issue.Poster, projectID, 0); err != nil { return err } } diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 3e000660e285..47f214a44e0d 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -64,7 +64,7 @@
-
+
{{range .Columns}}
diff --git a/tests/integration/org_project_test.go b/tests/integration/org_project_test.go index a14004f6b028..ca39cf513020 100644 --- a/tests/integration/org_project_test.go +++ b/tests/integration/org_project_test.go @@ -5,17 +5,17 @@ package integration import ( "net/http" + "slices" "testing" unit_model "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" ) func TestOrgProjectAccess(t *testing.T) { defer tests.PrepareTestEnv(t)() - - // disable repo project unit - unit_model.DisabledRepoUnits = []unit_model.Type{unit_model.TypeProjects} + defer test.MockVariableValue(&unit_model.DisabledRepoUnits, append(slices.Clone(unit_model.DisabledRepoUnits), unit_model.TypeProjects))() // repo project, 404 req := NewRequest(t, "GET", "/user2/repo1/projects") diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 45061c5b24f1..1d9c3aae5361 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -4,10 +4,18 @@ package integration import ( + "fmt" "net/http" "testing" + "code.gitea.io/gitea/models/db" + project_model "code.gitea.io/gitea/models/project" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" ) func TestPrivateRepoProject(t *testing.T) { @@ -21,3 +29,59 @@ func TestPrivateRepoProject(t *testing.T) { req = NewRequest(t, "GET", "/user31/-/projects") sess.MakeRequest(t, req, http.StatusOK) } + +func TestMoveRepoProjectColumns(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + + projectsUnit := repo2.MustGetUnit(db.DefaultContext, unit.TypeProjects) + assert.True(t, projectsUnit.ProjectsConfig().IsProjectsAllowed(repo_model.ProjectsModeRepo)) + + project1 := project_model.Project{ + Title: "new created project", + RepoID: repo2.ID, + Type: project_model.TypeRepository, + BoardType: project_model.BoardTypeNone, + } + err := project_model.NewProject(db.DefaultContext, &project1) + assert.NoError(t, err) + + for i := 0; i < 3; i++ { + err = project_model.NewBoard(db.DefaultContext, &project_model.Board{ + Title: fmt.Sprintf("column %d", i+1), + ProjectID: project1.ID, + }) + assert.NoError(t, err) + } + + columns, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, 0, columns[0].Sorting) + assert.EqualValues(t, 1, columns[1].Sorting) + assert.EqualValues(t, 2, columns[2].Sorting) + + sess := loginUser(t, "user1") + req := NewRequest(t, "GET", fmt.Sprintf("/%s/projects/%d", repo2.FullName(), project1.ID)) + resp := sess.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), repo2.FullName(), project1.ID), map[string]any{ + "columns": []map[string]any{ + {"columnID": columns[1].ID, "sorting": 0}, + {"columnID": columns[2].ID, "sorting": 1}, + {"columnID": columns[0].ID, "sorting": 2}, + }, + }) + sess.MakeRequest(t, req, http.StatusOK) + + columnsAfter, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) + assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) + assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) + + assert.NoError(t, project_model.DeleteProjectByID(db.DefaultContext, project1.ID)) +} diff --git a/web_src/js/features/repo-projects.js b/web_src/js/features/repo-projects.js index a869c24c823a..a1cc4b346bb8 100644 --- a/web_src/js/features/repo-projects.js +++ b/web_src/js/features/repo-projects.js @@ -2,7 +2,6 @@ import $ from 'jquery'; import {contrastColor} from '../utils/color.js'; import {createSortable} from '../modules/sortable.js'; import {POST, DELETE, PUT} from '../modules/fetch.js'; -import tinycolor from 'tinycolor2'; function updateIssueCount(cards) { const parent = cards.parentElement; @@ -63,17 +62,20 @@ async function initRepoProjectSortable() { delay: 500, onSort: async () => { boardColumns = mainBoard.getElementsByClassName('project-column'); - for (let i = 0; i < boardColumns.length; i++) { - const column = boardColumns[i]; - if (parseInt(column.getAttribute('data-sorting')) !== i) { - try { - const bgColor = column.style.backgroundColor; // will be rgb() string - const color = bgColor ? tinycolor(bgColor).toHexString() : ''; - await PUT(column.getAttribute('data-url'), {data: {sorting: i, color}}); - } catch (error) { - console.error(error); - } - } + + const columnSorting = { + columns: Array.from(boardColumns, (column, i) => ({ + columnID: parseInt(column.getAttribute('data-id')), + sorting: i, + })), + }; + + try { + await POST(mainBoard.getAttribute('data-url'), { + data: columnSorting, + }); + } catch (error) { + console.error(error); } }, }); From 2f91a461f7f25f9b59c7c54406b4dbe2a44e2fc2 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 9 May 2024 00:38:46 +0800 Subject: [PATCH 10/58] Fix misspelling of mergable (#30896) (#30905) Backport #30896 by @yp05327 https://github.com/go-gitea/gitea/pull/25812#issuecomment-2099833692 Follow #30573 Co-authored-by: yp05327 <576951401@qq.com> --- routers/api/v1/repo/pull.go | 4 ++-- routers/web/repo/pull.go | 4 ++-- services/automerge/automerge.go | 4 ++-- services/pull/check.go | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 8bd4ddf64b65..38a32a73c78e 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -881,7 +881,7 @@ func MergePullRequest(ctx *context.APIContext) { } // start with merging by checking - if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil { + if err := pull_service.CheckPullMergeable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil { if errors.Is(err, pull_service.ErrIsClosed) { ctx.NotFound() } else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) { @@ -890,7 +890,7 @@ func MergePullRequest(ctx *context.APIContext) { ctx.Error(http.StatusMethodNotAllowed, "PR already merged", "") } else if errors.Is(err, pull_service.ErrIsWorkInProgress) { ctx.Error(http.StatusMethodNotAllowed, "PR is a work in progress", "Work in progress PRs cannot be merged") - } else if errors.Is(err, pull_service.ErrNotMergableState) { + } else if errors.Is(err, pull_service.ErrNotMergeableState) { ctx.Error(http.StatusMethodNotAllowed, "PR not in mergeable state", "Please try again later") } else if models.IsErrDisallowedToMerge(err) { ctx.Error(http.StatusMethodNotAllowed, "PR is not ready to be merged", err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 7041175d9ae1..bbdc6ca631ae 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1007,7 +1007,7 @@ func MergePullRequest(ctx *context.Context) { } // start with merging by checking - if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil { + if err := pull_service.CheckPullMergeable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, mergeCheckType, form.ForceMerge); err != nil { switch { case errors.Is(err, pull_service.ErrIsClosed): if issue.IsPull { @@ -1021,7 +1021,7 @@ func MergePullRequest(ctx *context.Context) { ctx.JSONError(ctx.Tr("repo.pulls.has_merged")) case errors.Is(err, pull_service.ErrIsWorkInProgress): ctx.JSONError(ctx.Tr("repo.pulls.no_merge_wip")) - case errors.Is(err, pull_service.ErrNotMergableState): + case errors.Is(err, pull_service.ErrNotMergeableState): ctx.JSONError(ctx.Tr("repo.pulls.no_merge_not_ready")) case models.IsErrDisallowedToMerge(err): ctx.JSONError(ctx.Tr("repo.pulls.no_merge_not_ready")) diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index bd427bef9f73..bd1317c7f48b 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -229,12 +229,12 @@ func handlePull(pullID int64, sha string) { return } - if err := pull_service.CheckPullMergable(ctx, doer, &perm, pr, pull_service.MergeCheckTypeGeneral, false); err != nil { + if err := pull_service.CheckPullMergeable(ctx, doer, &perm, pr, pull_service.MergeCheckTypeGeneral, false); err != nil { if errors.Is(pull_service.ErrUserNotAllowedToMerge, err) { log.Info("%-v was scheduled to automerge by an unauthorized user", pr) return } - log.Error("%-v CheckPullMergable: %v", pr, err) + log.Error("%-v CheckPullMergeable: %v", pr, err) return } diff --git a/services/pull/check.go b/services/pull/check.go index 9495e8ad5f3f..7d93ff7a8a4b 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -39,7 +39,7 @@ var ( ErrHasMerged = errors.New("has already been merged") ErrIsWorkInProgress = errors.New("work in progress PRs cannot be merged") ErrIsChecking = errors.New("cannot merge while conflict checking is in progress") - ErrNotMergableState = errors.New("not in mergeable state") + ErrNotMergeableState = errors.New("not in mergeable state") ErrDependenciesLeft = errors.New("is blocked by an open dependency") ) @@ -66,8 +66,8 @@ const ( MergeCheckTypeAuto // Auto Merge (Scheduled Merge) After Checks Succeed ) -// CheckPullMergable check if the pull mergeable based on all conditions (branch protection, merge options, ...) -func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, adminSkipProtectionCheck bool) error { +// CheckPullMergeable check if the pull mergeable based on all conditions (branch protection, merge options, ...) +func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, adminSkipProtectionCheck bool) error { return db.WithTx(stdCtx, func(ctx context.Context) error { if pr.HasMerged { return ErrHasMerged @@ -97,7 +97,7 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce } if !pr.CanAutoMerge() && !pr.IsEmpty() { - return ErrNotMergableState + return ErrNotMergeableState } if pr.IsChecking() { From 5f1af1f37dcc519fc4bb52c90859f0301c89bf0b Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 9 May 2024 07:26:53 +0800 Subject: [PATCH 11/58] Fix incorrect issue form (#30881) (#30904) Backport #30881 by wxiaoguang Co-authored-by: wxiaoguang --- .../repo/issue/branch_selector_field.tmpl | 6 +---- web_src/js/features/repo-legacy.js | 27 ++++++++----------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/templates/repo/issue/branch_selector_field.tmpl b/templates/repo/issue/branch_selector_field.tmpl index cbf7929fdb31..5793a8bfda88 100644 --- a/templates/repo/issue/branch_selector_field.tmpl +++ b/templates/repo/issue/branch_selector_field.tmpl @@ -1,12 +1,8 @@ {{if and (not .Issue.IsPull) (not .PageIsComparePull)}} - -
- {{$.CsrfTokenHtml}} -