Skip to content

Conversation

@richardlau
Copy link
Member

Add a check that the gitref being built has not been updated since the build was requested. Requires COMMIT_SHA_CHECK to be set to the SHA of the commit to build/test.

Add a check that the gitref being built has not been updated since the
build was requested. Requires `COMMIT_SHA_CHECK` to be set to the SHA
of the commit to build/test.
Comment on lines 30 to 35
# COMMIT_SHA_CHECK needs to be set in the job. Check that the gitref
# that is checked out hasn't been updated since the job was requested.
if [ "$(git rev-parse HEAD)" != "$(git rev-parse ${COMMIT_SHA_CHECK})" ]; then
echo "HEAD does not match expected COMMIT_SHA_CHECK"
exit 1
fi
Copy link
Member Author

@richardlau richardlau Mar 21, 2025

Choose a reason for hiding this comment

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

We might be able to change the checkout on L20 above to checkout $COMMIT_SHA_CHECK but I don't know if anything will break (for example, the subsequent possible rebase?) if the checkout is a "detached HEAD" state and not a checkout of a branch.

Copy link
Member

Choose a reason for hiding this comment

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

We can deploy the change and monitor for errors.

@richardlau
Copy link
Member Author

I've added an extra check that COMMIT_SHA_CHECK looks like a SHA and not some other gitref (such as a branch, e.g. refs/pull/<pr id>/head).

@richardlau richardlau merged commit a65c8bc into main Mar 25, 2025
4 checks passed
@richardlau richardlau deleted the shacheck branch March 25, 2025 15:55
richardlau added a commit that referenced this pull request Mar 27, 2025
Only enforce the COMMIT_SHA_CHECK verification when:
- The org/repo is not nodejs/node (i.e. outside of our project).
- The org/repo is nodejs/node and the reference is for a pull request.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants