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 <cfont...@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,$^)
>>>
>
