git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: "Étienne SERVAIS" <etienne.servais@voucoux.fr>,
	git@vger.kernel.org, "Rafael Ascensão" <rafa.almas@gmail.com>
Subject: Re: Simplify-by-decoration with decorate-refs-exclude
Date: Fri, 02 Aug 2019 12:14:33 -0700
Message-ID: <xmqq36ijl6qu.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <37283d4e-3f79-a6b1-425a-f90704fbcce2@web.de> (=?utf-8?Q?=22R?= =?utf-8?Q?en=C3=A9?= Scharfe"'s message of "Fri, 2 Aug 2019 18:52:51 +0200")

René Scharfe <l.s.r@web.de> writes:

> Am 02.08.19 um 10:47 schrieb Étienne SERVAIS:
>> Thus, when I enter
>>
>> ```
>> git log --oneline --graph  --decorate=full --decorate-refs-exclude='refs/tags/<pattern>'
>> ```
>> The selected tags are properly excluded but once I add the
>> `simplify-by-decoration` option
>>
>> ```
>> git log --oneline --graph  --decorate=full --decorate-refs-exclude='refs/tags/<pattern>' --simplify-by-decoration
>> ```
>> The excluded tags pop back again.
>
> Does this help?

I can see how this would help, but it somehow feels a bit brittle
to rely on where the decorations get loaded.

I wonder if it would help to move the ability to handle decoration
filter down from the log layer to revisions.c API layer.

It looks to me that this caller of setup_revisions() can prepare
decoration_filter before it calls setup_revisions(); we can let the
revisions.c layer call load_ref_decorations() in setup_revisions()
if that is the case, no?

Other two callers of load_ref_decorations() are deep inside pretty.c
but I wonder in the longer term if we would want to turn them into
an "a lot higher level should have already loaded decorations"
assert.

Thanks.

> -- >8 --
> Subject: [PATCH] revision: load decorations lazily for --simplify-by-decoration
>
> Let setup_revisions() and friends respect a filtered set of decoration
> refs loaded by callers by postponing its own load_ref_decorations() call
> to just before decorations are used to simplify history.  That function
> only does any actual work the first time it is called.
>
> This allows using the revision option --simplify-by-decoration together
> with the log option --decorate-refs-exclude and having it simplify over
> the restricted set of refs.
>
> Reported-by: Étienne SERVAIS <etienne.servais@voucoux.fr>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  revision.c     |  8 +++++++-
>  t/t4202-log.sh | 15 +++++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index 07412297f0..d3456c959b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -633,6 +633,13 @@ static int rev_compare_tree(struct rev_info *revs,
>  		return REV_TREE_OLD;
>
>  	if (revs->simplify_by_decoration) {
> +		/*
> +		 * Load decorations lazily; later calls have no effect.
> +		 * This gives callers a chance to load a restricted set
> +		 * beforehand.
> +		 */
> +		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
> +
>  		/*
>  		 * If we are simplifying by decoration, then the commit
>  		 * is worth showing if it has a tag pointing at it.
> @@ -2063,7 +2070,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  		revs->simplify_by_decoration = 1;
>  		revs->limited = 1;
>  		revs->prune = 1;
> -		load_ref_decorations(NULL, DECORATE_SHORT_REFS);
>  	} else if (!strcmp(arg, "--date-order")) {
>  		revs->sort_order = REV_SORT_BY_COMMIT_DATE;
>  		revs->topo_order = 1;
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index c20209324c..bb66d1d93c 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -837,6 +837,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '
>  	test_cmp expect.decorate actual
>  '
>
> +test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
> +	cat >expect.decorate <<-\EOF &&
> +	Merge-tag-reach (HEAD -> master)
> +	reach (tag: reach, reach)
> +	seventh (tag: seventh)
> +	Merge-branch-tangle
> +	Merge-branch-side-early-part-into-tangle (tangle)
> +	tangle-a (tag: tangle-a)
> +	EOF
> +	git log -n6 --decorate=short --pretty="tformat:%f%d" \
> +		--decorate-refs-exclude="*octopus*" \
> +		--simplify-by-decoration >actual &&
> +	test_cmp expect.decorate actual
> +'
> +
>  test_expect_success 'log.decorate config parsing' '
>  	git log --oneline --decorate=full >expect.full &&
>  	git log --oneline --decorate=short >expect.short &&
> --
> 2.22.0

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02  8:47 Étienne SERVAIS
2019-08-02 16:52 ` René Scharfe
2019-08-02 19:14   ` Junio C Hamano [this message]
2019-08-02 20:36     ` René Scharfe
2019-08-02 21:20       ` Junio C Hamano
2019-08-02 23:21         ` Jeff King
2019-08-03  6:51         ` René Scharfe
2019-08-03 14:44           ` Junio C Hamano
2019-09-08 17:58             ` [PATCH 1/2] log: test --decorate-refs-exclude with --simplify-by-decoration René Scharfe
2019-09-09 18:17               ` Junio C Hamano
2019-09-08 17:58             ` [PATCH 2/2] log-tree: call load_ref_decorations() in get_name_decoration() René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq36ijl6qu.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=etienne.servais@voucoux.fr \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=rafa.almas@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git