Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4675337e-d71d-2045-1cab-b81ba9fa2c25@tessares.net>
Date:   Tue, 29 Mar 2022 17:12:12 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH perf] perf tools: .git/ORIG_HEAD might not exist
Content-Language: en-GB
To:     John Garry <john.garry@huawei.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>
Cc:     mptcp@lists.linux.dev, Arnaldo Carvalho de Melo <acme@redhat.com>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220329093120.4173283-1-matthieu.baerts@tessares.net>
 <e9969b97-3998-d7b2-9fd2-77ddd27969ef@huawei.com>
From:   Matthieu Baerts <matthieu.baerts@tessares.net>
In-Reply-To: <e9969b97-3998-d7b2-9fd2-77ddd27969ef@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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