[PATCH 1/3] build: avoid an extra shell out in quiet variable function

[PATCH 1/3] build: avoid an extra shell out in quiet variable function

[Jani Nikula]

$(word 1, $1) yields the same result as the more complicated
$(shell echo $1 | sed -e s'/ .*//')
---
 Makefile.local | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index af12ca7f2ef8..c51b9afb8fe6 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -182,14 +182,14 @@ verify-newer:
 # user how to enable verbose compiles.
 ifeq ($(V),)
 quiet_DOC := "Use \"$(MAKE) V=1\" to see the verbose compile lines.\n"
-quiet = @printf $(quiet_DOC)$(eval quiet_DOC:=)"$1 $@\n"; $($(shell echo $1 | sed -e s'/ .*//'))
+quiet = @printf $(quiet_DOC)$(eval quiet_DOC:=)"$1 $@\n"; $($(word 1, $(1)))
 endif
 # The user has explicitly enabled quiet compilation.
 ifeq ($(V),0)
-quiet = @printf "$1 $@\n"; $($(shell echo $1 | sed -e s'/ .*//'))
+quiet = @printf "$1 $@\n"; $($(word 1, $(1)))
 endif
 # Otherwise, print the full command line.
-quiet ?= $($(shell echo $1 | sed -e s'/ .*//'))
+quiet ?= $($(word 1, $(1)))
 
 %.o: %.cc $(global_deps)
 	@mkdir -p $(patsubst %/.,%,.deps/$(@D))
-- 
2.11.0

[PATCH 2/3] build: add optional target parameter to quiet variable function

[Jani Nikula]

Sometimes using $@ as the target in the quiet build lines can be
confusing. Accept an optional second parameter in the quiet variable
function to specify the target.
---
 Makefile.local | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index c51b9afb8fe6..2eb55e6ce1cd 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -182,11 +182,11 @@ verify-newer:
 # user how to enable verbose compiles.
 ifeq ($(V),)
 quiet_DOC := "Use \"$(MAKE) V=1\" to see the verbose compile lines.\n"
-quiet = @printf $(quiet_DOC)$(eval quiet_DOC:=)"$1 $@\n"; $($(word 1, $(1)))
+quiet = @printf $(quiet_DOC)$(eval quiet_DOC:=)"$(1) $(or $(2),$@)\n"; $($(word 1, $(1)))
 endif
 # The user has explicitly enabled quiet compilation.
 ifeq ($(V),0)
-quiet = @printf "$1 $@\n"; $($(word 1, $(1)))
+quiet = @printf "$(1) $(or $(2),$@)\n"; $($(word 1, $(1)))
 endif
 # Otherwise, print the full command line.
 quiet ?= $($(word 1, $(1)))
-- 
2.11.0

[PATCH 3/3] build: add .deps to CLEAN instead of listing in clean target

[Jani Nikula]

Seems, uh, cleaner this way.
---
 Makefile.local | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.local b/Makefile.local
index 2eb55e6ce1cd..9e2111f5cc0d 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -201,7 +201,7 @@ quiet ?= $($(word 1, $(1)))
 
 .PHONY : clean
 clean:
-	rm -rf $(CLEAN); rm -rf .deps
+	rm -rf $(CLEAN)
 
 .PHONY: distclean
 distclean: clean
@@ -280,6 +280,7 @@ endif
 SRCS  := $(SRCS) $(notmuch_client_srcs)
 CLEAN := $(CLEAN) notmuch notmuch-shared $(notmuch_client_modules)
 CLEAN := $(CLEAN) version.stamp notmuch-*.tar.gz.tmp
+CLEAN := $(CLEAN) .deps
 
 DISTCLEAN := $(DISTCLEAN) .first-build-message Makefile.config sh.config
 
-- 
2.11.0

Re: [PATCH 2/3] build: add optional target parameter to quiet variable function

[Tomi Ollila]

On Tue, Aug 29 2017, Jani Nikula wrote:

> Sometimes using $@ as the target in the quiet build lines can be
> confusing. Accept an optional second parameter in the quiet variable
> function to specify the target.

+1 for 1/3, +1 for 3/3. this I did not quite understand (and don't bother
to figure out -- but I trust this works so +1 too)

Tomi


> ---
>  Makefile.local | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.local b/Makefile.local
> index c51b9afb8fe6..2eb55e6ce1cd 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -182,11 +182,11 @@ verify-newer:
>  # user how to enable verbose compiles.
>  ifeq ($(V),)
>  quiet_DOC := "Use \"$(MAKE) V=1\" to see the verbose compile lines.\n"
> -quiet = @printf $(quiet_DOC)$(eval quiet_DOC:=)"$1 $@\n"; $($(word 1, $(1)))
> +quiet = @printf $(quiet_DOC)$(eval quiet_DOC:=)"$(1) $(or $(2),$@)\n"; $($(word 1, $(1)))
>  endif
>  # The user has explicitly enabled quiet compilation.
>  ifeq ($(V),0)
> -quiet = @printf "$1 $@\n"; $($(word 1, $(1)))
> +quiet = @printf "$(1) $(or $(2),$@)\n"; $($(word 1, $(1)))
>  endif
>  # Otherwise, print the full command line.
>  quiet ?= $($(word 1, $(1)))
> -- 
> 2.11.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/3] build: add optional target parameter to quiet variable function

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Sometimes using $@ as the target in the quiet build lines can be
> confusing. Accept an optional second parameter in the quiet variable
> function to specify the target.

I'm OK with this, but I wondered about a simpler change: always use $<.
I doesn't strictly make sense to write "CXX  -g -O2
test/make-db-version.o". When is $@ better here?

d

Re: [PATCH 2/3] build: add optional target parameter to quiet variable function

[Jani Nikula]

On Tue, 29 Aug 2017, David Bremner <david@tethera.net> wrote:
> Jani Nikula <jani@nikula.org> writes:
>
>> Sometimes using $@ as the target in the quiet build lines can be
>> confusing. Accept an optional second parameter in the quiet variable
>> function to specify the target.
>
> I'm OK with this, but I wondered about a simpler change: always use $<.
> I doesn't strictly make sense to write "CXX  -g -O2
> test/make-db-version.o". When is $@ better here?

For compiling C, C++, and elisp $< does seem better. However, for AR and
linking $@ seems like the way to go. $< only prints the first
dependency, which seems silly for them.

We could also remove the default, and have all callers of quiet pass in
$< or $@ or whatever as part of the first parameter. It's more verbose
everywhere, but I think simpler overall.

BR,
Jani.

Re: [PATCH 2/3] build: add optional target parameter to quiet variable function

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> We could also remove the default, and have all callers of quiet pass in
> $< or $@ or whatever as part of the first parameter. It's more verbose
> everywhere, but I think simpler overall.

Since I've got a rebased version of my patch on top of your existing
proposal to add an optional parameter, I guess we'll go with that for
now. For me the hard part to understand is not this change, but how
'quiet' works before the change.

d

Re: [PATCH 1/3] build: avoid an extra shell out in quiet variable function

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> $(word 1, $1) yields the same result as the more complicated
> $(shell echo $1 | sed -e s'/ .*//')

series pushed.

d