Re: breakage in revision traversal with pathspec
On Fri, Sep 20, 2013 at 10:51:55AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: My original question was going to be: why bother peeling at all if we are just going to push the outer objects, anyway? And after staring at it, I somehow convinced myself that the answer was that you were pushing both. But that is not the case. Sorry for the noise. But that is still a valid point, and the patch to avoid peeling for non symmetric diff does not look too bad, either. revision.c | 59 ++-- t/t6000-rev-list-misc.sh | 8 +++ 2 files changed, 45 insertions(+), 22 deletions(-) FWIW, the flow of this version makes more sense to me. It also allows things like: git rev-list --objects $blob..$tree which I cannot see anybody actually wanting, but it somehow seems simpler to me to say A..B is syntactic sugar for ^B A, without qualifying except that A and B must be commit-ishes. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Jeff King p...@peff.net writes:
My original question was going to be: why bother peeling at all if we
are just going to push the outer objects, anyway?
And after staring at it, I somehow convinced myself that the answer was
that you were pushing both. But that is not the case. Sorry for the
noise.
But that is still a valid point, and the patch to avoid peeling for
non symmetric diff does not look too bad, either.
revision.c | 59 ++--
t/t6000-rev-list-misc.sh | 8 +++
2 files changed, 45 insertions(+), 22 deletions(-)
diff --git a/revision.c b/revision.c
index 68545c8..7010aff 100644
--- a/revision.c
+++ b/revision.c
@@ -1157,41 +1157,56 @@ int handle_revision_arg(const char *arg_, struct
rev_info *revs, int flags, unsi
}
if (!get_sha1_committish(this, from_sha1)
!get_sha1_committish(next, sha1)) {
- struct commit *a, *b;
- struct commit_list *exclude;
-
- a = lookup_commit_reference(from_sha1);
- b = lookup_commit_reference(sha1);
- if (!a || !b) {
- if (revs-ignore_missing)
- return 0;
- die(symmetric ?
- Invalid symmetric difference expression
%s...%s :
- Invalid revision range %s..%s,
- arg, next);
- }
+ struct object *a_obj, *b_obj;
if (!cant_be_filename) {
*dotdot = '.';
verify_non_filename(revs-prefix, arg);
}
- if (symmetric) {
+ a_obj = parse_object(from_sha1);
+ b_obj = parse_object(sha1);
+ if (!a_obj || !b_obj) {
+ missing:
+ if (revs-ignore_missing)
+ return 0;
+ die(symmetric
+ ? Invalid symmetric difference expression
%s
+ : Invalid revision range %s, arg);
+ }
+
+ if (!symmetric) {
+ /* just A..B */
+ a_flags = flags_exclude;
+ } else {
+ /* A...B -- find merge bases between the two */
+ struct commit *a, *b;
+ struct commit_list *exclude;
+
+ a = (a_obj-type == OBJ_COMMIT
+? (struct commit *)a_obj
+: lookup_commit_reference(a_obj-sha1));
+ b = (b_obj-type == OBJ_COMMIT
+? (struct commit *)b_obj
+: lookup_commit_reference(b_obj-sha1));
+ if (!a || !b)
+ goto missing;
exclude = get_merge_bases(a, b, 1);
add_pending_commit_list(revs, exclude,
flags_exclude);
free_commit_list(exclude);
+
a_flags = flags | SYMMETRIC_LEFT;
- } else
- a_flags = flags_exclude;
- a-object.flags |= a_flags;
- b-object.flags |= flags;
- add_rev_cmdline(revs, a-object, this,
+ }
+
+ a_obj-flags |= a_flags;
+ b_obj-flags |= flags;
+ add_rev_cmdline(revs, a_obj, this,
REV_CMD_LEFT, a_flags);
- add_rev_cmdline(revs, b-object, next,
+ add_rev_cmdline(revs, b_obj, next,
REV_CMD_RIGHT, flags);
- add_pending_object(revs, a-object, this);
- add_pending_object(revs, b-object, next);
+ add_pending_object(revs, a_obj, this);
+ add_pending_object(revs, b_obj, next);
return 0;
}
*dotdot = '.';
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index b10685a..15e3d64 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -48,4 +48,12 @@ test_expect_success 'rev-list --objects with pathspecs and
copied files' '
! grep one output
'
+test_expect_success 'rev-list A..B and rev-list ^A B are the same' '
+
Re: breakage in revision traversal with pathspec
Junio C Hamano gits...@pobox.com writes:
Kevin Bracey ke...@bracey.fi writes:
To see the effect at the command line: git log v1.8.3..v.1.8.4 hides
the merge, but git log ^v1.8.3 v1.8.4 shows it. Whoops. A new
example of a dotty shorthand not being exactly equivalent.
In the .. case the v1.8.3 tag gets peeled before being sent to
add_rev_cmdline , and the mark bottom commits logic works. But in
the ^ case, the v1.8.3 doesn't get peeled.
That sounds like a bug. ^v1.8.3 should mark v1.8.3^0 as
uninteresting.
OK, so git rev-list ^v1.8.3 v1.8.4 throws two objects into
revs-pending.objects[] array. Two tags, v1.8.3 marked as
UNINTERESTING and v1.8.4. The revision walking machinery will peel
the tag by calling handle_commit() (which by the way arguably is
misnamed because it has to be called for any type of object) when it
starts to walk in prepare_revision_walk().
But git rev-list v1.8.3..v1.8.4 throws two commits (v1.8.3^0 with
UNINTERESTING bit and v1.8.4^0) to revs-pending.objects[] after
peeling. I _think_ it is wrong. Because the range is only defined
over commit DAG, and because the same codepath handles the symmetric
difference v1.8.3...v1.8.4 as well, both ends of dots operator do
need to be peeled to commits, but I think it is wrong to throw these
peeled results into revs-pending.objects[].
Where it makes a difference is when rev-list is used with --objects.
$ git rev-list --objects v1.8.4^1..v1.8.4 | grep $(git rev-parse v1.8.4)
$ git rev-list --objects v1.8.4 ^v1.8.4^1 | grep $(git rev-parse v1.8.4)
04f013dc38d7512eadb915eba22efc414f18b869 v1.8.4
-- 8 --
Subject: revision: do not peel tags used in range notation
A range notation A..B means exactly the same thing as what ^A B
means, i.e. the set of commits that are reachable from B but not
from A. But the internal representation after the revision parser
parsed these two notations are subtly different.
- rev-list ^A B leaves A and B in the revs-pending.objects[]
array, with the former marked as UNINTERESTING and the revision
traversal machinery propagates the mark to underlying commit
objects A^0 and B^0.
- rev-list A..B peels tags and leaves A^0 (marked as
UNINTERESTING) and B^0 in revs-pending.objects[] array before
the traversal machinery kicks in.
This difference usually does not matter, but starts to matter when
the --objects option is used. For example, we see this:
$ git rev-list --objects v1.8.4^1..v1.8.4 | grep $(git rev-parse v1.8.4)
$ git rev-list --objects v1.8.4 ^v1.8.4^1 | grep $(git rev-parse v1.8.4)
04f013dc38d7512eadb915eba22efc414f18b869 v1.8.4
With the former invocation, the revision traversal machinery never
hears about the tag v1.8.4 (it only sees the result of peeling it,
i.e. the commit v1.8.4^0), and the tag itself does not appear in the
output. The latter does send the tag object itself to the output.
Make the range notation keep the unpeeled objects and feed them to
the traversal machinery to fix this inconsistency.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Made against maint-1.8.0 just in case we may want to fix older
maintenance series.
revision.c | 19 +--
t/t6000-rev-list-misc.sh | 8
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/revision.c b/revision.c
index 68545c8..a6d2150 100644
--- a/revision.c
+++ b/revision.c
@@ -1159,6 +1159,7 @@ int handle_revision_arg(const char *arg_, struct rev_info
*revs, int flags, unsi
!get_sha1_committish(next, sha1)) {
struct commit *a, *b;
struct commit_list *exclude;
+ struct object *a_obj, *b_obj;
a = lookup_commit_reference(from_sha1);
b = lookup_commit_reference(sha1);
@@ -1184,14 +1185,20 @@ int handle_revision_arg(const char *arg_, struct
rev_info *revs, int flags, unsi
a_flags = flags | SYMMETRIC_LEFT;
} else
a_flags = flags_exclude;
- a-object.flags |= a_flags;
- b-object.flags |= flags;
- add_rev_cmdline(revs, a-object, this,
+ a_obj = (!hashcmp(a-object.sha1, from_sha1)
+? a-object
+: lookup_object(from_sha1));
+ b_obj = (!hashcmp(b-object.sha1, sha1)
+? b-object
+: lookup_object(sha1));
+ a_obj-flags |= a_flags;
+ b_obj-flags |= flags;
+ add_rev_cmdline(revs, a_obj, this,
REV_CMD_LEFT, a_flags);
- add_rev_cmdline(revs, b-object, next,
+ add_rev_cmdline(revs, b_obj, next,
Re: breakage in revision traversal with pathspec
On Thu, Sep 19, 2013 at 02:35:40PM -0700, Junio C Hamano wrote: -- 8 -- Subject: revision: do not peel tags used in range notation A range notation A..B means exactly the same thing as what ^A B means, i.e. the set of commits that are reachable from B but not from A. But the internal representation after the revision parser parsed these two notations are subtly different. [...] Thanks for a very clear explanation. This definitely seems like an improvement, and the patch looks good to me. One question, though. With your patch, if I do tag1..tag2, I get both the tags and the peeled commits in the pending object list. Whereas with ^tag1 tag2, we put only the tags into the list, and we expect the traversal machinery to peel them later. I cannot off-hand think of a reason this difference should be a problem, but I am wondering if there is some code path that does not traverse, but just looks at pending objects, that might care. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Jeff King p...@peff.net writes: One question, though. With your patch, if I do tag1..tag2, I get both the tags and the peeled commits in the pending object list. Whereas with ^tag1 tag2, we put only the tags into the list, and we expect the traversal machinery to peel them later. I cannot off-hand think of a reason this difference should be a problem, but I am wondering if there is some code path that does not traverse, but just looks at pending objects, that might care. Did I really do that? I thought that the original was pushing peeled tag1^0 and tag2^0 (and nothing else) for tag1..tag2, and the intent of the patch was to see if a (which is tag1^0 in this case) has the same object name as the object originally given on the side of the dots (i.e. tag1). If they differ, that means a is the peeled object, and instead use the original tag1 for a_obj that is pushed into the pending (and if they are the same, a_obj is just a-object, the object itself). The same for b, tag2 and b_obj. So at least I didn't mean to push four objects into the pending list before prepare_revision_walk() kicks in. Perhaps I missed something? Now, when prepare_revision_walk() picks up objects from the pending list, they are fed to handle_commit(), and these two tags will be peeled and their commits are returned to be queued in revs-commits linked list, while the tags themselves are sent to the pending list to be emitted in --objects output. But that should be the same between tag1..tag2 and ^tag1 tag2. A possible difference in behaviour is that with ^tag1 tag2, we do not instantiate the commit objects pointed at by these tags until prepare_revision_walk() sends these tags to handle_commit(), while with tag1..tag2, these tags and the commit objects would already be parsed when setup_revisions() returns (and the updated code does rely on this behaviour by saying if a-object.sha1 and from_sha1 are different, we know the tag whose name is from_sha1 is already parsed, so we can just call lookup_object() on from_sha1 to grab it). But I do not think any code just tries to grab an object using a random object name outside the revision traversal and decide to do things that results in semantically different behaviour if the resulting object has (or has not) already been parsed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
On Thu, Sep 19, 2013 at 09:58:23PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: One question, though. With your patch, if I do tag1..tag2, I get both the tags and the peeled commits in the pending object list. Whereas with ^tag1 tag2, we put only the tags into the list, and we expect the traversal machinery to peel them later. I cannot off-hand think of a reason this difference should be a problem, but I am wondering if there is some code path that does not traverse, but just looks at pending objects, that might care. Did I really do that? I thought that the original was pushing peeled tag1^0 and tag2^0 (and nothing else) for tag1..tag2, and the intent of the patch was to see if a (which is tag1^0 in this case) has the same object name as the object originally given on the side of the dots (i.e. tag1). If they differ, that means a is the peeled object, and instead use the original tag1 for a_obj that is pushed into the pending (and if they are the same, a_obj is just a-object, the object itself). The same for b, tag2 and b_obj. So at least I didn't mean to push four objects into the pending list before prepare_revision_walk() kicks in. Perhaps I missed something? Hrm, no, it is me misreading the diff. My original question was going to be: why bother peeling at all if we are just going to push the outer objects, anyway? And after staring at it, I somehow convinced myself that the answer was that you were pushing both. But that is not the case. Sorry for the noise. The other reason I considered is that we want to make sure they do peel to commits. I do not think that is technically required for A..B, which can operate on non-commits. But it is for A...B, and I do not see any advantage in loosening A..B for the non-commit case. It would just complicate the code. Now, when prepare_revision_walk() picks up objects from the pending list, they are fed to handle_commit(), and these two tags will be peeled and their commits are returned to be queued in revs-commits linked list, while the tags themselves are sent to the pending list to be emitted in --objects output. But that should be the same between tag1..tag2 and ^tag1 tag2. Yes, I was specifically concerned about sites that did not call prepare_revision_walk(), but since the state is the same for both cases, it's a non-issue. it). But I do not think any code just tries to grab an object using a random object name outside the revision traversal and decide to do things that results in semantically different behaviour if the resulting object has (or has not) already been parsed. Yeah, I think any code relying on that would be insane, from a modularity perspective. The caching of parsed object state is an optimization, and callers have no business making assumptions about it. Otherwise they are fragile with respect to a previous traversal being added in the same in-memory process. So I think your patch is doing the right thing, and my concern was just from mis-reading. Again, sorry for the confusion. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
On 11/09/2013 01:23, Junio C Hamano wrote: Kevin Bracey ke...@bracey.fi writes: On 10/09/2013 20:19, Junio C Hamano wrote: This command $ git log v1.8.3.1..v1.8.4 -- git-cvsserver.perl reports that a merge 766f0f8ef7 (which did not touch the specified path at all) touches it. Bisecting points at d0af663e (revision.c: Make --full-history consider more merges, 2013-05-16). That merge appearing *with* --full-history would seem like correct behaviour to me. Or at least it's what I intended. ... But it shouldn't appear if the user does not ask for --full-history. Well, there is a functioning semi-work-around for now: avoid difficult non-linear questions like v1.8.3.1..v1.8.4. A question like v1.8.3..v1.8.4 is a lot easier to visualise, and it does already omit the merge. On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x In effect, we do have a linear section of history to follow, and the file does change in the middle of that line. It may be quite hard to come up with a solid rule to hide the merge that doesn't go wrong somewhere else. The current rules for this are 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. For this query, the mainline link to 1.8.3 is the uninteresting side branch! If you do specify v1.8.3..v1.8.4, then v1.8.3 becomes on-graph thanks to other new rules, and this rule does kick in, hiding the merge. 2) If rule 1 doesn't activate, and it remains as a merge, hide it if treesame to all on-graph parents. Previously this rule was hide if treesame to any parent, and so that would have hidden the merge. Now, when I changed rule 2, I did not think this would affect the default log. See my commit message: Now redefine a commit's TREESAME flag to be true only if a commit is TREESAME to _all_ of its [later: on-graph] parent. This doesn't affect ... the default simplify_history behaviour (because partially TREESAME merges are turned into normal commits)... Whoops - partially TREESAME merges are not always turned into normal commits. Maybe the fix is to define TREESAME differently for simplify_history - to use the old definition of identical to any parent in that case. I'm not sure that's right though. I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Kevin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Kevin Bracey wrote: On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x [...] 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. [...] 2) If rule 1 doesn't activate, and it remains as a merge, hide it if treesame to all on-graph parents. Previously this rule was hide if treesame to any parent, and so that would have hidden the merge. Now, when I changed rule 2, I did not think this would affect the default log. See my commit message: [...] I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Thanks for this analysis. Interesting. The rule (1) comes from v1.3.0-rc1~13^2~6: commit f3219fbbba32b5100430c17468524b776eb869d6 Author: Junio C Hamano jun...@cox.net Date: Fri Mar 10 21:59:37 2006 -0800 try_to_simplify_commit(): do not skip inspecting tree change at boundary. When git-rev-list (and git-log) collapsed ancestry chain to commits that touch specified paths, we failed to inspect and notice tree changes when we are about to hit uninteresting parent. This resulted in git rev-list since.. -- file to always show the child commit after the lower bound, even if it does not touch the file. This commit fixes it. Thanks for Catalin for reporting this. See also: 461cf59f8924f174d7a0dcc3d77f576d93ed29a4 Signed-off-by: Junio C Hamano jun...@cox.net I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Jonathan Nieder jrnie...@gmail.com writes: I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. Sounds sensible. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
On 11/09/2013 21:24, Jonathan Nieder wrote: Kevin Bracey wrote: On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x [...] 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. [...] I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Thanks for this analysis. Interesting. The rule (1) comes from v1.3.0-rc1~13^2~6: ... I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. I agree about the rationale still applying - why not follow off-graph, unless you're doing --ancestry-path? (Fortunately ancestry_path already disables simplify_history). That makes more sense if you try to ignore the misleading comment. In a typical v1..v3 range, the temporal limiting means that it's paths to the mainline that will tend to be marked UNINTERESTING, not to the topic branches... But I can imagine going off graph it may previously have tripped up other parts of the code. It could be that this Git 1.3.0 rule ended up covering over some of the older merge hiding logic flakiness. Maybe it's no longer necessary. I'll do some experiments. Now, one bit of news - I have just figured out why gitk is behaving differently. It transforms .. before it reaches git. To see the effect at the command line: git log v1.8.3..v.1.8.4 hides the merge, but git log ^v1.8.3 v1.8.4 shows it. Whoops. A new example of a dotty shorthand not being exactly equivalent. In the .. case the v1.8.3 tag gets peeled before being sent to add_rev_cmdline , and the mark bottom commits logic works. But in the ^ case, the v1.8.3 doesn't get peeled. Junio - any thoughts on the correct place to fix that? (And gitk actually does ^tag-sha, just to be odd, so that needs to be handled too). Should these things be peeled in revs-cmdline or not? We should be consistent. Kevin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Kevin Bracey ke...@bracey.fi writes: To see the effect at the command line: git log v1.8.3..v.1.8.4 hides the merge, but git log ^v1.8.3 v1.8.4 shows it. Whoops. A new example of a dotty shorthand not being exactly equivalent. In the .. case the v1.8.3 tag gets peeled before being sent to add_rev_cmdline , and the mark bottom commits logic works. But in the ^ case, the v1.8.3 doesn't get peeled. That sounds like a bug. ^v1.8.3 should mark v1.8.3^0 as uninteresting. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
breakage in revision traversal with pathspec
I am grumpy X-. It appears that we introduced a large breakage during 1.8.4 cycle to the revision traversal machinery and made pathspec-limited git log pretty much useless. This command $ git log v1.8.3.1..v1.8.4 -- git-cvsserver.perl reports that a merge 766f0f8ef7 (which did not touch the specified path at all) touches it. Bisecting points at d0af663e (revision.c: Make --full-history consider more merges, 2013-05-16). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Kevin Bracey ke...@bracey.fi writes: On 10/09/2013 20:19, Junio C Hamano wrote: I am grumpy X-. It appears that we introduced a large breakage during 1.8.4 cycle to the revision traversal machinery and made pathspec-limited git log pretty much useless. This command $ git log v1.8.3.1..v1.8.4 -- git-cvsserver.perl reports that a merge 766f0f8ef7 (which did not touch the specified path at all) touches it. Bisecting points at d0af663e (revision.c: Make --full-history consider more merges, 2013-05-16). That merge appearing *with* --full-history would seem like correct behaviour to me. Or at least it's what I intended. Oh, of course. --full-history is about showing any pointless change, the mainline was a lot more up-to-date and there were changes relative to a fork based on an older baseline, so your updated log should show that in the mainline git-cvsserver.perl has been more fresh when that merge happened. But it shouldn't appear if the user does not ask for --full-history. However, your particular example occurs *without*--full-history, which suggests a problem. Yes. I note that gitk v1.8.3^0..v1.8.4 and git log --parents v1.8.3..v1.8.4 show that merge in Git 1.8.3, but not in Git 1.8.4. So we're going partially forwards, at least. With the testcases demonstrating the cases your series fixed that all look sensible, I think it is not really an option for us to revert them; you do not have to defend it with we are going partially forwards ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
