search

libgit2 / libgit2

A cross-platform, linkable library implementation of Git that you can use in your application.
https://libgit2.org/
Other
9.69k stars 2.41k forks source link

Status and Submodules #756

Closed dahlbyk closed 12 years ago

dahlbyk commented 12 years ago

Two unexpected behaviors:

  1. For an uninitialized submodule, status for the submodule path returns GIT_STATUS_WT_MODIFIED.
  2. For a clean initialized submodule, status for files within the submodule return GIT_STATUS_NOTFOUND.

In both cases git status comes back clean.

arrbee commented 12 years ago

Hey @dahlbyk! Are you running git_status_file and asking specifically about a file inside the submodule, or running git_status_foreach and getting results for each of the items inside the submodule? Because my belief is that the files inside the submodule should not even show up in git_status_foreach - the workdir iterator should skip the contents of an initialized submodule.

dahlbyk commented 12 years ago

I'm using git_status_foreach.

arrbee commented 12 years ago

Ok, hrm. Well, I'll see what I can recreate. Thanks.

dahlbyk commented 12 years ago

For a bit of context, I'm working on dahlbyk/posh-git#37 using libgit2sharp. When I switch into a fresh libgit2sharp clone, it shows one file modified: libgit2 (the submodule). After submodule init/update, every file in the module shows as deleted.

arrbee commented 12 years ago

Hey @dahlbyk!

A major reworking of the submodule code got merged and may likely have fixed these problems with submodule status. For some reason I'm having a deja vu that we discussed this already, but can you confirm if this problem is now resolved? Thanks!

dahlbyk commented 12 years ago

I won't be able to look at this until I can find time to rebuild the machine that had my C++ compiler. Unless someone would be so kind as to push a libgit2sharp branch with updated binaries? /cc @nulltoken

ben commented 12 years ago

The benstraub/clone branch will have what you need. It's libgit2 is built on @carlosmn's winhttp branch, which is experimental, but has the new submodule stuff integrated.

nulltoken commented 12 years ago

@ben @dahlbyk LibGit2Sharp/vNext has been updated with latest libgit2 :)

dahlbyk commented 12 years ago

Thanks @ben and @nulltoken :)

dahlbyk commented 12 years ago

Finally having a look at this and trying to reconcile this: https://github.com/libgit2/libgit2/blob/development/include/git2/status.h#L22-37 With our mapping: https://github.com/libgit2/libgit2sharp/blob/vNext/LibGit2Sharp/FileStatus.cs

Shouldn't we have failing tests?

arrbee commented 12 years ago

@dahlbyk Good question! @nulltoken thoughts?

I should have made more of a public notice of the fact that I wanted to renumber those constants - sorry that I didn't. I tried to space them out a bit so there would be space if we need to insert more in the middle.

I could have added the new constants at the end of the enumeration, and I could still be convinced to go back and do that before we tag a new release if folks feel very strongly, but I felt like I'd rather have the API and numbering that we want instead of a wonky one due to accidents of history (at least now, since we are still pre v1.0).

nulltoken commented 12 years ago

@dahlbyk You're right. That's very strange. Would you have some time to investigate further?

dahlbyk commented 12 years ago

Digging in now.

dahlbyk commented 12 years ago

My mistake, I was looking at the development branch; LG2# does not have that change yet. When we get it, I have a FileStatus update ready. :)

On the submodule front, an uninitialized or unmodified submodule directory now correctly reports Current. Is it expected that querying status on a file within the submodule reports Ignored? I have not tested status on modified files within a submodule.

arrbee commented 12 years ago

I believe with the latest ignore update that testing an individual file in a submodule will say it is ignored.

arrbee commented 12 years ago

But I should probably write a test :-)

dahlbyk commented 12 years ago

I wonder if it's worth having a separate status? It's not strictly "ignored"...

I realize this will never come up from normal git_status_foreach usage...maybe not worth worrying about?

dahlbyk commented 12 years ago

As for the original issue, as far as I can tell libgit2 status/submodule tests don't cover the uninitialized or unmodified cases. Am I missing something?

arrbee commented 12 years ago

I think tests-clar/submodule/status.c does cover uninitialized and unmodified submodules, albeit with a call to git_submodule_status() instead of a call to a git_status_... API.

Given that workdir iterators try git_submodule_lookup() on every subdirectory and that now diff.c:maybe_modified() calls through to git_submodule_status() when it sees a GITLINK entry, I think that the existing tests for submodule lookup and status are probably sufficient...

dahlbyk commented 12 years ago

If you're satisfied, I'm satisfied.