Repository navigation
Fix forced branch creation on HEAD of a bare repo. - #3977
jfultz wants to merge 2 commits into
Conversation
Sorry, something went wrong.
The code correctly detects that forced creation of a branch on a nonbare repo should not be able to overwrite a branch which is the HEAD reference. But there's no reason to prevent this on a bare repo, and in fact, git allows this. I.e., git branch -f master new_sha works on a bare repo with HEAD set to master. This change fixes that problem, and updates tests so that, for this case, both the bare and nonbare cases are checked for correct behavior.
pks-t
left a comment
There was a problem hiding this comment.
Looks good besides the memory leak. Thanks for working on this 👍
Sorry, something went wrong.
| oid = git_commit_id(target); | ||
|
|
||
| cl_git_pass(git_branch_create(&branch, repo, "master", target, 1)); | ||
| branch = NULL; |
There was a problem hiding this comment.
This leaks memory as you do not free branch here and overwrite it afterwards.
Sorry, something went wrong.
There was a problem hiding this comment.
Copied and pasted the code from the legacy test case, but in the legacy test case it should fail, so there's nothing to clean up. Oops. :P
Sorry, something went wrong.
|
Looks good. Will wait for comments and merge later this week if nothing comes up. Thanks 🎉 |
Sorry, something went wrong.
|
Manually squashed in the fixup and merged via 6e2fab9. Thanks for your work |
Sorry, something went wrong.
The code correctly detects that forced creation of a branch on a
nonbare repo should not be able to overwrite a branch which is
the HEAD reference. But there's no reason to prevent this on
a bare repo, and in fact, git allows this. I.e.,
git branch -f master new_sha
works on a bare repo with HEAD set to master. This change fixes
that problem, and updates tests so that, for this case, both the
bare and nonbare cases are checked for correct behavior.