Re: [PATCH] tools: perf: Fix liblk not built when using perf_install target

[Sedat Dilek <sedat.dilek@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2880 bytes --]

On Tue, Sep 3, 2013 at 10:56 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Tue, Sep 3, 2013 at 10:48 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>> Hi,
>>
>> On Fri, 30 Aug 2013 18:49:05 +0200, Robert Richter wrote:
>>> On 30.08.13 17:18:36, Sedat Dilek wrote:
>>>> So, I tried to build perf the "official" way:
>>>>
>>>>    $ make -C tools/ perf_install
>>>>
>>>> Unfortunately, my build breaks like this:
>>>> ...
>>>> make[2]: Entering directory
>>>> `~/src/linux-kernel/linux/tools/lib/traceevent'
>>>> make[2]: Leaving directory
>>>> `~/src/linux-kernel/linux/tools/lib/traceevent'
>>>>     LINK perf
>>>> gcc: error: ~/src/linux-kernel/linux/tools/lib/lk/liblk.a: No such file or directory
>>>> make[1]: *** [perf] Error 1
>>>> make[1]: Leaving directory `~/src/linux-kernel/linux/tools/perf'
>>>> make: *** [perf_install] Error 2
>>>>
>>>> After some discussion on IRC with peterz and acme and a closer look at
>>>> the targets in "tools/Makefile", I have noticed that the perf_install
>>>> target misses liblk to be built beforehand.
>>>
>>> There are no build prerequisites on purpose for install targets.
>>> Install targets are intended to be run with root permissions. We don't
>>> want to create/overwrite files in the build directory as root while
>>> running install rules. Thus, you always need to run:
>>>
>>>  $ make -C tools/ perf
>>>
>>> or similar first.
>>
>> But AFAIK install target usually depends on build targets on many
>> projects.  And perf too already does the same:
>>
>>   [ tools/perf/Makefile ]
>>   install-bin: all
>>         ...
>>
>>   install: install-bin
>>
>>
>> So if you really concern the permission problem, install target should
>> not build perf itself, right?  But in this case, it tried to build perf
>> but failed to do it on liblk.
>>
>>>
>>>> On the contrary the perf_clean target invokes to clean liblk when perf
>>>> is cleaned.
>>>>
>>>> [ tools/Makefile ]
>>>> ...
>>>> perf_clean: liblk_clean
>>>>      $(call descend,$(@:_clean=),clean)
>>>> ...
>>>>
>>>> Fix this by adding liblk target to perf_install target.
>>>
>>> So no, this is not the proper fix.
>>>
>>> Before installing you *need* to build. You might want to fix the
>>> documenation...
>>
>> Hmm.. I don't know.  What I'm curious about is why it didn't build liblk
>> during building perf.
>>
>
> The day I fell over that perf stuff, I created a pile of patches
> fixing that unbeloved tools stuff.
> Some tools listed in tools/Makefile are not buildable via...
>
>      $ make -C tools/ <tool>
>      $ make -C tools/ <tool>_install
>
> ...because some <tool> have no "install" target.
>
> I attach the patchset here. It has no "proper" changelogs.
> The series is "incomplete"... needs some more love... eat it carefully.
>

Attached 0008 v2.

* move cpupower to tools where sources are below power-dir.
* selftests is always last target

- Sedat -

[-- Attachment #2: 0008-tools-Reorder-tool-_install-install-and-clean-target.patch --]
[-- Type: application/octet-stream, Size: 3604 bytes --]

From dfef2887f084d2ccf7e37f6b319254c3b73cbe65 Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Tue, 3 Sep 2013 11:09:43 +0200
Subject: [PATCH 8/8] tools: Reorder <tool>_install, install and clean targets
 (v2)

Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 tools/Makefile | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index 8238e5f..5566829 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -34,10 +34,7 @@ help:
 	@echo '    the respective build directory.'
 	@echo '  clean: a summary clean target to clean _all_ folders'
 
-cpupower: FORCE
-	$(call descend,power/$@)
-
-cgroup firewire lguest usb virtio net: FORCE
+cgroup firewire lguest net usb virtio: FORCE
 	$(call descend,$@)
 
 liblk: FORCE
@@ -46,34 +43,35 @@ liblk: FORCE
 perf vm: liblk FORCE
 	$(call descend,$@)
 
-selftests: FORCE
-	$(call descend,testing/$@)
+cpupower: FORCE
+        $(call descend,power/$@)
 
 turbostat x86_energy_perf_policy: FORCE
 	$(call descend,power/x86/$@)
 
-all: cpupower cgroup firewire lguest net usb virtio perf vm selftests turbostat x86_energy_perf_policy
+selftests: FORCE
+        $(call descend,testing/$@)
 
-cpupower_install:
-	$(call descend,power/$(@:_install=),install)
+all: cgroup firewire lguest net usb virtio perf vm cpupower turbostat x86_energy_perf_policy selftests
 
-cgroup_install firewire_install lguest_install perf_install usb_install virtio_install vm_install net_install:
+cgroup_install firewire_install lguest_install net_install usb_install virtio_install perf_install vm_install:
 	$(call descend,$(@:_install=),install)
 
-selftests_install:
-	$(call descend,testing/$(@:_install=),install)
+cpupower_install:
+        $(call descend,power/$(@:_install=),install)
 
 turbostat_install x86_energy_perf_policy_install:
 	$(call descend,power/x86/$(@:_install=),install)
 
-install: cgroup_install cpupower_install firewire_install lguest_install \
-		perf_install selftests_install turbostat_install usb_install \
-		virtio_install vm_install net_install x86_energy_perf_policy_install
+selftests_install:
+        $(call descend,testing/$(@:_install=),install)
 
-cpupower_clean:
-	$(call descend,power/cpupower,clean)
+install: cgroup_install firewire_install lguest_install net_install usb_install virtio_install \
+               perf_install vm_install \
+               cpupower_install turbostat_install x86_energy_perf_policy_install \
+               selftests_install
 
-cgroup_clean firewire_clean lguest_clean usb_clean virtio_clean net_clean:
+cgroup_clean firewire_clean lguest_clean net_clean usb_clean virtio_clean:
 	$(call descend,$(@:_clean=),clean)
 
 liblk_clean:
@@ -82,14 +80,18 @@ liblk_clean:
 perf_clean vm_clean: liblk_clean
 	$(call descend,$(@:_clean=),clean)
 
-selftests_clean:
-	$(call descend,testing/$(@:_clean=),clean)
+cpupower_clean:
+        $(call descend,power/cpupower,clean)
 
 turbostat_clean x86_energy_perf_policy_clean:
 	$(call descend,power/x86/$(@:_clean=),clean)
 
-clean: cgroup_clean cpupower_clean firewire_clean lguest_clean perf_clean \
-		selftests_clean turbostat_clean usb_clean virtio_clean \
-		vm_clean net_clean x86_energy_perf_policy_clean
+selftests_clean:
+        $(call descend,testing/$(@:_clean=),clean)
+
+clean: cgroup_clean firewire_clean lguest_clean net_clean usb_clean virtio_clean \
+               perf_clean vm_clean \
+               cpupower_clean turbostat_clean x86_energy_perf_policy_clean \
+               selftests_clean
 
 .PHONY: FORCE
-- 
1.8.4