mirror of
https://github.com/go-gitea/gitea
synced 2024-09-10 08:27:04 +02:00
Fix reverting a merge commit failing (#28794)
Fixes #22236
---
Error occurring currently while trying to revert commit using read-tree
-m approach:
> 2022/12/26 16:04:43 ...rvices/pull/patch.go:240:AttemptThreeWayMerge()
[E] [63a9c61a] Unable to run read-tree -m! Error: exit status 128 -
fatal: this operation must be run in a work tree
> - fatal: this operation must be run in a work tree
We need to clone a non-bare repository for `git read-tree -m` to work.
bb371aee6e
adds support to create a non-bare cloned temporary upload repository.
After cloning a non-bare temporary upload repository, we [set default
index](https://github.com/go-gitea/gitea/blob/main/services/repository/files/cherry_pick.go#L37)
(`git read-tree HEAD`).
This operation ends up resetting the git index file (see investigation
details below), due to which, we need to call `git update-index
--refresh` afterward.
Here's the diff of the index file before and after we execute
SetDefaultIndex: https://www.diffchecker.com/hyOP3eJy/
Notice the **ctime**, **mtime** are set to 0 after SetDefaultIndex.
You can reproduce the same behavior using these steps:
```bash
$ git clone https://try.gitea.io/me-heer/test.git -s -b main
$ cd test
$ git read-tree HEAD
$ git read-tree -m 1f085d7ed8 1f085d7ed8 9933caed00
error: Entry '1' not uptodate. Cannot merge.
```
After which, we can fix like this:
```
$ git update-index --refresh
$ git read-tree -m 1f085d7ed8 1f085d7ed8 9933caed00
```
This commit is contained in:
parent
edfb57e699
commit
b8270240bf
@ -669,3 +669,10 @@
|
|||||||
type: 10
|
type: 10
|
||||||
config: "{}"
|
config: "{}"
|
||||||
created_unix: 946684810
|
created_unix: 946684810
|
||||||
|
|
||||||
|
-
|
||||||
|
id: 101
|
||||||
|
repo_id: 59
|
||||||
|
type: 1
|
||||||
|
config: "{}"
|
||||||
|
created_unix: 946684810
|
||||||
|
@ -1693,3 +1693,16 @@
|
|||||||
size: 0
|
size: 0
|
||||||
is_fsck_enabled: true
|
is_fsck_enabled: true
|
||||||
close_issues_via_commit_in_any_branch: false
|
close_issues_via_commit_in_any_branch: false
|
||||||
|
|
||||||
|
-
|
||||||
|
id: 59
|
||||||
|
owner_id: 2
|
||||||
|
owner_name: user2
|
||||||
|
lower_name: test_commit_revert
|
||||||
|
name: test_commit_revert
|
||||||
|
default_branch: main
|
||||||
|
is_empty: false
|
||||||
|
is_archived: false
|
||||||
|
is_private: true
|
||||||
|
status: 0
|
||||||
|
num_issues: 0
|
||||||
|
@ -66,7 +66,7 @@
|
|||||||
num_followers: 2
|
num_followers: 2
|
||||||
num_following: 1
|
num_following: 1
|
||||||
num_stars: 2
|
num_stars: 2
|
||||||
num_repos: 14
|
num_repos: 15
|
||||||
num_teams: 0
|
num_teams: 0
|
||||||
num_members: 0
|
num_members: 0
|
||||||
visibility: 0
|
visibility: 0
|
||||||
|
@ -267,7 +267,7 @@ func alterRepositoryContent(ctx context.Context, doer *user_model.User, repo *re
|
|||||||
defer t.Close()
|
defer t.Close()
|
||||||
|
|
||||||
var lastCommitID string
|
var lastCommitID string
|
||||||
if err := t.Clone(repo.DefaultBranch); err != nil {
|
if err := t.Clone(repo.DefaultBranch, true); err != nil {
|
||||||
if !git.IsErrBranchNotExist(err) || !repo.IsEmpty {
|
if !git.IsErrBranchNotExist(err) || !repo.IsEmpty {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
@ -31,12 +31,15 @@ func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_mod
|
|||||||
log.Error("%v", err)
|
log.Error("%v", err)
|
||||||
}
|
}
|
||||||
defer t.Close()
|
defer t.Close()
|
||||||
if err := t.Clone(opts.OldBranch); err != nil {
|
if err := t.Clone(opts.OldBranch, false); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := t.SetDefaultIndex(); err != nil {
|
if err := t.SetDefaultIndex(); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
if err := t.RefreshIndex(); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
// Get the commit of the original branch
|
// Get the commit of the original branch
|
||||||
commit, err := t.GetBranchCommit(opts.OldBranch)
|
commit, err := t.GetBranchCommit(opts.OldBranch)
|
||||||
|
@ -21,7 +21,7 @@ func GetDiffPreview(ctx context.Context, repo *repo_model.Repository, branch, tr
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
defer t.Close()
|
defer t.Close()
|
||||||
if err := t.Clone(branch); err != nil {
|
if err := t.Clone(branch, true); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := t.SetDefaultIndex(); err != nil {
|
if err := t.SetDefaultIndex(); err != nil {
|
||||||
|
@ -113,7 +113,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user
|
|||||||
log.Error("%v", err)
|
log.Error("%v", err)
|
||||||
}
|
}
|
||||||
defer t.Close()
|
defer t.Close()
|
||||||
if err := t.Clone(opts.OldBranch); err != nil {
|
if err := t.Clone(opts.OldBranch, true); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := t.SetDefaultIndex(); err != nil {
|
if err := t.SetDefaultIndex(); err != nil {
|
||||||
|
@ -51,8 +51,13 @@ func (t *TemporaryUploadRepository) Close() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Clone the base repository to our path and set branch as the HEAD
|
// Clone the base repository to our path and set branch as the HEAD
|
||||||
func (t *TemporaryUploadRepository) Clone(branch string) error {
|
func (t *TemporaryUploadRepository) Clone(branch string, bare bool) error {
|
||||||
if _, _, err := git.NewCommand(t.ctx, "clone", "-s", "--bare", "-b").AddDynamicArguments(branch, t.repo.RepoPath(), t.basePath).RunStdString(nil); err != nil {
|
cmd := git.NewCommand(t.ctx, "clone", "-s", "-b").AddDynamicArguments(branch, t.repo.RepoPath(), t.basePath)
|
||||||
|
if bare {
|
||||||
|
cmd.AddArguments("--bare")
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, _, err := cmd.RunStdString(nil); err != nil {
|
||||||
stderr := err.Error()
|
stderr := err.Error()
|
||||||
if matched, _ := regexp.MatchString(".*Remote branch .* not found in upstream origin.*", stderr); matched {
|
if matched, _ := regexp.MatchString(".*Remote branch .* not found in upstream origin.*", stderr); matched {
|
||||||
return git.ErrBranchNotExist{
|
return git.ErrBranchNotExist{
|
||||||
@ -97,6 +102,14 @@ func (t *TemporaryUploadRepository) SetDefaultIndex() error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RefreshIndex looks at the current index and checks to see if merges or updates are needed by checking stat() information.
|
||||||
|
func (t *TemporaryUploadRepository) RefreshIndex() error {
|
||||||
|
if _, _, err := git.NewCommand(t.ctx, "update-index", "--refresh").RunStdString(&git.RunOpts{Dir: t.basePath}); err != nil {
|
||||||
|
return fmt.Errorf("RefreshIndex: %w", err)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// LsFiles checks if the given filename arguments are in the index
|
// LsFiles checks if the given filename arguments are in the index
|
||||||
func (t *TemporaryUploadRepository) LsFiles(filenames ...string) ([]string, error) {
|
func (t *TemporaryUploadRepository) LsFiles(filenames ...string) ([]string, error) {
|
||||||
stdOut := new(bytes.Buffer)
|
stdOut := new(bytes.Buffer)
|
||||||
|
@ -146,7 +146,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
|
|||||||
}
|
}
|
||||||
defer t.Close()
|
defer t.Close()
|
||||||
hasOldBranch := true
|
hasOldBranch := true
|
||||||
if err := t.Clone(opts.OldBranch); err != nil {
|
if err := t.Clone(opts.OldBranch, true); err != nil {
|
||||||
for _, file := range opts.Files {
|
for _, file := range opts.Files {
|
||||||
if file.Operation == "delete" {
|
if file.Operation == "delete" {
|
||||||
return nil, err
|
return nil, err
|
||||||
|
@ -87,7 +87,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
|
|||||||
defer t.Close()
|
defer t.Close()
|
||||||
|
|
||||||
hasOldBranch := true
|
hasOldBranch := true
|
||||||
if err = t.Clone(opts.OldBranch); err != nil {
|
if err = t.Clone(opts.OldBranch, true); err != nil {
|
||||||
if !git.IsErrBranchNotExist(err) || !repo.IsEmpty {
|
if !git.IsErrBranchNotExist(err) || !repo.IsEmpty {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
@ -0,0 +1 @@
|
|||||||
|
ref: refs/heads/main
|
@ -0,0 +1,8 @@
|
|||||||
|
[core]
|
||||||
|
repositoryformatversion = 0
|
||||||
|
filemode = true
|
||||||
|
bare = true
|
||||||
|
ignorecase = true
|
||||||
|
precomposeunicode = true
|
||||||
|
[remote "origin"]
|
||||||
|
url = https://try.gitea.io/me-heer/test_commit_revert.git
|
@ -0,0 +1 @@
|
|||||||
|
Unnamed repository; edit this file 'description' to name the repository.
|
@ -0,0 +1,6 @@
|
|||||||
|
# git ls-files --others --exclude-from=.git/info/exclude
|
||||||
|
# Lines that start with '#' are comments.
|
||||||
|
# For a project mostly in C, the following would be a good set of
|
||||||
|
# exclude patterns (uncomment them if you want to use them):
|
||||||
|
# *.[oa]
|
||||||
|
# *~
|
Binary file not shown.
Binary file not shown.
@ -0,0 +1,3 @@
|
|||||||
|
# pack-refs with: peeled fully-peeled sorted
|
||||||
|
46aa6ab2c881ae90e15d9ccfc947d1625c892ce5 refs/heads/develop
|
||||||
|
deebcbc752e540bab4ce3ee713d3fc8fdc35b2f7 refs/heads/main
|
34
tests/integration/repo_mergecommit_revert_test.go
Normal file
34
tests/integration/repo_mergecommit_revert_test.go
Normal file
@ -0,0 +1,34 @@
|
|||||||
|
package integration
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/tests"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestRepoMergeCommitRevert(t *testing.T) {
|
||||||
|
defer tests.PrepareTestEnv(t)()
|
||||||
|
session := loginUser(t, "user2")
|
||||||
|
|
||||||
|
req := NewRequest(t, "GET", "/user2/test_commit_revert/_cherrypick/deebcbc752e540bab4ce3ee713d3fc8fdc35b2f7/main?ref=main&refType=branch&cherry-pick-type=revert")
|
||||||
|
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||||
|
|
||||||
|
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||||
|
req = NewRequestWithValues(t, "POST", "/user2/test_commit_revert/_cherrypick/deebcbc752e540bab4ce3ee713d3fc8fdc35b2f7/main", map[string]string{
|
||||||
|
"_csrf": htmlDoc.GetCSRF(),
|
||||||
|
"last_commit": "deebcbc752e540bab4ce3ee713d3fc8fdc35b2f7",
|
||||||
|
"page_has_posted": "true",
|
||||||
|
"revert": "true",
|
||||||
|
"commit_summary": "reverting test commit",
|
||||||
|
"commit_message": "test message",
|
||||||
|
"commit_choice": "direct",
|
||||||
|
"new_branch_name": "test-revert-branch-1",
|
||||||
|
})
|
||||||
|
resp = session.MakeRequest(t, req, http.StatusSeeOther)
|
||||||
|
|
||||||
|
// A successful revert redirects to the main branch
|
||||||
|
assert.EqualValues(t, "/user2/test_commit_revert/src/branch/main", resp.Header().Get("Location"))
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user