public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Makefules: fix MAKEFLAGS assignment for upcoming make-4.4
@ 2022-09-09 11:50 Sergei Trofimovich
  2022-09-12 20:27 ` Siddhesh Poyarekar
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Trofimovich @ 2022-09-09 11:50 UTC (permalink / raw)
  To: libc-alpha; +Cc: Sergei Trofimovich

make-4.4 will add long flags to MAKEFLAGS variable:

    * WARNING: Backward-incompatibility!
      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.

This causes locale builds to fail when long options are used:

    $ make --shuffle
    ...
    make  -C localedata install-locales
    make: invalid shuffle mode: '1662724426r'

The change fixes it by passing eash option via whitespace and dashes.
That way option is appended to both single-word form and whitespace
separated form.

Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
---
 Makerules | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makerules b/Makerules
index d1e139d03c..17c16fdada 100644
--- a/Makerules
+++ b/Makerules
@@ -794,7 +794,7 @@ endif
 # Maximize efficiency by minimizing the number of rules.
 .SUFFIXES:	# Clear the suffix list.  We don't use suffix rules.
 # Don't define any builtin rules.
-MAKEFLAGS := $(MAKEFLAGS)r
+MAKEFLAGS := $(MAKEFLAGS) -r
 
 # Generic rule for making directories.
 %/:
-- 
2.37.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] Makefules: fix MAKEFLAGS assignment for upcoming make-4.4
  2022-09-09 11:50 [PATCH] Makefules: fix MAKEFLAGS assignment for upcoming make-4.4 Sergei Trofimovich
@ 2022-09-12 20:27 ` Siddhesh Poyarekar
  2022-09-12 21:27   ` [PATCH v2] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564] Sergei Trofimovich
  0 siblings, 1 reply; 6+ messages in thread
From: Siddhesh Poyarekar @ 2022-09-12 20:27 UTC (permalink / raw)
  To: Sergei Trofimovich, libc-alpha



On 2022-09-09 07:50, Sergei Trofimovich via Libc-alpha wrote:
> make-4.4 will add long flags to MAKEFLAGS variable:
> 
>      * WARNING: Backward-incompatibility!
>        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.
> 
> This causes locale builds to fail when long options are used:
> 
>      $ make --shuffle
>      ...
>      make  -C localedata install-locales
>      make: invalid shuffle mode: '1662724426r'
> 
> The change fixes it by passing eash option via whitespace and dashes.
> That way option is appended to both single-word form and whitespace
> separated form.
> 
> Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> ---

Thanks, that's the right direction IMO but allowing word form flags will 
break this check too:

     ifeq    "$(findstring s,$(MAKEFLAGS))" ""       # if not -s

which is present in Makerules, Makeconfig and elf/rtld-Rules since the s 
will also match, e.g. --shuffle.  Those checks need to be fixed too.

Thanks,
Siddhesh

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564]
  2022-09-12 20:27 ` Siddhesh Poyarekar
@ 2022-09-12 21:27   ` Sergei Trofimovich
  2022-09-12 22:47     ` Paul Smith
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Trofimovich @ 2022-09-12 21:27 UTC (permalink / raw)
  To: libc-alpha; +Cc: Sergei Trofimovich, psmith, Siddhesh Poyarekar

make-4.4 will add long flags to MAKEFLAGS variable:

    * WARNING: Backward-incompatibility!
      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.

This causes locale builds to fail when long options are used:

    $ make --shuffle
    ...
    make  -C localedata install-locales
    make: invalid shuffle mode: '1662724426r'

The change fixes it by passing eash option via whitespace and dashes.
That way option is appended to both single-word form and whitespace
separated form.

While at it fixed --silent mode detection in $(MAKEFLAGS) by filtering
out --long-options. Otherwise options like --shuffle flag enable silent
mode unintentionally. $(silent-make) variable consolidates the checks.

CC: psmith@gnu.org
CC: Siddhesh Poyarekar <siddhesh@gotplt.org>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
---
Change since v1:
- Fixed $(MAKEFLAGS) introspection to avoid false positives on long
  options.
- Fixed 'Makefules' typo in the subject line.
 Makeconfig     | 16 +++++++++++++++-
 Makerules      |  4 ++--
 elf/rtld-Rules |  2 +-
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Makeconfig b/Makeconfig
index f8164a0025..b47dcea251 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -43,6 +43,20 @@ else
 $(error objdir must be defined by the build-directory Makefile)
 endif
 
+# Did we request 'make -s' run? "yes" or "no".
+# Starting from make-4.4 MAKEFLAGS now contains long
+# options like '--shuffle'. To detect presence of 's'
+# we filter out long options first and then check
+# for 's' in short options.
+# Typical MAKEFLAGS values to watch for:
+#   "rs --shuffle=42" (silent)
+#   " --shuffle" (not silent)
+ifeq ($(findstring s, $(filter-out --% , $(MAKEFLAGS))),)
+silent-make := no
+else
+silent-make := yes
+endif
+
 # Root of the sysdeps tree.
 sysdep_dir := $(..)sysdeps
 export sysdep_dir := $(sysdep_dir)
@@ -917,7 +931,7 @@ endif
 # umpteen zillion filenames along with it (we use `...' instead)
 # but we don't want this echoing done when the user has said
 # he doesn't want to see commands echoed by using -s.
-ifneq	"$(findstring s,$(MAKEFLAGS))" ""	# if -s
+ifeq ($(silent-make),yes)			# if -s
 +cmdecho	:= echo >/dev/null
 else						# not -s
 +cmdecho	:= echo
diff --git a/Makerules b/Makerules
index d1e139d03c..09c0cf8357 100644
--- a/Makerules
+++ b/Makerules
@@ -794,7 +794,7 @@ endif
 # Maximize efficiency by minimizing the number of rules.
 .SUFFIXES:	# Clear the suffix list.  We don't use suffix rules.
 # Don't define any builtin rules.
-MAKEFLAGS := $(MAKEFLAGS)r
+MAKEFLAGS := $(MAKEFLAGS) -r
 
 # Generic rule for making directories.
 %/:
@@ -811,7 +811,7 @@ MAKEFLAGS := $(MAKEFLAGS)r
 .PRECIOUS: $(foreach l,$(libtypes),$(patsubst %,$(common-objpfx)$l,c))
 \f
 # Use the verbose option of ar and tar when not running silently.
-ifeq	"$(findstring s,$(MAKEFLAGS))" ""	# if not -s
+ifeq ($(silent-make),no)			# if not -s
 verbose := v
 else	   					# -s
 verbose	:=
diff --git a/elf/rtld-Rules b/elf/rtld-Rules
index ca00dd1fe2..3c5e273f2b 100644
--- a/elf/rtld-Rules
+++ b/elf/rtld-Rules
@@ -52,7 +52,7 @@ $(objpfx)rtld-libc.a: $(foreach dir,$(rtld-subdirs),\
 	mv -f $@T $@
 
 # Use the verbose option of ar and tar when not running silently.
-ifeq	"$(findstring s,$(MAKEFLAGS))" ""	# if not -s
+ifeq ($(silent-make),no)			# if not -s
 verbose := v
 else						# -s
 verbose	:=
-- 
2.37.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564]
  2022-09-12 21:27   ` [PATCH v2] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564] Sergei Trofimovich
@ 2022-09-12 22:47     ` Paul Smith
  2022-09-12 23:07       ` [PATCH v3] " Sergei Trofimovich
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Smith @ 2022-09-12 22:47 UTC (permalink / raw)
  To: Sergei Trofimovich, libc-alpha; +Cc: Siddhesh Poyarekar

On Mon, 2022-09-12 at 22:27 +0100, Sergei Trofimovich wrote:
> +ifeq ($(findstring s, $(filter-out --% , $(MAKEFLAGS))),)

A more reliable, and guaranteed correct, way to do this is:

  ifeq ($(findstring s,$(firstword -$(MAKEFLAGS)),)

GNU make guarantees that all options which can possibly be represented
as single-letter options will always appear in the first word of
MAKEFLAGS (even if the user used the long option on make's command
line).

It also guarantees that there will be a space before any non-short
options, including if there are no short options given.  In that case
"-$(MAKEFLAGS)" would expand to "- --shuffle=random" (for example) and
firstword would give you just "-".

The filter-out is not as reliable, as it could be possible, someday, to
not filter out something and match incorrectly.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564]
  2022-09-12 22:47     ` Paul Smith
@ 2022-09-12 23:07       ` Sergei Trofimovich
  2022-09-13 14:21         ` Siddhesh Poyarekar
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Trofimovich @ 2022-09-12 23:07 UTC (permalink / raw)
  To: libc-alpha; +Cc: Sergei Trofimovich, Paul Smith, Siddhesh Poyarekar

make-4.4 will add long flags to MAKEFLAGS variable:

    * WARNING: Backward-incompatibility!
      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.

This causes locale builds to fail when long options are used:

    $ make --shuffle
    ...
    make  -C localedata install-locales
    make: invalid shuffle mode: '1662724426r'

The change fixes it by passing eash option via whitespace and dashes.
That way option is appended to both single-word form and whitespace
separated form.

While at it fixed --silent mode detection in $(MAKEFLAGS) by filtering
out --long-options. Otherwise options like --shuffle flag enable silent
mode unintentionally. $(silent-make) variable consolidates the checks.

CC: Paul Smith <psmith@gnu.org>
CC: Siddhesh Poyarekar <siddhesh@gotplt.org>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
---
Change since v2:
- Used more robust '$(firstword' for suggested by Paul Smith.
Change since v1:
- Fixed $(MAKEFLAGS) introspection to avoid false positives on long
  options.
- Fixed 'Makefules' typo in the subject line.
 Makeconfig     | 18 +++++++++++++++++-
 Makerules      |  4 ++--
 elf/rtld-Rules |  2 +-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/Makeconfig b/Makeconfig
index f8164a0025..842f49eb58 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -43,6 +43,22 @@ else
 $(error objdir must be defined by the build-directory Makefile)
 endif
 
+# Did we request 'make -s' run? "yes" or "no".
+# Starting from make-4.4 MAKEFLAGS now contains long
+# options like '--shuffle'. To detect presence of 's'
+# we pick first word with short options. Long options
+# are guaranteed to come after whitespace. We use '-'
+# prefix to always have a word before long options
+# even if no short options were passed.
+# Typical MAKEFLAGS values to watch for:
+#   "rs --shuffle=42" (silent)
+#   " --shuffle" (not silent)
+ifeq ($(findstring s, $(firstword -$(MAKEFLAGS))),)
+silent-make := no
+else
+silent-make := yes
+endif
+
 # Root of the sysdeps tree.
 sysdep_dir := $(..)sysdeps
 export sysdep_dir := $(sysdep_dir)
@@ -917,7 +933,7 @@ endif
 # umpteen zillion filenames along with it (we use `...' instead)
 # but we don't want this echoing done when the user has said
 # he doesn't want to see commands echoed by using -s.
-ifneq	"$(findstring s,$(MAKEFLAGS))" ""	# if -s
+ifeq ($(silent-make),yes)			# if -s
 +cmdecho	:= echo >/dev/null
 else						# not -s
 +cmdecho	:= echo
diff --git a/Makerules b/Makerules
index d1e139d03c..09c0cf8357 100644
--- a/Makerules
+++ b/Makerules
@@ -794,7 +794,7 @@ endif
 # Maximize efficiency by minimizing the number of rules.
 .SUFFIXES:	# Clear the suffix list.  We don't use suffix rules.
 # Don't define any builtin rules.
-MAKEFLAGS := $(MAKEFLAGS)r
+MAKEFLAGS := $(MAKEFLAGS) -r
 
 # Generic rule for making directories.
 %/:
@@ -811,7 +811,7 @@ MAKEFLAGS := $(MAKEFLAGS)r
 .PRECIOUS: $(foreach l,$(libtypes),$(patsubst %,$(common-objpfx)$l,c))
 \f
 # Use the verbose option of ar and tar when not running silently.
-ifeq	"$(findstring s,$(MAKEFLAGS))" ""	# if not -s
+ifeq ($(silent-make),no)			# if not -s
 verbose := v
 else	   					# -s
 verbose	:=
diff --git a/elf/rtld-Rules b/elf/rtld-Rules
index ca00dd1fe2..3c5e273f2b 100644
--- a/elf/rtld-Rules
+++ b/elf/rtld-Rules
@@ -52,7 +52,7 @@ $(objpfx)rtld-libc.a: $(foreach dir,$(rtld-subdirs),\
 	mv -f $@T $@
 
 # Use the verbose option of ar and tar when not running silently.
-ifeq	"$(findstring s,$(MAKEFLAGS))" ""	# if not -s
+ifeq ($(silent-make),no)			# if not -s
 verbose := v
 else						# -s
 verbose	:=
-- 
2.37.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564]
  2022-09-12 23:07       ` [PATCH v3] " Sergei Trofimovich
@ 2022-09-13 14:21         ` Siddhesh Poyarekar
  0 siblings, 0 replies; 6+ messages in thread
From: Siddhesh Poyarekar @ 2022-09-13 14:21 UTC (permalink / raw)
  To: Sergei Trofimovich, libc-alpha; +Cc: Paul Smith



On 2022-09-12 19:07, Sergei Trofimovich wrote:
> make-4.4 will add long flags to MAKEFLAGS variable:
> 
>      * WARNING: Backward-incompatibility!
>        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.
> 
> This causes locale builds to fail when long options are used:
> 
>      $ make --shuffle
>      ...
>      make  -C localedata install-locales
>      make: invalid shuffle mode: '1662724426r'
> 
> The change fixes it by passing eash option via whitespace and dashes.
> That way option is appended to both single-word form and whitespace
> separated form.
> 
> While at it fixed --silent mode detection in $(MAKEFLAGS) by filtering
> out --long-options. Otherwise options like --shuffle flag enable silent
> mode unintentionally. $(silent-make) variable consolidates the checks.
> 
> CC: Paul Smith <psmith@gnu.org>
> CC: Siddhesh Poyarekar <siddhesh@gotplt.org>
> Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> ---

LGTM.  I'll commit this in a while.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> Change since v2:
> - Used more robust '$(firstword' for suggested by Paul Smith.
> Change since v1:
> - Fixed $(MAKEFLAGS) introspection to avoid false positives on long
>    options.
> - Fixed 'Makefules' typo in the subject line.
>   Makeconfig     | 18 +++++++++++++++++-
>   Makerules      |  4 ++--
>   elf/rtld-Rules |  2 +-
>   3 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/Makeconfig b/Makeconfig
> index f8164a0025..842f49eb58 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -43,6 +43,22 @@ else
>   $(error objdir must be defined by the build-directory Makefile)
>   endif
>   
> +# Did we request 'make -s' run? "yes" or "no".
> +# Starting from make-4.4 MAKEFLAGS now contains long
> +# options like '--shuffle'. To detect presence of 's'
> +# we pick first word with short options. Long options
> +# are guaranteed to come after whitespace. We use '-'
> +# prefix to always have a word before long options
> +# even if no short options were passed.
> +# Typical MAKEFLAGS values to watch for:
> +#   "rs --shuffle=42" (silent)
> +#   " --shuffle" (not silent)
> +ifeq ($(findstring s, $(firstword -$(MAKEFLAGS))),)
> +silent-make := no
> +else
> +silent-make := yes
> +endif
> +
>   # Root of the sysdeps tree.
>   sysdep_dir := $(..)sysdeps
>   export sysdep_dir := $(sysdep_dir)
> @@ -917,7 +933,7 @@ endif
>   # umpteen zillion filenames along with it (we use `...' instead)
>   # but we don't want this echoing done when the user has said
>   # he doesn't want to see commands echoed by using -s.
> -ifneq	"$(findstring s,$(MAKEFLAGS))" ""	# if -s
> +ifeq ($(silent-make),yes)			# if -s
>   +cmdecho	:= echo >/dev/null
>   else						# not -s
>   +cmdecho	:= echo
> diff --git a/Makerules b/Makerules
> index d1e139d03c..09c0cf8357 100644
> --- a/Makerules
> +++ b/Makerules
> @@ -794,7 +794,7 @@ endif
>   # Maximize efficiency by minimizing the number of rules.
>   .SUFFIXES:	# Clear the suffix list.  We don't use suffix rules.
>   # Don't define any builtin rules.
> -MAKEFLAGS := $(MAKEFLAGS)r
> +MAKEFLAGS := $(MAKEFLAGS) -r
>   
>   # Generic rule for making directories.
>   %/:
> @@ -811,7 +811,7 @@ MAKEFLAGS := $(MAKEFLAGS)r
>   .PRECIOUS: $(foreach l,$(libtypes),$(patsubst %,$(common-objpfx)$l,c))
>   \f
>   # Use the verbose option of ar and tar when not running silently.
> -ifeq	"$(findstring s,$(MAKEFLAGS))" ""	# if not -s
> +ifeq ($(silent-make),no)			# if not -s
>   verbose := v
>   else	   					# -s
>   verbose	:=
> diff --git a/elf/rtld-Rules b/elf/rtld-Rules
> index ca00dd1fe2..3c5e273f2b 100644
> --- a/elf/rtld-Rules
> +++ b/elf/rtld-Rules
> @@ -52,7 +52,7 @@ $(objpfx)rtld-libc.a: $(foreach dir,$(rtld-subdirs),\
>   	mv -f $@T $@
>   
>   # Use the verbose option of ar and tar when not running silently.
> -ifeq	"$(findstring s,$(MAKEFLAGS))" ""	# if not -s
> +ifeq ($(silent-make),no)			# if not -s
>   verbose := v
>   else						# -s
>   verbose	:=

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-13 14:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 11:50 [PATCH] Makefules: fix MAKEFLAGS assignment for upcoming make-4.4 Sergei Trofimovich
2022-09-12 20:27 ` Siddhesh Poyarekar
2022-09-12 21:27   ` [PATCH v2] Makerules: fix MAKEFLAGS assignment for upcoming make-4.4 [BZ# 29564] Sergei Trofimovich
2022-09-12 22:47     ` Paul Smith
2022-09-12 23:07       ` [PATCH v3] " Sergei Trofimovich
2022-09-13 14:21         ` Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).