From 62f2d717b7e04565c5ff260746e43ed64a87b0e0 Mon Sep 17 00:00:00 2001
From: Mihir Joshi <mihir67mj@gmail.com>
Date: Sun, 21 Jan 2024 19:48:37 +0530
Subject: [PATCH] Fix reverting a merge commit failing (#28794) (#28825)
Backport https://github.com/go-gitea/gitea/pull/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.
https://github.com/go-gitea/gitea/commit/bb371aee6ecf5e570cdf7b5f7f0d6f47a607a325
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:
```bash
$ git update-index --refresh
$ git read-tree -m 1f085d7ed8 1f085d7ed8 9933caed00
```
---
models/fixtures/repo_unit.yml | 7 ++++
models/fixtures/repository.yml | 13 +++++++
models/fixtures/user.yml | 2 +-
services/packages/cargo/index.go | 2 +-
services/repository/files/cherry_pick.go | 5 ++-
services/repository/files/diff.go | 2 +-
services/repository/files/patch.go | 2 +-
services/repository/files/temp_repo.go | 17 +++++++--
services/repository/files/update.go | 2 +-
services/repository/files/upload.go | 2 +-
.../user2/test_commit_revert.git/HEAD | 1 +
.../user2/test_commit_revert.git/config | 8 +++++
.../user2/test_commit_revert.git/description | 1 +
.../user2/test_commit_revert.git/info/exclude | 6 ++++
...200c8e6707636a6cc3e0d8101fba08b19dcb91.idx | Bin 0 -> 1268 bytes
...00c8e6707636a6cc3e0d8101fba08b19dcb91.pack | Bin 0 -> 609 bytes
.../user2/test_commit_revert.git/packed-refs | 3 ++
.../repo_mergecommit_revert_test.go | 34 ++++++++++++++++++
18 files changed, 98 insertions(+), 9 deletions(-)
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/HEAD
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/config
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/description
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/info/exclude
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/objects/pack/pack-91200c8e6707636a6cc3e0d8101fba08b19dcb91.idx
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/objects/pack/pack-91200c8e6707636a6cc3e0d8101fba08b19dcb91.pack
create mode 100644 tests/gitea-repositories-meta/user2/test_commit_revert.git/packed-refs
create mode 100644 tests/integration/repo_mergecommit_revert_test.go
diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml
index c22eb8c2a2..98b1f47401 100644
--- a/models/fixtures/repo_unit.yml
+++ b/models/fixtures/repo_unit.yml
@@ -649,3 +649,10 @@
repo_id: 49
type: 2
created_unix: 946684810
+
+-
+ id: 98
+ repo_id: 59
+ type: 1
+ config: "{}"
+ created_unix: 946684810
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index 373c1caa62..f4e8376735 100644
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -1693,3 +1693,16 @@
size: 0
is_fsck_enabled: true
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
diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index fd51379816..79fbb981f6 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -66,7 +66,7 @@
num_followers: 2
num_following: 1
num_stars: 2
- num_repos: 14
+ num_repos: 15
num_teams: 0
num_members: 0
visibility: 0
diff --git a/services/packages/cargo/index.go b/services/packages/cargo/index.go
index 8164ffb01c..e580de2fe0 100644
--- a/services/packages/cargo/index.go
+++ b/services/packages/cargo/index.go
@@ -267,7 +267,7 @@ func alterRepositoryContent(ctx context.Context, doer *user_model.User, repo *re
defer t.Close()
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 {
return err
}
diff --git a/services/repository/files/cherry_pick.go b/services/repository/files/cherry_pick.go
index c1c5bfb617..5e9a0900a7 100644
--- a/services/repository/files/cherry_pick.go
+++ b/services/repository/files/cherry_pick.go
@@ -31,12 +31,15 @@ func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_mod
log.Error("%v", err)
}
defer t.Close()
- if err := t.Clone(opts.OldBranch); err != nil {
+ if err := t.Clone(opts.OldBranch, false); err != nil {
return nil, err
}
if err := t.SetDefaultIndex(); err != nil {
return nil, err
}
+ if err := t.RefreshIndex(); err != nil {
+ return nil, err
+ }
// Get the commit of the original branch
commit, err := t.GetBranchCommit(opts.OldBranch)
diff --git a/services/repository/files/diff.go b/services/repository/files/diff.go
index 373249b114..bf8b938e21 100644
--- a/services/repository/files/diff.go
+++ b/services/repository/files/diff.go
@@ -21,7 +21,7 @@ func GetDiffPreview(ctx context.Context, repo *repo_model.Repository, branch, tr
return nil, err
}
defer t.Close()
- if err := t.Clone(branch); err != nil {
+ if err := t.Clone(branch, true); err != nil {
return nil, err
}
if err := t.SetDefaultIndex(); err != nil {
diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go
index fdf0b32f1a..5bd59251df 100644
--- a/services/repository/files/patch.go
+++ b/services/repository/files/patch.go
@@ -108,7 +108,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user
log.Error("%v", err)
}
defer t.Close()
- if err := t.Clone(opts.OldBranch); err != nil {
+ if err := t.Clone(opts.OldBranch, true); err != nil {
return nil, err
}
if err := t.SetDefaultIndex(); err != nil {
diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go
index 7f6b8137ae..4c8cfb2481 100644
--- a/services/repository/files/temp_repo.go
+++ b/services/repository/files/temp_repo.go
@@ -51,8 +51,13 @@ func (t *TemporaryUploadRepository) Close() {
}
// Clone the base repository to our path and set branch as the HEAD
-func (t *TemporaryUploadRepository) Clone(branch string) error {
- if _, _, err := git.NewCommand(t.ctx, "clone", "-s", "--bare", "-b").AddDynamicArguments(branch, t.repo.RepoPath(), t.basePath).RunStdString(nil); err != nil {
+func (t *TemporaryUploadRepository) Clone(branch string, bare bool) error {
+ 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()
if matched, _ := regexp.MatchString(".*Remote branch .* not found in upstream origin.*", stderr); matched {
return git.ErrBranchNotExist{
@@ -98,6 +103,14 @@ func (t *TemporaryUploadRepository) SetDefaultIndex() error {
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
func (t *TemporaryUploadRepository) LsFiles(filenames ...string) ([]string, error) {
stdOut := new(bytes.Buffer)
diff --git a/services/repository/files/update.go b/services/repository/files/update.go
index f01092d360..9d32876eff 100644
--- a/services/repository/files/update.go
+++ b/services/repository/files/update.go
@@ -141,7 +141,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
}
defer t.Close()
hasOldBranch := true
- if err := t.Clone(opts.OldBranch); err != nil {
+ if err := t.Clone(opts.OldBranch, true); err != nil {
for _, file := range opts.Files {
if file.Operation == "delete" {
return nil, err
diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go
index f4e1da7bb1..c893c5c43e 100644
--- a/services/repository/files/upload.go
+++ b/services/repository/files/upload.go
@@ -87,7 +87,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
defer t.Close()
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 {
return err
}
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/HEAD b/tests/gitea-repositories-meta/user2/test_commit_revert.git/HEAD
new file mode 100644
index 0000000000..b870d82622
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/test_commit_revert.git/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/main
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/config b/tests/gitea-repositories-meta/user2/test_commit_revert.git/config
new file mode 100644
index 0000000000..57bbcba5be
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/test_commit_revert.git/config
@@ -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
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/description b/tests/gitea-repositories-meta/user2/test_commit_revert.git/description
new file mode 100644
index 0000000000..498b267a8c
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/test_commit_revert.git/description
@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/info/exclude b/tests/gitea-repositories-meta/user2/test_commit_revert.git/info/exclude
new file mode 100644
index 0000000000..a5196d1be8
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/test_commit_revert.git/info/exclude
@@ -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]
+# *~
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/objects/pack/pack-91200c8e6707636a6cc3e0d8101fba08b19dcb91.idx b/tests/gitea-repositories-meta/user2/test_commit_revert.git/objects/pack/pack-91200c8e6707636a6cc3e0d8101fba08b19dcb91.idx
new file mode 100644
index 0000000000000000000000000000000000000000..77bcbe7fb4caeaf783d375068e6ffad1a61ed6f9
GIT binary patch
literal 1268
zcmexg;-AdGz`(>nd%(!RKsE-sZ=f(U0|QMl3j@O_9-uH_WndU^%*Mb#HfCpFpbnPU
z->08CXNQwdPL65s%82#e3p^^-eHeo7hH(5?r+vbOLGyq5_1Q-cyRFLFbfR(Hgom+n
z&YyI@m=x2g^EB??V(VbJ#K@;xHu&frp3&v9&+yY;^W(8%TSdHYUR6>)@BCf-&aq!%
z_g<eq9`w{<*Oqg3&xJ4l>Az#T>HAx$&y2TTFEcp*oWW#%U$m0Z)=mMznA>`cHe2?8
z-}-&)E`38kvvX?F5B_JqO#kv&o`He!90LQB0|Nu2Bm)DZKLZ2fKL!Q{9tH*`fr$z{
red+AUSviLv+z^o8#j$bj>4{T1%ou`>&%0ZEj@95$+VLx%XWjGxmLGEK
literal 0
HcmV?d00001
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/objects/pack/pack-91200c8e6707636a6cc3e0d8101fba08b19dcb91.pack b/tests/gitea-repositories-meta/user2/test_commit_revert.git/objects/pack/pack-91200c8e6707636a6cc3e0d8101fba08b19dcb91.pack
new file mode 100644
index 0000000000000000000000000000000000000000..7271cdaeb87741f0ed005bdae83367b7e3dd76ff
GIT binary patch
literal 609
zcmV-n0-pU)K|@Ob00062000M=8F-wXj!jO(FcgIMImMG$6oi_@f2my|#0A(Ou|~Et
zZX+dea9<0(Jw+6WH8Y!8jJ_Ez00|qCg-o4sF<32i>@HTu_TCxmlMcSscH^qW7tBls
z30|pSlt3qpg9$pcI!Z+4to2$4D3RoEUw7VjKFDaT@Ng}J>0OK|I=$&dCrz|YMwrSU
zd7leSSnq*C57+QC=YnNC*Idr^ZeaL&$d4p8tx-x!A?d!osET=-24?)9P(7pUK+AC)
zso?#H1Jmkt={<weFhbP)$t$|O5qFeIo~XwOV_s;OIn(5axvE}p8t{RBo#!p>nU}-1
zY5wju*HaB&@NYk--^vA4xcOc_G@|+fiMoNz>jHrdc$}NT{E=}&+{8XJCIiEX#~h>$
z3>CBufPjlDv>+w1Bvm0TGbhzhucV@c3ji-z4N{&9c$}TgK@P$o6h+ZFt6**<9}p>t
zF}f5e4@**{A>YLAUI(tgJ36Dw1CYo%7E3xuq<FS87kDCj3T1*z##ocw6f}wW;-wGS
za9n^PFF)L#ePCO6#bZ~G%807=tJI#HwMP3O5A;pUy|v1W{}1Axvn+LmR{Uo10VDxB
zh@}E}oHH~qFf%bxNXyJgHPkDqC}G%ZemquetBCi_t4hk}oxh9UIrb|I0O*ho_BnW*
zGc+<b;W9Ndw*UYLJpr$!0(hJ=G%zqTF;Pg%%t<xWE2$`9Q1@X7x*NjrW1aR17Y5D$
v>DOl;Jq!TW;|>!vc$_mdGByDK0r&vRksu6?X9r_yY{TH#5Ffe-v7O72@1GJp
literal 0
HcmV?d00001
diff --git a/tests/gitea-repositories-meta/user2/test_commit_revert.git/packed-refs b/tests/gitea-repositories-meta/user2/test_commit_revert.git/packed-refs
new file mode 100644
index 0000000000..1f546d7fd5
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/test_commit_revert.git/packed-refs
@@ -0,0 +1,3 @@
+# pack-refs with: peeled fully-peeled sorted
+46aa6ab2c881ae90e15d9ccfc947d1625c892ce5 refs/heads/develop
+deebcbc752e540bab4ce3ee713d3fc8fdc35b2f7 refs/heads/main
diff --git a/tests/integration/repo_mergecommit_revert_test.go b/tests/integration/repo_mergecommit_revert_test.go
new file mode 100644
index 0000000000..4d612bdcdb
--- /dev/null
+++ b/tests/integration/repo_mergecommit_revert_test.go
@@ -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"))
+}