diff mbox series

[XEN,v2,02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

Message ID 20200117105358.607910-3-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Build system improvements | expand

Commit Message

Anthony PERARD Jan. 17, 2020, 10:53 a.m. UTC
This is part of upgrading our build system and import more of Linux's
one.

In Linux, subdir-y in Makefiles is only used to descend into
subdirectory when there are no object to build, Xen doesn't have that
and all subdir have object to be included in the final binary.

To allow the new syntax, the "obj-y" and "subdir-*" calculation in
Rules.mk is changed and partially imported from Linux's Kbuild.

The command used to modify the Makefile was:
    sed -i -r 's#^subdir-(.*)#obj-\1/#;' **/Makefile

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk                         | 21 ++++++++++-----------
 xen/arch/arm/Makefile                | 14 +++++++-------
 xen/arch/arm/arm32/Makefile          |  2 +-
 xen/arch/arm/arm64/Makefile          |  2 +-
 xen/arch/x86/Makefile                | 18 +++++++++---------
 xen/arch/x86/acpi/Makefile           |  2 +-
 xen/arch/x86/cpu/Makefile            |  4 ++--
 xen/arch/x86/guest/Makefile          |  4 ++--
 xen/arch/x86/hvm/Makefile            |  6 +++---
 xen/arch/x86/mm/Makefile             |  4 ++--
 xen/arch/x86/x86_64/Makefile         |  2 +-
 xen/common/Makefile                  |  8 ++++----
 xen/drivers/Makefile                 | 14 +++++++-------
 xen/drivers/acpi/Makefile            |  6 +++---
 xen/drivers/passthrough/Makefile     |  8 ++++----
 xen/drivers/passthrough/vtd/Makefile |  2 +-
 xen/lib/Makefile                     |  2 +-
 xen/xsm/Makefile                     |  2 +-
 xen/xsm/flask/Makefile               |  2 +-
 19 files changed, 61 insertions(+), 62 deletions(-)

Comments

Jan Beulich Jan. 29, 2020, 2:19 p.m. UTC | #1
On 17.01.2020 11:53, Anthony PERARD wrote:
> This is part of upgrading our build system and import more of Linux's
> one.
> 
> In Linux, subdir-y in Makefiles is only used to descend into
> subdirectory when there are no object to build, Xen doesn't have that
> and all subdir have object to be included in the final binary.
> 
> To allow the new syntax, the "obj-y" and "subdir-*" calculation in
> Rules.mk is changed and partially imported from Linux's Kbuild.
> 
> The command used to modify the Makefile was:
>     sed -i -r 's#^subdir-(.*)#obj-\1/#;' **/Makefile
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with two remarks:

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -105,17 +105,16 @@ define gendep
>  endef
>  $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gendep,$(o))))
>  
> -# Ensure each subdirectory has exactly one trailing slash.
> -subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
> -subdir-y := $(patsubst %,%/,$(patsubst %/,%,$(subdir-y)))
> -
> -# Add explicitly declared subdirectories to the object lists.
> -obj-y += $(patsubst %/,%/built_in.o,$(subdir-y))
> -
> -# Add implicitly declared subdirectories (in the object lists) to the
> -# subdirectory list, and rewrite the object-list entry.
> -subdir-y += $(filter %/,$(obj-y))
> -obj-y    := $(patsubst %/,%/built-in.o,$(obj-y))
> +# Handle objects in subdirs
> +# ---------------------------------------------------------------------------
> +# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
> +#   and add the directory to the list of dirs to descend into: $(subdir-y)
> +__subdir-y	:= $(filter %/, $(obj-y))
> +subdir-y	+= $(__subdir-y)

I realize I'll be called guilty of bike-shedding again, and I also
realize this is the way Linux does it, but what use is the
intermediate __subdir-y? Linux has no 2nd use, and hence I also
don't see why we would gain one. I further think according to our
style there should be no use of tabs here.

> +obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
> +
> +subdir-n := $(subdir-n) $(subdir-) \
> +		$(filter %/, $(obj-n) $(obj-))

This will easily fit on one line (and isn't anything cloned from
Linux).

Jan
Anthony PERARD Jan. 30, 2020, 4:54 p.m. UTC | #2
On Wed, Jan 29, 2020 at 03:19:05PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > +# Handle objects in subdirs
> > +# ---------------------------------------------------------------------------
> > +# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
> > +#   and add the directory to the list of dirs to descend into: $(subdir-y)
> > +__subdir-y	:= $(filter %/, $(obj-y))
> > +subdir-y	+= $(__subdir-y)
> 
> I realize I'll be called guilty of bike-shedding again, and I also
> realize this is the way Linux does it, but what use is the
> intermediate __subdir-y? Linux has no 2nd use, and hence I also
> don't see why we would gain one. I further think according to our
> style there should be no use of tabs here.

I though the extra __subdir-y that Linux does was to do the filtering on
obj-y right way and not at a later time when subdir-y is used. But in
Linux (now that I look more closely at it), subdir-y is initialised with
:= to have the right type, so the extra __subdir-y doesn't appear to be
useful. (And I didn't find any subdir-y=)

So, I'll add a "subdir-y :=" somewhere and remove the need for
__subdir-y. (And hopefully, no one will add a subdir-y=dir somewhere and
break the build.)

As for using space instead of tabs, I can do that. I just need to figure
out how to configure my editor properly to use tab only when needed.

> > +obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
> > +
> > +subdir-n := $(subdir-n) $(subdir-) \
> > +		$(filter %/, $(obj-n) $(obj-))
> 
> This will easily fit on one line (and isn't anything cloned from
> Linux).

Will do.

Thanks,
Jan Beulich Jan. 31, 2020, 8:35 a.m. UTC | #3
On 30.01.2020 17:54, Anthony PERARD wrote:
> On Wed, Jan 29, 2020 at 03:19:05PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> +# Handle objects in subdirs
>>> +# ---------------------------------------------------------------------------
>>> +# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
>>> +#   and add the directory to the list of dirs to descend into: $(subdir-y)
>>> +__subdir-y	:= $(filter %/, $(obj-y))
>>> +subdir-y	+= $(__subdir-y)
>>
>> I realize I'll be called guilty of bike-shedding again, and I also
>> realize this is the way Linux does it, but what use is the
>> intermediate __subdir-y? Linux has no 2nd use, and hence I also
>> don't see why we would gain one. I further think according to our
>> style there should be no use of tabs here.
> 
> I though the extra __subdir-y that Linux does was to do the filtering on
> obj-y right way and not at a later time when subdir-y is used. But in
> Linux (now that I look more closely at it), subdir-y is initialised with
> := to have the right type, so the extra __subdir-y doesn't appear to be
> useful. (And I didn't find any subdir-y=)
> 
> So, I'll add a "subdir-y :=" somewhere and remove the need for
> __subdir-y. (And hopefully, no one will add a subdir-y=dir somewhere and
> break the build.)

Alternatively, to retain this "latching" effect, how about

subdir-y := $(subdir-y) $(filter %/, $(obj-y))

?

Jan
Anthony PERARD Feb. 3, 2020, 11:31 a.m. UTC | #4
On Fri, Jan 31, 2020 at 09:35:16AM +0100, Jan Beulich wrote:
> Alternatively, to retain this "latching" effect, how about
> 
> subdir-y := $(subdir-y) $(filter %/, $(obj-y))

Sounds good.

Thanks,
diff mbox series

Patch

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 22f25c5b2be8..8b04cbdd24ca 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -105,17 +105,16 @@  define gendep
 endef
 $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gendep,$(o))))
 
-# Ensure each subdirectory has exactly one trailing slash.
-subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
-subdir-y := $(patsubst %,%/,$(patsubst %/,%,$(subdir-y)))
-
-# Add explicitly declared subdirectories to the object lists.
-obj-y += $(patsubst %/,%/built_in.o,$(subdir-y))
-
-# Add implicitly declared subdirectories (in the object lists) to the
-# subdirectory list, and rewrite the object-list entry.
-subdir-y += $(filter %/,$(obj-y))
-obj-y    := $(patsubst %/,%/built-in.o,$(obj-y))
+# Handle objects in subdirs
+# ---------------------------------------------------------------------------
+# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
+#   and add the directory to the list of dirs to descend into: $(subdir-y)
+__subdir-y	:= $(filter %/, $(obj-y))
+subdir-y	+= $(__subdir-y)
+obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
+
+subdir-n := $(subdir-n) $(subdir-) \
+		$(filter %/, $(obj-n) $(obj-))
 
 subdir-all := $(subdir-y) $(subdir-n)
 
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 70f532e42a06..1044c2298a05 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -1,11 +1,11 @@ 
-subdir-$(CONFIG_ARM_32) += arm32
-subdir-$(CONFIG_ARM_64) += arm64
-subdir-$(CONFIG_ARM_64) += efi
-subdir-$(CONFIG_ACPI) += acpi
+obj-$(CONFIG_ARM_32) += arm32/
+obj-$(CONFIG_ARM_64) += arm64/
+obj-$(CONFIG_ARM_64) += efi/
+obj-$(CONFIG_ACPI) += acpi/
 ifneq ($(CONFIG_NO_PLAT),y)
-subdir-y += platforms
+obj-y += platforms/
 endif
-subdir-$(CONFIG_TEE) += tee
+obj-$(CONFIG_TEE) += tee/
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.init.o
@@ -48,7 +48,7 @@  obj-y += sysctl.o
 obj-y += time.o
 obj-y += traps.o
 obj-y += vcpreg.o
-subdir-$(CONFIG_NEW_VGIC) += vgic
+obj-$(CONFIG_NEW_VGIC) += vgic/
 ifneq ($(CONFIG_NEW_VGIC),y)
 obj-y += gic-vgic.o
 obj-y += vgic.o
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 0ac254f34714..539bbef298a7 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -1,4 +1,4 @@ 
-subdir-y += lib
+obj-y += lib/
 
 obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c4f3a28a0d0b..db8565b71a33 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,4 +1,4 @@ 
-subdir-y += lib
+obj-y += lib/
 
 obj-y += cache.o
 obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 6783688b00be..461d1f3dc2a6 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -1,12 +1,12 @@ 
-subdir-y += acpi
-subdir-y += cpu
-subdir-y += genapic
-subdir-$(CONFIG_GUEST) += guest
-subdir-$(CONFIG_HVM) += hvm
-subdir-y += mm
-subdir-$(CONFIG_XENOPROF) += oprofile
-subdir-$(CONFIG_PV) += pv
-subdir-y += x86_64
+obj-y += acpi/
+obj-y += cpu/
+obj-y += genapic/
+obj-$(CONFIG_GUEST) += guest/
+obj-$(CONFIG_HVM) += hvm/
+obj-y += mm/
+obj-$(CONFIG_XENOPROF) += oprofile/
+obj-$(CONFIG_PV) += pv/
+obj-y += x86_64/
 
 alternative-y := alternative.init.o
 alternative-$(CONFIG_LIVEPATCH) :=
diff --git a/xen/arch/x86/acpi/Makefile b/xen/arch/x86/acpi/Makefile
index 27b4aa30b0ca..1b9e62571301 100644
--- a/xen/arch/x86/acpi/Makefile
+++ b/xen/arch/x86/acpi/Makefile
@@ -1,4 +1,4 @@ 
-subdir-y += cpufreq
+obj-y += cpufreq/
 
 obj-y += lib.o power.o suspend.o cpu_idle.o cpuidle_menu.o
 obj-bin-y += boot.init.o wakeup_prot.o
diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index 466acc8b10e5..de983006a1b1 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -1,5 +1,5 @@ 
-subdir-y += mcheck
-subdir-y += mtrr
+obj-y += mcheck/
+obj-y += mtrr/
 
 obj-y += amd.o
 obj-y += centaur.o
diff --git a/xen/arch/x86/guest/Makefile b/xen/arch/x86/guest/Makefile
index f164196772e8..a1e370d69df8 100644
--- a/xen/arch/x86/guest/Makefile
+++ b/xen/arch/x86/guest/Makefile
@@ -1,4 +1,4 @@ 
 obj-y += hypervisor.o
 
-subdir-$(CONFIG_HYPERV_GUEST) += hyperv
-subdir-$(CONFIG_XEN_GUEST) += xen
+obj-$(CONFIG_HYPERV_GUEST) += hyperv/
+obj-$(CONFIG_XEN_GUEST) += xen/
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 43e5f3a21f8b..346419154460 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -1,6 +1,6 @@ 
-subdir-y += svm
-subdir-y += vmx
-subdir-y += viridian
+obj-y += svm/
+obj-y += vmx/
+obj-y += viridian/
 
 obj-y += asid.o
 obj-y += dm.o
diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index 5010a29d6cb0..d87dc0aa6eeb 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -1,5 +1,5 @@ 
-subdir-y += shadow
-subdir-$(CONFIG_HVM) += hap
+obj-y += shadow/
+obj-$(CONFIG_HVM) += hap/
 
 obj-$(CONFIG_HVM) += altp2m.o
 obj-$(CONFIG_HVM) += guest_walk_2.o guest_walk_3.o guest_walk_4.o
diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile
index 4bfa1480eb7e..2bb1eb0a8131 100644
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -1,4 +1,4 @@ 
-subdir-$(CONFIG_PV) += compat
+obj-$(CONFIG_PV) += compat/
 
 obj-bin-y += entry.o
 obj-y += traps.o
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 62b34e69e95c..d4db0a6d466a 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -73,8 +73,8 @@  obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall
 
 extra-y := symbols-dummy.o
 
-subdir-$(CONFIG_COVERAGE) += coverage
-subdir-$(CONFIG_UBSAN) += ubsan
+obj-$(CONFIG_COVERAGE) += coverage/
+obj-$(CONFIG_UBSAN) += ubsan/
 
-subdir-$(CONFIG_NEEDS_LIBELF) += libelf
-subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
+obj-$(CONFIG_NEEDS_LIBELF) += libelf/
+obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
diff --git a/xen/drivers/Makefile b/xen/drivers/Makefile
index 30bab3cfdb36..2a1ae8ad130a 100644
--- a/xen/drivers/Makefile
+++ b/xen/drivers/Makefile
@@ -1,7 +1,7 @@ 
-subdir-y += char
-subdir-$(CONFIG_HAS_CPUFREQ) += cpufreq
-subdir-$(CONFIG_HAS_PCI) += pci
-subdir-$(CONFIG_HAS_VPCI) += vpci
-subdir-$(CONFIG_HAS_PASSTHROUGH) += passthrough
-subdir-$(CONFIG_ACPI) += acpi
-subdir-$(CONFIG_VIDEO) += video
+obj-y += char/
+obj-$(CONFIG_HAS_CPUFREQ) += cpufreq/
+obj-$(CONFIG_HAS_PCI) += pci/
+obj-$(CONFIG_HAS_VPCI) += vpci/
+obj-$(CONFIG_HAS_PASSTHROUGH) += passthrough/
+obj-$(CONFIG_ACPI) += acpi/
+obj-$(CONFIG_VIDEO) += video/
diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
index 444b11d5839d..4f8e97228ee2 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,6 +1,6 @@ 
-subdir-y += tables
-subdir-y += utilities
-subdir-$(CONFIG_X86) += apei
+obj-y += tables/
+obj-y += utilities/
+obj-$(CONFIG_X86) += apei/
 
 obj-bin-y += tables.init.o
 obj-$(CONFIG_NUMA) += numa.o
diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index d50ab188c83c..e973e16c7484 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -1,7 +1,7 @@ 
-subdir-$(CONFIG_X86) += vtd
-subdir-$(CONFIG_X86) += amd
-subdir-$(CONFIG_X86) += x86
-subdir-$(CONFIG_ARM) += arm
+obj-$(CONFIG_X86) += vtd/
+obj-$(CONFIG_X86) += amd/
+obj-$(CONFIG_X86) += x86/
+obj-$(CONFIG_ARM) += arm/
 
 obj-y += iommu.o
 obj-$(CONFIG_HAS_PCI) += pci.o
diff --git a/xen/drivers/passthrough/vtd/Makefile b/xen/drivers/passthrough/vtd/Makefile
index f302653858a0..fde7555fac07 100644
--- a/xen/drivers/passthrough/vtd/Makefile
+++ b/xen/drivers/passthrough/vtd/Makefile
@@ -1,4 +1,4 @@ 
-subdir-$(CONFIG_X86) += x86
+obj-$(CONFIG_X86) += x86/
 
 obj-y += iommu.o
 obj-y += dmar.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index dcdb75931378..7019ca00e8fd 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1 +1 @@ 
-subdir-$(CONFIG_X86) += x86
+obj-$(CONFIG_X86) += x86/
diff --git a/xen/xsm/Makefile b/xen/xsm/Makefile
index e4d581e065f8..cf0a728f1c96 100644
--- a/xen/xsm/Makefile
+++ b/xen/xsm/Makefile
@@ -3,4 +3,4 @@  obj-$(CONFIG_XSM) += xsm_policy.o
 obj-$(CONFIG_XSM) += dummy.o
 obj-$(CONFIG_XSM_SILO) += silo.o
 
-subdir-$(CONFIG_XSM_FLASK) += flask
+obj-$(CONFIG_XSM_FLASK) += flask/
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 7c3f381287be..b1fd45421993 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -2,7 +2,7 @@  obj-y += avc.o
 obj-y += hooks.o
 obj-y += flask_op.o
 
-subdir-y += ss
+obj-y += ss/
 
 CFLAGS += -I./include