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

Run hpc-ci on PRs filed from forks #78

Merged
merged 1 commit into from
Jan 31, 2025
Merged

Run hpc-ci on PRs filed from forks #78

merged 1 commit into from
Jan 31, 2025

Conversation

awnawab
Copy link
Collaborator

@awnawab awnawab commented Jan 28, 2025

This PR adds the ability to run the hpc-ci on PRs filed from forks. Importantly, this will only run once a maintainer has added the "approved-for-ci" label. Any further pushes to the PR will remove the label, and it will have to be manually re-added to re-run the hpc-ci. Thanks a lot @samhatfield for showing us how to do this 🙏

@awnawab awnawab added the approved-for-ci Approved to run hpc-ci label Jan 28, 2025
@awnawab awnawab removed the approved-for-ci Approved to run hpc-ci label Jan 28, 2025
@awnawab awnawab changed the title Setup to run hpc-ci on PRs filed from forks Run hpc-ci on PRs filed from forks Jan 28, 2025
@awnawab awnawab marked this pull request as ready for review January 28, 2025 10:39
@awnawab awnawab requested review from reuterbal and dareg January 28, 2025 10:39
@reuterbal reuterbal added the approved-for-ci Approved to run hpc-ci label Jan 28, 2025
Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just yesterday we realized that for the pull_request_target trigger, this action checks out the wrong branch because ${{ github.sha }} will point to the target branch top commit instead of the HEAD of the pull request branch.

We haven't confirmed this, yet, but it might be that we need to use ${{ github.ref }} instead (in l. 106 of your workflow file).

@awnawab
Copy link
Collaborator Author

awnawab commented Jan 29, 2025

Thanks @reuterbal for picking up on that! I'm turning this back into a draft whilst I investigate the above. (seems like I can't turn it into a draft 😅 )

@samhatfield
Copy link

github.ref_name is another contender. I think we just have to put some print statements in to the workflow file to figure it out. There is also github.event.pull_request which may have all the properties we need.

@awnawab
Copy link
Collaborator Author

awnawab commented Jan 29, 2025

To test the above I pushed a commit with a cmake debug print, and this print does indeed show up in the hpc-ci. The commit hash printed in the hpc-ci is however not the same as the HEAD of the PR branch, but that is explained by the following:

github.sha
This is the SHA for a temporary commit created for validating the pull request. The commit represents the results of a point-in-time merge of the pull_request.head to the pull_request.base. This value is also exposed through the environment variable GITHUB_SHA. By default, actions/checkout will pull this commit into the local workspace.

So it seems like github.sha does indeed correspond to the correct code, but the commit id is different because github.sha represents the merge commit of the PR head and PR base.

Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, I fell into a trap with the different hash there. All good then!

@awnawab
Copy link
Collaborator Author

awnawab commented Jan 31, 2025

Thanks @reuterbal and @samhatfield for the feedback! @dareg could you please have a look at this at your earliest convenience? I would like to merge this and #75 soon if you approve.

@awnawab awnawab merged commit 6f853b0 into main Jan 31, 2025
29 checks passed
@awnawab awnawab deleted the hpc-ci-from-fork branch January 31, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved-for-ci Approved to run hpc-ci
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants