Skip to contentSkip to conversation

git-gui: silence install recipes under "make -s"

PATCH Superseded Harald Nordgren via GitGitGadget 4 participants 14 messages Jun 3 – Jun 11

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 git-gui: silence install recipes under "make -s"

From: Harald Nordgren <[email protected]>

The split install/uninstall recipes embed "echo" calls that fire even under "make -s", so install still prints "DEST /path" and "INSTALL 644 about.tcl" banners. The existing "-s" block only clears QUIET_GEN.

Wrap the whole "ifndef V" block in the canonical "-s" guard from shared.mak, and drop the now-redundant narrow block.

---
git-gui: silence install recipes under "make -s"

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2318%2FHaraldNordgren%2Fgit-gui-respect-silent-flag-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2318/HaraldNordgren/git-gui-respect-silent-flag-v1
Pull-Request: https://github.com/git/git/pull/2318

 git-gui/Makefile | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
git-gui/Makefile +2 -4
index ca01068810..d33204e875 100644
@@ -64,6 +64,7 @@ REMOVE_F0  = $(RM_RF) # space is required here
 REMOVE_F1  =
 CLEAN_DST  = true
 
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 ifndef V
 	QUIET          = @
 	QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
@@ -89,6 +90,7 @@ ifndef V
 	REMOVE_F0 = dst=
 	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
 endif
+endif
 
 TCLTK_PATH ?= wish
 ifeq (./,$(dir $(TCLTK_PATH)))
@@ -97,10 +99,6 @@ else
 	TCL_PATH ?= $(dir $(TCLTK_PATH))$(notdir $(subst wish,tclsh,$(TCLTK_PATH)))
 endif
 
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)
-QUIET_GEN =
-endif
-
 -include config.mak
 
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))

base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e

Signature · 1 line
gitgitgadget
#1 lore raw

Am 03.06.26 um 11:04 schrieb Harald Nordgren via GitGitGadget:

Quoted 6 lines

From: Harald Nordgren <[email protected]>

The split install/uninstall recipes embed "echo" calls that fire even under "make -s", so install still prints "DEST /path" and "INSTALL 644 about.tcl" banners. The existing "-s" block only clears QUIET_GEN.

Good catch.

Quoted 2 lines

Wrap the whole "ifndef V" block in the canonical "-s" guard from shared.mak, and drop the now-redundant narrow block.

Can we please mention shared.mak in a way that doesn't assume that this patch was made in the Git repository?

git-gui/Makefile ifndef V · line 93

Quoted 11 lines
git-gui/Makefile +2 -1
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 ifndef V
 	QUIET          = @
 	QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
@@ -89,6 +90,7 @@ ifndef V
 	REMOVE_F0 = dst=
 	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
 endif
+endif
 
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)

I would have expected that the old and the new condition expressions only differ in the ifeq vs. ifneq, but they are different in more than that. Assuming that the new expression is correct, was the old one incorrect?

git-gui/Makefile else · line 102

Quoted 2 lines
git-gui/Makefile +0 -2
-QUIET_GEN =
-endif

-- Hannes

#2 lore raw
Quoted 2 lines

Can we please mention shared.mak in a way that doesn't assume that this patch was made in the Git repository?

Good point!

Quoted 10 lines
Quoted 9 lines
git-gui/Makefile +2 -0
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 ifndef V
      QUIET          = @
      QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
@@ -89,6 +90,7 @@ ifndef V
      REMOVE_F0 = dst=
      REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
 endif
+endif
Quoted 6 lines

git-gui/Makefile else · line 100

Quoted 1 line
git-gui/Makefile +0 -1
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)

I would have expected that the old and the new condition expressions only differ in the ifeq vs. ifneq, but they are different in more than that. Assuming that the new expression is correct, was the old one incorrect?

Yeah, the old format doesn't seem to work when I use that with ifneq. So I took this other format that is also used twice in shared.mak (lines 40 and 46).

Harald

#3 lore raw

PATCH v2 1/1 git-gui: silence install recipes under "make -s"

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

Several install and uninstall recipes embed "echo" calls that fire as part of the recipe itself, so the install banners (DEST, INSTALL, LINK, REMOVE) were visible whenever the variables expand non-empty.

Guard the whole "ifndef V" block on "-s" so the loud variants are selected only when "-s" is absent and V=1 is unset.

---
git-gui: silence install recipes under "make -s"

* Clarified commit message.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2318%2FHaraldNordgren%2Fgit-gui-respect-silent-flag-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2318/HaraldNordgren/git-gui-respect-silent-flag-v2
Pull-Request: https://github.com/git/git/pull/2318

Range-diff vs v1:

1: b9f2b16a8d ! 1: 4e4029c8e8 git-gui: silence install recipes under "make -s"
@@ Metadata
## Commit message ##
git-gui: silence install recipes under "make -s"

- The split install/uninstall recipes embed "echo" calls that fire
- even under "make -s", so install still prints "DEST /path" and
- "INSTALL 644 about.tcl" banners. The existing "-s" block only
- clears QUIET_GEN.
+ Several install and uninstall recipes embed "echo" calls that fire as
+ part of the recipe itself, so the install banners (DEST, INSTALL,
+ LINK, REMOVE) were visible whenever the variables expand non-empty.

- Wrap the whole "ifndef V" block in the canonical "-s" guard from
- shared.mak, and drop the now-redundant narrow block.
+ Guard the whole "ifndef V" block on "-s" so the loud variants are
+ selected only when "-s" is absent and V=1 is unset.

 git-gui/Makefile | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
git-gui/Makefile +2 -4
index ca01068810..d33204e875 100644
@@ -64,6 +64,7 @@ REMOVE_F0  = $(RM_RF) # space is required here
 REMOVE_F1  =
 CLEAN_DST  = true
 
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 ifndef V
 	QUIET          = @
 	QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
@@ -89,6 +90,7 @@ ifndef V
 	REMOVE_F0 = dst=
 	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
 endif
+endif
 
 TCLTK_PATH ?= wish
 ifeq (./,$(dir $(TCLTK_PATH)))
@@ -97,10 +99,6 @@ else
 	TCL_PATH ?= $(dir $(TCLTK_PATH))$(notdir $(subst wish,tclsh,$(TCLTK_PATH)))
 endif
 
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)
-QUIET_GEN =
-endif
-
 -include config.mak
 
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))

base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0

Signature · 1 line
gitgitgadget
#4 lore raw

Am 03.06.26 um 20:13 schrieb Harald Nordgren via GitGitGadget:

Quoted 6 lines

From: Harald Nordgren <[email protected]>

Several install and uninstall recipes embed "echo" calls that fire as part of the recipe itself, so the install banners (DEST, INSTALL, LINK, REMOVE) were visible whenever the variables expand non-empty.

Quoted 8 lines

Guard the whole "ifndef V" block on "-s" so the loud variants are selected only when "-s" is absent and V=1 is unset.

---
git-gui: silence install recipes under "make -s"

* Clarified commit message.

I appreciate that you made it more suitable to be used outside of the Git repository, but it still does not explain why the change from ifeq to ifneq is not sufficient to negate the condition.

In fact, the old version of the condition never worked as intended. The parameters of findstring are in the order needle,haystack. The arguments are -,s for normal make and -s,s for make -s. In no case is the needle found in the haystack. The new version is correct. This is worth to be mentioned.

git-gui/Makefile else · line 100

Quoted 3 lines
git-gui/Makefile +1 -1
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)

-- Hannes

#5 lore raw

PATCH v3 1/1 git-gui: silence install recipes under "make -s"

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

Several install and uninstall recipes embed "echo" calls that fire as part of the recipe itself, so the install banners (DEST, INSTALL, LINK, REMOVE) were visible whenever the variables expand non-empty.

Guard the whole "ifndef V" block on "-s" so the loud variants are selected only when "-s" is absent and V=1 is unset. The existing "-s" check also had its findstring arguments in the wrong order (needle "-s" never fit in haystack "s"), so swap them while moving the check to wrap the block.

---
git-gui: silence install recipes under "make -s"

Added sentences to the commit message noting that the old findstring arg order was broken (needle never fit haystack).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2318%2FHaraldNordgren%2Fgit-gui-respect-silent-flag-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2318/HaraldNordgren/git-gui-respect-silent-flag-v3
Pull-Request: https://github.com/git/git/pull/2318

Range-diff vs v2:

1: 4e4029c8e8 ! 1: 1375fdc1aa git-gui: silence install recipes under "make -s"
@@ Commit message
LINK, REMOVE) were visible whenever the variables expand non-empty.

Guard the whole "ifndef V" block on "-s" so the loud variants are
- selected only when "-s" is absent and V=1 is unset.
+ selected only when "-s" is absent and V=1 is unset. The existing
+ "-s" check also had its findstring arguments in the wrong order
+ (needle "-s" never fit in haystack "s"), so swap them while moving
+ the check to wrap the block.

 git-gui/Makefile | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
git-gui/Makefile +2 -4
index ca01068810..d33204e875 100644
@@ -64,6 +64,7 @@ REMOVE_F0  = $(RM_RF) # space is required here
 REMOVE_F1  =
 CLEAN_DST  = true
 
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 ifndef V
 	QUIET          = @
 	QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
@@ -89,6 +90,7 @@ ifndef V
 	REMOVE_F0 = dst=
 	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
 endif
+endif
 
 TCLTK_PATH ?= wish
 ifeq (./,$(dir $(TCLTK_PATH)))
@@ -97,10 +99,6 @@ else
 	TCL_PATH ?= $(dir $(TCLTK_PATH))$(notdir $(subst wish,tclsh,$(TCLTK_PATH)))
 endif
 
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)
-QUIET_GEN =
-endif
-
 -include config.mak
 
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))

base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0

Signature · 1 line
gitgitgadget
#6 lore raw

Am 04.06.26 um 08:48 schrieb Harald Nordgren via GitGitGadget:

Quoted 6 lines

From: Harald Nordgren <[email protected]>

Several install and uninstall recipes embed "echo" calls that fire as part of the recipe itself, so the install banners (DEST, INSTALL, LINK, REMOVE) were visible whenever the variables expand non-empty.

Quoted 7 lines

Guard the whole "ifndef V" block on "-s" so the loud variants are selected only when "-s" is absent and V=1 is unset. The existing "-s" check also had its findstring arguments in the wrong order (needle "-s" never fit in haystack "s"), so swap them while moving the check to wrap the block.

The new text looks good. However, the email addresses of author and signer-off are different. They should be the same. I notice that you use the gmail address in both places in other patch submissions, so I can use that if you agree (and you don't need to send another round).

-- Hannes

#7 lore raw

PATCH v4 1/1 git-gui: silence install recipes under "make -s"

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

Several install and uninstall recipes embed "echo" calls that fire as part of the recipe itself, so the install banners (DEST, INSTALL, LINK, REMOVE) were visible whenever the variables expand non-empty.

Guard the whole "ifndef V" block on "-s" so the loud variants are selected only when "-s" is absent and V=1 is unset. The existing "-s" check also had its findstring arguments in the wrong order (needle "-s" never fit in haystack "s"), so swap them while moving the check to wrap the block.

---
git-gui: silence install recipes under "make -s"

Change sign-off email from work email to correct personal email.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2318%2FHaraldNordgren%2Fgit-gui-respect-silent-flag-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2318/HaraldNordgren/git-gui-respect-silent-flag-v4
Pull-Request: https://github.com/git/git/pull/2318

Range-diff vs v3:

1: 1375fdc1aa ! 1: 27d9fcf26b git-gui: silence install recipes under "make -s"
@@ Commit message
(needle "-s" never fit in haystack "s"), so swap them while moving the check to wrap the block.

- Signed-off-by: Harald Nordgren <[email protected]>
+ Signed-off-by: Harald Nordgren <[email protected]>

## git-gui/Makefile ##
@@ git-gui/Makefile: REMOVE_F0 = $(RM_RF) # space is required here

 git-gui/Makefile | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
git-gui/Makefile +2 -4
index ca01068810..d33204e875 100644
@@ -64,6 +64,7 @@ REMOVE_F0  = $(RM_RF) # space is required here
 REMOVE_F1  =
 CLEAN_DST  = true
 
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s)
 ifndef V
 	QUIET          = @
 	QUIET_GEN      = $(QUIET)echo '   ' GEN '$@' &&
@@ -89,6 +90,7 @@ ifndef V
 	REMOVE_F0 = dst=
 	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
 endif
+endif
 
 TCLTK_PATH ?= wish
 ifeq (./,$(dir $(TCLTK_PATH)))
@@ -97,10 +99,6 @@ else
 	TCL_PATH ?= $(dir $(TCLTK_PATH))$(notdir $(subst wish,tclsh,$(TCLTK_PATH)))
 endif
 
-ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s)
-QUIET_GEN =
-endif
-
 -include config.mak
 
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))

base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0

Signature · 1 line
gitgitgadget
#9 lore raw
Quoted 5 lines · Johannes Sixt

Thanks, queued.

-- Hannes

Hi!

Thanks for the help!

What does it mean for it to be queued here, should I expect it to show up on seen or next?

Harald

#11 lore raw
Quoted 7 lines · Johannes Sixt

Am 10.06.26 um 15:19 schrieb Harald Nordgren:

Quoted 2 lines

What does it mean for it to be queued here, should I expect it to show up on seen or next?

It means that I'll arrange that it will appear in the next Git release. Until then you can find the commit in
https://github.com/j6t/git-gui/tree/hn/silence-make-s .

Thanks! So does that mean that 'seen' and 'next' are branches that are added to only by Junio Hamano?

Harald

#12 lore raw
Quoted 11 lines · Harald Nordgren
Quoted 7 lines · Johannes Sixt

Am 10.06.26 um 15:19 schrieb Harald Nordgren:

Quoted 2 lines

What does it mean for it to be queued here, should I expect it to show up on seen or next?

It means that I'll arrange that it will appear in the next Git release. Until then you can find the commit in
https://github.com/j6t/git-gui/tree/hn/silence-make-s .

Thanks! So does that mean that 'seen' and 'next' are branches that are added to only by Junio Hamano?

Yes. Because the git/git repository (on GitHub) is a mirror of his own gitster/git repo (for these branches but not topic branches).

When he is away and there is an interim maintainer, they may push to these branches on their own repos and use git/git as a mirror. So I guess they all have commit access to git/git.

That is my understanding of the matter and what I have observed.