headers
Simon J. Gerraty | 1 Jun 2011 17:43

make: sysV subst with variables

A colleague came accross this.

While make handles variables on the rhs of a sysV style substituion,
our handling of recursive modifiers prevented handling variables
on the lhs.

For the test case below, we'd get:

fun
make: Unknown modifier '/'
{S}fun=fun}
make: Unknown modifier '/'
{S}%=%}
In the Sun

Index: var.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/var.c,v
retrieving revision 1.166
diff -u -p -r1.166 var.c
--- var.c	21 May 2011 07:30:42 -0000	1.166
+++ var.c	1 Jun 2011 15:42:24 -0000
 <at>  <at>  -2498,14 +2498,51  <at>  <at>  ApplyModifiers(char *nstr, const char *t

 	if (*tstr == '$') {
 	    /*
-	     * We have some complex modifiers in a variable.
+	     * We may have some complex modifiers in a variable.
 	     */
 	    void *freeIt;
(Continue reading)

Permalink | Reply | 7 comments |
headers
Simon Gerraty | 1 Jun 2011 18:51
Favicon

Re: Compiler selection support

I would have to agree that ?= should be better for setting CC etc.
Anything set by sys.mk can be overridden by the Makefile
having bsd.*.mk clobber things set by the Makefile is impolite.

Also the comment "# Duplicated to avoid make bugs"
which bugs would that be?
Permalink | Reply | 0 comments |
headers
David Holland | 1 Jun 2011 20:49
Picon

Re: make: sysV subst with variables

On Wed, Jun 01, 2011 at 08:43:34AM -0700, Simon J. Gerraty wrote:
 > While make handles variables on the rhs of a sysV style substituion,
 > our handling of recursive modifiers prevented handling variables
 > on the lhs.
 > [...]

What's an example of the recursive modifier behavior that conflicts
with handling those variables the way you want?

--

-- 
David A. Holland
dholland <at> netbsd.org
Permalink | Reply | 6 comments |
headers
Simon J. Gerraty | 1 Jun 2011 21:27

Re: make: sysV subst with variables


On Wed, 1 Jun 2011 18:49:00 +0000, David Holland writes:
>What's an example of the recursive modifier behavior that conflicts
>with handling those variables the way you want?

Sure - these are all from my sys.mk and get used heavily.

# some useful modifiers

# A useful trick for testing multiple :M's against something
# :L says to use the variable's name as its value - ie. literal
# got = ${clean* destroy:${M_ListToMatch:S,V,.TARGETS,}}
M_ListToMatch = L: <at> m <at> $${V:M$$m} <at> 
# match against our initial targets (see above)
M_L_TARGETS = ${M_ListToMatch:S,V,_TARGETS,}

# turn a list into a set of :N modifiers
# NskipFoo = ${Foo:${M_ListToSkip}}
M_ListToSkip= O:u:ts::S,:,:N,g:S,^,N,

# type should be a builtin in any sh since about 1980,
# AUTOCONF := ${autoconf:L:${M_whence}}
M_type =  <at> x <at> (type $$x 2> /dev/null); echo; <at> :sh:[0]:N* found*:[ <at> ]:C,[()],,g
M_whence = ${M_type}:M/*

# convert a path to a valid shell variable
M_P2V = tu:C,[./-],_,g

So take the example M_ListToSkip as an example:
(Continue reading)

Permalink | Reply | 5 comments |
headers
David Holland | 2 Jun 2011 19:51
Picon

Re: make: sysV subst with variables

On Wed, Jun 01, 2011 at 12:27:52PM -0700, Simon J. Gerraty wrote:
 > >What's an example of the recursive modifier behavior that conflicts
 > >with handling those variables the way you want?
 > 
 > Sure - these are all from my sys.mk and get used heavily.
 > [snip]

Ok, that makes my eyes burn but I think I understand. Symbolically the
variable expansion syntax is supposed

   ${VAR:modifiers}

where modifiers is arbitrary text possibly including variable
expansions, which has to be collected before it can be split into
individual modifiers and applied. The bug is that if it sees
${VAR:${...}} it assumes the internal variable reference is a complete
modifier (or a complete group of modifiers) and if this isn't true
because it's e.g. part of a substitution string, it belches.

 > So this patch adds a check that what Var_Parse() consumed is what we
 > expect in this case it would not be since '/' is not ':', '}' or ""
 > so it consumes up to one of those, with the result that  ApplyModifiers
 > is called with ${BAR}/%.x=% instead.

Wouldn't it be better to collect the complete modifier text up to the
terminating right-brace, do a full variable expansion on it, and only
then split it into individual modifiers and apply them? No, I guess
that breaks things like ${FOO:S/${OLDPATH}/${NEWPATH}/} where either
or both paths contains slashes.
(Continue reading)

Permalink | Reply | 4 comments |
headers
Simon J. Gerraty | 2 Jun 2011 21:29

Re: make: sysV subst with variables


On Thu, 2 Jun 2011 17:51:01 +0000, David Holland writes:
> > Sure - these are all from my sys.mk and get used heavily.
> > [snip]
>
>Ok, that makes my eyes burn but I think I understand. 

Which is exactly why you want to capture them in macros,
so that the usage is clear.

>individual modifiers and applied. The bug is that if it sees
>${VAR:${...}} it assumes the internal variable reference is a complete
>modifier (or a complete group of modifiers) and if this isn't true
>because it's e.g. part of a substitution string, it belches.

Yes.

> > So this patch adds a check that what Var_Parse() consumed is what we
> > expect in this case it would not be since '/' is not ':', '}' or ""
> > so it consumes up to one of those, with the result that  ApplyModifiers
> > is called with ${BAR}/%.x=% instead.
>
>Wouldn't it be better to collect the complete modifier text up to the
>terminating right-brace, do a full variable expansion on it, and only

Possibly - ie. always do the above?
The reason I chose to avoid that was just caution.
The existing code has been working well for many years.
My for loop is thus just dealing with a corner case in which we may be
about to fail anyway.
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon J. Gerraty | 2 Jun 2011 21:47

Re: make: sysV subst with variables


On Thu, 2 Jun 2011 17:51:01 +0000, David Holland writes:
>Or... hmm, maybe, since we already insist that a variable reference in
>the modifier position is a complete modifier, the right answer is to
>demand that it be followed by : or }, and if it isn't, treat it as
>plain text rather than a modifier block and therefore part of a svr4
>substitution. I believe this is grammatically sound, and it solves
>your problem. It does prohibit the case

I just took a *quick* look at this...
At the expense of a goto, its not too bad...

The important tests cases work and the corner cases you mention 
(which we don't need to support) fail as expected.

Index: var.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/var.c,v
retrieving revision 1.166
diff -u -p -r1.166 var.c
--- var.c	21 May 2011 07:30:42 -0000	1.166
+++ var.c	2 Jun 2011 19:45:06 -0000
 <at>  <at>  -2498,14 +2498,28  <at>  <at>  ApplyModifiers(char *nstr, const char *t

 	if (*tstr == '$') {
 	    /*
-	     * We have some complex modifiers in a variable.
+	     * We may have some complex modifiers in a variable.
 	     */
 	    void *freeIt;
(Continue reading)

Permalink | Reply | 0 comments |
headers
David Laight | 2 Jun 2011 22:31
Picon

Re: make: sysV subst with variables

On Thu, Jun 02, 2011 at 05:51:01PM +0000, David Holland wrote:
> It does prohibit the case
> 
>    MOD=tl:
>    ${FOO:${MOD}S/^/blah/}
> 
> from above, but I can't think of any particular reason we should cater
> to this when it's just as reasonable to write
> 
>    MOD=tl
>    ${FOO:${MOD}:S/^/blah/}

I can think that it might be useful to generate a list of modifiers
and then apply it - when the above translation can't be done.

    MOD=tl:
    MOD+=S/^/blah/
    ${FOO:${MOD}}

might be more reasonable - but probably suffers if 'blah' is actually a variable
that contains '/' since it probably expanded at the wrong time.

Or is the expansion of ${MOD} in my example one of the few places where make
doesn't do a recursive expansion?

	David

--

-- 
David Laight: david <at> l8s.co.uk
(Continue reading)

Permalink | Reply | 1 comment |
headers
Simon J. Gerraty | 2 Jun 2011 23:02

Re: make: sysV subst with variables


On Thu, 2 Jun 2011 21:31:46 +0100, David Laight writes:
>I can think that it might be useful to generate a list of modifiers
>and then apply it - when the above translation can't be done.
>
>    MOD=tl:
>    MOD+=S/^/blah/
>    ${FOO:${MOD}}
>
>might be more reasonable - but probably suffers if 'blah' is actually a variab
>le
>that contains '/' since it probably expanded at the wrong time.
>
>Or is the expansion of ${MOD} in my example one of the few places where make
>doesn't do a recursive expansion?

If you made it

MOD=tl
MOD+= S/^/blah/

${FOO:${MOD:ts:}}

it would work (with or without patch), but without the :ts: 
you'd get a space after tl: which would muck up the parsing.
Permalink | Reply | 0 comments |
headers
David Holland | 6 Jun 2011 02:58
Picon

bsd.lib.mk question

Does anyone know why bsd.lib.mk contains the following logic?

   _YLSRCS=        ${SRCS:M*.[ly]:C/\..$/.c/} ${YHEADER:D${SRCS:M*.y:.y=.h}}
      :
   DPSRCS+=        ${_YLSRCS}
      :
   ${STOBJS} ${POBJS} ${GOBJS} ${SOBJS} ${LOBJS}: ${DPSRCS}

It appears that this forces a complete recompile any time a .l or .y
file is touched, which seems entirely wrong.

--

-- 
David A. Holland
dholland <at> netbsd.org
Permalink | Reply | 31 comments |

Gmane