| Message ID | patch-v3-1.1-432518b2dd7-20221130T081835Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 67b36879fc06581131fa7e57c9ee1e560ea9d1fc |
| Headers | show |
| Series | Makefiles: GNU make 4.4 fixes | expand |
On Wed, 2022-11-30 at 09:23 +0100, Ævar Arnfjörð Bjarmason wrote: > Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has > changed in a backward-incompatible way, as its "NEWS" file notes: Hrm. I did try to look through the other makefiles to find similar constructs and get them all, but apparently my grep fu was insufficient. Bother. Thanks.
Paul Smith <psmith@gnu.org> writes: > On Wed, 2022-11-30 at 09:23 +0100, Ævar Arnfjörð Bjarmason wrote: >> Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has >> changed in a backward-incompatible way, as its "NEWS" file notes: > > Hrm. I did try to look through the other makefiles to find similar > constructs and get them all, but apparently my grep fu was > insufficient. Bother. > > Thanks. Thanks, both. Will queue.
Hi Junio, On Thu, 1 Dec 2022, Junio C Hamano wrote: > Paul Smith <psmith@gnu.org> writes: > > > On Wed, 2022-11-30 at 09:23 +0100, Ævar Arnfjörð Bjarmason wrote: > >> Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has > >> changed in a backward-incompatible way, as its "NEWS" file notes: > > > > Hrm. I did try to look through the other makefiles to find similar > > constructs and get them all, but apparently my grep fu was > > insufficient. Bother. > > > > Thanks. > > Thanks, both. Will queue. I noticed that this patch also touches Git GUI, a change which technically should have come in via https://github.com/prati0100/git-gui, not directly via git/git. So let's make Pratyush [Cc:ed] aware of this change. We probably want to avoid applying Git GUI changes directly to git/git in the future. In the meantime, because I know that Pratyush is busy, I opened https://github.com/prati0100/git-gui/pull/83 with a (partial) backport of this patch. The following command demonstrates that this change is the only divergence that would need backporting into Git GUI (the first SHA is the current tip of git/git's `main` and the second SHA is the latest git-gui tip that was merged into git/git): git diff 2e71cbbddd6:git-gui df4f9e28f64: For the record, there is one change in git-gui's main branch that has not yet made it into git/git [*1*], but it merely appends a full stop character to the end of a sentence in the `README.md` file, therefore there is probably no urgency in pulling git-gui into git any time soon. That typo fix waited over a year to make it into git/git, it can easily wait some more. Ciao, Johannes Footnote *1*: https://github.com/prati0100/git-gui/commit/8cf36407cab
On Tue, Dec 06 2022, Johannes Schindelin wrote: > Hi Junio, > > On Thu, 1 Dec 2022, Junio C Hamano wrote: > >> Paul Smith <psmith@gnu.org> writes: >> >> > On Wed, 2022-11-30 at 09:23 +0100, Ævar Arnfjörð Bjarmason wrote: >> >> Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has >> >> changed in a backward-incompatible way, as its "NEWS" file notes: >> > >> > Hrm. I did try to look through the other makefiles to find similar >> > constructs and get them all, but apparently my grep fu was >> > insufficient. Bother. >> > >> > Thanks. >> >> Thanks, both. Will queue. > > I noticed that this patch also touches Git GUI, a change which technically > should have come in via https://github.com/prati0100/git-gui, not directly > via git/git. > > I noticed that this patch also touches Git GUI, a change which technically > should have come in via https://github.com/prati0100/git-gui, not directly > via git/git. > > So let's make Pratyush [Cc:ed] aware of this change. > > We probably want to avoid applying Git GUI changes directly to git/git in > the future. In the meantime, because I know that Pratyush is busy, I > opened https://github.com/prati0100/git-gui/pull/83 with a (partial) > backport of this patch. Should it? I looked at https://github.com/prati0100/git-gui#contributing before including git-gui in that change, which says: Even though the project is hosted at GitHub, the development does not happen over GitHub Issues and Pull Requests. Instead, an email based workflow is used. The Git mailing list [git@vger.kernel.org](mailto:git@vger.kernel.org) is where the patches are discussed and reviewed. As a bit of deja-vu when trying to find if that was outdated or not I found that you seemed to have had pretty much this exact exchange already with the git-gui maintainer at https://lore.kernel.org/git/20190924122306.bcwe37wlahjimve7@yadavpratyush.com/ Which seems to have been followed-up by https://lore.kernel.org/git/pull.361.git.gitgitgadget@gmail.com/; I.e. you sent a git-gui change to this ML. Or do you mean that it should have been sent to this ML, Pratyush should have pulled it, and Junio would have pulled upstream after that?
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Which seems to have been followed-up by > https://lore.kernel.org/git/pull.361.git.gitgitgadget@gmail.com/; > I.e. you sent a git-gui change to this ML. > > Or do you mean that it should have been sent to this ML, Pratyush should > have pulled it, and Junio would have pulled upstream after that? The destination of the e-mailed patch was fine. I think what Dscho is saying is that the patch for git-gui should have been split into its own patch that is rooted at that project, i.e. the "diff --git" line shouldn't have had "a/git-gui/Makefile" but just "a/Makefile" if the patch were to modify the top-level Makefile of that project. Then the git-gui maintainer picks up the patch (after possible review iterations), applies to his or her tree, and tells me to pull the result with "-Xsubtree=git-gui" option. At least that was how the world worked, when we had an active git-gui maintainer. The same story goes for gitk part of the tree. These days, neither subtree is very active and I am not sure how much value we are getting out of this "clean separation". Thanks.
diff --git a/git-gui/Makefile b/git-gui/Makefile index 56c85a85c1e..a0d5a4b28e1 100644 --- a/git-gui/Makefile +++ b/git-gui/Makefile @@ -116,7 +116,7 @@ ifeq ($(uname_S),Darwin) TKEXECUTABLE = $(shell basename "$(TKFRAMEWORK)" .app) endif -ifeq ($(findstring $(MAKEFLAGS),s),s) +ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s) QUIET_GEN = endif diff --git a/shared.mak b/shared.mak index be1f30ff206..aeb80fc4d5a 100644 --- a/shared.mak +++ b/shared.mak @@ -37,13 +37,13 @@ space := $(empty) $(empty) QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir QUIET_SUBDIR1 = -ifneq ($(findstring w,$(MAKEFLAGS)),w) +ifneq ($(findstring w,$(firstword -$(MAKEFLAGS))),w) PRINT_DIR = --no-print-directory else # "make -w" NO_SUBDIR = : endif -ifneq ($(findstring s,$(MAKEFLAGS)),s) +ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),s) ifndef V ## common QUIET_SUBDIR0 = +@subdir=
Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has changed in a backward-incompatible way, as its "NEWS" file notes: Previously only simple (one-letter) options were added to the MAKEFLAGS variable that was visible while parsing makefiles. Now, all options are available in MAKEFLAGS. If you want to check MAKEFLAGS for a one-letter option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return the set of one-letter options which can be examined via findstring, etc. This upstream change meant that e.g.: make man Would become very noisy, because in shared.mak we rely on extracting "s" from the $(MAKEFLAGS), which now contains long options like "--jobserver-auth=fifo:<path>", which we'll conflate with the "-s" option. So, let's change this idiom we've been carrying since [1], [2] and [3] as the "NEWS" suggests. Note that the "-" in "-$(MAKEFLAGS)" is critical here, as the variable will always contain leading whitespace if there are no short options, but long options are present. Without it e.g. "make --debug=all" would yield "--debug=all" as the first word, but with it we'll get "-" as intended. Then "-s" for "-s", "-Bs" for "-s -B" etc. 1. 0c3b4aac8ec (git-gui: Support of "make -s" in: do not output anything of the build itself, 2007-03-07) 2. b777434383b (Support of "make -s": do not output anything of the build itself, 2007-03-07) 3. bb2300976ba (Documentation/Makefile: make most operations "quiet", 2009-03-27) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-gui/Makefile | 2 +- shared.mak | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)