Re: [PATCH] revision: add --except option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 30, 2013, 06:32 UTC
- Message-ID
- <7vhae7k7t1.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1377838805-7693-1-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 8 quoted lines
> So that it's possible to remove certain refs from the list without > removing the objects that are referenced by other refs. > > For example this repository: > > * 374e8dd (crap) crap > * 4cbbf7b (test) two > * d025ae0 (HEAD, master) one
Can we make it more clear that your assumption is "crap" is a child of "test" which is a child of "master"? Without that, the "nothing will come out" will not follow.
Show 6 quoted lines
> When using '--branches --except crap': > > * 4cbbf7b (test) two > * d025ae0 (HEAD, master) one > > But when using '--branches --not crap' nothing will come out.
If you have a history where
- branches "master" and "maint" point at commit A; - branch "next" points at commit B that is a descendant of A; and - there are tags X and Y pointing at commits that are ahead of B or behind A
i.e.
----X----A----B----Y
what are the desired semantics for these?
(1) --branches --except maint
(2) --all --not --branches --except maint
(3) ^master next --except maint
"--branches" wants to include "master", "next", and "maint", and the "--except" tells us we do not want to take "maint" into account, but should that affect what "master" wants to do (either include or exclude what are reachable from it)?
As the way we parse the revisions from the command line is to mark "objects", not "refs", as we go, it looks like that the flag SKIP in this patch is placed conceptually at a wrong level.
I agree "--branches --except maint" is a good concept, but to implement what this patch wants to do properly, I suspect that the revision parser may need to be extended to be a two-phase process; the first phase will collect list of objects involved in the range expression without marking them with UNINTERESTING mark (that would also involve expanding things like --all or --branches), while remembering those given with --except, exclude the "except" set from the first set, and then finally marking the objects using the remainder, or something like that.
Show 6 quoted lines
> @@ -2585,12 +2588,19 @@ int prepare_revision_walk(struct rev_info *revs)
> revs->pending.objects = NULL;
> while (--nr >= 0) {
> struct commit *commit = handle_commit(revs, e->item, e->name);
> + for (i = 0; i < revs->cmdline.nr; i++) {
> + struct rev_cmdline_entry *ce;"ce" will have a strong association with "cache entry"; avoid using that variable name for anything else to avoid confusion.
> + ce = &revs->cmdline.rev[i]; > + if ((ce->flags & SKIP) && !strcmp(ce->name, e->name)) > + goto next;
I think this SKIP will not help an object that is already tainted by UNINTERESTING; if it is discovered during a traversal from another object that will remain in the rev->commits, the travesal will stop there, even if a ref that is marked with SKIP will "goto next" here.
Show 11 quoted lines
> + }
> if (commit) {
> if (!(commit->object.flags & SEEN)) {
> commit->object.flags |= SEEN;
> next = commit_list_append(commit, next);
> }
> }
> +next:
> e++;
> }
> if (!revs->leak_pending)Show 15 quoted lines
> diff --git a/t/t6112-rev-list-except.sh b/t/t6112-rev-list-except.sh > new file mode 100755 > index 0000000..b8f9a61 > --- /dev/null > +++ b/t/t6112-rev-list-except.sh > @@ -0,0 +1,35 @@ > +#!/bin/sh > + > +test_description='test for rev-list --except' > + > +. ./test-lib.sh > + > +test_expect_success 'setup' ' > + > + echo one > content &&
Style.