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,
depending on whether we're on a branch or not, a "git cherry-pick" may
not lead to the version being updated.
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
[0] https://lore.kernel.org/git/[email protected]/T/#m4a4dd6de52fdbe21179306cd57b3761eb07f45f8
[1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file creation")
Reported-by: Matthieu Baerts <[email protected]>
Signed-off-by: John Garry <[email protected]>
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) '[email protected]' '$(OUTPUT)$@'
-$(OUTPUT)PERF-VERSION-FILE: ../../.git/HEAD ../../.git/ORIG_HEAD
+$(OUTPUT)PERF-VERSION-FILE: .FORCE-PERF-VERSION-FILE
$(Q)$(SHELL_PATH) util/PERF-VERSION-GEN $(OUTPUT)
- $(Q)touch $(OUTPUT)PERF-VERSION-FILE
# These can record PERF_VERSION
perf.spec $(SCRIPTS) \
@@ -1139,21 +1138,12 @@ else
@echo "FEATURE-DUMP file available in $(OUTPUT)FEATURE-DUMP"
endif
-#
-# Trick: if ../../.git does not exist - we are building out of tree for example,
-# then force version regeneration:
-#
-ifeq ($(wildcard ../../.git/HEAD),)
- GIT-HEAD-PHONY = ../../.git/HEAD ../../.git/ORIG_HEAD
-else
- GIT-HEAD-PHONY =
-endif
FORCE:
.PHONY: all install clean config-clean strip install-gtk
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-.PHONY: $(GIT-HEAD-PHONY) TAGS tags cscope FORCE prepare
+.PHONY: .FORCE-PERF-VERSION-FILE TAGS tags cscope FORCE prepare
.PHONY: libtraceevent_plugins archheaders
endif # force_fixdep
--
2.26.2
Hi Arnaldo,
On 30/03/2022 16:12, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 30, 2022 at 03:22:37PM +0200, Matthieu Baerts escreveu:
>> Hi John,
>>
>> On 30/03/2022 12:22, John Garry wrote:
>>> 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,
>>> depending on whether we're on a branch or not, a "git cherry-pick" may
>>> not lead to the version being updated.
>>>
>>> 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
>>>
>>> [0] https://lore.kernel.org/git/[email protected]/T/#m4a4dd6de52fdbe21179306cd57b3761eb07f45f8
>>> [1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
>>>
>>> Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file creation")
>>> Reported-by: Matthieu Baerts <[email protected]>
>>> Signed-off-by: John Garry <[email protected]>
>>
>> Thank you for your patch, I just tested it and it also fixes the issue I
>> reported!
>
> I'm taking this as a:
>
> Tested-by: Matthieu Baerts <[email protected]>
>
> Ok?
Yes, OK for me, thank you!
I never know what maintainers prefer when there is already a
"Reported-by" tag so I didn't specify it but I will do next time.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net
Hi John,
On 30/03/2022 12:22, John Garry wrote:
> 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,
> depending on whether we're on a branch or not, a "git cherry-pick" may
> not lead to the version being updated.
>
> 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
>
> [0] https://lore.kernel.org/git/[email protected]/T/#m4a4dd6de52fdbe21179306cd57b3761eb07f45f8
> [1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
>
> Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file creation")
> Reported-by: Matthieu Baerts <[email protected]>
> Signed-off-by: John Garry <[email protected]>
Thank you for your patch, I just tested it and it also fixes the issue I
reported!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net
Em Wed, Mar 30, 2022 at 11:28:12AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 30, 2022 at 04:22:35PM +0200, Matthieu Baerts escreveu:
> > On 30/03/2022 16:12, Arnaldo Carvalho de Melo wrote:
> > >> Thank you for your patch, I just tested it and it also fixes the issue I
> > >> reported!
> > > I'm taking this as a:
> > > Tested-by: Matthieu Baerts <[email protected]>
> > > Ok?
> > Yes, OK for me, thank you!
> > I never know what maintainers prefer when there is already a
> > "Reported-by" tag so I didn't specify it but I will do next time.
> Right, I'll stick both R-by and T-by.
> There are cases where people just report and don't test, or just ack.
But when you actually provide the:
Tested-by: Matthieu Baerts <[email protected]>
It makes the lives of maintainers using b4 easier, as it'll collect
those.
About b4: https://people.kernel.org/monsieuricon/introducing-b4-and-patch-attestation
- Arnaldo
On 30/03/2022 16:30, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 30, 2022 at 11:28:12AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Wed, Mar 30, 2022 at 04:22:35PM +0200, Matthieu Baerts escreveu:
>>> On 30/03/2022 16:12, Arnaldo Carvalho de Melo wrote:
>>>>> Thank you for your patch, I just tested it and it also fixes the issue I
>>>>> reported!
>
>>>> I'm taking this as a:
>
>>>> Tested-by: Matthieu Baerts <[email protected]>
>
>>>> Ok?
>
>>> Yes, OK for me, thank you!
>
>>> I never know what maintainers prefer when there is already a
>>> "Reported-by" tag so I didn't specify it but I will do next time.
>
>> Right, I'll stick both R-by and T-by.
>
>> There are cases where people just report and don't test, or just ack.
>
> But when you actually provide the:
>
> Tested-by: Matthieu Baerts <[email protected]>
>
> It makes the lives of maintainers using b4 easier, as it'll collect
> those.
I agree it is clearer having both tags.
On my side, some people are often asking not to their name in multiple
tags but instead combining tags, e.g. Reported-and-tested-by. But
Patchwork doesn't like that and I have to add the tag manually when
applying the patch. I guess many tools don't like these combined tags!
Anyway, next time I add it and let the maintainer drop / modify it if
they prefer :)
> About b4: https://people.kernel.org/monsieuricon/introducing-b4-and-patch-attestation
Excellent and very useful tool, I agree :-)
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net
Em Wed, Mar 30, 2022 at 03:22:37PM +0200, Matthieu Baerts escreveu:
> Hi John,
>
> On 30/03/2022 12:22, John Garry wrote:
> > 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,
> > depending on whether we're on a branch or not, a "git cherry-pick" may
> > not lead to the version being updated.
> >
> > 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
> >
> > [0] https://lore.kernel.org/git/[email protected]/T/#m4a4dd6de52fdbe21179306cd57b3761eb07f45f8
> > [1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
> >
> > Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file creation")
> > Reported-by: Matthieu Baerts <[email protected]>
> > Signed-off-by: John Garry <[email protected]>
>
> Thank you for your patch, I just tested it and it also fixes the issue I
> reported!
I'm taking this as a:
Tested-by: Matthieu Baerts <[email protected]>
Ok?
- Arnaldo
Em Wed, Mar 30, 2022 at 04:22:35PM +0200, Matthieu Baerts escreveu:
> Hi Arnaldo,
>
> On 30/03/2022 16:12, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Mar 30, 2022 at 03:22:37PM +0200, Matthieu Baerts escreveu:
> >> Hi John,
> >>
> >> On 30/03/2022 12:22, John Garry wrote:
> >>> 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,
> >>> depending on whether we're on a branch or not, a "git cherry-pick" may
> >>> not lead to the version being updated.
> >>>
> >>> 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
> >>>
> >>> [0] https://lore.kernel.org/git/[email protected]/T/#m4a4dd6de52fdbe21179306cd57b3761eb07f45f8
> >>> [1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
> >>>
> >>> Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file creation")
> >>> Reported-by: Matthieu Baerts <[email protected]>
> >>> Signed-off-by: John Garry <[email protected]>
> >>
> >> Thank you for your patch, I just tested it and it also fixes the issue I
> >> reported!
> >
> > I'm taking this as a:
> >
> > Tested-by: Matthieu Baerts <[email protected]>
> >
> > Ok?
>
> Yes, OK for me, thank you!
>
> I never know what maintainers prefer when there is already a
> "Reported-by" tag so I didn't specify it but I will do next time.
Right, I'll stick both R-by and T-by.
There are cases where people just report and don't test, or just ack.
- Arnaldo
Em Wed, Mar 30, 2022 at 03:22:37PM +0200, Matthieu Baerts escreveu:
> Hi John,
>
> On 30/03/2022 12:22, John Garry wrote:
> > 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,
> > depending on whether we're on a branch or not, a "git cherry-pick" may
> > not lead to the version being updated.
> >
> > 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
> >
> > [0] https://lore.kernel.org/git/[email protected]/T/#m4a4dd6de52fdbe21179306cd57b3761eb07f45f8
> > [1] https://lore.kernel.org/linux-perf-users/[email protected]/T/#u
> >
> > Fixes: 4e666cdb06ee ("perf tools: Fix dependency for version file creation")
> > Reported-by: Matthieu Baerts <[email protected]>
> > Signed-off-by: John Garry <[email protected]>
>
> Thank you for your patch, I just tested it and it also fixes the issue I
Thanks, applied.
And added this "Committer testing" section:
-----
Committer testing:
After a fresh rebuild using 'make -C tools/perf O=/tmp/build/perf install-bin':
$ perf -v
perf version 5.17.g162f9db407b6
$ git log --oneline -1
162f9db407b6a6e5 (HEAD -> perf/core) perf tools: Stop depending on .git files for building PERF-VERSION-FILE
$
Now using a detached tarball, i.e. outside the kernel source tree:
$ ls -la perf*tar
ls: cannot access 'perf*tar': No such file or directory
$ make perf-tar-src-pkg
TAR
PERF_VERSION = 5.17.g31d10b3ef133
$ ls -la perf*tar
-rw-r--r--. 1 acme acme 22241280 Mar 30 13:26 perf-5.17.0.tar
$ mv perf-5.17.0.tar /tmp
$ cd /tmp
$ tar xf perf-5.17.0.tar
$ cd perf-5.17.0/
$ make -C tools/perf |& tail
CC util/pmu.o
CC util/pmu-flex.o
CC util/expr-flex.o
CC util/expr.o
LD util/scripting-engines/perf-in.o
LD util/intel-pt-decoder/perf-in.o
LD util/perf-in.o
LD perf-in.o
LINK perf
make: Leaving directory '/tmp/perf-5.17.0/tools/perf'
$ tools/perf/perf -v
perf version 5.17.g31d10b3ef133
$ pwd
/tmp/perf-5.17.0
$ cat PERF-VERSION-FILE
#define PERF_VERSION "5.17.g31d10b3ef133"
$
-----