Ref slash prefix workaround (empty branches collection) #108
Conversation
|
@tanoku This seems like a libgit2 bug, yes? |
|
Certainly sounds like it. Checking for |
|
@markus-olsson Nice spotting and awesome analysis!
... and indeed LibGit2Sharp should support this as well. Would you be so kind as to add to your commit a change to the xml documentation of the @arrbee @carlosmn unless you're already working on this, I'm going to send a PR to fix this issue in libgit2 |
ligit2sharp doesn't need to to anything, libgit2 already does. This is just a bug in the underlying library. I'm not sure where we should fix this though. We could move the path prettyfying to after trying to find |
My wording was inaccurate. I was referring to the documentation which was not in sync with libgit2 abilities.
It looks like replacing
|
|
@nulltoken sure, I can open a new pull-request with the documentation change if you want me to? Or should I add it to this PR (I'm guessing not since it doesn't look like it will be merged)? |
|
@markus-olsson I'm indeed going to send a PR to fix libgit2. But, it has to be reviewed, accepted (or patched, or even rejected) and merged. This might take some time. Thus, my proposal is to merge your workaround into LibGit2Sharp with the documentation change. Once libgit2 is fixed, we'll remove the "workaround" while embedding a new version of libgit2 binaries. It this fits you, you can amend your commit and force push it against your |
|
@nulltoken right, no problem. I'll fix it. I'm curious though. Is a force-pushed amended commit preferable to simply pushing an additional commit with just the documentation changes to my branch and, if so, why? |
|
@markus-olsson It would make the commit more cohesive. As this fixes the opening of a repository through the working directory path, and makes it work as it should, it makes sense to add the change of the documentation in the same change. |
When libgit2 is passed a path to a working directory instead of a git directory the names returned from git_reference_listall (and perhaps other similar methods) will be prefixed with a slash such that insteaf of refs/heads/master it'll return /refs/heads/master. LibGitSharp always does it's string prefix comparisons without a starting slash (which seems to be the correct thing to do). Includes test which verifies the problem by copying the sample working directory and performing the same test (CanListAllBranches) that's run for the bare repository. Update constructor documentation to reflect that it's possible to pass the path to a working directory.
|
@nulltoken ah, yes that makes sense :) The commit has been amended, just ping me if there's anything else! |
|
Thanks for this one. Merged! BTW, a PR has been sent to fix this in the native library (see libgit2/libgit2#549) |
…rward slash when opening a repository through a working directory path This fixes an issue which was detected while using one of the libgit2 bindings [0]. The lack of the trailing forward slash led the name of references returned by git_reference_listall() to be prefixed with a forward slash. [0]: libgit2/libgit2sharp#108
When libgit2 is passed a path to a working directory instead of a git directory the names returned from git_reference_listall (and perhaps other similar methods) will apparently be prefixed with a slash such that instead of refs/heads/master it'll return /refs/heads/master.
I'm at fault for not reading the constructor documentation for Repository which does say that you should give it the path to the .git-directory, not the working directory. But since it worked for the trivial stuff I was doing I didn't notice it until I tried retrieving all branches and repo.Branches was empty. Digging into the code I saw that LibGit2Sharp calls git_reference_listall (through Libgit2UnsafeHelper.ListAllReferenceNames) and then filters the output using the LooksLikeABranchName predicate.
When I called ListAllReferenceNames myself I saw that it was returning the refs with a slash prefixed (/refs/heads/...) and then noticed that it was only doing so when I opened a repository with the path to a working directory instead of a git directory.
Perhaps I shouldn't try to load a repo witha working directory at all but since libgit2 seems to support it (see https://github.com/libgit2/libgit2/blob/development/src/repository.c#L164-171) I thought I should at least open an issue.
My workaround does more or less exactly what libgit2 does, it checks for the presence of a .git-directory under the path given to the constructor and if it exists it replaces the path provided with the path to the .git-directory before passing it on to libgit2.