Re: [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary

[Claudio Fontana <cfontana@suse.de>]

On 5/11/20 8:33 AM, David Gibson wrote:
> On Wed, Apr 15, 2020 at 10:16:52AM +0200, Claudio Fontana wrote:
>> On 4/14/20 4:03 AM, David Gibson wrote:
>>> On Sat, Apr 11, 2020 at 11:31:48AM +0200, Claudio Fontana wrote:
>>>> v2 -> v3:
>>>>
>>>> * changed into a 2 patch series; in the second patch we remove the old
>>>>   compatibility gunks that were meant for removal some time after 4.1.
>>>>
>>>> * renamed the libfdt PHONY rule to dtc/all, with the intent to make
>>>>   existing working trees forward and backward compatible across the change.
>>>>
>>>> v1 -> v2:
>>>>
>>>> * fix error generated when running UNCHECKED_GOALS without prior configure,
>>>>   for example during make docker-image-fedora. Without configure, DSOSUF is
>>>>   empty, and the module pattern rule in rules.mak that uses this variable
>>>>   can match too much; provide a default in the Makefile to avoid it.
>>>>
>>>> * only attempt to build the archive when there is a non-empty list of objects.
>>>>   This could be done in general for the %.a: pattern in rules.mak, but maybe
>>>>   there are valid reasons to build an empty .a?
>>>>
>>>> * removed some intermediate variables that did not add much value
>>>>   (LIBFDT_srcdir, LIBFDT_archive)
>>>>
>>>> Tested locally with 3 VPATH configurations (no-, VPATH, VPATH in src subdir),
>>>> and with docker-image-fedora, docker-test-debug@fedora that failed
>>>> before.
>>>
>>> Seems reasonable to me.  It's a bit of a shame that we can't use the
>>> dtc makefiles more simply for this.  But I don't quickly know how to
>>> fix them upstream to allow that.
>>
>>
>> Hi David,
> 
> Sorry it's taken so long to reply.
> 
>> I tried to look at dtc upstream makefiles, I don't see a perfect
>> solution at the moment.
>>
>> I came up with this idea though (not fully tested..) that _could_
>> work, ie special casing the libfdt target when it is the only goal
>> in MAKECMDGOALS.
>>
>> Any thoughts?
> 
> Bit of a hack, but still better than what we have now.  If you post
> that for dtc upstream <devicetree-compiler@vger.kernel.org>, I'd apply
> it.
> 
> Another improvement would be to not include these files on a "make
> clean" - it's kind of annoying how a make clean will regenerate all
> these before removing them.
> 

Hi David,

seems it works well also integrated with qemu, and there should be no problem in make clean,
as the version_gen should not get created anymore, the overall clean rule should take care
of objects and libs.

While bolting it to qemu I noticed that _avoiding_ passing down CFLAGS _and_ QEMU_CFLAGS is
necessary to avoid compilation errors. With those changes made, the integration of the
modified dtc seems to work.

the diff needed in qemu (note the absence of DTC_CFLAGS passed down as CFLAGS):

From: Claudio Fontana <cfontana@suse.de>
Date: Tue, 12 May 2020 12:24:55 +0200
Subject: [PATCH] Makefile: dtc: build the libfdt target

call the libfdt target from the dtc Makefile, which has been
changed to not require bison, flex, etc.

scripts/ symlink and tests directory creation are not necessary,
and neither is calling the clean rule explicitly.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 Makefile  | 8 +++-----
 configure | 1 -
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 34275f57c9..3b88e5eb99 100644
--- a/Makefile
+++ b/Makefile
@@ -527,12 +527,11 @@ $(TARGET_DIRS_RULES):
 	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" TARGET_DIR="$(dir $@)" $(notdir $@),)
 
 DTC_MAKE_ARGS=-I$(SRC_PATH)/dtc VPATH=$(SRC_PATH)/dtc -C dtc V="$(V)" LIBFDT_srcdir=$(SRC_PATH)/dtc/libfdt
-DTC_CFLAGS=$(CFLAGS) $(QEMU_CFLAGS)
-DTC_CPPFLAGS=-I$(BUILD_DIR)/dtc -I$(SRC_PATH)/dtc -I$(SRC_PATH)/dtc/libfdt
+DTC_CPPFLAGS=-I$(SRC_PATH)/dtc/libfdt
 
 .PHONY: dtc/all
-dtc/all: .git-submodule-status dtc/libfdt dtc/tests
-	$(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) CPPFLAGS="$(DTC_CPPFLAGS)" CFLAGS="$(DTC_CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)" ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) libfdt/libfdt.a,)
+dtc/all: .git-submodule-status dtc/libfdt
+	$(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) CPPFLAGS="$(DTC_CPPFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)" ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) libfdt,)
 
 dtc/%: .git-submodule-status
 	@mkdir -p $@
@@ -820,7 +819,6 @@ distclean: clean
 	rm -rf $$d || exit 1 ; \
         done
 	rm -Rf .sdk
-	if test -f dtc/version_gen.h; then $(MAKE) $(DTC_MAKE_ARGS) clean; fi
 
 KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  no  pt-br  sv \
 ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \
diff --git a/configure b/configure
index 0d69c360c0..42554792ec 100755
--- a/configure
+++ b/configure
@@ -4281,7 +4281,6 @@ EOF
           mkdir -p dtc
           if [ "$pwd_is_source_path" != "y" ] ; then
               symlink "$source_path/dtc/Makefile" "dtc/Makefile"
-              symlink "$source_path/dtc/scripts" "dtc/scripts"
           fi
           fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
           fdt_ldflags="-L\$(BUILD_DIR)/dtc/libfdt"
-- 
2.16.4