* [PATCH v2] Makefile: libfdt: build only the strict necessary
@ 2020-04-09 12:43 Claudio Fontana
2020-04-09 16:33 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Claudio Fontana @ 2020-04-09 12:43 UTC (permalink / raw)
To: Peter Maydell, David Gibson
Cc: qemu-devel, Alex Bennee, Claudio Fontana, Laurent Vivier
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 .
Remove the subdir-dtc "compatibility gunk" for recursion,
since we are not recursing anymore.
Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
Makefile | 23 +++++++++++++----------
configure | 6 +-----
rules.mak | 2 ++
3 files changed, 16 insertions(+), 15 deletions(-)
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.
diff --git a/Makefile b/Makefile
index 84ef881600..92bc853b5f 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,16 @@ $(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
-
-.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,)
+LIBFDT_objdir = dtc/libfdt
+-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
+LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
+.PHONY: libfdt
+libfdt: .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.
@@ -563,7 +568,6 @@ slirp/all: .git-submodule-status
# 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
@@ -821,7 +825,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..36f83ffc5a 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"
@@ -8151,7 +8147,7 @@ echo "PIXMAN_CFLAGS=$pixman_cflags" >> $config_host_mak
echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak
if [ "$fdt" = "git" ]; then
- echo "config-host.h: dtc/all" >> $config_host_mak
+ echo "config-host.h: libfdt" >> $config_host_mak
fi
if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
echo "config-host.h: capstone/all" >> $config_host_mak
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] 5+ messages in thread
* Re: [PATCH v2] Makefile: libfdt: build only the strict necessary
2020-04-09 12:43 [PATCH v2] Makefile: libfdt: build only the strict necessary Claudio Fontana
@ 2020-04-09 16:33 ` Philippe Mathieu-Daudé
2020-04-10 13:00 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-09 16:33 UTC (permalink / raw)
To: Claudio Fontana, Peter Maydell, David Gibson
Cc: Alex Bennee, qemu-devel, Laurent Vivier
Hi Claudio,
On 4/9/20 2:43 PM, 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 .
>
> Remove the subdir-dtc "compatibility gunk" for recursion,
> since we are not recursing anymore.
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
> Makefile | 23 +++++++++++++----------
> configure | 6 +-----
> rules.mak | 2 ++
> 3 files changed, 16 insertions(+), 15 deletions(-)
>
> 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.
>
> diff --git a/Makefile b/Makefile
> index 84ef881600..92bc853b5f 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,16 @@ $(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
> -
> -.PHONY: dtc/all
> -dtc/all: .git-submodule-status dtc/libfdt dtc/tests
I'm getting:
config-host.mak is out-of-date, running configure
make: *** No rule to make target 'dtc/all', needed by 'config-host.h'.
Stop.
On second try it works.
Instead of alarming users, we could keep this target as a silent no-op,
then remove it after some time.
For the rest, patch looks good, nice cleanup!
Regards,
Phil.
> - $(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,)
> +LIBFDT_objdir = dtc/libfdt
> +-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
> +LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
> +.PHONY: libfdt
> +libfdt: .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.
> @@ -563,7 +568,6 @@ slirp/all: .git-submodule-status
>
> # 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
>
> @@ -821,7 +825,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..36f83ffc5a 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"
> @@ -8151,7 +8147,7 @@ echo "PIXMAN_CFLAGS=$pixman_cflags" >> $config_host_mak
> echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak
>
> if [ "$fdt" = "git" ]; then
> - echo "config-host.h: dtc/all" >> $config_host_mak
> + echo "config-host.h: libfdt" >> $config_host_mak
> fi
> if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
> echo "config-host.h: capstone/all" >> $config_host_mak
> 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] 5+ messages in thread
* Re: [PATCH v2] Makefile: libfdt: build only the strict necessary
2020-04-09 16:33 ` Philippe Mathieu-Daudé
@ 2020-04-10 13:00 ` Philippe Mathieu-Daudé
2020-04-10 14:32 ` Claudio Fontana
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-10 13:00 UTC (permalink / raw)
To: Claudio Fontana, Peter Maydell, David Gibson
Cc: Alex Bennee, qemu-devel, Laurent Vivier
On 4/9/20 6:33 PM, Philippe Mathieu-Daudé wrote:
> Hi Claudio,
>
> On 4/9/20 2:43 PM, 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 .
>>
>> Remove the subdir-dtc "compatibility gunk" for recursion,
>> since we are not recursing anymore.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>> Makefile | 23 +++++++++++++----------
>> configure | 6 +-----
>> rules.mak | 2 ++
>> 3 files changed, 16 insertions(+), 15 deletions(-)
>>
>> 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.
>>
>> diff --git a/Makefile b/Makefile
>> index 84ef881600..92bc853b5f 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,16 @@ $(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
>> -
>> -.PHONY: dtc/all
>> -dtc/all: .git-submodule-status dtc/libfdt dtc/tests
>
> I'm getting:
>
> config-host.mak is out-of-date, running configure
> make: *** No rule to make target 'dtc/all', needed by 'config-host.h'.
> Stop.
>
> On second try it works.
FYI same happens when going back (previous this patch applied) but there
is nothing we can do to prevent that afaik:
config-host.mak is out-of-date, running configure
make: *** No rule to make target 'libfdt', needed by 'config-host.h'. Stop.
>
> Instead of alarming users, we could keep this target as a silent no-op,
> then remove it after some time.
>
> For the rest, patch looks good, nice cleanup!
>
> Regards,
>
> Phil.
>
>> - $(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,)
>> +LIBFDT_objdir = dtc/libfdt
>> +-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
>> +LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
>> +.PHONY: libfdt
>> +libfdt: .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.
>> @@ -563,7 +568,6 @@ slirp/all: .git-submodule-status
>> # 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
>> @@ -821,7 +825,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..36f83ffc5a 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"
>> @@ -8151,7 +8147,7 @@ echo "PIXMAN_CFLAGS=$pixman_cflags" >>
>> $config_host_mak
>> echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak
>> if [ "$fdt" = "git" ]; then
>> - echo "config-host.h: dtc/all" >> $config_host_mak
>> + echo "config-host.h: libfdt" >> $config_host_mak
>> fi
>> if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
>> echo "config-host.h: capstone/all" >> $config_host_mak
>> 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] 5+ messages in thread
* Re: [PATCH v2] Makefile: libfdt: build only the strict necessary
2020-04-10 13:00 ` Philippe Mathieu-Daudé
@ 2020-04-10 14:32 ` Claudio Fontana
2020-04-14 7:20 ` Markus Armbruster
0 siblings, 1 reply; 5+ messages in thread
From: Claudio Fontana @ 2020-04-10 14:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Peter Maydell, David Gibson, Markus Armbruster
Cc: Alex Bennee, qemu-devel, Laurent Vivier
Hello Philippe, Markus,
On 4/10/20 3:00 PM, Philippe Mathieu-Daudé wrote:
> On 4/9/20 6:33 PM, Philippe Mathieu-Daudé wrote:
>> Hi Claudio,
>>
>> On 4/9/20 2:43 PM, 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 .
>>>
>>> Remove the subdir-dtc "compatibility gunk" for recursion,
>>> since we are not recursing anymore.
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>> ---
>>> Makefile | 23 +++++++++++++----------
>>> configure | 6 +-----
>>> rules.mak | 2 ++
>>> 3 files changed, 16 insertions(+), 15 deletions(-)
>>>
>>> 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.
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 84ef881600..92bc853b5f 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,16 @@ $(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
>>> -
>>> -.PHONY: dtc/all
>>> -dtc/all: .git-submodule-status dtc/libfdt dtc/tests
>>
>> I'm getting:
>>
>> config-host.mak is out-of-date, running configure
>> make: *** No rule to make target 'dtc/all', needed by 'config-host.h'.
>> Stop.
>>
>> On second try it works.
>
> FYI same happens when going back (previous this patch applied) but there
> is nothing we can do to prevent that afaik:
hmm maybe preserving the previous name for the phony rule "dtc/all" could make it work both forward and backward..
I'll try it in v3.
>
> config-host.mak is out-of-date, running configure
> make: *** No rule to make target 'libfdt', needed by 'config-host.h'. Stop.
>
>>
>> Instead of alarming users, we could keep this target as a silent no-op,
>> then remove it after some time.
>>
>> For the rest, patch looks good, nice cleanup!
I am tempted to remove the old compatibility gunks that are marked as "to be removed some time after 4.1" as the second patch in the series,
any thoughts? (Markus?)
Ciao,
Claudio
>>
>> Regards,
>>
>> Phil.
>>
>>> - $(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,)
>>> +LIBFDT_objdir = dtc/libfdt
>>> +-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
>>> +LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
>>> +.PHONY: libfdt
>>> +libfdt: .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.
>>> @@ -563,7 +568,6 @@ slirp/all: .git-submodule-status
>>> # 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
>>> @@ -821,7 +825,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..36f83ffc5a 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"
>>> @@ -8151,7 +8147,7 @@ echo "PIXMAN_CFLAGS=$pixman_cflags" >>
>>> $config_host_mak
>>> echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak
>>> if [ "$fdt" = "git" ]; then
>>> - echo "config-host.h: dtc/all" >> $config_host_mak
>>> + echo "config-host.h: libfdt" >> $config_host_mak
>>> fi
>>> if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
>>> echo "config-host.h: capstone/all" >> $config_host_mak
>>> 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] 5+ messages in thread
* Re: [PATCH v2] Makefile: libfdt: build only the strict necessary
2020-04-10 14:32 ` Claudio Fontana
@ 2020-04-14 7:20 ` Markus Armbruster
0 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2020-04-14 7:20 UTC (permalink / raw)
To: Claudio Fontana
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Laurent Vivier, qemu-devel, Alex Bennee, David Gibson
Claudio Fontana <cfontana@suse.de> writes:
[...]
> I am tempted to remove the old compatibility gunks that are marked as "to be removed some time after 4.1" as the second patch in the series,
> any thoughts? (Markus?)
Yes, please!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-14 7:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 12:43 [PATCH v2] Makefile: libfdt: build only the strict necessary Claudio Fontana
2020-04-09 16:33 ` Philippe Mathieu-Daudé
2020-04-10 13:00 ` Philippe Mathieu-Daudé
2020-04-10 14:32 ` Claudio Fontana
2020-04-14 7:20 ` Markus Armbruster
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.