GNU bug report logs - #60535
[PATCH] depend2: switch echo|sed to automatic vars

Previous Next

To reply to this bug, email your comments to 60535 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: automake-patches <at> gnu.org
Subject: [PATCH] depend2: switch echo|sed to automatic vars
Date: Tue,  3 Jan 2023 21:06:46 -0500

The echo|sed is used to split the dirname & filename so it can insert
$(DEPDIR) in the middle, and then chop the trailing object suffix.  In
the generic case, %OBJ% is $@, so we can leverage the POSIX vars $(@D)
and $(@F) to do the pathname splitting and insert $(DEPDIR) in between.
For chopping the object suffix, we can use the $(xxx:...=...) syntax.
This avoids invoking sed completely.

These dirname/filename vars have been in POSIX since at least 2004.
https://pubs.opengroup.org/onlinepubs/009695399/utilities/make.html
> Each of the internal macros has an alternative form. When an
> uppercase 'D' or 'F' is appended to any of the macros, the
> meaning shall be changed to the directory part for 'D' and
> filename part for 'F'. The directory part is the path prefix
> of the file without a trailing slash; for the current directory,
> the directory part is '.'.

* lib/am/depend2.am: Change echo|sed logic to make variables.
---
 lib/am/depend2.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/am/depend2.am b/lib/am/depend2.am
index 507fa40b54a0..87546f9d8274 100644
--- a/lib/am/depend2.am
+++ b/lib/am/depend2.am
@@ -42,7 +42,7 @@ if %FASTDEP%
 ?!GENERIC?	%SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
 ?GENERIC??!SUBDIROBJ?	%VERBOSE%%COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJ% %SOURCEFLAG%%SOURCE%
 ?GENERIC??!SUBDIROBJ?	%SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
-?GENERIC??SUBDIROBJ?	%VERBOSE%depbase=`echo %OBJ% | sed 's|[^/]*$$|$(DEPDIR)/&|;s|\.o$$||'`;\
+?GENERIC??SUBDIROBJ?	%VERBOSE%depbase='$(@D)/$(DEPDIR)/$(@F:.o=)';\
 ?GENERIC??SUBDIROBJ?	%COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJ% %SOURCEFLAG%%SOURCE% &&\
 ?GENERIC??SUBDIROBJ?	$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
 else !%FASTDEP%
@@ -69,7 +69,7 @@ if %FASTDEP%
 ?!GENERIC?	%SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
 ?GENERIC??!SUBDIROBJ?	%VERBOSE%%COMPILE% -MT %OBJOBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJOBJ% %SOURCEFLAG%`$(CYGPATH_W) '%SOURCE%'`
 ?GENERIC??!SUBDIROBJ?	%SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
-?GENERIC??SUBDIROBJ?	%VERBOSE%depbase=`echo %OBJ% | sed 's|[^/]*$$|$(DEPDIR)/&|;s|\.obj$$||'`;\
+?GENERIC??SUBDIROBJ?	%VERBOSE%depbase='$(@D)/$(DEPDIR)/$(@F:.obj=)';\
 ?GENERIC??SUBDIROBJ?	%COMPILE% -MT %OBJOBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJOBJ% %SOURCEFLAG%`$(CYGPATH_W) '%SOURCE%'` &&\
 ?GENERIC??SUBDIROBJ?	$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
 else !%FASTDEP%
@@ -98,7 +98,7 @@ if %FASTDEP%
 ?!GENERIC?	%SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Plo
 ?GENERIC??!SUBDIROBJ?	%VERBOSE%%LTCOMPILE% -MT %LTOBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %LTOBJ% %SOURCEFLAG%%SOURCE%
 ?GENERIC??!SUBDIROBJ?	%SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Plo
-?GENERIC??SUBDIROBJ?	%VERBOSE%depbase=`echo %OBJ% | sed 's|[^/]*$$|$(DEPDIR)/&|;s|\.lo$$||'`;\
+?GENERIC??SUBDIROBJ?	%VERBOSE%depbase='$(@D)/$(DEPDIR)/$(@F:.lo=)';\
 ?GENERIC??SUBDIROBJ?	%LTCOMPILE% -MT %LTOBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %LTOBJ% %SOURCEFLAG%%SOURCE% &&\
 ?GENERIC??SUBDIROBJ?	$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Plo
 else !%FASTDEP%
-- 
2.39.0

Message #8 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Nick Bowler <nbowler <at> draconx.ca>
To: Mike Frysinger <vapier <at> gentoo.org>
Cc: 60535 <at> debbugs.gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Wed, 4 Jan 2023 21:10:55 -0500

On 2023-01-03, Mike Frysinger <vapier <at> gentoo.org> wrote:
> The echo|sed is used to split the dirname & filename so it can insert
> $(DEPDIR) in the middle, and then chop the trailing object suffix.  In
> the generic case, %OBJ% is $@, so we can leverage the POSIX vars $(@D)
> and $(@F) to do the pathname splitting and insert $(DEPDIR) in between.
> For chopping the object suffix, we can use the $(xxx:...=...) syntax.
> This avoids invoking sed completely.

I think this is a generally good idea.

> These dirname/filename vars have been in POSIX since at least 2004.

Except for one minor detail, $(@F) and $(@D) are highly portable.  I
expect they were in the very first POSIX.2 specs as they predate the
earliest standards; I believe they first appeared in UNIX System V (ca.
1983) and were later added to BSD in 4.3BSD-Reno (ca. 1990).

The only potential problem I am aware of with these features is one
implementation (dmake) which does not follow the POSIX rule of excluding
the trailing slash from the directory part, and (a bit more troublesome)
will expand $(@D) to the empty string if the directory part is empty,
instead of . as required by POSIX.  Usually this is not a major issue,
as writing something like ./$(@D)/$(@F) is usually sufficient to avoid
any problems with empty $(@D).

It's worth noting that $(?F), $(?D), $(%F) and $(%D) were never added
to BSD and as a result are missing from many BSD derivatives.  However,
these variables are of limited utility and are unlikely to be missed.

The $(x:a=b) suffix substitutions are of identical vintage, and as
far as I know every make supporting file/directory variables also,
in principle, supports suffix substitution too.  However, there are
many bugs in real-world implementations of this feature.

Of particular relevance to this patch: older versions of NetBSD make,
while they do support suffix substitution, fail to correctly parse
$(@F:.o=):

  % cat >Makefile <<'EOF'
foo/bar.o:
	echo $(@F:.o=)
EOF
  % make
  echo bar.o.o=)
  Syntax error: ")" unexpected
  *** Error code 2

NetBSD make was widely adopted by other systems, so this particular
issue may actually be found elsewhere.  This bug was present as recently
as NetBSD 7.2 (ca. 2018).  Furthermore, old versions of FreeBSD (before
they ditched their own make in favour of NetBSD's version) also fail to
correctly handle $(@F:.o=), but in a different way.

The good news is that these problems can be worked around pretty easily
simply by using another variable:

  % cat >Makefile <<'EOF'
at_f = $(@F)
foo/bar.o:
	echo $(at_f:.o=)
EOF
  % make
  echo bar
  bar

But this workaround trips over a different suffix substitution bug on
ULTRIX, which would otherwise work just fine with plain $(@F:.o=).
Maybe ULTRIX is not worth worrying about these days, but nevertheless,
it is not too hard to detect and work around both problems in pure
shell, which should still work to avoid the additional forks/sed,
maybe something like:

  % cat >Makefile <<'EOF'
at_f = $(@F)
foo/bar.o:
	a='$(@F:.o=)' b='$(at_f:.o=)'; test x"$$a.o" = x"$(@F)" || a=$$b;\
	  echo $$a
EOF

Cheers,
  Nick

Message #11 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: Nick Bowler <nbowler <at> draconx.ca>
Cc: 60535 <at> debbugs.gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Wed, 4 Jan 2023 22:41:15 -0500

[Message part 1 (text/plain, inline)]

On 04 Jan 2023 21:10, Nick Bowler wrote:
> Except for one minor detail, $(@F) and $(@D) are highly portable.  I
> expect they were in the very first POSIX.2 specs as they predate the
> earliest standards; I believe they first appeared in UNIX System V (ca.
> 1983) and were later added to BSD in 4.3BSD-Reno (ca. 1990).
> 
> The only potential problem I am aware of with these features is one
> implementation (dmake) which does not follow the POSIX rule of excluding
> the trailing slash from the directory part, and (a bit more troublesome)
> will expand $(@D) to the empty string if the directory part is empty,
> instead of . as required by POSIX.  Usually this is not a major issue,
> as writing something like ./$(@D)/$(@F) is usually sufficient to avoid
> any problems with empty $(@D).

thanks ... i vaguely recall seeing a thread somewhere in the last year or
two talking about portable $(@D)/$(@F) issues somewhere which is how i
learned about these in the first place, but i can't seem to locate it, and
the syntax is not conducive to Google searches.  you don't happen to know
what i'm remembering ?  or have a link to some docs about how portable
these vars are ?  i thought it'd be useful to include them in our docs
and/or manual somewhere.

i can build & install dmake easily since someone is maintaining it in GH:
	https://github.com/jimjag/dmake
i'll throw some bugs at them :)

> It's worth noting that $(?F), $(?D), $(%F) and $(%D) were never added
> to BSD and as a result are missing from many BSD derivatives.  However,
> these variables are of limited utility and are unlikely to be missed.

that'll be good to note in our docs too

> The $(x:a=b) suffix substitutions are of identical vintage, and as
> far as I know every make supporting file/directory variables also,
> in principle, supports suffix substitution too.  However, there are
> many bugs in real-world implementations of this feature.
> 
> Of particular relevance to this patch: older versions of NetBSD make,
> while they do support suffix substitution, fail to correctly parse
> $(@F:.o=):
> 
>   % cat >Makefile <<'EOF'
> foo/bar.o:
> 	echo $(@F:.o=)
> EOF
>   % make
>   echo bar.o.o=)
>   Syntax error: ")" unexpected
>   *** Error code 2
> 
> NetBSD make was widely adopted by other systems, so this particular
> issue may actually be found elsewhere.  This bug was present as recently
> as NetBSD 7.2 (ca. 2018).  Furthermore, old versions of FreeBSD (before
> they ditched their own make in favour of NetBSD's version) also fail to
> correctly handle $(@F:.o=), but in a different way.
> 
> The good news is that these problems can be worked around pretty easily
> simply by using another variable:
> 
>   % cat >Makefile <<'EOF'
> at_f = $(@F)
> foo/bar.o:
> 	echo $(at_f:.o=)
> EOF
>   % make
>   echo bar
>   bar
> 
> But this workaround trips over a different suffix substitution bug on
> ULTRIX, which would otherwise work just fine with plain $(@F:.o=).
> Maybe ULTRIX is not worth worrying about these days, but nevertheless,
> it is not too hard to detect and work around both problems in pure
> shell, which should still work to avoid the additional forks/sed,
> maybe something like:
> 
>   % cat >Makefile <<'EOF'
> at_f = $(@F)
> foo/bar.o:
> 	a='$(@F:.o=)' b='$(at_f:.o=)'; test x"$$a.o" = x"$(@F)" || a=$$b;\
> 	  echo $$a
> EOF

this is interesting.  we actually have some uses of this already in
the tree.  no one seems to have complained though afaict.

automake-1.13+ has:
lib/am/subdirs.am:AM_RECURSIVE_TARGETS += $(am__recursive_targets:-recursive=)

automake-1.9+ has:
lib/am/texibuild.am:      for f in $@ $@-[0-9] $@-[0-9][0-9] $(@:.info=).i[0-9] $(@:.info=).i[0-9][0-9]; do \

dmake is one implementation that fails, and your suggestion doesn't work :/.
$ dmake foo/bar.o
dmake:  Error: -- Incomplete macro expression [)' b='$(at_f:.o=)'; test x"$$a.o" = x"$(@F)" || a=$$b;\
                echo $$a]

how portable is $() ?  that seems to work in GNU make & dmake.
$ cat Makefile
DEPDIR = ./$(@D)/depdir/$(@F:.o=$())
foo/bar.o:
	@echo $(DEPDIR)
$ dmake
./foo//depdir/bar
-mike

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Nick Bowler <nbowler <at> draconx.ca>
To: Mike Frysinger <vapier <at> gentoo.org>
Cc: 60535 <at> debbugs.gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Thu, 5 Jan 2023 01:02:01 -0500

On 2023-01-04, Mike Frysinger <vapier <at> gentoo.org> wrote:
> On 04 Jan 2023 21:10, Nick Bowler wrote:
[...]
>> maybe something like:
>>
>>   % cat >Makefile <<'EOF'
>> at_f = $(@F)
>> foo/bar.o:
>> 	a='$(@F:.o=)' b='$(at_f:.o=)'; test x"$$a.o" = x"$(@F)" || a=$$b;\
>> 	  echo $$a
>> EOF
>
> this is interesting.  we actually have some uses of this already in
> the tree.  no one seems to have complained though afaict.

Note that the NetBSD bug affects only substitutions on file/directory
variants like $(@F:a=b), there is no parse problem (that I know of)
with substitutions on the normal forms like $(@:a=b).

> automake-1.13+ has:
> lib/am/subdirs.am:AM_RECURSIVE_TARGETS +=
> $(am__recursive_targets:-recursive=)
>
> automake-1.9+ has:
> lib/am/texibuild.am:      for f in $@ $@-[0-9] $@-[0-9][0-9]
> $(@:.info=).i[0-9] $(@:.info=).i[0-9][0-9]; do \
>
> dmake is one implementation that fails, and your suggestion doesn't work
> :/.
> $ dmake foo/bar.o
> dmake:  Error: -- Incomplete macro expression [)' b='$(at_f:.o=)'; test
> x"$$a.o" = x"$(@F)" || a=$$b;\
>                 echo $$a]

Huh, I somehow have never noticed this problem with dmake.  So the
right-hand side cannot be empty on this one.

> how portable is $() ?  that seems to work in GNU make & dmake.

To expand to the empty string?  I expect that's probably fine, but not
sure about using a make variable on the RHS of a suffix substitution.
At least, this does not appear to work on ULTRIX (the substituted string
is output literally, not expanded at all).

I think my trick can be salvaged to work on all of these crazy
implementations by substituting no-op shell syntax instead:

  % cat >Makefile <<'EOF'
at_f = $(@F)
foo/bar.o:
	a='$(@F:.o='')' b='$(at_f:.o='')'; \
	  test x"$$a.o" = x"$(@F)" || a=$$b; \
	  echo $$a
EOF

Cheers,
  Nick

Message #17 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Nick Bowler <nbowler <at> draconx.ca>
To: Mike Frysinger <vapier <at> gentoo.org>
Cc: 60535 <at> debbugs.gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Thu, 5 Jan 2023 18:00:44 -0500

On 2023-01-04, Mike Frysinger <vapier <at> gentoo.org> wrote:
[...]
> dmake is one implementation that fails, and your suggestion doesn't work
> :/.
> $ dmake foo/bar.o
> dmake:  Error: -- Incomplete macro expression [)' b='$(at_f:.o=)'; test
> x"$$a.o" = x"$(@F)" || a=$$b;\
>                 echo $$a]

It might also be worth considering that all of these issues go away
if we can arrange the rules to use plain $(*F) instead of doing suffix
substitution on $(@F).

I think this code is used in both suffix and target rules, and $(*F) is
only valid in the former, but perhaps Automake can just substitute the
appropriate literal string into generated target rules?

I'm not familiar with how Automake actually uses this code to generate
rules, so maybe this is not a workable idea.

Cheers,
  Nick

Message #20 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: nbowler <at> draconx.ca
Cc: 60535 <at> debbugs.gnu.org, vapier <at> gentoo.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Thu, 5 Jan 2023 16:47:30 -0700

Please excuse my curmudgeonness, but it's not clear to me that avoiding
sed is worth these hassles in working around the implementation-specific
bugs in the automatic variables. Especially if we have to invoke a shell
and various commands anyway, how about keeping things as they are? -k

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Zack Weinberg" <zack <at> owlfolio.org>
To: automake-patches <at> gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Thu, 05 Jan 2023 23:56:11 -0500

On Thu, Jan 5, 2023, at 6:47 PM, Karl Berry wrote:
> Please excuse my curmudgeonness, but it's not clear to me that avoiding
> sed is worth these hassles in working around the implementation-specific
> bugs in the automatic variables.

It seems to me that this would be worth doing *if* we could get it to the
point where gmake (and any other implementation with similar optimizations)
didn't need to invoke a shell for the command at all.  I thought the rules
for that were documented but I can't find them at the moment, and I got lost
in gmake's job.c's maze of #ifdefs; anyone know what they are?

It might also be worth doing if we could guarantee that `sed` would not be
used by *any* rules, as that would be a solid step in the direction of
removing `sed` from the set of shell utilities potentially needed at build time
for a generic package (of course, if the package's own build logic uses sed,
there's nothing we can do about that).  ISTR there are several existing automake
rules with complex embedded shell scripts that use sed, though.

zw

Message #26 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: nbowler <at> draconx.ca, 60535 <at> debbugs.gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Wed, 11 Jan 2023 22:36:53 -0500

[Message part 1 (text/plain, inline)]

On 05 Jan 2023 16:47, Karl Berry wrote:
> Please excuse my curmudgeonness, but it's not clear to me that avoiding
> sed is worth these hassles in working around the implementation-specific
> bugs in the automatic variables. Especially if we have to invoke a shell
> and various commands anyway, how about keeping things as they are? -k

i don't quite buy the argument of "we've got a lot of per-object overhead,
so what's a little bit more".  we should be trying to minimize overhead of
generated code as much as possible.  forking a subshell to fork sed just to
munge a string that we can have make itself generate is pure overhead.

but you're appealing to avoiding hassle.  to that end, i'll point out the
current depdir logic is already in an unhealthy state: we're generating
similar logic 3 times with automake-processed conditionals depending on 3
runtime settings:
?!GENERIC?              %VERBOSE%%COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJ% %SOURCEFLAG%`test -f '%SOURCE%' || echo '$(srcdir)/'`%SOURCE%
?!GENERIC?              %SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
?GENERIC??!SUBDIROBJ?   %VERBOSE%%COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJ% %SOURCEFLAG%%SOURCE%
?GENERIC??!SUBDIROBJ?   %SILENT%$(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po
?GENERIC??SUBDIROBJ?    %VERBOSE%depbase=`echo %OBJ% | sed 's|[^/]*$$|$(DEPDIR)/&|;s|\.o$$||'`;\
?GENERIC??SUBDIROBJ?    %COMPILE% -MT %OBJ% -MD -MP -MF %DEPBASE%.Tpo %-c% -o %OBJ% %SOURCEFLAG%%SOURCE% &&\
?GENERIC??SUBDIROBJ?    $(am__mv) %DEPBASE%.Tpo %DEPBASE%.Po

this directly translates to overhead in the generated Makefile.in:
.c.o:
@am__fastdepCC_TRUE@                $(AM_V_CC)depbase=`echo $@ | sed 's|[^/]*$$|$(DEPDIR)/&|;s|\.o$$||'`;\
@am__fastdepCC_TRUE@                $(COMPILE) -MT $@ -MD -MP -MF $$depbase.Tpo -c -o $@ $< &&\
@am__fastdepCC_TRUE@                $(am__mv) $$depbase.Tpo $$depbase.Po
@AMDEP_TRUE@@am__fastdepCC_FALSE@   $(AM_V_CC)source='$<' object='$@' libtool=no @AMDEPBACKSLASH@
@AMDEP_TRUE@@am__fastdepCC_FALSE@   DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
@am__fastdepCC_FALSE@               $(AM_V_CC <at> am__nodep@)$(COMPILE) -c -o $@ $<

switching to make variables will allow us to collapse these.  i'm fairly
confident i can at least collapse the last 5 lines ?GENERIC? into 2 lines.
-mike

[signature.asc (application/pgp-signature, inline)]

Message #29 received at 60535 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: vapier <at> gentoo.org
Cc: nbowler <at> draconx.ca, 60535 <at> debbugs.gnu.org
Subject: Re: [bug#60535] [PATCH] depend2: switch echo|sed to automatic vars
Date: Thu, 12 Jan 2023 16:33:43 -0700

Well, stated more positively, my concern is about not replacing a bunch
of (granted) complicated old code with a bunch of complicated new code,
i.e., creating new-and-different maintainer and portability hassles, for
the sake of (what nowadays feels to me like) a micro-optimization. Let's
not underestimate the value of stable code that is known to work.

From Nick's and your previous discussion about the non-portability of
the make variables and the (what looks to me like) rather complicated
workarounds, I have my doubts that any replacement will work without a
few rounds of portability and bug fixes. With yet more complications
resulting.

I've never used those make variables myself, nor have I ever touched any
of this dependency code in Automake, and could certainly be overstating
the case. Nevertheless, that's my concern.

That said, if your replacement code passes all the tests on NetBSD,
dmake, whatever else is known to be problematic (I doubt we have to care
about Ultrix though), I guess it would be safe enough. --thanks, karl.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.