Felipe Contreras <felipe.contre...@gmail.com> writes:

> 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.

> 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



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.

> @@ -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.

> +             }
>               if (commit) {
>                       if (!(commit->object.flags & SEEN)) {
>                               commit->object.flags |= SEEN;
>                               next = commit_list_append(commit, next);
>                       }
>               }
> +next:
>               e++;
>       }
>       if (!revs->leak_pending)

> 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 &&

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

Reply via email to