Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v3] Makefile: fix use of many br2-external trees
@ 2022-09-20 19:46 Yann E. MORIN
  2022-09-21 18:13 ` Thomas Petazzoni
  0 siblings, 1 reply; 5+ messages in thread
From: Yann E. MORIN @ 2022-09-20 19:46 UTC (permalink / raw)
  To: buildroot; +Cc: David Lawson, Yann E. MORIN

The top level Makefile in buildroot has a recursive rule which causes
the appearance of a hang as the number of directories in BR2_EXTERNAL
increases. When the number of directories in BR2_EXTERNAL is small, the
recursion occurs, but make detects the recursion and determines the
target does not have to be remade. This allows make to progress.

This is the failing rule:

    define percent_defconfig
    # Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
    %_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
        @$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
                $$< --defconfig=$(1)/configs/$$@ $$(CONFIG_CONFIG_IN)
    endef
    $(eval $(foreach d,$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(call percent_defconfig,$(d))$(sep)))

The rule for %defconfig is created for each directory in BR2_EXTERNAL.
When the rule is matched, the stem is 'defconfig_name'. The second
prerequisite is expanded to $(1)/configs/defconfig_name_defconfig. The
rule, and all of the other rules defined by this macro, are invoked
again, but the stem is now $(1)/configs/defconfig_name_defconfig. The
second prerequisite is now expanded to
$(1)/configs/($1)/configs/defconfig_name_defconfig. This expansion
continues until make detects the infinite recursion.

With up to 5 br2-external trees, the time is very small, so that it is
not noticeable. But starting with 6 br2-external trees, the time is
insanely big (so much so that we did not even let it finish after it ran
for hours); see timings toward the end of the commit log.

One of the rationale behind this code, is that we want the defconfig
files from br2-external trees further down the list, to override
defconfig files from those earlier in the list, even overriding the
defconfig files from Buildroot itself.

We fix that by only creating explicit rules for defconfig files.

To keep the promise that later defconfig files override previous ones
(which we do document in our manual), we need to memorise what defconfig
file we already created a rule for, and only create a rule for the
first-seen-in-reverse-order (aka the last one) defconfig.

Since some people appear to be bold enough (or insane enough?) to use
defconfig files that start with a dot, also handle those explictly.

Fixes: #14996

Reported-by: David Lawson <david.lawson1@tx.rr.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

----
How to test many br2-external trees:

$ for i in $(seq 1 1000); do
    mkdir -p br2-external-${i}/configs
    touch br2-external-${i}/{Config.in,external.mk}
    echo "name: BR_TEST_${i}" >br2-external-${i}/external.desc
    touch br2-external-${i}/configs/foo{,_${i}}_defconfig
done

$ time make V=1 \
    BR2_EXTERNAL="$(for i in $(seq 1 N); do printf '%s ' "$(pwd)/foo²/br2-external-${i}"; done)" \
    foo_1_defconfig

Timings ('real' as reported by 'time'):

   N    Before     After
   1    0.325s     0.328s
   5    0.957s     0.358s
   6    n/a        0.394s
  10    n/a        0.432s
 100    n/a        1.851s
1000    n/a        15.887s

So, not only does that work for a large number of br2-external trees, it
is also a little bit faster when using just a few.

---
Changes v2 -> v3:
  - fix order of comment documenting $1 and $2  (David)
  - don't use immediate assignment :=, use simple assignment
  - add timing information and a way to reproduce

Changes v1 -> v2:
  - keep comment
  - fix typo
---
 Makefile | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index ec7c034ac1..9d4b1626ae 100644
--- a/Makefile
+++ b/Makefile
@@ -1010,13 +1010,27 @@ oldconfig syncconfig olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmake
 defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(DEFCONFIG),=$(DEFCONFIG)) $(CONFIG_CONFIG_IN)
 
-define percent_defconfig
-# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
-%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
-	@$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
+ALL_DEFCONFIGS =
+# $1: br2-external directory, without trailing /configs/
+# $2: defconfig name with trailing _defconfig
+define defconfig_rule
+ifeq ($$(filter $(2),$$(ALL_DEFCONFIGS)),)
+# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the actual defconfig
+$(2): $$(BUILD_DIR)/buildroot-config/conf outputmakefile
+	$$(Q)$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
 		$$< --defconfig=$(1)/configs/$$@ $$(CONFIG_CONFIG_IN)
+ALL_DEFCONFIGS += $(2)
+endif
 endef
-$(eval $(foreach d,$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)),$(call percent_defconfig,$(d))$(sep)))
+$(eval \
+	$(foreach d, \
+		$(call reverse,$(TOPDIR) $(BR2_EXTERNAL_DIRS)), \
+		$(foreach c, \
+			$(wildcard $(d)/configs/*_defconfig $(d)/configs/.*_defconfig), \
+			$(call defconfig_rule,$(d),$(notdir $(c)))$(sep) \
+		) \
+	) \
+)
 
 update-defconfig: savedefconfig
 
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-21 18:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-20 19:46 [Buildroot] [PATCH v3] Makefile: fix use of many br2-external trees Yann E. MORIN
2022-09-21 18:13 ` Thomas Petazzoni
2022-09-21 18:28   ` Yann E. MORIN
2022-09-21 18:32   ` Arnout Vandecappelle
2022-09-21 18:57     ` Yann E. MORIN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox