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

Actions: pull_request event checks out PR head, not merge commit #27039

Open
bilderbuchi opened this issue Sep 12, 2023 · 6 comments
Open

Actions: pull_request event checks out PR head, not merge commit #27039

bilderbuchi opened this issue Sep 12, 2023 · 6 comments
Assignees
Labels
topic/gitea-actions related to the actions of Gitea type/bug

Comments

@bilderbuchi
Copy link

bilderbuchi commented Sep 12, 2023

Description

tl;dr: When using a CI configuration set up with on: pull_request, this fires on PR updates, but checks out not the merge commit, but the PR HEAD.
This is unexpected, because

  • This behaviour differs from Github's
  • This makes it harder to do Continuous Integration, as the PR does not get tested as integrated into the target branch.

Observed behaviour

As documented, Gitea Actions implement the pull_request workflow trigger event, and it is "compatible with Github".
Githubs docs, linked from the above page, say that GITHUB_SHA will be the "Last merge commit on the GITHUB_REF branch" and GITHUB_REF will be the "PR merge branch refs/pull/:prNumber/merge".

I have a workflow configured with

on: 
  push:
    branches:
      - main
  pull_request:

and use actions/checkout@v3.

On a pull request update, in the "Set up job" log of the Workflow run, I unexpectedly see "GITHUB_REF:refs/pull/61/head" and a GITHUB_SHA entry that corresponds to the PR HEAD (i.e. the last pushed commit), and not the merge commit, as documented.

Expected behaviour

A pull_request event builds the merge commit of the PR commit that triggered the event, or some other way to continuously build the integrated branch. Otherwise a PR could test green, but break the tests after merging.

Gitea Version

1.20.3

Can you reproduce the bug on the Gitea demo site?

Could not check

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

This is a self-hosted instance, but I'm not running it so I can't offer more details.

Database

None

@bilderbuchi bilderbuchi changed the title pull_request action checks out pr head, not merge commit pull_request Action checks out PR head, not merge commit Sep 12, 2023
@bilderbuchi bilderbuchi changed the title pull_request Action checks out PR head, not merge commit Actions: pull_request event checks out PR head, not merge commit Sep 12, 2023
@lunny
Copy link
Member

lunny commented Sep 12, 2023

@Zettat123 Could you have a look at this?

@techknowlogick techknowlogick added the topic/gitea-actions related to the actions of Gitea label Sep 12, 2023
@Zettat123 Zettat123 self-assigned this Sep 18, 2023
@Zettat123
Copy link
Contributor

Thanks for the bug report. The reason of this bug is Gitea doesn't have refs/pull/:prNumber/merge references.

refs/pull/:prNumber/merge is a reference pointing to the merge commit between the PR's base reference and head reference. It is created by Github. Gitea currently has no such reference as refs/pull/:prNumber/merge

There has been a proposal(#13134) to add this kind of reference. This bug can only be fixed when Gitea supports such a reference. I'll update the documentation later to explain this difference between Gitea Actions and Github Actions.

@bilderbuchi
Copy link
Author

Thank you. That's a pity, as it also increases the conceptual distance between Gitea and Github CICD at a pretty base level. If indeed

the merge commit id is stored in the pull_request table.

#13134 does not sound like a complicated enhancement?

@Zettat123
Copy link
Contributor

the merge commit id is stored in the pull_request table.

#13134 does not sound like a complicated enhancement?

The refs/pull/:prNumber/merge reference in GitHub is different from the merge commit id in Gitea's pull_request table. In GitHub, refs/pull/:prNumber/merge is a reference to an intermediate state where the head branch is hypothetically merged to the base branch.

However, in Gitea, the commit id in the pull_request table is the actual commit id and the id can only be set after the head branch is actually merged into the base branch. So we cannot get the id before the PR is merged.

@bilderbuchi
Copy link
Author

bilderbuchi commented Sep 19, 2023

I'm not sure I get the distinction between "hypothetical" and "actual" here -- I'd think that Github actually does the PR merge in the background (maybe as part of the merge conflict checking?), because how else would it be able to derive a correct GITHUB_SHA for "Last merge commit on the GITHUB_REF branch"?
If that's the case, can Gitea do the same thing? e.g. also setting that id in the table as part of the merge conflict checking?

lunny pushed a commit that referenced this issue Sep 20, 2023
Related to #27039

The `ref` property in Gitea Actions is different from GitHub Actions.
This PR improves the documentation to explain the difference.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Sep 20, 2023
Related to go-gitea#27039

The `ref` property in Gitea Actions is different from GitHub Actions.
This PR improves the documentation to explain the difference.
silverwind pushed a commit that referenced this issue Sep 20, 2023
Backport #27126 by @Zettat123

Related to #27039

The `ref` property in Gitea Actions is different from GitHub Actions.
This PR improves the documentation to explain the difference.

Co-authored-by: Zettat123 <zettat123@gmail.com>
@Zettat123
Copy link
Contributor

Sorry for the ambiguity. I use "hypothetical" here simply because the head branch of the PR is not actually merged into the base branch.

From the documentation, we can find

There’s also a refs/pull/#/merge ref on the GitHub side, which represents the commit that would result if you push the “merge” button on the site. This can allow you to test the merge before even hitting the button.

I'm not sure how it is implemented, but I think Gitea should be able to do the same thing in theory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/gitea-actions related to the actions of Gitea type/bug
Projects
None yet
Development

No branches or pull requests

4 participants