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

Index operations using globs #1661

Merged
merged 4 commits into from Jun 19, 2013
Merged

Index operations using globs #1661

merged 4 commits into from Jun 19, 2013

Conversation

arrbee
Copy link
Member

@arrbee arrbee commented Jun 19, 2013

This adds three new public APIs for manipulating the index:

  1. git_index_add_all is similar to git add -A and adds files in the working directory that match a pathspec to the index while honoring ignores, etc.
  2. git_index_remove_all removes files from the index that match a pathspec.
  3. git_index_update_all is similar to git add -u and updates entries in the index based on the current contents of the working directory, either added the new information or removing the entry from the index.

There are a variety of arguments to the functions that allow for fairly fine grained control of the behavior. See include/git2/index.h documentation for more detail.

This work should address #156 and #157, and I also hope that it will help with a number of libgit2/rugged API changes that are in the works.

Right now, setting up a pathspec to be parsed and processed
requires several data structures and a couple of API calls.  This
adds a new high level data structure that contains all the items
that you'll need and high-level APIs that do all of the setup and
all of the teardown.  This will make it easier to use pathspecs
in various places with less repeated code.
Command line Git sometimes generates an error message if given a
pathspec that contains an exact match to an ignored file (provided
--force isn't also given).  This adds an internal function that
makes it easy to check it that has happened.  Right now, I'm not
creating a public API for this because that would get a little
more complicated with a need for callbacks for all invalid paths.
This adds three new public APIs for manipulating the index:

1. `git_index_add_all` is similar to `git add -A` and will add
   files in the working directory that match a pathspec to the
   index while honoring ignores, etc.
2. `git_index_remove_all` removes files from the index that match
   a pathspec.
3. `git_index_update_all` updates entries in the index based on
   the current contents of the working directory, either added
   the new information or removing the entry from the index.
This adds some tests for updating the index and having it remove
items to make sure that the iteration over the index still works
even as earlier items are removed.

In testing with valgrind, this found a path that would use the
path string from the index entry after it had been freed.  The
bug fix is simply to copy the path of the index entry before
doing any actual index manipulation.
@arthurschreiber
Copy link
Member

This looks very nice! 👍 I'll see if I can get an initial implementation for this into Rugged tomorrow (unless someone else is already workin on this - @vmg?).

@arrbee
Copy link
Member Author

arrbee commented Jun 19, 2013

I'd love feedback on the APIs. I went back and forth with this for a while and I think the current form is fairly easy to use and gives most of the power necessary to emulate the various git command line options.

The one thing that can't be done easily given the current design is list all the individual pathspec entries that exactly matched ignored items in the directory. I did add a flag that lets you reject the update if the pathspec contains such items (just a git add will reject it), but once you know that the pathspec is rejected, you'd have to go through yourself to check which entries were problematic.

@arrbee
Copy link
Member Author

arrbee commented Jun 19, 2013

@arthurschreiber I'm glad it looks good to you! I was hoping it would make some of things you're doing a little bit easier. As you're playing around with it, let me know if you want any changes - I'm still on the fence about some parts of the API design.

@vmg
Copy link
Member

vmg commented Jun 19, 2013

The strarray, as always, makes this a PITA to use in bindings, but I guess we need to pass multiple pathspecs and this is the only way around it.

For everything else, this API is awesome. I love how you can do dry runs by using an empty callback. No complaints from me. :)

@vmg
Copy link
Member

vmg commented Jun 19, 2013

Oh, and on a related note, I just gave @arthurschreiber push bit to Rugged because he's been doing an awesome job at it. GO GO PORT THIS OVER.

vmg pushed a commit that referenced this pull request Jun 19, 2013
Index operations using globs
@vmg vmg merged commit 8b2fa18 into libgit2:development Jun 19, 2013
@arrbee arrbee deleted the index-add-all branch June 19, 2013 23:19
@arrbee
Copy link
Member Author

arrbee commented Jun 19, 2013

If we can thing about a better way to handle pathspecs, I'm all for it. We could make them a first-class object and provide an API for creating and manipulating them if that makes it easier for bindings.

@arrbee arrbee mentioned this pull request Jun 19, 2013
@arthurschreiber
Copy link
Member

@vmg Oh, thanks - I'm feeling honored. 😄

@arrbee arrbee mentioned this pull request Jun 19, 2013
phatblat pushed a commit to phatblat/libgit2 that referenced this pull request Sep 13, 2014
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.

None yet

3 participants