Re: [PATCH 2/2] describe: Exclude --all --match=PATTERN

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Greg Price <price@xxxxxxx> writes:

> Currently when --all is passed, the effect of --match is only
> to demote non-matching tags to be treated like non-tags.  This
> is puzzling behavior and not consistent with the documentation,
> especially with the suggested usage of avoiding information leaks.
> The combination of --all and --match is an oxymoron anyway, so
> just forbid it.

I am not sure if this is (1) "behaviour is sometimes useful in
narrow cases but is not explained well", (2) "behaviour does not
make sense in any situation", or (3) "the combination can make sense
if corrected, but the current behaviour is buggy".  If it is (2) or
(3), I think it makes sense to forbid the combination. Also, if it
is (3), we should later come up with an improved behaviour and then
re-enable the combination.

Without "--all" the command considers only the annotated tags to
base the descripion on, and with "--all", a ref that is not
annotated tags can be used as a base, but with a lower priority (if
an annotated tag can describe a given commit, that tag is used).

So naïvely I would expect "--all" and "--match" to base the
description on refs that match the pattern without limiting the
choice of base to annotated tags, and refs that do not match the
given pattern should not appear even as the last resort.  It appears
to me that the current situation is (3).

Will queue and cook in 'next'; thanks.

>
> Signed-off-by: Greg Price <price@xxxxxxx>
> ---
> This should be applied after the preceding patch; I mistakenly omitted
> the '1/2' in its subject line.
>
>  Documentation/git-describe.txt | 3 ++-
>  builtin/describe.c             | 3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
> index 711040d..fd5d8f2 100644
> --- a/Documentation/git-describe.txt
> +++ b/Documentation/git-describe.txt
> @@ -83,7 +83,8 @@ OPTIONS
>  --match <pattern>::
>  	Only consider tags matching the given `glob(7)` pattern,
>  	excluding the "refs/tags/" prefix.  This can be used to avoid
> -	leaking private tags from the repository.
> +	leaking private tags from the repository.  This option is
> +	incompatible with `--all`.
>  
>  --always::
>  	Show uniquely abbreviated commit object as fallback.
> diff --git a/builtin/describe.c b/builtin/describe.c
> index 04c185b..90a72af 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -435,6 +435,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
>  	if (longformat && abbrev == 0)
>  		die(_("--long is incompatible with --abbrev=0"));
>  
> +	if (pattern && all)
> +		die(_("--match is incompatible with --all"));
> +
>  	if (contains) {
>  		const char **args = xmalloc((7 + argc) * sizeof(char *));
>  		int i = 0;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]