[Buildroot] [PATCH 0/8] Top-level Makefile fixes

[Buildroot] [PATCH 0/8] Top-level Makefile fixes

[Arnout Vandecappelle]

 I saw this StackOverflow question [1], to which Luca gave a satisfactory
answer, and noticed that this really shouldn't happen: when Buildroot
isn't configured and you type 'make toolchain', you should get an error
instead of "Nothing to be done.".

 So I set out to fix that, one thing led to another, and here are 8
patches :-)

 As described in the last patch, there are in fact two different ways
to fix it. I already have a patch for the alternative so I can post it
if necessary.

 Regards,
 Arnout


Arnout Vandecappelle (8):
  Makefile: remove 'toolchain' from .PHONY
  Makefile: document noconfig_targets variable
  Makefile: use pattern for manual-% in noconfig_targets
  Makefile: declare targets PHONY where they are defined
  Makefile: add missing PHONY targets
  doc-asciidoc: add missing .PHONY declarations
  printvars: remove "Nothing to be done for 'printvars'."
  Makefile: unconfigured "make toolchain" should report error

 Makefile                | 50 +++++++++++++++++++++++++++++++++++++++++--------
 package/doc-asciidoc.mk |  8 +++++++-
 2 files changed, 49 insertions(+), 9 deletions(-)

-- 
2.11.0

[Buildroot] [PATCH 1/8] Makefile: remove 'toolchain' from .PHONY

[Arnout Vandecappelle]

toolchain is a package, so it is already defined as .PHONY in the
inner-generic-package macro.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 499a39fe20..5f348b2513 100644
--- a/Makefile
+++ b/Makefile
@@ -548,7 +548,7 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
 
 world: target-post-image
 
-.PHONY: all world toolchain dirs clean distclean source outputmakefile \
+.PHONY: all world dirs clean distclean source outputmakefile \
 	legal-info legal-info-prepare legal-info-clean printvars help \
 	list-defconfigs target-finalize target-post-image source-check
 
-- 
2.11.0

[Buildroot] [PATCH 2/8] Makefile: document noconfig_targets variable

[Arnout Vandecappelle]

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 5f348b2513..089989c624 100644
--- a/Makefile
+++ b/Makefile
@@ -125,6 +125,7 @@ DATE := $(shell date +%Y%m%d)
 # Need to export it, so it can be got from environment in children (eg. mconf)
 export BR2_VERSION_FULL := $(BR2_VERSION)$(shell $(TOPDIR)/support/scripts/setlocalversion)
 
+# List of targets and target patterns for which .config doesn't need to be read in
 noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
 	defconfig %_defconfig allyesconfig allnoconfig silentoldconfig release \
 	randpackageconfig allyespackageconfig allnopackageconfig \
-- 
2.11.0

[Buildroot] [PATCH 3/8] Makefile: use pattern for manual-% in noconfig_targets

[Arnout Vandecappelle]

This simplifies the variable a little

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 089989c624..1e22858082 100644
--- a/Makefile
+++ b/Makefile
@@ -129,8 +129,7 @@ export BR2_VERSION_FULL := $(BR2_VERSION)$(shell $(TOPDIR)/support/scripts/setlo
 noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
 	defconfig %_defconfig allyesconfig allnoconfig silentoldconfig release \
 	randpackageconfig allyespackageconfig allnopackageconfig \
-	print-version olddefconfig distclean manual manual-html manual-split-html \
-	manual-pdf manual-text manual-epub
+	print-version olddefconfig distclean manual manual-%
 
 # Some global targets do not trigger a build, but are used to collect
 # metadata, or do various checks. When such targets are triggered,
-- 
2.11.0

[Buildroot] [PATCH 4/8] Makefile: declare targets PHONY where they are defined

[Arnout Vandecappelle]

Currently, a lot of targets are declared PHONY together in the middle
of the Makefile. This has two important shortcomings:

- it is more difficult to see if a target is missing from PHONY;

- it is currently inside the ifeq ($(BR2_HAVE_DOT_CONFIG),y) condition,
  but some of these targets are also defined when there is no .config;
  in that case, these targets are not declared as PHONY.

Both issues can easily be solved by putting the PHONY declaration next
to the definition of the target.

The noconfig_targets are also all declared PHONY together; however,
for these we anyway have to keep the noconfig_targets variable
up-to-date, and that PHONY declaration is outside all conditions, so
there is no benefit of splitting them.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 1e22858082..fca528ca19 100644
--- a/Makefile
+++ b/Makefile
@@ -84,6 +84,7 @@ else # umask / $(CURDIR) / $(O)
 
 # This is our default rule, so must come first
 all:
+.PHONY: all
 
 # Set and export the version string
 export BR2_VERSION := 2017.08-git
@@ -538,6 +539,7 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\
 
 endif
 
+.PHONY: dirs
 dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
 	$(HOST_DIR) $(BINARIES_DIR)
 
@@ -546,12 +548,9 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 
 prepare: $(BUILD_DIR)/buildroot-config/auto.conf
 
+.PHONY: world
 world: target-post-image
 
-.PHONY: all world dirs clean distclean source outputmakefile \
-	legal-info legal-info-prepare legal-info-clean printvars help \
-	list-defconfigs target-finalize target-post-image source-check
-
 # Populating the staging with the base directories is handled by the skeleton package
 $(STAGING_DIR):
 	@mkdir -p $(STAGING_DIR)
@@ -654,6 +653,7 @@ endif
 
 $(TARGETS_ROOTFS): target-finalize
 
+.PHONY: target-finalize
 target-finalize: $(PACKAGES)
 	@$(call MESSAGE,"Finalizing target directory")
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
@@ -714,11 +714,13 @@ endif
 		$(call MESSAGE,"Executing post-build script $(s)"); \
 		$(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
+.PHONY: target-post-image
 target-post-image: $(TARGETS_ROOTFS) target-finalize
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \
 		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
+.PHONY: source
 source: $(foreach p,$(PACKAGES),$(p)-all-source)
 
 _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
@@ -726,11 +728,14 @@ external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
 
 # check if download URLs are outdated
+.PHONY: source-check
 source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
 
+.PHONY: legal-info-clean
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)
 
+.PHONY: legal-info-prepare
 legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call MESSAGE,"Collecting legal info")
 	@$(call legal-license-file,buildroot,COPYING,COPYING,HOST)
@@ -740,6 +745,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call legal-warning,the Buildroot source code has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
 
+.PHONY: legal-info
 legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
 		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
 	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
@@ -921,6 +927,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 # outputmakefile generates a Makefile in the output directory, if using a
 # separate output directory. This allows convenient use of make in the
 # output directory.
+.PHONY: outputmakefile
 outputmakefile:
 ifeq ($(NEED_WRAPPER),y)
 	$(Q)$(TOPDIR)/support/scripts/mkmakefile $(TOPDIR) $(O)
@@ -937,6 +944,7 @@ $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
 # Makefiles. Alternatively, if a non-empty VARS variable is passed,
 # only the variables matching the make pattern passed in VARS are
 # displayed.
+.PHONY: printvars
 printvars:
 	@$(foreach V, \
 		$(sort $(if $(VARS),$(filter $(VARS),$(.VARIABLES)),$(.VARIABLES))), \
@@ -947,11 +955,13 @@ printvars:
 			$(info $V=$(if $(RAW_VARS),$(value $V),$($V))))))
 # ' Syntax colouring...
 
+.PHONY: clean
 clean:
 	rm -rf $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
 		$(BUILD_DIR) $(BASE_DIR)/staging \
 		$(LEGAL_INFO_DIR) $(GRAPHS_DIR)
 
+.PHONY: distclean
 distclean: clean
 ifeq ($(O),$(CURDIR)/output)
 	rm -rf $(O)
@@ -959,6 +969,7 @@ endif
 	rm -rf $(TOPDIR)/dl $(BR2_CONFIG) $(CONFIG_DIR)/.config.old $(CONFIG_DIR)/..config.tmp \
 		$(CONFIG_DIR)/.auto.deps $(BR2_EXTERNAL_FILE)
 
+.PHONY: help
 help:
 	@echo 'Cleaning:'
 	@echo '  clean                  - delete all files created by build'
@@ -1055,6 +1066,7 @@ endef
 
 # We iterate over BR2_EXTERNAL_NAMES rather than BR2_EXTERNAL_DIRS,
 # because we want to display the name of the br2-external tree.
+.PHONY: list-defconfigs
 list-defconfigs:
 	$(call list-defconfigs,$(TOPDIR))
 	$(foreach name,$(BR2_EXTERNAL_NAMES),\
-- 
2.11.0

[Buildroot] [PATCH 5/8] Makefile: add missing PHONY targets

[Arnout Vandecappelle]

Quite a few targets in the top-level Makefile were missing the .PHONY
marking. Now that the .PHONY declarations are next to the definition
of the targets, they are much easier to find.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Makefile b/Makefile
index fca528ca19..320f74a30d 100644
--- a/Makefile
+++ b/Makefile
@@ -546,6 +546,7 @@ dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
 $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" silentoldconfig
 
+.PHONY: prepare
 prepare: $(BUILD_DIR)/buildroot-config/auto.conf
 
 .PHONY: world
@@ -723,6 +724,7 @@ target-post-image: $(TARGETS_ROOTFS) target-finalize
 .PHONY: source
 source: $(foreach p,$(PACKAGES),$(p)-all-source)
 
+.PHONY: _external-deps external-deps
 _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
 external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
@@ -760,11 +762,14 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
 		mv .legal-info.sha256 legal-info.sha256)
 	@echo "Legal info produced in $(LEGAL_INFO_DIR)"
 
+.PHONY: show-targets
 show-targets:
 	@echo $(PACKAGES) $(TARGETS_ROOTFS)
 
+.PHONY: show-build-order
 show-build-order: $(patsubst %,%-show-build-order,$(PACKAGES))
 
+.PHONY: graph-build
 graph-build: $(O)/build/build-time.log
 	@install -d $(GRAPHS_DIR)
 	$(foreach o,name build duration,./support/scripts/graph-build-time \
@@ -776,10 +781,12 @@ graph-build: $(O)/build/build-time.log
 				   --output=$(GRAPHS_DIR)/build.pie-$(t).$(BR_GRAPH_OUT) \
 				   $(if $(BR2_GRAPH_ALT),--alternate-colors)$(sep))
 
+.PHONY: graph-depends-requirements
 graph-depends-requirements:
 	@dot -? >/dev/null 2>&1 || \
 		{ echo "ERROR: The 'dot' program from Graphviz is needed for graph-depends" >&2; exit 1; }
 
+.PHONY: graph-depends
 graph-depends: graph-depends-requirements
 	@$(INSTALL) -d $(GRAPHS_DIR)
 	@cd "$(CONFIG_DIR)"; \
@@ -789,6 +796,7 @@ graph-depends: graph-depends-requirements
 		-o $(GRAPHS_DIR)/$(@).$(BR_GRAPH_OUT) \
 		$(GRAPHS_DIR)/$(@).dot
 
+.PHONY: graph-size
 graph-size:
 	$(Q)mkdir -p $(GRAPHS_DIR)
 	$(Q)$(TOPDIR)/support/scripts/size-stats --builddir $(BASE_DIR) \
@@ -796,6 +804,7 @@ graph-size:
 		--file-size-csv $(GRAPHS_DIR)/file-size-stats.csv \
 		--package-size-csv $(GRAPHS_DIR)/package-size-stats.csv
 
+.PHONY: check-dependencies
 check-dependencies:
 	@cd "$(CONFIG_DIR)"; \
 	$(TOPDIR)/support/scripts/graph-depends -C
-- 
2.11.0

[Buildroot] [PATCH 6/8] doc-asciidoc: add missing .PHONY declarations

[Arnout Vandecappelle]

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/doc-asciidoc.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/doc-asciidoc.mk b/package/doc-asciidoc.mk
index cb6e616603..be92496c2e 100644
--- a/package/doc-asciidoc.mk
+++ b/package/doc-asciidoc.mk
@@ -1,5 +1,6 @@
 # we can't use suitable-host-package here because that's not available in
 # the context of 'make release'
+.PHONY: asciidoc-check-dependencies
 asciidoc-check-dependencies:
 	$(Q)if [ -z "$(shell support/dependencies/check-host-asciidoc.sh)" ]; then \
 		echo "You need a sufficiently recent asciidoc on your host" \
@@ -58,8 +59,9 @@ $(1): $(1)-$(5)
 .PHONY: $(1)-$(5)
 $(1)-$(5): $$(O)/docs/$(1)/$(1).$(6)
 
-# Single line, because splitting a foreach is not easy...
 asciidoc-check-dependencies-$(5):
+.PHONY: $(1)-check-dependencies-$(5)
+# Single line, because splitting a foreach is not easy...
 $(1)-check-dependencies-$(5): asciidoc-check-dependencies-$(5)
 	$$(Q)$$(foreach hook,$$($(2)_CHECK_DEPENDENCIES_$$(call UPPERCASE,$(5))_HOOKS),$$(call $$(hook))$$(sep))
 
@@ -141,6 +143,7 @@ endef
 ################################################################################
 define ASCIIDOC
 # Single line, because splitting a foreach is not easy...
+.PHONY: $(1)-check-dependencies
 $(1)-check-dependencies: asciidoc-check-dependencies $$($(2)_DEPENDENCIES)
 	$$(Q)$$(foreach hook,$$($(2)_CHECK_DEPENDENCIES_HOOKS),$$(call $$(hook))$$(sep))
 
@@ -153,6 +156,7 @@ $$(BUILD_DIR)/docs/$(1)/.stamp_doc_rsynced:
 	$$(Q)rsync -a $(3) $$(@D)
 	$$(Q)$$(foreach hook,$$($(2)_POST_RSYNC_HOOKS),$$(call $$(hook))$$(sep))
 
+.PHONY: $(1)-prepare-sources
 $(1)-prepare-sources: $$(BUILD_DIR)/docs/$(1)/.stamp_doc_rsynced
 
 $(2)_ASCIIDOC_CONF = $(3)/asciidoc.conf
-- 
2.11.0

[Buildroot] [PATCH 7/8] printvars: remove "Nothing to be done for 'printvars'."

[Arnout Vandecappelle]

When calling 'make printvars' without -s, it ends with
"Nothing to be done for 'printvars'." That's because the rule only
contains $(info ...) calls and no actual shell commands to execute.

To avoid this, make sure there is a shell command by adding :.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 320f74a30d..88d98e0405 100644
--- a/Makefile
+++ b/Makefile
@@ -955,7 +955,7 @@ $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
 # displayed.
 .PHONY: printvars
 printvars:
-	@$(foreach V, \
+	@:$(foreach V, \
 		$(sort $(if $(VARS),$(filter $(VARS),$(.VARIABLES)),$(.VARIABLES))), \
 		$(if $(filter-out environment% default automatic, \
 				$(origin $V)), \
-- 
2.11.0

[Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error

[Arnout Vandecappelle]

As reported by Alessandro Power on StackOverflow [1], the behaviour
of "make toolchain" in an unconfigured tree is misleading.

When .config doesn't exist, we don't read in the package .mk files, so
"make <package>" doesn't work:

    $ make busybox
    make: *** No rule to make target 'busybox'.  Stop.

However, for "linux" and "toolchain", the corresponding file (or
actually directory) already exists. So instead, we get:

    $ make linux
    make: Nothing to be done for 'linux'.

This is confusing, because it looks as if the build succeeded.

The obvious solution would be to make linux and toolchain PHONY targets
when .config doesn't exist. However, that actually does the reverse,
because then a rule _does_ exist for them and since they don't have
dependencies, make will consider them to be ready.

Instead, we could define linux and toolchain as targets and make them
depend on menuconfig. But then their behaviour is different from other
packages.

So instead, we add a catch-all rule when .config doesn't exist. This
rule will be triggered whenever the user requests a target that is not
explicitly defined and prints a more user-friendly error message. To
make this work correctly, we have to make sure that any valid target
has commands so it avoids the catch-all rule.

[1] https://stackoverflow.com/questions/44521150

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
This solution is admittedly a bit fragile... I also have a patch for
the alternative "define linux and toolchain as targets that depend
on menuconfig".
---
 Makefile                | 15 ++++++++++++++-
 package/doc-asciidoc.mk |  4 +++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 88d98e0405..6eab289c00 100644
--- a/Makefile
+++ b/Makefile
@@ -813,6 +813,19 @@ else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
 all: menuconfig
 
+%:
+	@echo 'Please configure Buildroot first (e.g. "make menuconfig")' 1>&2
+	@exit 1
+
+# Since linux and toolchain already exist as directories, we must force them to
+# fall under the catch-all rule defined above
+linux toolchain: FORCE
+FORCE: ;
+
+# .config doesn't exist yet but is -include'd above, so make will try to rebuild
+# it with the catch-all rule. Add an explicit rule to avoid that.
+$(BR2_CONFIG): ;
+
 endif # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
 # configuration
@@ -937,7 +950,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 # separate output directory. This allows convenient use of make in the
 # output directory.
 .PHONY: outputmakefile
-outputmakefile:
+outputmakefile: ;
 ifeq ($(NEED_WRAPPER),y)
 	$(Q)$(TOPDIR)/support/scripts/mkmakefile $(TOPDIR) $(O)
 endif
diff --git a/package/doc-asciidoc.mk b/package/doc-asciidoc.mk
index be92496c2e..89317910a6 100644
--- a/package/doc-asciidoc.mk
+++ b/package/doc-asciidoc.mk
@@ -18,6 +18,9 @@ asciidoc-check-dependencies-pdf:
 		exit 1; \
 	fi
 
+# catch-all pattern rule for when there are no dependencies
+asciidoc-check-dependencies-%: ;
+
 # PDF generation is broken because of a bug in xsltproc program provided
 # by libxslt <=1.1.28, which does not honor an option we need to set.
 # Fortunately, this bug is already fixed upstream:
@@ -59,7 +62,6 @@ $(1): $(1)-$(5)
 .PHONY: $(1)-$(5)
 $(1)-$(5): $$(O)/docs/$(1)/$(1).$(6)
 
-asciidoc-check-dependencies-$(5):
 .PHONY: $(1)-check-dependencies-$(5)
 # Single line, because splitting a foreach is not easy...
 $(1)-check-dependencies-$(5): asciidoc-check-dependencies-$(5)
-- 
2.11.0

[Buildroot] [PATCH 0/8] Top-level Makefile fixes

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:

 >  I saw this StackOverflow question [1], to which Luca gave a satisfactory
 > answer, and noticed that this really shouldn't happen: when Buildroot
 > isn't configured and you type 'make toolchain', you should get an error
 > instead of "Nothing to be done.".

ENOURL?

 >  So I set out to fix that, one thing led to another, and here are 8
 > patches :-)

 >  As described in the last patch, there are in fact two different ways
 > to fix it. I already have a patch for the alternative so I can post it
 > if necessary.

Committed patch 1-7. I would like to think a bit more about patch 8 and
perhaps get feedback from others.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 0/8] Top-level Makefile fixes

[Arnout Vandecappelle]

On 15-06-17 11:59, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:
> 
>  >  I saw this StackOverflow question [1], to which Luca gave a satisfactory
>  > answer, and noticed that this really shouldn't happen: when Buildroot
>  > isn't configured and you type 'make toolchain', you should get an error
>  > instead of "Nothing to be done.".
> 
> ENOURL?

 It was in the last patch as well :-)

[1] https://stackoverflow.com/questions/44521150


>  >  So I set out to fix that, one thing led to another, and here are 8
>  > patches :-)
> 
>  >  As described in the last patch, there are in fact two different ways
>  > to fix it. I already have a patch for the alternative so I can post it
>  > if necessary.
> 
> Committed patch 1-7. I would like to think a bit more about patch 8 and
> perhaps get feedback from others.

 I'll send the simpler alternative as well.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig

[Arnout Vandecappelle]

As reported by Alessandro Power on StackOverflow [1], the behaviour
of "make toolchain" in an unconfigured tree is misleading.

When .config doesn't exist, we don't read in the package .mk files, so
"make <package>" doesn't work:

    $ make busybox
    make: *** No rule to make target 'busybox'.  Stop.

However, for "linux" and "toolchain", the corresponding file (or
actually directory) already exists. So instead, we get:

    $ make linux
    make: Nothing to be done for 'linux'.

This is confusing, because it looks as if the build succeeded.

The obvious solution would be to make linux and toolchain PHONY targets
when .config doesn't exist. However, that actually does the reverse,
because then a rule _does_ exist for them and since they don't have
dependencies, make will consider them to be ready.

Instead, we define linux and toolchain as targets and make them depend
on menuconfig. The behaviour is still different from other packages,
but at least it is less confusing.

[1] https://stackoverflow.com/questions/44521150

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 88d98e0405..f1ae9b0c17 100644
--- a/Makefile
+++ b/Makefile
@@ -813,6 +813,11 @@ else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
 all: menuconfig
 
+# Some subdirectories are also package names. To avoid that "make linux"
+# on an unconfigured tree produces "Nothing to be done", add an explicit
+# rule for it.
+linux toolchain: menuconfig
+
 endif # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
 # configuration
-- 
2.11.0

[Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error

[Luca Ceresoli]

Hi Arnout,

On 15/06/2017 00:11, Arnout Vandecappelle (Essensium/Mind) wrote:
> As reported by Alessandro Power on StackOverflow [1], the behaviour
> of "make toolchain" in an unconfigured tree is misleading.
> 
> When .config doesn't exist, we don't read in the package .mk files, so
> "make <package>" doesn't work:
> 
>     $ make busybox
>     make: *** No rule to make target 'busybox'.  Stop.
> 
> However, for "linux" and "toolchain", the corresponding file (or
> actually directory) already exists. So instead, we get:
> 
>     $ make linux
>     make: Nothing to be done for 'linux'.
> 
> This is confusing, because it looks as if the build succeeded.
> 
> The obvious solution would be to make linux and toolchain PHONY targets
> when .config doesn't exist. However, that actually does the reverse,
> because then a rule _does_ exist for them and since they don't have
> dependencies, make will consider them to be ready.
> 
> Instead, we could define linux and toolchain as targets and make them
> depend on menuconfig. But then their behaviour is different from other
> packages.
> 
> So instead, we add a catch-all rule when .config doesn't exist. This
> rule will be triggered whenever the user requests a target that is not
> explicitly defined and prints a more user-friendly error message. To
> make this work correctly, we have to make sure that any valid target
> has commands so it avoids the catch-all rule.
> 
> [1] https://stackoverflow.com/questions/44521150
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> This solution is admittedly a bit fragile... I also have a patch for
> the alternative "define linux and toolchain as targets that depend
> on menuconfig".
> ---
>  Makefile                | 15 ++++++++++++++-
>  package/doc-asciidoc.mk |  4 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 88d98e0405..6eab289c00 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -813,6 +813,19 @@ else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>  
>  all: menuconfig
>  
> +%:
> +	@echo 'Please configure Buildroot first (e.g. "make menuconfig")' 1>&2
> +	@exit 1
> +
> +# Since linux and toolchain already exist as directories, we must force them to
> +# fall under the catch-all rule defined above
> +linux toolchain: FORCE
> +FORCE: ;
> +
> +# .config doesn't exist yet but is -include'd above, so make will try to rebuild
> +# it with the catch-all rule. Add an explicit rule to avoid that.
> +$(BR2_CONFIG): ;
> +
>  endif # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>  
>  # configuration
> @@ -937,7 +950,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
>  # separate output directory. This allows convenient use of make in the
>  # output directory.
>  .PHONY: outputmakefile
> -outputmakefile:
> +outputmakefile: ;

The ';' code looks a bit like black magic to non-make-gurus. And since
there are several sprinkled in the code, a verbose comment on why it is
needed is not quite doable. Moreover it's easy to forget to put one when
adding new targets.

So IMO the alternative solution is better for both readability and
maintainability.

>  ifeq ($(NEED_WRAPPER),y)
>  	$(Q)$(TOPDIR)/support/scripts/mkmakefile $(TOPDIR) $(O)
>  endif
> diff --git a/package/doc-asciidoc.mk b/package/doc-asciidoc.mk
> index be92496c2e..89317910a6 100644
> --- a/package/doc-asciidoc.mk
> +++ b/package/doc-asciidoc.mk
> @@ -18,6 +18,9 @@ asciidoc-check-dependencies-pdf:
>  		exit 1; \
>  	fi
>  
> +# catch-all pattern rule for when there are no dependencies
> +asciidoc-check-dependencies-%: ;
> +
>  # PDF generation is broken because of a bug in xsltproc program provided
>  # by libxslt <=1.1.28, which does not honor an option we need to set.
>  # Fortunately, this bug is already fixed upstream:
> @@ -59,7 +62,6 @@ $(1): $(1)-$(5)
>  .PHONY: $(1)-$(5)
>  $(1)-$(5): $$(O)/docs/$(1)/$(1).$(6)
>  
> -asciidoc-check-dependencies-$(5):
>  .PHONY: $(1)-check-dependencies-$(5)
>  # Single line, because splitting a foreach is not easy...
>  $(1)-check-dependencies-$(5): asciidoc-check-dependencies-$(5)
> 

-- 
Luca

[Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig

[Luca Ceresoli]

Hi Arnout,

On 15/06/2017 22:19, Arnout Vandecappelle (Essensium/Mind) wrote:
> As reported by Alessandro Power on StackOverflow [1], the behaviour
> of "make toolchain" in an unconfigured tree is misleading.
> 
> When .config doesn't exist, we don't read in the package .mk files, so
> "make <package>" doesn't work:
> 
>     $ make busybox
>     make: *** No rule to make target 'busybox'.  Stop.
> 
> However, for "linux" and "toolchain", the corresponding file (or
> actually directory) already exists. So instead, we get:
> 
>     $ make linux
>     make: Nothing to be done for 'linux'.
> 
> This is confusing, because it looks as if the build succeeded.
> 
> The obvious solution would be to make linux and toolchain PHONY targets
> when .config doesn't exist. However, that actually does the reverse,
> because then a rule _does_ exist for them and since they don't have
> dependencies, make will consider them to be ready.
> 
> Instead, we define linux and toolchain as targets and make them depend
> on menuconfig. The behaviour is still different from other packages,
> but at least it is less confusing.
> 
> [1] https://stackoverflow.com/questions/44521150
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 88d98e0405..f1ae9b0c17 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -813,6 +813,11 @@ else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>  
>  all: menuconfig
>  
> +# Some subdirectories are also package names. To avoid that "make linux"
> +# on an unconfigured tree produces "Nothing to be done", add an explicit
> +# rule for it.
> +linux toolchain: menuconfig
> +

This is very localized and documented w.r.t. the other solution, so I
prefer this one.

However I prefer how the other solution behaves, i.e. print an
explicit error message, not call menuconfig.

How about the following solution?

.PHONY: linux toolchain
linux toolchain:
  @echo 'Please configure Buildroot first (e.g. "make menuconfig")' 1>&2
  @exit 1

I'm not sure it's correct, but seems like it works:

$ make toolchain
Please configure Buildroot first (e.g. "make menuconfig")
Makefile:818: recipe for target 'toolchain' failed
make[1]: *** [toolchain] Error 1
Makefile:79: recipe for target '_all' failed
make: *** [_all] Error 2
$ make defconfig
[...]
$ make toolchain
[...the build starts...]

>  endif # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>  
>  # configuration
> 

-- 
Luca

[Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig

[Thomas Petazzoni]

Hello,

On Fri, 16 Jun 2017 13:19:27 +0200, Luca Ceresoli wrote:

> > +# Some subdirectories are also package names. To avoid that "make linux"
> > +# on an unconfigured tree produces "Nothing to be done", add an explicit
> > +# rule for it.
> > +linux toolchain: menuconfig
> > +  
> 
> This is very localized and documented w.r.t. the other solution, so I
> prefer this one.

Agreed.

> 
> However I prefer how the other solution behaves, i.e. print an
> explicit error message, not call menuconfig.
> 
> How about the following solution?
> 
> .PHONY: linux toolchain
> linux toolchain:
>   @echo 'Please configure Buildroot first (e.g. "make menuconfig")' 1>&2
>   @exit 1
> 
> I'm not sure it's correct, but seems like it works:

+1 with Luca here. Running menuconfig is confusing, I very much prefer
an error message.

Arnout, what do you think? Can you respin with this idea?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com