[PATCH 0/5] kbuild: Use relative paths if possible

[PATCH 0/5] kbuild: Use relative paths if possible

[Michal Marek]

Hi,

This series changes the Makefile to use relative paths for $(objtree),
always setting it to '.', and for $(srctree) setting either to '.' or
'..', when building in the source tree or a subdirectory thereof,
respectively. The goal is to make compiler messages and Ooopses more
readable by avoiding the full path to the source/object directory.

Patch 2/5 is just an unrelated cleanup.

Michal

Michal Marek (5):
  firmware: Simplify directory creation
  firmware: Use $(quote) in the Makefile
  kbuild: Use relative path for $(objtree)
  kbuild: Use relative path when building in the source tree
  kbuild: Use relative path when building in a subdir of the source tree

 Makefile                | 14 ++++++++++++--
 firmware/Makefile       | 34 +++++++---------------------------
 scripts/Makefile.fwinst | 24 +++++++++++-------------
 3 files changed, 30 insertions(+), 42 deletions(-)

-- 
1.9.2

[PATCH 1/5] firmware: Simplify directory creation

[Michal Marek]

When building the firmware blobs, use a simple loop to create
directories in $(objtree), like in Makefile.build. This simplifies the
rules and also makes it possible to set $(objtree) to '.' later. Before
this change, a dependency on $(objtree)/<dir> would be satisfied by
<dir> in $(srctree).

When installing the firmware blobs, call mkdir like in Makefile.modinst.

Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 firmware/Makefile       | 30 +++++-------------------------
 scripts/Makefile.fwinst | 24 +++++++++++-------------
 2 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/firmware/Makefile b/firmware/Makefile
index cbb09ce..6af62cf 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -138,12 +138,6 @@ fw-shipped-$(CONFIG_YAM) += yam/1200.bin yam/9600.bin
 
 fw-shipped-all := $(fw-shipped-y) $(fw-shipped-m) $(fw-shipped-)
 
-# Directories which we _might_ need to create, so we have a rule for them.
-firmware-dirs := $(sort $(addprefix $(objtree)/$(obj)/,$(dir $(fw-external-y) $(fw-shipped-all))))
-
-quiet_cmd_mkdir = MKDIR   $(patsubst $(objtree)/%,%,$@)
-      cmd_mkdir = mkdir -p $@
-
 quiet_cmd_ihex  = IHEX    $@
       cmd_ihex  = $(OBJCOPY) -Iihex -Obinary $< $@
 
@@ -184,21 +178,10 @@ wordsize_deps := $(wildcard include/config/64bit.h include/config/32bit.h \
 		include/config/superh32.h include/config/superh64.h \
 		include/config/x86_32.h include/config/x86_64.h)
 
-# Workaround for make < 3.81, where .SECONDEXPANSION doesn't work.
-# It'll end up depending on these targets, so make them a PHONY rule which
-# depends on _all_ the directories in $(firmware-dirs), and it'll work out OK.
-PHONY += $(objtree)/$$(%) $(objtree)/$(obj)/$$(%)
-$(objtree)/$$(%) $(objtree)/$(obj)/$$(%): $(firmware-dirs)
-	@true
-
-# For the $$(dir %) trick, where we need % to be expanded first.
-.SECONDEXPANSION:
-
-$(patsubst %,$(obj)/%.gen.S, $(fw-shipped-y)): %: $(wordsize_deps) \
-		| $(objtree)/$$(dir %)
+$(patsubst %,$(obj)/%.gen.S, $(fw-shipped-y)): %: $(wordsize_deps)
 	$(call cmd,fwbin,$(patsubst %.gen.S,%,$@))
 $(patsubst %,$(obj)/%.gen.S, $(fw-external-y)): %: $(wordsize_deps) \
-		include/config/extra/firmware/dir.h | $(objtree)/$$(dir %)
+		include/config/extra/firmware/dir.h
 	$(call cmd,fwbin,$(fwabs)/$(patsubst $(obj)/%.gen.S,%,$@))
 
 # The .o files depend on the binaries directly; the .S files don't.
@@ -207,7 +190,7 @@ $(patsubst %,$(obj)/%.gen.o, $(fw-external-y)): $(obj)/%.gen.o: $(fwdir)/%
 
 # .ihex is used just as a simple way to hold binary files in a source tree
 # where binaries are frowned upon. They are directly converted with objcopy.
-$(obj)/%: $(obj)/%.ihex | $(objtree)/$(obj)/$$(dir %)
+$(obj)/%: $(obj)/%.ihex
 	$(call cmd,ihex)
 
 # Don't depend on ihex2fw if we're installing and it already exists.
@@ -226,16 +209,13 @@ endif
 # is actually meaningful, because the firmware has to be loaded in a certain
 # order rather than as a single binary blob. Thus, we convert them into our
 # more compact binary representation of ihex records (<linux/ihex.h>)
-$(obj)/%.fw: $(obj)/%.HEX $(ihex2fw_dep) | $(objtree)/$(obj)/$$(dir %)
+$(obj)/%.fw: $(obj)/%.HEX $(ihex2fw_dep)
 	$(call cmd,ihex2fw)
 
 # .H16 is our own modified form of Intel HEX, with 16-bit length for records.
-$(obj)/%.fw: $(obj)/%.H16 $(ihex2fw_dep) | $(objtree)/$(obj)/$$(dir %)
+$(obj)/%.fw: $(obj)/%.H16 $(ihex2fw_dep)
 	$(call cmd,h16tofw)
 
-$(firmware-dirs):
-	$(call cmd,mkdir)
-
 obj-y				 += $(patsubst %,%.gen.o, $(fw-external-y))
 obj-$(CONFIG_FIRMWARE_IN_KERNEL) += $(patsubst %,%.gen.o, $(fw-shipped-y))
 
diff --git a/scripts/Makefile.fwinst b/scripts/Makefile.fwinst
index 4d908d1..1a5639e 100644
--- a/scripts/Makefile.fwinst
+++ b/scripts/Makefile.fwinst
@@ -24,25 +24,23 @@ ifndef CONFIG_FIRMWARE_IN_KERNEL
 mod-fw += $(fw-shipped-y)
 endif
 
+ifneq ($(KBUILD_SRC),)
+# Create output directory if not already present
+_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
+
+firmware-dirs := $(sort $(addprefix $(objtree)/$(obj)/,$(dir $(fw-external-y) $(fw-shipped-all))))
+# Create directories for firmware in subdirectories
+_dummy := $(foreach d,$(firmware-dirs), $(shell [ -d $(d) ] || mkdir -p $(d)))
+endif
+
 installed-mod-fw := $(addprefix $(INSTALL_FW_PATH)/,$(mod-fw))
 
 installed-fw := $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-all))
-installed-fw-dirs := $(sort $(dir $(installed-fw))) $(INSTALL_FW_PATH)/./
-
-# Workaround for make < 3.81, where .SECONDEXPANSION doesn't work.
-PHONY += $(INSTALL_FW_PATH)/$$(%) install-all-dirs
-$(INSTALL_FW_PATH)/$$(%): install-all-dirs
-	@true
-install-all-dirs: $(installed-fw-dirs)
-	@true
 
 quiet_cmd_install = INSTALL $(subst $(srctree)/,,$@)
-      cmd_install = $(INSTALL) -m0644 $< $@
-
-$(installed-fw-dirs):
-	$(call cmd,mkdir)
+      cmd_install = mkdir -p $(@D); $(INSTALL) -m0644 $< $@
 
-$(installed-fw): $(INSTALL_FW_PATH)/%: $(obj)/% | $(INSTALL_FW_PATH)/$$(dir %)
+$(installed-fw): $(INSTALL_FW_PATH)/%: $(obj)/%
 	$(call cmd,install)
 
 PHONY +=  __fw_install __fw_modinst FORCE
-- 
1.9.2

[PATCH 2/5] firmware: Use $(quote) in the Makefile

[Michal Marek]

The literal " confuses syntax highlighting in vim.

Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 firmware/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/firmware/Makefile b/firmware/Makefile
index 6af62cf..5747417 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -4,10 +4,10 @@
 
 # Create $(fwabs) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a
 # leading /, it's relative to $(srctree).
-fwdir := $(subst ",,$(CONFIG_EXTRA_FIRMWARE_DIR))
+fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
 fwabs := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))
 
-fw-external-y := $(subst ",,$(CONFIG_EXTRA_FIRMWARE))
+fw-external-y := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE))
 
 # There are three cases to care about:
 # 1. Building kernel with CONFIG_FIRMWARE_IN_KERNEL=y -- $(fw-shipped-y) should
-- 
1.9.2

[PATCH 3/5] kbuild: Use relative path for $(objtree)

[Michal Marek]

The main Makefile sets its working directory to the object tree and
never changes it again. Therefore, we can use '.' instead of the
absolute path.

Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 60ccbfe..7273a3b 100644
--- a/Makefile
+++ b/Makefile
@@ -154,7 +154,7 @@ _all: modules
 endif
 
 srctree		:= $(if $(KBUILD_SRC),$(KBUILD_SRC),$(CURDIR))
-objtree		:= $(CURDIR)
+objtree		:= .
 src		:= $(srctree)
 obj		:= $(objtree)
 
-- 
1.9.2

[PATCH 4/5] kbuild: Use relative path when building in the source tree

[Michal Marek]

When not using O=, $(srctree) refers to the same directory as
$(objtree), so we can set it to '.' as well. This makes the default
include path more compact and results in more readable messages from the
compiler.

Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7273a3b..7ae9e9d 100644
--- a/Makefile
+++ b/Makefile
@@ -153,7 +153,7 @@ else
 _all: modules
 endif
 
-srctree		:= $(if $(KBUILD_SRC),$(KBUILD_SRC),$(CURDIR))
+srctree		:= $(if $(KBUILD_SRC),$(KBUILD_SRC),.)
 objtree		:= .
 src		:= $(srctree)
 obj		:= $(objtree)
-- 
1.9.2

[PATCH 5/5] kbuild: Use relative path when building in a subdir of the source tree

[Michal Marek]

When doing make O=<subdir>, use '..' to refer to the source tree. This
allows for more readable compiler messages, and, more importantly, it
sets the VPATH to '..', so filenames in WARN_ON() etc. will be shorter.

Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7ae9e9d..3d311b6 100644
--- a/Makefile
+++ b/Makefile
@@ -153,7 +153,17 @@ else
 _all: modules
 endif
 
-srctree		:= $(if $(KBUILD_SRC),$(KBUILD_SRC),.)
+ifeq ($(KBUILD_SRC),)
+        # building in the source tree
+        srctree := .
+else
+        ifeq ($(KBUILD_SRC)/,$(dir $(CURDIR)))
+                # building in a subdirectory of the source tree
+                srctree := ..
+        else
+                srctree := $(KBUILD_SRC)
+        endif
+endif
 objtree		:= .
 src		:= $(srctree)
 obj		:= $(objtree)
-- 
1.9.2

Re: [PATCH 1/5] firmware: Simplify directory creation

[Sam Ravnborg]

On Tue, Apr 29, 2014 at 04:08:09PM +0200, Michal Marek wrote:
> When building the firmware blobs, use a simple loop to create
> directories in $(objtree), like in Makefile.build. This simplifies the
> rules and also makes it possible to set $(objtree) to '.' later. Before
> this change, a dependency on $(objtree)/<dir> would be satisfied by
> <dir> in $(srctree).
> 
> When installing the firmware blobs, call mkdir like in Makefile.modinst.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Michal Marek <mmarek@suse.cz>

Hi Michal.

The patch looks good. I di not stare enough to fully grasp it,
but it is a nice cleanup.

	Sam

Re: [PATCH 0/5] kbuild: Use relative paths if possible

[Sam Ravnborg]

On Tue, Apr 29, 2014 at 04:08:08PM +0200, Michal Marek wrote:
> Hi,
> 
> This series changes the Makefile to use relative paths for $(objtree),
> always setting it to '.', and for $(srctree) setting either to '.' or
> '..', when building in the source tree or a subdirectory thereof,
> respectively. The goal is to make compiler messages and Ooopses more
> readable by avoiding the full path to the source/object directory.
> 
> Patch 2/5 is just an unrelated cleanup.
> 
> Michal
> 
> Michal Marek (5):
>   firmware: Simplify directory creation
>   firmware: Use $(quote) in the Makefile
>   kbuild: Use relative path for $(objtree)
>   kbuild: Use relative path when building in the source tree
>   kbuild: Use relative path when building in a subdir of the source tree

For patch 2,3,4 and 5 you can add my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Patch 1 also looks good, but I did not fully grasp it and this is MO
implied by an ack. So please do not hold it back due to this as
the cleanup looked good.

	Sam

Re: [PATCH 1/5] firmware: Simplify directory creation

[Michal Marek]

On 2014-04-30 18:12, Sam Ravnborg wrote:
> On Tue, Apr 29, 2014 at 04:08:09PM +0200, Michal Marek wrote:
>> When building the firmware blobs, use a simple loop to create
>> directories in $(objtree), like in Makefile.build. This simplifies the
>> rules and also makes it possible to set $(objtree) to '.' later. Before
>> this change, a dependency on $(objtree)/<dir> would be satisfied by
>> <dir> in $(srctree).
>>
>> When installing the firmware blobs, call mkdir like in Makefile.modinst.
>>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Signed-off-by: Michal Marek <mmarek@suse.cz>
> 
> Hi Michal.
> 
> The patch looks good. I di not stare enough to fully grasp it,
> but it is a nice cleanup.

Previously, there were make rules for

$(objtree)/firmware/<dir>

to create each <dir> under firmware. If we change $(objtree) to '.', the
rule becomes ./firmware/<dir>, but since there already is
$(srctree)/firmware/<dir> and VPATH is set to $(srctree), the rule will
be satisfied by the directory in the $(srctree) and no directory in
$(objtree) will be created. So I changed it to create the directories
unconditionally when parsing the Makefile, just like
scripts/Makefile.build does it.

Michal

Re: [PATCH 0/5] kbuild: Use relative paths if possible

[Michal Marek]

On 2014-04-30 18:13, Sam Ravnborg wrote:
> On Tue, Apr 29, 2014 at 04:08:08PM +0200, Michal Marek wrote:
>> Hi,
>>
>> This series changes the Makefile to use relative paths for $(objtree),
>> always setting it to '.', and for $(srctree) setting either to '.' or
>> '..', when building in the source tree or a subdirectory thereof,
>> respectively. The goal is to make compiler messages and Ooopses more
>> readable by avoiding the full path to the source/object directory.
>>
>> Patch 2/5 is just an unrelated cleanup.
>>
>> Michal
>>
>> Michal Marek (5):
>>   firmware: Simplify directory creation
>>   firmware: Use $(quote) in the Makefile
>>   kbuild: Use relative path for $(objtree)
>>   kbuild: Use relative path when building in the source tree
>>   kbuild: Use relative path when building in a subdir of the source tree
> 
> For patch 2,3,4 and 5 you can add my:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Thanks!

Michal

Re: [PATCH 0/5] kbuild: Use relative paths if possible

[Michal Marek]

On Wed, Apr 30, 2014 at 06:13:59PM +0200, Sam Ravnborg wrote:
> On Tue, Apr 29, 2014 at 04:08:08PM +0200, Michal Marek wrote:
> > Hi,
> > 
> > This series changes the Makefile to use relative paths for $(objtree),
> > always setting it to '.', and for $(srctree) setting either to '.' or
> > '..', when building in the source tree or a subdirectory thereof,
> > respectively. The goal is to make compiler messages and Ooopses more
> > readable by avoiding the full path to the source/object directory.
> > 
> > Patch 2/5 is just an unrelated cleanup.
> > 
> > Michal
> > 
> > Michal Marek (5):
> >   firmware: Simplify directory creation
> >   firmware: Use $(quote) in the Makefile
> >   kbuild: Use relative path for $(objtree)
> >   kbuild: Use relative path when building in the source tree
> >   kbuild: Use relative path when building in a subdir of the source tree
> 
> For patch 2,3,4 and 5 you can add my:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> Patch 1 also looks good, but I did not fully grasp it and this is MO
> implied by an ack. So please do not hold it back due to this as
> the cleanup looked good.

I found a bug caused by 3/5 and 4/5: The source and build symlinks
created by modules_install were wrong. The fix is simple:

diff --git a/Makefile b/Makefile
index 3d311b6..9872c53 100644
--- a/Makefile
+++ b/Makefile
@@ -1064,10 +1064,10 @@ _modinst_:
 	@rm -rf $(MODLIB)/kernel
 	@rm -f $(MODLIB)/source
 	@mkdir -p $(MODLIB)/kernel
-	@ln -s $(srctree) $(MODLIB)/source
+	@ln -s `cd $(srctree) && /bin/pwd` $(MODLIB)/source
 	@if [ ! $(objtree) -ef  $(MODLIB)/build ]; then \
 		rm -f $(MODLIB)/build ; \
-		ln -s $(objtree) $(MODLIB)/build ; \
+		ln -s $(CURDIR) $(MODLIB)/build ; \
 	fi
 	@cp -f $(objtree)/modules.order $(MODLIB)/
 	@cp -f $(objtree)/modules.builtin $(MODLIB)/

I will fold it into the respective patches and send a V2.

Michal