Closed pull requests retain no information about deleted branches

[themightychris]

• Gitea version (or commit ref): 1.10.0
• Git version: n/a
• Operating system: Docker
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes: https://try.gitea.io/themightychris/test/pulls/2
  • No
  • Not relevant

Description

When the branch behind a closed pull request gets deleted, all information about the branch appears to be lost (whether the branch is deleted through Git or through the provided button in the pull request)

Observed behavior: The pull request does not retain the name of the branch, the pushed commits, or the diffs and does not offer a restore branch button

Expected behavior: A closed PR will permanently store a ref to the tip of the deleted branch, enabling its commit list and files changed views to continue working, contextual comments to continue to be read, and the branch to be restorable from the commit preserved by the PR.

Screenshots

[guillep2k]

I may be wrong about this, but I think this will be a problem with squash&merge and git gc. AFAIU git will drop any commits that are no longer referenced, so even if we keep a reference in the database, git won't know about it and the commits themselves will be lost.

[themightychris]

Pull requests are typically stored inside git's ref database under refs/pull/* which prevents GC from dropping the commits.

Indeed, Gitea still has the correct ref stored for the example:

$ git ls-remote https://try.gitea.io/themightychris/test.git
841afc0554da25a0610bcf09676fd8a5890d4548	HEAD
279983dad2964bbb74bbc9c087a289f32f74800a	refs/heads/develop
841afc0554da25a0610bcf09676fd8a5890d4548	refs/heads/master
279983dad2964bbb74bbc9c087a289f32f74800a	refs/pull/1/head
7bc8abcfa4ffb3f23632d18e2d6898cf9bb953cc	refs/pull/2/head

All needed content is still available:

$ git clone https://try.gitea.io/themightychris/test.git /tmp/test
Cloning into 'test'...
remote: Enumerating objects: 6, done.
remote: Counting objects: 100% (6/6), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 6 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (6/6), done.

$ cd /tmp/test
$ git fetch origin refs/pull/2/head:refs/heads/deleted-branch
remote: Enumerating objects: 5, done.
remote: Counting objects: 100% (5/5), done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From https://try.gitea.io/themightychris/test
 * [new ref]         refs/pull/2/head -> deleted-branch

$ git log deleted-branch
commit 7bc8abcfa4ffb3f23632d18e2d6898cf9bb953cc (deleted-branch)
Author: themightychris <chris@jarv.us>
Date:   Mon Nov 25 15:27:47 2019 +0000

    Update 'file.txt'

commit 841afc0554da25a0610bcf09676fd8a5890d4548 (HEAD -> master, origin/master, origin/HEAD)
Author: Chris Alfano <chris@jarv.us>
Date:   Tue Jun 25 01:04:45 2019 +0000

    first commit

$ git ls-tree deleted-branch
100644 blob 2a82ed1226e978932bec374e454a6f3906d3450e	file.txt

$ git cat-file -p 2a82ed1226e978932bec374e454a6f3906d3450e
one
three?

[lunny]

This should be a bug and I think this is a broken again?

[zeripath]

@themightychris Is this still a problem?

On current master - admittedly looking at merges from the same repo - I can't seem to replicate.

[themightychris]

@zeripath you would not see the issue with merged PRs, only PRs closed without merging

[zeripath]

Ah!

I understand.

The problem lies in here:

gitea/routers/repo/pull.go

Lines 328 to 427 in 5749b26

	// PrepareViewPullInfo show meta information for a pull request preview page
	func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.CompareInfo {
	repo := ctx.Repo.Repository
	pull := issue.PullRequest
	var err error
	if err = pull.GetHeadRepo(); err != nil {
	ctx.ServerError("GetHeadRepo", err)
	return nil
	}
	setMergeTarget(ctx, pull)
	if err = pull.LoadProtectedBranch(); err != nil {
	ctx.ServerError("GetLatestCommitStatus", err)
	return nil
	}
	ctx.Data["EnableStatusCheck"] = pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck
	var headGitRepo *git.Repository
	var headBranchExist bool
	// HeadRepo may be missing
	if pull.HeadRepo != nil {
	headGitRepo, err = git.OpenRepository(pull.HeadRepo.RepoPath())
	if err != nil {
	ctx.ServerError("OpenRepository", err)
	return nil
	}
	defer headGitRepo.Close()
	headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch)
	if headBranchExist {
	sha, err := headGitRepo.GetBranchCommitID(pull.HeadBranch)
	if err != nil {
	ctx.ServerError("GetBranchCommitID", err)
	return nil
	}
	commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
	if err != nil {
	ctx.ServerError("GetLatestCommitStatus", err)
	return nil
	}
	if len(commitStatuses) > 0 {
	ctx.Data["LatestCommitStatuses"] = commitStatuses
	ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
	}
	if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck {
	ctx.Data["is_context_required"] = func(context string) bool {
	for _, c := range pull.ProtectedBranch.StatusCheckContexts {
	if c == context {
	return true
	}
	}
	return false
	}
	ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts)
	}
	}
	}
	if pull.HeadRepo == nil || !headBranchExist {
	ctx.Data["IsPullRequestBroken"] = true
	ctx.Data["HeadTarget"] = "deleted"
	ctx.Data["NumCommits"] = 0
	ctx.Data["NumFiles"] = 0
	return nil
	}
	compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(repo.Owner.Name, repo.Name),
	pull.BaseBranch, pull.HeadBranch)
	if err != nil {
	if strings.Contains(err.Error(), "fatal: Not a valid object name") {
	ctx.Data["IsPullRequestBroken"] = true
	ctx.Data["BaseTarget"] = "deleted"
	ctx.Data["NumCommits"] = 0
	ctx.Data["NumFiles"] = 0
	return nil
	}
	ctx.ServerError("GetCompareInfo", err)
	return nil
	}
	if pull.IsWorkInProgress() {
	ctx.Data["IsPullWorkInProgress"] = true
	ctx.Data["WorkInProgressPrefix"] = pull.GetWorkInProgressPrefix()
	}
	if pull.IsFilesConflicted() {
	ctx.Data["IsPullFilesConflicted"] = true
	ctx.Data["ConflictedFiles"] = pull.ConflictedFiles
	}
	ctx.Data["NumCommits"] = compareInfo.Commits.Len()
	ctx.Data["NumFiles"] = compareInfo.NumFiles
	return compareInfo
	}

This should really look at the refs/pulls/x/head to get the commits not the headRepo. I guess the question is whether refs/pulls/x/head is always available on this template.

I'll take a look.

[zeripath]

So the next thing that needs to be done is to proffer to re-create from the previous pull head. Now that's a bit difficult as we may not have the permission to write to the original headRepo, or the headBranch may have moved on - however, that could be a separate issue. (Similarly for actually deleting PRs)

[themightychris]

If you're talking about having "Restore branch" button, perhaps that could just be visible but disabled in the case that the branch name isn't unused

[zeripath]

Because of the way the permissions work I think that needs more thought.

For example The repo owner should be able to restore to a branch in their repo from any pull - probably with the option to takeover a closed pr.

[tanrui8765]

Hello friend, I met the same problem on v1.11.7 too. Some closed pull requests with deleted branch, N/A commits and N/A changed files.

I tried to fix the problem with cmd doctor --run recalculate_merge_bases --fix and doctor -all -fix, but didn't work. The cmd results gave "2 PR mergebases updated of 173 PRs total in 71 repos" several times.

Is there any possible solution for this problem? Many Thanks~

[zeripath]

@tanrui8765 for us to be able to figure out if there is anything we can do here we need more information.

For example, does the Gitea repository still have the pull ref on the server? I.e. if you go to the repository on the filesystem does refs/pull/X/head exist? If not then there's nothing we can do - all of the information about that pull is gone.

Was the pull merged? If it wasn't does the base branch of the pull request still exist? If it doesn't then there's no base we can compare against to restore a merge base - we may still be able to use a mergebase if it's in the pull request table but previously PRs had very incorrect mergebases so...

In the pull table is there a merge base listed for the pr? Does that merge base produce a reasonable diff if you do: git diff mergebase refs/pull/X/head?

You need to tell us more about these PRs.

[tanrui8765]

Hello @zeripath , sorry for the insufficient information, I don't have much understanding about git & gitea (learning though), many thanks for your help.

In my case, I have a PR like this:

This PR is "Closed" because of the branch was needed to be renamed. The PR displays N/A commits and N/A changed files.
I upgraded gitea to V1.12.1 and executed "doctor -all -fix", I got the result below, but PR still have the problem:

I checked the repo, "refs\pull\466\head" exists. This 466 PR is not merged, but the branch has been deleted. Could you please tell me how to get the pull table?

However, I can find the branch origin tag and its head:

I did "git diff 649405ede055eedc015e75e851c6a87fcbcce0ea refs/pull/466/head", returned diff information, I think they are right.

Are these information reasonable & enough? Thanks~