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

Enable creation of worktree from bare repo's default branch #4982

Merged
merged 5 commits into from
Feb 14, 2019

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Feb 14, 2019

Previously, we have determined whether a branch is checked out or not by simply checking the repo and all its worktree HEADs. This is the wrong thing to do in case where we have a bare repo, as a bare repo's HEAD will not be checked out but only indicate the "default" branch. This is getting fixed by this PR.

Furthermore, we'll now verify a given ref when adding new worktrees before creating the on-disk data structures. In case where the ref is invalid, we'll now stop leaving behind those files.

This fixes #4977

When adding a new worktree, we only verify that an optionally given
reference is valid half-way through the function. At this point, some
data structures have already been created on-disk. If we bail out due to
an invalid reference, these will be left behind and need to be manually
cleaned up by the user.

Improve the situation by moving the reference checks to the function's
preamble. Like this, we error out as early as possible and will not
leave behind any files.
We currently do not have any tests at all for the
`git_branch_is_checked_out` function. Add some basic ones.
Libraries should use assert(3P) only very scarcely. First, we usually
shouldn't cause the caller of our library to abort in case where the
assert fails. Second, if code is compiled with -DNDEBUG, then the assert
will not be included at all.

In our `git_branch_is_checked_out` function, we have an assert that
verifies that the given reference parameter is non-NULL and in fact a
branch. While the first check is fine, the second is not. E.g. when
compiled with -DNDEBUG, we'd proceed and treat the given reference as a
branch in all cases.

Fix the issue by instead treating a non-branch reference as not being
checked out. This is the obvious solution, as references other than
branches cannot be directly checked out.
Right now, the function `git_repository_foreach_head` will always
iterate over all HEADs of the main repository and its worktrees. In some
cases, it might be required to skip either of those, though. Add a flag
in preparation for the following commit that enables this behaviour.
In a bare repository, HEAD usually points to the branch that is
considered the "default" branch. As the current implementation for
`git_branch_is_checked_out` only does a comparison of HEAD with the
branch that is to be checked, it will say that the branch pointed to by
HEAD in such a bare repo is checked out.

Fix this by skipping the main repo's HEAD when it is bare.
@ethomson
Copy link
Member

Nice, thanks @pks-t.

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.

git_worktree_add fails on master branch with already checked out
2 participants