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

Fail push if a ref update is rejected #195

Merged
merged 1 commit into from
Jul 14, 2021
Merged

Conversation

squaremo
Copy link
Member

libgit2's Push method will succeed even when ref updates are rejected, meaning it can silently fail if you e.g., use branch protection in GitHub.

To make these errors visible, a callback is supplied to Push, which checks for a non-empty status (on the advice of https://libgit2.org/libgit2/#HEAD/group/callback/git_push_update_reference_cb).

For whatever reason, gogit seems overly sensitive to hook errors (in a way that git and libgit2 aren't), and reports "invalid pkg-len found" when it sees a rejected ref message. This doesn't affect the runtime code, since that uses libgit2 -- but it does affect the test code, which initialises the git repo used in many tests, so more care is needed to push only the main branch, so as not to trigger a rejection.

Fixes #194.

libgit2's Push method will succeed even when ref updates are rejected,
meaning it can silently fail if you e.g., use branch protection in
GitHub.

To make these errors visible, a callback is supplied to Push, which
checks for a non-empty status (on the advice of
https://libgit2.org/libgit2/#HEAD/group/callback/git_push_update_reference_cb).

For whatever reason, gogit seems overly sensitive to hook errors (in a
way that `git` and libgit2 aren't), and reports "invalid pkg-len
found" when it sees a rejected ref message. This doesn't affect the
runtime code, since that uses libgit2 -- but it does affect the test
code, which initialises the git repo used in many tests, so more care
is needed to push only the main branch, so as not to trigger a
rejection.

Signed-off-by: Michael Bridgen <michael@weave.works>
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Seems to do what the commit describes, thanks @squaremo 🍏

@squaremo squaremo merged commit 70eb6c4 into main Jul 14, 2021
@squaremo squaremo deleted the rejected-branch-push branch July 14, 2021 09:50
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 this pull request may close these issues.

Silent push failures on GitHub protected branch
2 participants