2016-03-13 00:16:30

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] kbuild: drop FORCE from PHONY targets

These targets are marked as PHONY. No need to add FORCE to their
dependency.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 8 ++++----
arch/arm/vdso/Makefile | 2 +-
arch/ia64/Makefile | 4 ++--
arch/x86/entry/vdso/Makefile | 4 ++--
4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 2d519d2..e3ef6b6 100644
--- a/Makefile
+++ b/Makefile
@@ -142,7 +142,7 @@ PHONY += $(MAKECMDGOALS) sub-make
$(filter-out _all sub-make $(CURDIR)/Makefile, $(MAKECMDGOALS)) _all: sub-make
@:

-sub-make: FORCE
+sub-make:
$(Q)$(MAKE) -C $(KBUILD_OUTPUT) KBUILD_SRC=$(CURDIR) \
-f $(CURDIR)/Makefile $(filter-out _all sub-make,$(MAKECMDGOALS))

@@ -989,7 +989,7 @@ prepare1: prepare2 $(version_h) include/generated/utsrelease.h \

archprepare: archheaders archscripts prepare1 scripts_basic

-prepare0: archprepare FORCE
+prepare0: archprepare
$(Q)$(MAKE) $(build)=.

# All the preparing..
@@ -1034,7 +1034,7 @@ INSTALL_FW_PATH=$(INSTALL_MOD_PATH)/lib/firmware
export INSTALL_FW_PATH

PHONY += firmware_install
-firmware_install: FORCE
+firmware_install:
@mkdir -p $(objtree)/firmware
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.fwinst obj=firmware __fw_install

@@ -1054,7 +1054,7 @@ PHONY += archscripts
archscripts:

PHONY += __headers
-__headers: $(version_h) scripts_basic asm-generic archheaders archscripts FORCE
+__headers: $(version_h) scripts_basic asm-generic archheaders archscripts
$(Q)$(MAKE) $(build)=scripts build_unifdef

PHONY += headers_install_all
diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile
index 1160434..59a8fa7 100644
--- a/arch/arm/vdso/Makefile
+++ b/arch/arm/vdso/Makefile
@@ -74,5 +74,5 @@ $(MODLIB)/vdso: FORCE
@mkdir -p $(MODLIB)/vdso

PHONY += vdso_install
-vdso_install: $(obj)/vdso.so.dbg $(MODLIB)/vdso FORCE
+vdso_install: $(obj)/vdso.so.dbg $(MODLIB)/vdso
$(call cmd,vdso_install)
diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile
index 970d0bd..648f1ce 100644
--- a/arch/ia64/Makefile
+++ b/arch/ia64/Makefile
@@ -95,8 +95,8 @@ define archhelp
echo '* unwcheck - Check vmlinux for invalid unwind info'
endef

-archprepare: make_nr_irqs_h FORCE
+archprepare: make_nr_irqs_h
PHONY += make_nr_irqs_h FORCE

-make_nr_irqs_h: FORCE
+make_nr_irqs_h:
$(Q)$(MAKE) $(build)=arch/ia64/kernel include/generated/nr-irqs.h
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index c854541..a81b539 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -188,10 +188,10 @@ vdso_img_insttargets := $(vdso_img_sodbg:%.dbg=install_%)
$(MODLIB)/vdso: FORCE
@mkdir -p $(MODLIB)/vdso

-$(vdso_img_insttargets): install_%: $(obj)/%.dbg $(MODLIB)/vdso FORCE
+$(vdso_img_insttargets): install_%: $(obj)/%.dbg $(MODLIB)/vdso
$(call cmd,vdso_install)

PHONY += vdso_install $(vdso_img_insttargets)
-vdso_install: $(vdso_img_insttargets) FORCE
+vdso_install: $(vdso_img_insttargets)

clean-files := vdso32.so vdso32.so.dbg vdso64* vdso-image-*.c vdsox32.so*
--
1.9.1


2016-03-14 00:40:12

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
<[email protected]> wrote:
>
> These targets are marked as PHONY. No need to add FORCE to their
> dependency.

If this is, in fact, correct, can you update
Documentation/kbuild/makefiles.txt as well?

Thanks,
Andy

2016-03-14 04:08:27

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

Hi Andy

2016-03-14 9:39 GMT+09:00 Andy Lutomirski <[email protected]>:
> On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
> <[email protected]> wrote:
>>
>> These targets are marked as PHONY. No need to add FORCE to their
>> dependency.
>
> If this is, in fact, correct, can you update
> Documentation/kbuild/makefiles.txt as well?

Which line do you want me to update?




--
Best Regards
Masahiro Yamada

2016-03-14 04:29:22

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
<[email protected]> wrote:
>
> Hi Andy
>
> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <[email protected]>:
> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
> > <[email protected]> wrote:
> >>
> >> These targets are marked as PHONY. No need to add FORCE to their
> >> dependency.
> >
> > If this is, in fact, correct, can you update
> > Documentation/kbuild/makefiles.txt as well?
>
> Which line do you want me to update?
>

All the references to FORCE should probably mention .PHONY as an alternative.

--Andy

>
>
>
>
> --
> Best Regards
> Masahiro Yamada




--
Andy Lutomirski
AMA Capital Management, LLC

2016-03-14 04:37:05

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

2016-03-14 13:28 GMT+09:00 Andy Lutomirski <[email protected]>:
> On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
> <[email protected]> wrote:
>>
>> Hi Andy
>>
>> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <[email protected]>:
>> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
>> > <[email protected]> wrote:
>> >>
>> >> These targets are marked as PHONY. No need to add FORCE to their
>> >> dependency.
>> >
>> > If this is, in fact, correct, can you update
>> > Documentation/kbuild/makefiles.txt as well?
>>
>> Which line do you want me to update?
>>
>
> All the references to FORCE should probably mention .PHONY as an alternative.

I do not get your point.

All the examples in the makefile.txt correctly reference to FORCE.
They are not PHONY targets.
No need to update.




--
Best Regards
Masahiro Yamada

2016-03-14 04:45:23

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

On Sun, Mar 13, 2016 at 9:36 PM, Masahiro Yamada
<[email protected]> wrote:
> 2016-03-14 13:28 GMT+09:00 Andy Lutomirski <[email protected]>:
>> On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
>> <[email protected]> wrote:
>>>
>>> Hi Andy
>>>
>>> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <[email protected]>:
>>> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
>>> > <[email protected]> wrote:
>>> >>
>>> >> These targets are marked as PHONY. No need to add FORCE to their
>>> >> dependency.
>>> >
>>> > If this is, in fact, correct, can you update
>>> > Documentation/kbuild/makefiles.txt as well?
>>>
>>> Which line do you want me to update?
>>>
>>
>> All the references to FORCE should probably mention .PHONY as an alternative.
>
> I do not get your point.
>
> All the examples in the makefile.txt correctly reference to FORCE.
> They are not PHONY targets.
> No need to update.

But they could be. For example:

$(obj)/image: vmlinux FORCE
$(call if_changed,objcopy)

could be:

.PHONY: $(obj)/image
$(obj)/image: vmlinux
$(call if_changed,objcopy)

I would at least change:

Note: It is a typical mistake to forget the FORCE prerequisite.

to:

Note: if-changed is only useful if make executes it, which won't
happen if it determines that the inputs have not changed since the
output was built. This can be avoided by specifying FORCE as a
prerequisite or by making declaring the output as .PHONY.

--Andy

>
>
>
>
> --
> Best Regards
> Masahiro Yamada



--
Andy Lutomirski
AMA Capital Management, LLC

2016-03-14 06:26:34

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

Hi Andy,


2016-03-14 13:44 GMT+09:00 Andy Lutomirski <[email protected]>:
> On Sun, Mar 13, 2016 at 9:36 PM, Masahiro Yamada
> <[email protected]> wrote:
>> 2016-03-14 13:28 GMT+09:00 Andy Lutomirski <[email protected]>:
>>> On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
>>> <[email protected]> wrote:
>>>>
>>>> Hi Andy
>>>>
>>>> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <[email protected]>:
>>>> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
>>>> > <[email protected]> wrote:
>>>> >>
>>>> >> These targets are marked as PHONY. No need to add FORCE to their
>>>> >> dependency.
>>>> >
>>>> > If this is, in fact, correct, can you update
>>>> > Documentation/kbuild/makefiles.txt as well?
>>>>
>>>> Which line do you want me to update?
>>>>
>>>
>>> All the references to FORCE should probably mention .PHONY as an alternative.
>>
>> I do not get your point.
>>
>> All the examples in the makefile.txt correctly reference to FORCE.
>> They are not PHONY targets.
>> No need to update.
>
> But they could be. For example:
>
> $(obj)/image: vmlinux FORCE
> $(call if_changed,objcopy)
>
> could be:
>
> .PHONY: $(obj)/image
> $(obj)/image: vmlinux
> $(call if_changed,objcopy)
>
> I would at least change:
>
> Note: It is a typical mistake to forget the FORCE prerequisite.
>
> to:
>
> Note: if-changed is only useful if make executes it, which won't
> happen if it determines that the inputs have not changed since the
> output was built. This can be avoided by specifying FORCE as a
> prerequisite or by making declaring the output as .PHONY.
>


No. This is absolutely wrong.
PHONY and FORCE are not interchangeable.

They have different behavior.
I will show you how they behave differently.


BTW, Linux 4.5 is out now.

Let's try a simple experiment on it.


The following shows that my source tree is v4.5

yamada@beagle:~/workspace/linux$ git describe
v4.5



Example 1)


Add the following code to the top Makefile.

Please note "FORCE" is used here.


yamada@beagle:~/workspace/linux$ git diff
diff --git a/Makefile b/Makefile
index 7b3ecdc..89b7d0d 100644
--- a/Makefile
+++ b/Makefile
@@ -914,6 +914,22 @@ export KBUILD_ALLDIRS := $(sort $(filter-out
arch/%,$(vmlinux-

vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_INIT) $(KBUILD_VMLINUX_MAIN)

+quiet_cmd_gen_foo = FOO $@
+ cmd_gen_foo = (cat $<; echo hello) > $@
+
+foo: bar FORCE
+ $(call if_changed,gen_foo)
+
+quiet_cmd_gen_bar = BAR $@
+ cmd_gen_bar = (cat $<; echo $(GREETING)) > $@
+
+bar: baz FORCE
+ $(call if_changed,gen_bar)
+
+baz:
+ @touch $@
+
+
# Final link of vmlinux
cmd_link-vmlinux = $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
quiet_cmd_link-vmlinux = LINK $@



Try the following.

yamada@beagle:~/workspace/linux$ git clean -x -f
yamada@beagle:~/workspace/linux$ make -s defconfig
yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
scripts/kconfig/conf --silentoldconfig Kconfig
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
make: `foo' is up to date.
yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
make: `foo' is up to date.
yamada@beagle:~/workspace/linux$ make GREETING=GoodEvening foo
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodEvening foo
make: `foo' is up to date.




Please notice "foo" and "bar" were not rebuilt
when I gave the same command line as the previous run.


When I changed the command line, "bar" was update
and "foo" was also updated because "foo" depends on "bar".


It means $(call if_changed,...) is working as expected.






Example 2)


Add the following to the top Makefile.
Please notice that I just replaced "FORCE" with ".PHONY".



diff --git a/Makefile b/Makefile
index 7b3ecdc..a0899c3 100644
--- a/Makefile
+++ b/Makefile
@@ -914,6 +914,24 @@ export KBUILD_ALLDIRS := $(sort $(filter-out
arch/%,$(vmlinux-

vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_INIT) $(KBUILD_VMLINUX_MAIN)

+quiet_cmd_gen_foo = FOO $@
+ cmd_gen_foo = (cat $<; echo hello) > $@
+
+.PHONY: foo
+foo: bar
+ $(call if_changed,gen_foo)
+
+quiet_cmd_gen_bar = BAR $@
+ cmd_gen_bar = (cat $<; echo $(GREETING)) > $@
+
+.PHONY: bar
+bar: baz
+ $(call if_changed,gen_bar)
+
+baz:
+ @touch $@
+
+
# Final link of vmlinux
cmd_link-vmlinux = $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
quiet_cmd_link-vmlinux = LINK $@



Let's try the same thing.



yamada@beagle:~/workspace/linux$ git clean -x -f
yamada@beagle:~/workspace/linux$ make -s defconfig
yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
scripts/kconfig/conf --silentoldconfig Kconfig
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
BAR bar
FOO foo
yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
BAR bar
FOO foo


Did you notice the difference?

The "foo" and "bar" are always rebuilt
regardless the command line is changed or not.

It means $(call if_changed,...) is not working.



--
Best Regards
Masahiro Yamada

2016-03-15 18:28:24

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

On Mar 13, 2016 11:26 PM, "Masahiro Yamada"
<[email protected]> wrote:
>
> Hi Andy,
>
>
> 2016-03-14 13:44 GMT+09:00 Andy Lutomirski <[email protected]>:
> > On Sun, Mar 13, 2016 at 9:36 PM, Masahiro Yamada
> > <[email protected]> wrote:
> >> 2016-03-14 13:28 GMT+09:00 Andy Lutomirski <[email protected]>:
> >>> On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
> >>> <[email protected]> wrote:
> >>>>
> >>>> Hi Andy
> >>>>
> >>>> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <[email protected]>:
> >>>> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
> >>>> > <[email protected]> wrote:
> >>>> >>
> >>>> >> These targets are marked as PHONY. No need to add FORCE to their
> >>>> >> dependency.
> >>>> >
> >>>> > If this is, in fact, correct, can you update
> >>>> > Documentation/kbuild/makefiles.txt as well?
> >>>>
> >>>> Which line do you want me to update?
> >>>>
> >>>
> >>> All the references to FORCE should probably mention .PHONY as an alternative.
> >>
> >> I do not get your point.
> >>
> >> All the examples in the makefile.txt correctly reference to FORCE.
> >> They are not PHONY targets.
> >> No need to update.
> >
> > But they could be. For example:
> >
> > $(obj)/image: vmlinux FORCE
> > $(call if_changed,objcopy)
> >
> > could be:
> >
> > .PHONY: $(obj)/image
> > $(obj)/image: vmlinux
> > $(call if_changed,objcopy)
> >
> > I would at least change:
> >
> > Note: It is a typical mistake to forget the FORCE prerequisite.
> >
> > to:
> >
> > Note: if-changed is only useful if make executes it, which won't
> > happen if it determines that the inputs have not changed since the
> > output was built. This can be avoided by specifying FORCE as a
> > prerequisite or by making declaring the output as .PHONY.
> >
>
>
> No. This is absolutely wrong.
> PHONY and FORCE are not interchangeable.
>
> They have different behavior.
> I will show you how they behave differently.
>
>
> BTW, Linux 4.5 is out now.
>
> Let's try a simple experiment on it.
>
>
> The following shows that my source tree is v4.5
>
> yamada@beagle:~/workspace/linux$ git describe
> v4.5
>
>
>
> Example 1)
>
>
> Add the following code to the top Makefile.
>
> Please note "FORCE" is used here.
>
>
> yamada@beagle:~/workspace/linux$ git diff
> diff --git a/Makefile b/Makefile
> index 7b3ecdc..89b7d0d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -914,6 +914,22 @@ export KBUILD_ALLDIRS := $(sort $(filter-out
> arch/%,$(vmlinux-
>
> vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_INIT) $(KBUILD_VMLINUX_MAIN)
>
> +quiet_cmd_gen_foo = FOO $@
> + cmd_gen_foo = (cat $<; echo hello) > $@
> +
> +foo: bar FORCE
> + $(call if_changed,gen_foo)
> +
> +quiet_cmd_gen_bar = BAR $@
> + cmd_gen_bar = (cat $<; echo $(GREETING)) > $@
> +
> +bar: baz FORCE
> + $(call if_changed,gen_bar)
> +
> +baz:
> + @touch $@
> +
> +
> # Final link of vmlinux
> cmd_link-vmlinux = $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> quiet_cmd_link-vmlinux = LINK $@
>
>
>
> Try the following.
>
> yamada@beagle:~/workspace/linux$ git clean -x -f
> yamada@beagle:~/workspace/linux$ make -s defconfig
> yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
> scripts/kconfig/conf --silentoldconfig Kconfig
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
> make: `foo' is up to date.
> yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
> make: `foo' is up to date.
> yamada@beagle:~/workspace/linux$ make GREETING=GoodEvening foo
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodEvening foo
> make: `foo' is up to date.
>
>
>
>
> Please notice "foo" and "bar" were not rebuilt
> when I gave the same command line as the previous run.
>
>
> When I changed the command line, "bar" was update
> and "foo" was also updated because "foo" depends on "bar".
>
>
> It means $(call if_changed,...) is working as expected.
>
>
>
>
>
>
> Example 2)
>
>
> Add the following to the top Makefile.
> Please notice that I just replaced "FORCE" with ".PHONY".
>
>
>
> diff --git a/Makefile b/Makefile
> index 7b3ecdc..a0899c3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -914,6 +914,24 @@ export KBUILD_ALLDIRS := $(sort $(filter-out
> arch/%,$(vmlinux-
>
> vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_INIT) $(KBUILD_VMLINUX_MAIN)
>
> +quiet_cmd_gen_foo = FOO $@
> + cmd_gen_foo = (cat $<; echo hello) > $@
> +
> +.PHONY: foo
> +foo: bar
> + $(call if_changed,gen_foo)
> +
> +quiet_cmd_gen_bar = BAR $@
> + cmd_gen_bar = (cat $<; echo $(GREETING)) > $@
> +
> +.PHONY: bar
> +bar: baz
> + $(call if_changed,gen_bar)
> +
> +baz:
> + @touch $@
> +
> +
> # Final link of vmlinux
> cmd_link-vmlinux = $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> quiet_cmd_link-vmlinux = LINK $@
>
>
>
> Let's try the same thing.
>
>
>
> yamada@beagle:~/workspace/linux$ git clean -x -f
> yamada@beagle:~/workspace/linux$ make -s defconfig
> yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
> scripts/kconfig/conf --silentoldconfig Kconfig
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodMorning foo
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
> BAR bar
> FOO foo
> yamada@beagle:~/workspace/linux$ make GREETING=GoodAfternoon foo
> BAR bar
> FOO foo
>
>
> Did you notice the difference?
>
> The "foo" and "bar" are always rebuilt
> regardless the command line is changed or not.
>
> It means $(call if_changed,...) is not working.
>

Fair enough, although I'm curious why this happens. It might be worth
changing the docs to say that .PHONY is *not* an substitute for FORCE
in that context, then.

--Andy

2016-03-15 20:45:33

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

Dne 15.3.2016 v 19:27 Andy Lutomirski napsal(a):
> Fair enough, although I'm curious why this happens. It might be worth
> changing the docs to say that .PHONY is *not* an substitute for FORCE
> in that context, then.

These two are unrelated, except that FORCE is redundant for a .PHONY
target. FORCE is our idiom to tell make to always remake the target and
let us handle the dependencies manually. Listing a target as .PHONY
tells make that the target will not produce a file of the same name
(typically, "all", "install", etc).

Michal

2016-03-15 20:48:40

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

On Tue, Mar 15, 2016 at 1:45 PM, Michal Marek <[email protected]> wrote:
> Dne 15.3.2016 v 19:27 Andy Lutomirski napsal(a):
>> Fair enough, although I'm curious why this happens. It might be worth
>> changing the docs to say that .PHONY is *not* an substitute for FORCE
>> in that context, then.
>
> These two are unrelated, except that FORCE is redundant for a .PHONY
> target. FORCE is our idiom to tell make to always remake the target and
> let us handle the dependencies manually. Listing a target as .PHONY
> tells make that the target will not produce a file of the same name
> (typically, "all", "install", etc).
>

Except that apparently if-changed doesn't work on .PHONY targets that
don't specify FORCE, which confuses me.

--Andy


--
Andy Lutomirski
AMA Capital Management, LLC

2016-03-17 03:52:58

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

Hi Andy,

2016-03-16 5:48 GMT+09:00 Andy Lutomirski <[email protected]>:
> On Tue, Mar 15, 2016 at 1:45 PM, Michal Marek <[email protected]> wrote:
>> Dne 15.3.2016 v 19:27 Andy Lutomirski napsal(a):
>>> Fair enough, although I'm curious why this happens. It might be worth
>>> changing the docs to say that .PHONY is *not* an substitute for FORCE
>>> in that context, then.
>>
>> These two are unrelated, except that FORCE is redundant for a .PHONY
>> target. FORCE is our idiom to tell make to always remake the target and
>> let us handle the dependencies manually. Listing a target as .PHONY
>> tells make that the target will not produce a file of the same name
>> (typically, "all", "install", etc).
>>
>
> Except that apparently if-changed doesn't work on .PHONY targets that
> don't specify FORCE, which confuses me.


OK, I will try to explain it.
I hope this will help you, not confuse you even more...


I think the difference of Kbuild behavior
comes down to "$?" behavior of GNU Make.




Please see the definition of "if_changed".


if_changed = $(if $(strip $(any-prereq) $(arg-check)), \
@set -e; \
$(echo-cmd) $(cmd_$(1)); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)



The "if_changed" does some actions if "any-prereq" or "arg-check" is non-empty.



Next, let's take a look at the definition of "any-prereq"

any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)



It seems $? makes the difference between FORCE and PHONY.


The GNU Make manual says as follows:
-------------------->8-----------------------------------
$? The names of all prerequisites that are newer than the target,
separated by spaces.
--------------------8<-----------------------------------

>From this statement, it is unclear what happens to $? if the target is
a PHONY target.
I am not a GNU Make developer, so I have no idea about the detailed
implementation,
but anyway let's try simple experiments.



[Example 3]

Let's write a simple Makefile as follows.

--------------------->8---------------------
foo: bar FORCE
echo $$? is "$?"
cp $< $@

bar:
touch $@

.PHONY: FORCE
FORCE:
----------------------8<--------------------

yamada@beagle:~/workspace/test$ rm -f foo bar
yamada@beagle:~/workspace/test$ make
touch bar
echo $? is "bar FORCE"
0 is bar FORCE
cp bar foo
yamada@beagle:~/workspace/test$ make
echo $? is ""
0 is
cp bar foo
yamada@beagle:~/workspace/test$ make
echo $? is ""
0 is
cp bar foo


As we expected, $? contains the prerequisite "bar",
but it is empty for the second run or later (because "foo" is newer than "bar").



[Example 4]

Let's replace the FORCE with PHONY.

--------------------->8---------------------
.PHONY: foo
foo: bar
echo $$? is "$?"
cp $< $@

bar:
touch $@
----------------------8<--------------------


yamada@beagle:~/workspace/test2$ rm -f foo bar
yamada@beagle:~/workspace/test2$ make
touch bar
echo $? is "bar"
0 is bar
cp bar foo
yamada@beagle:~/workspace/test2$ make
echo $? is "bar"
0 is bar
cp bar foo
yamada@beagle:~/workspace/test2$ make
echo $? is "bar"
0 is bar
cp bar foo


This time, $? always contains "bar"
nevertheless "foo" is newer than "bar" on the second run or later.


So, it looks like GNU Make assumes that
all the prerequisites of a PHONY target are always newer than the target.



Go back to the definition of "any-prereq",

any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)

If the prerequisite is not a PHONY target (like the "baz" in the
example 2 in my former email),
it is not filtered out from $?. So, any-prereq is non-empty and
if_changed updates the target.





--
Best Regards
Masahiro Yamada

2016-04-20 08:29:20

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop FORCE from PHONY targets

Dne 13.3.2016 v 01:13 Masahiro Yamada napsal(a):
> These targets are marked as PHONY. No need to add FORCE to their
> dependency.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Makefile | 8 ++++----
> arch/arm/vdso/Makefile | 2 +-
> arch/ia64/Makefile | 4 ++--
> arch/x86/entry/vdso/Makefile | 4 ++--
> 4 files changed, 9 insertions(+), 9 deletions(-)

Applied to kbuild.git#kbuild now, sorry for the delay.

Michal