All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary
@ 2020-04-11  9:31 Claudio Fontana
  2020-04-11  9:31 ` [PATCH RESEND v3 1/2] " Claudio Fontana
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Claudio Fontana @ 2020-04-11  9:31 UTC (permalink / raw)
  To: Peter Maydell, David Gibson, Philippe Mathieu-Daude, Markus Armbruster
  Cc: Claudio Fontana, Alex Bennee, Laurent Vivier, qemu-devel

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.

Claudio Fontana (2):
  Makefile: libfdt: build only the strict necessary
  Makefile: remove old compatibility gunks

 Makefile  | 32 ++++++++++++++++----------------
 configure |  6 +-----
 rules.mak |  2 ++
 3 files changed, 19 insertions(+), 21 deletions(-)

-- 
2.16.4



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH RESEND v3 1/2] Makefile: libfdt: build only the strict necessary
  2020-04-11  9:31 [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary Claudio Fontana
@ 2020-04-11  9:31 ` Claudio Fontana
  2020-04-29 11:47   ` Claudio Fontana
  2020-04-11  9:31 ` [PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks Claudio Fontana
  2020-04-14  2:03 ` [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary David Gibson
  2 siblings, 1 reply; 10+ messages in thread
From: Claudio Fontana @ 2020-04-11  9:31 UTC (permalink / raw)
  To: Peter Maydell, David Gibson, Philippe Mathieu-Daude, Markus Armbruster
  Cc: Claudio Fontana, Alex Bennee, Laurent Vivier, qemu-devel

when building dtc/libfdt, we were previously using dtc/Makefile,
which tries to build some artifacts that are not needed,
and can complain on stderr about the absence of tools that
are not required to build just libfdt.

Instead, build only the strict necessary to get libfdt.a .

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 Makefile  | 21 +++++++++++++--------
 configure |  4 ----
 rules.mak |  2 ++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 84ef881600..7be15eeb7c 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
   $(error main directory cannot contain spaces nor colons)
 endif
 
+# some pattern rules in rules.mak are confused by an empty DSOSUF,
+# and UNCHECKED_GOALS for testing (docker-) can run without prior configure.
+DSOSUF ?= ".so"
+
 # Always point to the root of the build tree (needs GNU make).
 BUILD_DIR=$(CURDIR)
 
@@ -526,15 +530,17 @@ $(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
 $(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
-
+LIBFDT_objdir = dtc/libfdt
+-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
+LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
+# the name dtc/all is for backward compatibility
 .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 $(LIBFDT_objdir)/libfdt.a
+$(LIBFDT_objdir)/libfdt.a: $(LIBFDT_objects)
+	$(if $(LIBFDT_objects),$(call quiet-command,rm -f $@ && $(AR) rcs $@ $^,"AR","$(TARGET_DIR)$@"),)
 
-dtc/%: .git-submodule-status
+$(LIBFDT_objects): | $(LIBFDT_objdir)
+$(LIBFDT_objdir): .git-submodule-status
 	@mkdir -p $@
 
 # Overriding CFLAGS causes us to lose defines added in the sub-makefile.
@@ -821,7 +827,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 233c671aaa..cf32bfb75b 100755
--- a/configure
+++ b/configure
@@ -4278,10 +4278,6 @@ EOF
       if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
           fdt=git
           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"
           fdt_libs="$fdt_libs"
diff --git a/rules.mak b/rules.mak
index 694865b63e..61eb474ba4 100644
--- a/rules.mak
+++ b/rules.mak
@@ -105,6 +105,8 @@ LINK = $(call quiet-command, $(LINKPROG) $(CFLAGS) $(QEMU_LDFLAGS) -o $@ \
 
 DSO_OBJ_CFLAGS := -fPIC -DBUILD_DSO
 module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
+
+# Note: DSOSUF must not be empty, or these rules will try to match too much
 %$(DSOSUF): QEMU_LDFLAGS += $(LDFLAGS_SHARED)
 %$(DSOSUF): %.mo
 	$(call LINK,$^)
-- 
2.16.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks
  2020-04-11  9:31 [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary Claudio Fontana
  2020-04-11  9:31 ` [PATCH RESEND v3 1/2] " Claudio Fontana
@ 2020-04-11  9:31 ` Claudio Fontana
  2020-04-14  9:40   ` Philippe Mathieu-Daudé
  2020-04-14  2:03 ` [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary David Gibson
  2 siblings, 1 reply; 10+ messages in thread
From: Claudio Fontana @ 2020-04-11  9:31 UTC (permalink / raw)
  To: Peter Maydell, David Gibson, Philippe Mathieu-Daude, Markus Armbruster
  Cc: Claudio Fontana, Alex Bennee, Laurent Vivier, qemu-devel

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 Makefile | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Makefile b/Makefile
index 7be15eeb7c..00377f28b9 100644
--- a/Makefile
+++ b/Makefile
@@ -567,12 +567,6 @@ slirp/all: .git-submodule-status
 		CC="$(CC)" AR="$(AR)" 	LD="$(LD)" RANLIB="$(RANLIB)"	\
 		CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)")
 
-# Compatibility gunk to keep make working across the rename of targets
-# for recursion, to be removed some time after 4.1.
-subdir-dtc: dtc/all
-subdir-capstone: capstone/all
-subdir-slirp: slirp/all
-
 $(filter %/all, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
 	$(qom-obj-y)
 
-- 
2.16.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary
  2020-04-11  9:31 [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary Claudio Fontana
  2020-04-11  9:31 ` [PATCH RESEND v3 1/2] " Claudio Fontana
  2020-04-11  9:31 ` [PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks Claudio Fontana
@ 2020-04-14  2:03 ` David Gibson
  2020-04-15  8:16   ` Claudio Fontana
  2 siblings, 1 reply; 10+ messages in thread
From: David Gibson @ 2020-04-14  2:03 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: Peter Maydell, Philippe Mathieu-Daude, qemu-devel,
	Markus Armbruster, Alex Bennee, Laurent Vivier

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

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.

> 
> Claudio Fontana (2):
>   Makefile: libfdt: build only the strict necessary
>   Makefile: remove old compatibility gunks
> 
>  Makefile  | 32 ++++++++++++++++----------------
>  configure |  6 +-----
>  rules.mak |  2 ++
>  3 files changed, 19 insertions(+), 21 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks
  2020-04-11  9:31 ` [PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks Claudio Fontana
@ 2020-04-14  9:40   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-14  9:40 UTC (permalink / raw)
  To: Claudio Fontana, Peter Maydell, David Gibson, Markus Armbruster
  Cc: Alex Bennee, Laurent Vivier, qemu-devel

On 4/11/20 11:31 AM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>   Makefile | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 7be15eeb7c..00377f28b9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -567,12 +567,6 @@ slirp/all: .git-submodule-status
>   		CC="$(CC)" AR="$(AR)" 	LD="$(LD)" RANLIB="$(RANLIB)"	\
>   		CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)")
>   
> -# Compatibility gunk to keep make working across the rename of targets
> -# for recursion, to be removed some time after 4.1.
> -subdir-dtc: dtc/all
> -subdir-capstone: capstone/all
> -subdir-slirp: slirp/all
> -
>   $(filter %/all, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
>   	$(qom-obj-y)
>   
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary
  2020-04-14  2:03 ` [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary David Gibson
@ 2020-04-15  8:16   ` Claudio Fontana
  2020-05-11  6:33     ` David Gibson
  0 siblings, 1 reply; 10+ messages in thread
From: Claudio Fontana @ 2020-04-15  8:16 UTC (permalink / raw)
  To: David Gibson
  Cc: Peter Maydell, Philippe Mathieu-Daude, qemu-devel,
	Markus Armbruster, Alex Bennee, Laurent Vivier

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,

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?

Ciao,

Claudio


diff --git a/Makefile b/Makefile
index f02aa19..cb256e8 100644
--- a/Makefile
+++ b/Makefile
@@ -176,6 +176,7 @@ endif
 
 
 ifneq ($(DEPTARGETS),)
+ifneq ($(MAKECMDGOALS),libfdt)
 -include $(DTC_OBJS:%.o=%.d)
 -include $(CONVERT_OBJS:%.o=%.d)
 -include $(FDTDUMP_OBJS:%.o=%.d)
@@ -183,6 +184,7 @@ ifneq ($(DEPTARGETS),)
 -include $(FDTPUT_OBJS:%.o=%.d)
 -include $(FDTOVERLAY_OBJS:%.o=%.d)
 endif
+endif
 
 
 
@@ -318,7 +320,9 @@ ifeq ($(NO_PYTHON),0)
 TESTS_PYLIBFDT += maybe_pylibfdt
 endif
 
+ifneq ($(MAKECMDGOALS),libfdt)
 include tests/Makefile.tests
+endif
 
 #
 # Clean rules


> 
>>
>> Claudio Fontana (2):
>>   Makefile: libfdt: build only the strict necessary
>>   Makefile: remove old compatibility gunks
>>
>>  Makefile  | 32 ++++++++++++++++----------------
>>  configure |  6 +-----
>>  rules.mak |  2 ++
>>  3 files changed, 19 insertions(+), 21 deletions(-)
>>
> 



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 1/2] Makefile: libfdt: build only the strict necessary
  2020-04-11  9:31 ` [PATCH RESEND v3 1/2] " Claudio Fontana
@ 2020-04-29 11:47   ` Claudio Fontana
  2020-05-04  8:24     ` David Gibson
  0 siblings, 1 reply; 10+ messages in thread
From: Claudio Fontana @ 2020-04-29 11:47 UTC (permalink / raw)
  To: Peter Maydell, David Gibson, Philippe Mathieu-Daude, Markus Armbruster
  Cc: Alex Bennee, Laurent Vivier, qemu-devel

just a gentle ping on this one, now that 5.0 is out, is proceeding with this ok?

Ciao,

Claudio


On 4/11/20 11:31 AM, Claudio Fontana wrote:
> when building dtc/libfdt, we were previously using dtc/Makefile,
> which tries to build some artifacts that are not needed,
> and can complain on stderr about the absence of tools that
> are not required to build just libfdt.
> 
> Instead, build only the strict necessary to get libfdt.a .
> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  Makefile  | 21 +++++++++++++--------
>  configure |  4 ----
>  rules.mak |  2 ++
>  3 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 84ef881600..7be15eeb7c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
>    $(error main directory cannot contain spaces nor colons)
>  endif
>  
> +# some pattern rules in rules.mak are confused by an empty DSOSUF,
> +# and UNCHECKED_GOALS for testing (docker-) can run without prior configure.
> +DSOSUF ?= ".so"
> +
>  # Always point to the root of the build tree (needs GNU make).
>  BUILD_DIR=$(CURDIR)
>  
> @@ -526,15 +530,17 @@ $(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
>  $(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
> -
> +LIBFDT_objdir = dtc/libfdt
> +-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
> +LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
> +# the name dtc/all is for backward compatibility
>  .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 $(LIBFDT_objdir)/libfdt.a
> +$(LIBFDT_objdir)/libfdt.a: $(LIBFDT_objects)
> +	$(if $(LIBFDT_objects),$(call quiet-command,rm -f $@ && $(AR) rcs $@ $^,"AR","$(TARGET_DIR)$@"),)
>  
> -dtc/%: .git-submodule-status
> +$(LIBFDT_objects): | $(LIBFDT_objdir)
> +$(LIBFDT_objdir): .git-submodule-status
>  	@mkdir -p $@
>  
>  # Overriding CFLAGS causes us to lose defines added in the sub-makefile.
> @@ -821,7 +827,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 233c671aaa..cf32bfb75b 100755
> --- a/configure
> +++ b/configure
> @@ -4278,10 +4278,6 @@ EOF
>        if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
>            fdt=git
>            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"
>            fdt_libs="$fdt_libs"
> diff --git a/rules.mak b/rules.mak
> index 694865b63e..61eb474ba4 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -105,6 +105,8 @@ LINK = $(call quiet-command, $(LINKPROG) $(CFLAGS) $(QEMU_LDFLAGS) -o $@ \
>  
>  DSO_OBJ_CFLAGS := -fPIC -DBUILD_DSO
>  module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
> +
> +# Note: DSOSUF must not be empty, or these rules will try to match too much
>  %$(DSOSUF): QEMU_LDFLAGS += $(LDFLAGS_SHARED)
>  %$(DSOSUF): %.mo
>  	$(call LINK,$^)
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 1/2] Makefile: libfdt: build only the strict necessary
  2020-04-29 11:47   ` Claudio Fontana
@ 2020-05-04  8:24     ` David Gibson
  0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2020-05-04  8:24 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: Peter Maydell, Philippe Mathieu-Daude, qemu-devel,
	Markus Armbruster, Alex Bennee, Laurent Vivier

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

On Wed, Apr 29, 2020 at 01:47:47PM +0200, Claudio Fontana wrote:
> just a gentle ping on this one, now that 5.0 is out, is proceeding
> with this ok?

Yes, go for it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary
  2020-04-15  8:16   ` Claudio Fontana
@ 2020-05-11  6:33     ` David Gibson
  2020-05-12 10:27       ` Claudio Fontana
  0 siblings, 1 reply; 10+ messages in thread
From: David Gibson @ 2020-05-11  6:33 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: Peter Maydell, Philippe Mathieu-Daude, qemu-devel,
	Markus Armbruster, Alex Bennee, Laurent Vivier

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

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.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary
  2020-05-11  6:33     ` David Gibson
@ 2020-05-12 10:27       ` Claudio Fontana
  0 siblings, 0 replies; 10+ messages in thread
From: Claudio Fontana @ 2020-05-12 10:27 UTC (permalink / raw)
  To: David Gibson
  Cc: Peter Maydell, Philippe Mathieu-Daude, Laurent Vivier,
	qemu-devel, Alex Bennee, Markus Armbruster

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








^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-12 10:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-11  9:31 [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary Claudio Fontana
2020-04-11  9:31 ` [PATCH RESEND v3 1/2] " Claudio Fontana
2020-04-29 11:47   ` Claudio Fontana
2020-05-04  8:24     ` David Gibson
2020-04-11  9:31 ` [PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks Claudio Fontana
2020-04-14  9:40   ` Philippe Mathieu-Daudé
2020-04-14  2:03 ` [PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary David Gibson
2020-04-15  8:16   ` Claudio Fontana
2020-05-11  6:33     ` David Gibson
2020-05-12 10:27       ` Claudio Fontana

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.