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

Pull request branch not updated when the diff is empty, even though there are changes in the tree #1191

Closed
oakkitten opened this issue May 31, 2022 · 9 comments · Fixed by #1307

Comments

@oakkitten
Copy link

oakkitten commented May 31, 2022

I suppose that this is working as intended, as the readme says,

If a pull request already exists and there are no further changes (i.e. no diff with the current pull request branch) then the action exits silently.

But this can lean to a situation where the PR branch (and hence the PR) is not updated even though there are changes in the tree, such as when the main branch was force pushed after rebase. I even ran into a situation with an empty diff and GitHub saying “This branch has conflicts that must be resolved”. While this problem is rare, probably extremely rare, perhaps there's a way to to update PR branch even when the diff is empty?

If you want to reproduce this behavior, force-pushing main branch without changes while PR is pending should trigger the issue if the workflow is set up in a more or less ordinary way. Here's one workflow run with this problem.

@oakkitten oakkitten changed the title Pull request branch not updated when diff is empty Pull request branch not updated when the diff is empty, even though there are changes in the tree May 31, 2022
@peter-evans
Copy link
Owner

Hi @oakkitten

What state is the PR in after you force push the main branch? Can it still be merged? Or is it complaining that there is a conflict?

If this is a rare problem, my inclination would be to suggest dealing with it by closing the PR and letting your workflow reopen it when it next runs. Note that if you don't want to wait for it to rerun you could add the workflow_dispatch event so you can trigger it manually from the UI.

@oakkitten
Copy link
Author

I once did run into a situation where there was a conflict so an easy merge wasn't possible.

I can resolve the conflict manually, this isn't a big deal. It's just that it seems that this action is designed to keep the PR up to date even if no merge conflicts exist—it seems that there's no way to opt out of that. But in this case the result depends on the state of the branch. I wonder if the issue can also arises if you change the number of new commits, or the order of new commits, or the commit message, or the author, or commiter? (I wonder if simply comparing old and new PR branches while ignoring dates would be suitable here?)

But as I said, it's not a big deal. If it's working as intended, sorry for bothering!

(P.S., I use this action for dependabot-like pull requests which I merge using fast-forward using another workflow, so I would want the PR branch to not only be conflict free but also fast-forwardable. However I guess there are even fewer people who care about this than those who run into the above issue.)

@peter-evans
Copy link
Owner

If you want to reproduce this behavior, force-pushing main branch without changes while PR is pending should trigger the issue if the workflow is set up in a more or less ordinary way.

When I have some time I'll test this scenario. The action only checks if there is a diff, so changes to commits after a force push on the target branch are probably being ignored. If such changes are leaving the PR in a state where it cannot be merged then I think that is not good behaviour, and perhaps there might be a way to detect that and update the PR by rebasing on to its target.

@highb

This comment was marked as off-topic.

@peter-evans
Copy link
Owner

@oakkitten I tried to reproduce the issue.

  1. Triggered a workflow that opens a PR with a change to merge to main
  2. Modified the commit message of the HEAD commit of main and force pushed. ("force-pushing main branch without changes")
  3. Trigger the workflow again to update the PR

After the force push on step 2, the diff of the PR is wrong. I guess this expected and just how git calculates the diff in this situation. However, after the workflow runs in step 3, the PR updates (rebases itself) and corrects the diff.

So I'm not seeing any issue here. Am I trying to reproduce this incorrectly?

@bgilbert
Copy link
Contributor

This demo repo has some workflows that should reproduce the problem:

  1. Fork the repo.
  2. Run the "PR" workflow in the fork to create a PR.
  3. Run the "Rewrite last commit" workflow to force-push the main branch.
  4. Run the "PR" workflow again. It won't rebase the update branch onto the new tip of main.

@peter-evans
Copy link
Owner

@bgilbert Thank you for creating that demo repo. It was very useful!

I've created a fix for this issue here. I also tested it using the demo repo @bgilbert created. You can see that this PR got updated after the force push of main to a new commit.

I'm about to go away for ~10 days so I don't want to merge this now, just in case it causes some unexpected issue and I need to roll it back. So you can either wait, or you can use the feature branch temporarily and update to v4 again after I release it.

If you want to use the feature branch:

      - uses: peter-evans/create-pull-request@base-force-push

@peter-evans
Copy link
Owner

Released as v4.2.1 / v4

Please let me know if/when it is safe to delete the base-force-push feature branch.

@bgilbert
Copy link
Contributor

Good to delete from my side. Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants