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

Recognize git worktree directories as valid git repositories #19832

merged 1 commit into from Nov 1, 2019


Copy link

lexi-lambda commented Aug 23, 2019

Identify the Bug

This PR fixes issue #8168.

Description of the Change

The cause of the bug is an outdated implementation of isValidGitDirectory. The existing implementation is based on a version of valid_repository_path from libgit2 that predates worktrees. This updates the implementation of isValidGitDirectory (and isValidGitDirectorySync) to reflect the current version of valid_repository_path, which recognizes the repository layout used by worktrees.

Alternate Designs

Since this bug arose in the first place due to code needing to be kept in sync with libgit2, an alternate approach would be to find a way to use the functionality in libgit2 directly, avoiding this issue from coming up in the future. However, that would be a relatively invasive change, and this functionality seems unlikely to change frequently, so this approach seems alright for now.

Possible Drawbacks

As far as I am aware, the only drawback of this change is that it makes the implementation of isValidGitDirectory somewhat more complicated.

Verification Process

I have both added additional tests for isValidGitDirectory and isValidGitDirectorySync and tested that this change works on a worktree directory for a real repository.

Release Notes

  • Fixed an issue that prevented directories created by git worktree from being recognized as git repositories.
@lexi-lambda lexi-lambda force-pushed the lexi-lambda:support-git-worktrees branch from 1b34428 to c78870b Aug 23, 2019
@lexi-lambda lexi-lambda force-pushed the lexi-lambda:support-git-worktrees branch from c78870b to 15c7ea0 Aug 23, 2019

This comment has been minimized.

Copy link

rsese commented Aug 26, 2019

Thanks @lexi-lambda! For completeness and to help whoever would review this pull request, can you update the PR body to discuss the Alternate Designs and Possible Drawbacks sections of the template?


This comment has been minimized.

Copy link
Contributor Author

lexi-lambda commented Aug 27, 2019

@rsese Sure thing, I have now done so.


This comment has been minimized.

Copy link

rsese commented Aug 27, 2019

Thank you! 🙇

@rsese rsese added the triaged label Aug 27, 2019
@darangi darangi merged commit a2a1277 into atom:master Nov 1, 2019
1 check passed
1 check passed
Atom Pull Requests #20190823.3 succeeded
@lexi-lambda lexi-lambda deleted the lexi-lambda:support-git-worktrees branch Nov 2, 2019
@darangi darangi self-assigned this Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
4 participants
You can’t perform that action at this time.