Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restore-file occasionally restores the wrong commit #4679

Closed
1 task done
fregante opened this issue Aug 18, 2021 · 2 comments · Fixed by #6596 or #6670
Closed
1 task done

restore-file occasionally restores the wrong commit #4679

fregante opened this issue Aug 18, 2021 · 2 comments · Fixed by #6596 or #6670
Assignees
Labels
bug help wanted Please! ♥︎ Particularly useful features that everyone would love!

Comments

@fregante
Copy link
Member

fregante commented Aug 18, 2021

Please ensure:

  • The bug is caused by Refined GitHub. It doesn't happen if I disable the extension.

Description

Rare occurrence but:

  1. Create X branch from main
  2. Force-push main to something else
  3. Create PR from branch X

Now you'll have a PR with the changes that the force-push made.

  1. Click "Restore file" on one of the files in the PR

You'll notice that the feature "succeeds" but the commits are empty. This is a live example: 419c37c (#4677)

Example URL

#4677, before it was rebased

Browser(s) used

Safari

@fregante fregante changed the title restore-file is not compatible with force-pushed branches restore-file occasionally restores the wrong commit Sep 4, 2021
@fregante
Copy link
Member Author

fregante commented Sep 4, 2021

Maybe it's not even due to force pushes. There were no force pushes in npmhub/npmhub#141, but this "Restore" commit is incorrect: 7485182 (#141)

The issue specifically is that it "correctly" fetched the file from the base branch (main), but GitHub still shows the file as a "change" in the PR (even though when merged nothing will actually change): https://github.com/npmhub/npmhub/pull/141/files#diff-719f723e7f36f1f0192a75b8750569dcb1319113ea7e038109894895d2ea7d74

You can try restoring the same file multiple times but that's what you get.

In short, I think we should fetch the file not from the base branch but from the base commit.

I think the repro is:

  1. Create PR with file X
  2. Change the same file X from the PR directly on the base branch
  3. Restore file X in the PR

@fregante
Copy link
Member Author

fregante commented May 12, 2023

"merge base" is what we're looking for: https://stackoverflow.com/q/1549146/288906

v3 seems to have merge_base_commit, which is likely what we want: https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#compare-two-commits

It seems that it also exists on v4 as baseTarget, I'll try it

{
  repository(owner: "refined-github", name: "sandbox") {
    pullRequest(number: 68) {
      baseRefOid
      mergeable
      viewerCanEditFiles
      headRef {
        compare(headRef: "default-a") {
          status
          aheadBy
          baseTarget {
            oid
          }
        }
      }
    }
  }
}

Edit: no. Both baseTarget and headTarget just show the latest commit on either branch, regardless of the comparison.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment