diff mbox

kbuild: drop FORCE from PHONY targets

Message ID CAK7LNATAeMmfwWiZha17mbyWhF6bR5RGjv+gbsQTvL4teoTiPQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masahiro Yamada March 14, 2016, 6:26 a.m. UTC
Hi Andy,


2016-03-14 13:44 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
> On Sun, Mar 13, 2016 at 9:36 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 2016-03-14 13:28 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
>>> On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
>>> <yamada.masahiro@socionext.com> wrote:
>>>>
>>>> Hi Andy
>>>>
>>>> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
>>>> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
>>>> > <yamada.masahiro@socionext.com> 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
 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.

Comments

Andy Lutomirski March 15, 2016, 6:27 p.m. UTC | #1
On Mar 13, 2016 11:26 PM, "Masahiro Yamada"
<yamada.masahiro@socionext.com> wrote:
>
> Hi Andy,
>
>
> 2016-03-14 13:44 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
> > On Sun, Mar 13, 2016 at 9:36 PM, Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> >> 2016-03-14 13:28 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
> >>> On Sun, Mar 13, 2016 at 9:08 PM, Masahiro Yamada
> >>> <yamada.masahiro@socionext.com> wrote:
> >>>>
> >>>> Hi Andy
> >>>>
> >>>> 2016-03-14 9:39 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
> >>>> > On Mar 12, 2016 4:14 PM, "Masahiro Yamada"
> >>>> > <yamada.masahiro@socionext.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek March 15, 2016, 8:45 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski March 15, 2016, 8:48 p.m. UTC | #3
On Tue, Mar 15, 2016 at 1:45 PM, Michal Marek <mmarek@suse.com> 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
Masahiro Yamada March 17, 2016, 3:52 a.m. UTC | #4
Hi Andy,

2016-03-16 5:48 GMT+09:00 Andy Lutomirski <luto@amacapital.net>:
> On Tue, Mar 15, 2016 at 1:45 PM, Michal Marek <mmarek@suse.com> 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.
diff mbox

Patch

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)