patch, 3 partsgit log --exclude

This series adds --exclude that uses .gitignore mechanism remove commits whose changes that are _entirely_ excluded. The main patch is 2/3 where it teaches diff_tree_* about struct exclude_list.

Because it uses .gitignore mechanism, beware that these patterns do not behave exactly like pathspecs (patterns without slashes match every directory, for example)

I tried these commands

  time git log --stat >/dev/null
  time git log --stat --exclude=Documentation >/dev/null

The former took 37 secs, the latter 40 secs. Not bad, but there is definitely room for improvement. skip_excludes() should be able to point out whether an entire directory is excluded and skip the whole directory (as opposed to descending in and checking files one by one now, in fear of negative patterns). These kinds of optimizations benefit sparse checkout too.

I think I made a mistake somewhere because the above command seems to remove more commits than it should... Regardless, "git log --exclude=po" on gnome-shell looks sooo clean.

Nguyễn Thái Ngọc Duy (3):
  diff-no-index: rename read_directory to avoid conflict from dir.h
  tree-diff: teach it to understand exclude patterns
  log: add --exclude option

 diff-no-index.c |    6 ++--
 diff.h          |   11 ++++++++++
 revision.c      |    7 ++++-
 revision.h      |    2 +
 tree-diff.c     |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 75 insertions(+), 9 deletions(-)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff-no-index.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

We introduce a new set of API, diff_tree_*_with_exclude, that also exclude entries based on .gitignore patterns.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.h      |   11 +++++++++++
 tree-diff.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 65 insertions(+), 4 deletions(-)

This only helps filtering out commits whose changes are _entirely_ filtered out. This does not affect patch generate (--patch, --stat and so on)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c |    7 +++++--
 revision.h |    2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

I'd see this --exclude as the opposite of specifying files, i.e. in a repository containing directories A, B and C,

git log --exclude=B

would be the same as

git log A C

If I understand correctly, it's not the case with your implementation because

and because "git log --stat A C" (or --patch) will show the diff only for A and C for commits touching all directories.

2011/10/5 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:

I think I emphasized it too much. "git log --exclude=B/ A B C" should be equivalent to "git log A C". If changes touch A or C, then no matter they touch B, the commit will always be in. If changes only touch B, neither A nor B, then the commit is removed, exactly the same case with "git log A C".

if you specify --exclude=B, then A/.../B, C/.../B are all removed. A subtle difference between pathspec and .gitignore.

I'll take care of --patch and friends later. They both (--patch and commit pruning) use the same diff mechanism, if we get it right for for commit pruning, --patch will come nicely.

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

I really do not want to see the exclude patterns nor mechanism used for this. This will unnecessarily impact the long term design of narrow clones I had in mind.

The way I envisioned the narrow cloning would work like this [*1*]:

 * The narrowed set of paths is an attribute of the local repository. It
   is not tied to the history nor the current working tree state, so the
   information does not live in the index or in the history. A new file
   $GIT_DIR/narrowed-paths specifies a list of pathspecs. We call a
   repository with such a file "a narrowed repository".

 * The objects that live in a narrowed repository are subset of the
   objects in an unnarrowed repository that records the same
   history. Objects are not modified in any way when transferring into
   a narrowed repository. E.g. if you clone git.git but limit the tree to
   Documentation/ and builtin/, you will get _all_ commit objects, even
   the ones that do _not_ touch these two directories, and the top level
   tree objects. These top level tree objects _do_ record the object names
   for paths outside the narrowed area. To facilitate local history
   traversal, we may add either grafts or replace entries to "gather" away
   commits that do not touch the narrowed area, but this is not essential.

 * "struct pathspec" gains a handful of new fields to carry the set of
   narrowing pathspecs taken from $GIT_DIR/narrowed-paths file down the
   callchain. When we decide to see if a given path matches the pathspec
   and/or is worth descending into, we use these pathspecs to reject
   anything that is outside of the narrowed area before applying user
   supplied pathspec. We use narrowing pathspecs even for the whole tree
   operation.

 * "fsck", "prune" and friends that do not use user-supplied pathspec will
   learn the narrowed-paths so that the tree entries that describe objects
   outside the narrowed area (e.g. "Makefile" in the top-level tree in a
   repository narrowed to "Documentation/" and "builtin/") are ignored.

   I looked at fsck and unpack-objects (surprise, but it does seem to need
   some work) a few months ago for this exact purpose, and it appeared
   doable.

We may want to be able to say something like "I want Documentation/ but not Documentaion/howto/", and for that to work, narrowed-paths needs to be able to express negative matches. And in _that_ context, we will need negative pathspec matches (and I think during the last round of pathspec updates we identified where the support for ":(no):pathspec" needs to be added in the codepath).

You could say "Use pathspec for inclusion and ignore patterns for exclusion", but I do not think that inconsistency has merits.

For the purpose of "log --exclude" [*2*], I do not mind too much if the UI expressed negative pathspecs using such a new command line option, but I think it would be more natural to say (notations aside):

	$ git log -- ':(no):po' .

and define the behaviour of user-supplied pathspec limiter this way:

 * Paths are matched from left to right;

 * First match determines the fate of the path;

 * A match with negative pathspec means "the path in question does _not_
   match".

[Footnote]

*1* Unlike the "This is _one_ possible way" I usually give in discussions, this is "The narrow clone _must_ work like this, if we were to add it".

*2* I would suggest not using --exclude to avoid giving users a wrong impression that this might have something to do with exclude patterns. How about calling it "--filter-out" instead?

On Wed, Oct 05, 2011 at 10:20:30AM -0700, Junio C Hamano wrote:

I'm really just a bystander on this topic, and haven't given it too much thought. But one stumbling block I see for narrow clone is how narrow repositories will interact with object transfer from other repositories.

For example, if I have a narrow git.git that omits Documentation, and I do a "git fetch" from a non-narrow repository, then how do we tell the non-narrow remote that we don't have blobs in Documentation, and that they should not be used as delta bases for any objects that are sent?

The current protocol relies on certain repository properties on the remote end that narrow clone will violate. I don't see a way around that without a protocol extension to communicate the narrowness. What will that extension look like?

-Peff

Jeff King <peff@peff.net> writes:

It would have to involve exchanging "I have/am interested in the paths that match these pathspecs". I do not mind if our initial implementation did not support anything other than fetching into a narrow from full and pushing from a narrow to full. The second iteration could add fetching and pushing between two narrows with the same set of narrowing pathspecs.

As to widening the area after a clone is made, I do not mind and if our initial impementation only supported widening the area in a stupid way (e.g. semi-equivalent of initial clone with the widened set of narrowing pathspecs).

2011/10/7 Jeff King <peff@peff.net>:

Pretty much how shallow cloned repos interacts. In shallow clone we send depth info. In narrow clone, we send width info, in terms of (restricted) pathspec. With pathspec (possibly plus exclude rules but this possibility is getting smaller), I can express "fetch this directory only", or "fetch this directory, but not this subdirectory (because I already have it)".

We can prohibit certain use cases at client side just like shallow clone.

Something like this

http://thread.gmane.org/gmane.comp.version-control.git/155427/focus=155613

2011/10/6 Junio C Hamano <gitster@pobox.com>:

Things have changed a bit since last time I touched negative pathspec. Because of depth limit support [1], pathspecs have to be sorted, which means we cannot keep pathspec in original order to match from left to right.

There may be a solution to mix depth limit and negative pathspec. I haven't thought carefully about that because I lean towards a simpler behaviour that only allows one negation level: a file is in if it matches any positive pathspecs and none of negative ones.

This is enough if narrow clones consist of positive pathspec only. I really doubt if users would want to make a narrow clone that "contains A but not A/B, storage-wise".

[1] 86e4ca6 (tree_entry_interesting(): fix depth limit with overlapping pathspecs)

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

I tend to think it probably is acceptable solution to define "struct pathspec" to have one positive and one negative set, traversing and declaring a match on what matches something from the positive set only if it does not match anything in the negative set.

That is similar to how we define revision ranges in the sense that the range notation to have two ranges (A..B C..D) does not mean union of two sets (A..B + C..D), but is difference between two unions ((B D)-(A C)).

And if you define "struct pathspec" to have one positive and one negative set, you do not have to limit narrow clone only to positive. The repository specific narrow pathspec will have one such "struct pathspec", but the user may give us from the command line another "struct pathspec". At runtime, we would merge them to form into one negative and one positive set, and apply the same rule: nothing that matches any negative will appear in the traversal or in the output.