* [Buildroot] [PATCH 0/8] Top-level Makefile fixes
@ 2017-06-14 22:11 Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 1/8] Makefile: remove 'toolchain' from .PHONY Arnout Vandecappelle
` (8 more replies)
0 siblings, 9 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 1/8] Makefile: remove 'toolchain' from .PHONY
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 2/8] Makefile: document noconfig_targets variable Arnout Vandecappelle
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 2/8] Makefile: document noconfig_targets variable
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 1/8] Makefile: remove 'toolchain' from .PHONY Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 3/8] Makefile: use pattern for manual-% in noconfig_targets Arnout Vandecappelle
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 3/8] Makefile: use pattern for manual-% in noconfig_targets
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 1/8] Makefile: remove 'toolchain' from .PHONY Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 2/8] Makefile: document noconfig_targets variable Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 4/8] Makefile: declare targets PHONY where they are defined Arnout Vandecappelle
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 4/8] Makefile: declare targets PHONY where they are defined
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
` (2 preceding siblings ...)
2017-06-14 22:11 ` [Buildroot] [PATCH 3/8] Makefile: use pattern for manual-% in noconfig_targets Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 5/8] Makefile: add missing PHONY targets Arnout Vandecappelle
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 5/8] Makefile: add missing PHONY targets
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
` (3 preceding siblings ...)
2017-06-14 22:11 ` [Buildroot] [PATCH 4/8] Makefile: declare targets PHONY where they are defined Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 6/8] doc-asciidoc: add missing .PHONY declarations Arnout Vandecappelle
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 6/8] doc-asciidoc: add missing .PHONY declarations
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
` (4 preceding siblings ...)
2017-06-14 22:11 ` [Buildroot] [PATCH 5/8] Makefile: add missing PHONY targets Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 7/8] printvars: remove "Nothing to be done for 'printvars'." Arnout Vandecappelle
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 7/8] printvars: remove "Nothing to be done for 'printvars'."
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
` (5 preceding siblings ...)
2017-06-14 22:11 ` [Buildroot] [PATCH 6/8] doc-asciidoc: add missing .PHONY declarations Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error Arnout Vandecappelle
2017-06-15 9:59 ` [Buildroot] [PATCH 0/8] Top-level Makefile fixes Peter Korsgaard
8 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
` (6 preceding siblings ...)
2017-06-14 22:11 ` [Buildroot] [PATCH 7/8] printvars: remove "Nothing to be done for 'printvars'." Arnout Vandecappelle
@ 2017-06-14 22:11 ` Arnout Vandecappelle
2017-06-16 11:18 ` Luca Ceresoli
2017-06-15 9:59 ` [Buildroot] [PATCH 0/8] Top-level Makefile fixes Peter Korsgaard
8 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-14 22:11 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 0/8] Top-level Makefile fixes
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
` (7 preceding siblings ...)
2017-06-14 22:11 ` [Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error Arnout Vandecappelle
@ 2017-06-15 9:59 ` Peter Korsgaard
2017-06-15 20:18 ` Arnout Vandecappelle
8 siblings, 1 reply; 15+ messages in thread
From: Peter Korsgaard @ 2017-06-15 9:59 UTC (permalink / raw)
To: buildroot
>>>>> "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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 0/8] Top-level Makefile fixes
2017-06-15 9:59 ` [Buildroot] [PATCH 0/8] Top-level Makefile fixes Peter Korsgaard
@ 2017-06-15 20:18 ` Arnout Vandecappelle
2017-06-15 20:19 ` [Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig Arnout Vandecappelle
0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-15 20:18 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig
2017-06-15 20:18 ` Arnout Vandecappelle
@ 2017-06-15 20:19 ` Arnout Vandecappelle
2017-06-16 11:19 ` Luca Ceresoli
0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-06-15 20:19 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error
2017-06-14 22:11 ` [Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error Arnout Vandecappelle
@ 2017-06-16 11:18 ` Luca Ceresoli
0 siblings, 0 replies; 15+ messages in thread
From: Luca Ceresoli @ 2017-06-16 11:18 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig
2017-06-15 20:19 ` [Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig Arnout Vandecappelle
@ 2017-06-16 11:19 ` Luca Ceresoli
2017-06-24 21:17 ` Thomas Petazzoni
0 siblings, 1 reply; 15+ messages in thread
From: Luca Ceresoli @ 2017-06-16 11:19 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig
2017-06-16 11:19 ` Luca Ceresoli
@ 2017-06-24 21:17 ` Thomas Petazzoni
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2017-06-24 21:17 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-06-24 21:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 22:11 [Buildroot] [PATCH 0/8] Top-level Makefile fixes Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 1/8] Makefile: remove 'toolchain' from .PHONY Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 2/8] Makefile: document noconfig_targets variable Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 3/8] Makefile: use pattern for manual-% in noconfig_targets Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 4/8] Makefile: declare targets PHONY where they are defined Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 5/8] Makefile: add missing PHONY targets Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 6/8] doc-asciidoc: add missing .PHONY declarations Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 7/8] printvars: remove "Nothing to be done for 'printvars'." Arnout Vandecappelle
2017-06-14 22:11 ` [Buildroot] [PATCH 8/8] Makefile: unconfigured "make toolchain" should report error Arnout Vandecappelle
2017-06-16 11:18 ` Luca Ceresoli
2017-06-15 9:59 ` [Buildroot] [PATCH 0/8] Top-level Makefile fixes Peter Korsgaard
2017-06-15 20:18 ` Arnout Vandecappelle
2017-06-15 20:19 ` [Buildroot] [PATCH] Makefile: unconfigured "make toolchain" should run menuconfig Arnout Vandecappelle
2017-06-16 11:19 ` Luca Ceresoli
2017-06-24 21:17 ` Thomas Petazzoni
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.