linux-kernel - Re: [PATCH v2] setlocalversion: work around "git describe" performance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877c92m4qc.fsf@prevas.dk>
Date: Sun, 17 Nov 2024 13:20:27 +0100
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>,  linux-kernel@...r.kernel.org,
  Jeff King <peff@...f.net>
Subject: Re: [PATCH v2] setlocalversion: work around "git describe" performance

On Sat, Nov 16 2024, Masahiro Yamada <masahiroy@...nel.org> wrote:

> This patch was not sent to linux-kbuild ML
> (and it can be one reason when a patch falls into a crack),
> but I guess I am expected to review and pick it.

Sorry, but get_maintainer.pl doesn't tell one to cc linux-kbuild.

>>
>> Reported-by: Josh Poimboeuf <jpoimboe@...nel.org>
>
>
> Maybe, the comprehensive tag list looks like this?
>
> Reported-by: Sean Christopherson <seanjc@...gle.com>
> Closes: https://lore.kernel.org/lkml/ZPtlxmdIJXOe0sEy@google.com/
> Reported-by: Josh Poimboeuf <jpoimboe@...nel.org>
> Closes: https://lore.kernel.org/lkml/309549cafdcfe50c4fceac3263220cc3d8b109b2.1730337435.git.jpoimboe@kernel.org/

Fine by me.

>>
>> +try_tag() {
>> +       tag="$1"
>> +
>> +       # Is $tag an annotated tag?
>> +       [ "$(git cat-file -t "$tag" 2> /dev/null)" = "tag" ] || return 1
>
> The double-quotes for tag are unneeded.
>
> "tag"  --> tag
>

OK. The current script isn't consistent here, though (--no-local and +
are quoted where they need not be), and I find having the quotes on both
sides of = more visually appealing. Not a hill I'm gonna die on.

> This function returns either 1 or 0, but how is it used?
>

Well, you're right that it's not used currently, but I might as well let
the return value reflect whether it succeeded or not. I played around
with some variation of

  if [ -n "${file_localversion#-}" ] && try_tag "${file_localversion#-}" ; then
    :
  elif [ -n "${file_localversion}" ] && try_tag "${version_tag}${file_localversion}" ; then
    :
  elif try_tag "${version_tag}"
    :
  else
    count=""
  fi

but in the end decided to keep the current logic of testing some shell
variable (previously $desc, not $count). Still, I see no reason to make
the early returns do "return 0".

Rasmus