Re: [PATCH perf] perf tools: .git/ORIG_HEAD might not exist

[Matthieu Baerts <matthieu.baerts@tessares.net>]

Hi John,

On 29/03/2022 12:32, John Garry wrote:
> On 29/03/2022 10:31, Matthieu Baerts wrote:
> 
> Hi Matthieu,
> 
> Sorry for the breakage.

That's alright, it happens.

(...)

> Could you please try this:
> 
> ---->8------
> 
> 
> From 694964709a7fc2b46c995bb7b1967cc6b129def8 Mon Sep 17 00:00:00 2001
> From: John Garry <john.garry@huawei.com>
> Date: Tue, 29 Mar 2022 11:06:46 +0100
> Subject: [PATCH] perf: Stop depending on .git internal files for
> building PERF-VERSION-FILE
> 
> This essentially reverts commit c72e3f04b45fb2e50cdd81a50c3778c6a57251d8
> and commit 4e666cdb06eede2069a7b1a96a1359d1c441a3eb.
> 
> In commit c72e3f04b45f ("tools/perf/build: Speed up git-version test on
> re-make"), a makefile dependency on .git/HEAD was added. The background
> is that running PERF-VERSION-FILE is relatively slow, and commands like
> "git describe" are particularly slow.
> 
> In commit 4e666cdb06ee ("perf tools: Fix dependency for version file
> creation"), an additional dependency on .git/ORIG_HEAD was added, as
> .git/HEAD may not change for "git reset --hard HEAD^" command.
> 
> However, as discussed with the git community in [0], using git internal
> files for dependencies is not reliable. Commit 4e666cdb06ee also breaks
> some build scenarios [1].
> 
> As mentioned, c72e3f04b45f was added to speed up the build. However in
> commit 7572733b8499 ("perf tools: Fix version kernel tag") we removed
> the call to "git describe", so just revert Makefile.perf back to same as
> pre c72e3f04b45f and the build should not be so slow, as below:
> 
> Pre 7572733b8499:
> $> time util/PERF-VERSION-GEN
>   PERF_VERSION = 5.17.rc8.g4e666cdb06ee
> 
> real    0m0.110s
> user    0m0.091s
> sys     0m0.019s
> 
> Post 7572733b8499:
> $> time util/PERF-VERSION-GEN
>   PERF_VERSION = 5.17.rc8.g7572733b8499
> 
> real    0m0.039s
> user    0m0.036s
> sys     0m0.007s

Probably the optimisation to avoid calling this script is indeed no
longer needed but it is not me to judge.

What I know is that indeed, 'git describe' can be slow while just
returning the current SHA is supposed to be always quick.

(...)

> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 9c935f86d172..ddd03b21bda2 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -691,9 +691,8 @@ $(OUTPUT)common-cmds.h: $(wildcard
> Documentation/perf-*.txt)
>  $(SCRIPTS) : % : %.sh
>      $(QUIET_GEN)$(INSTALL) '$@.sh' '$(OUTPUT)$@'
> 
> -$(OUTPUT)PERF-VERSION-FILE: ../../.git/HEAD ../../.git/ORIG_HEAD
> +$(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE

Regarding my issue, removing the dependence to .git/ORIG_HEAD file fixes
the issue. So I'm fine to drop my patch and revert the two commits you
mentioned.

If you send this patch, please do not forget to add a Fixes tag:

  Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file
creation")

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net