Bug 64327

Summary: [patch] make(1): document surprising behaviour of assign with expansion
Product: Base System Reporter: Oliver Eikemeier <eikemeier>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Open ---    
Severity: Affects Only Me Keywords: patch
Priority: Normal    
Version: 4.9-STABLE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description Oliver Eikemeier 2004-03-16 11:50:11 UTC
Document a surprising behaviour of make(1)s assign with expansion
operator, that is apparently inteded but had bitten my at least once.

How-To-Repeat: 
Try the Makefile from the patch:

VAR1:=	Assigned with${OUT} expansion
VAR2!=	echo Assigned with${OUT} expansion
OUT=	out
all:
	@echo := - ${VAR1}
	@echo != - ${VAR2}
Comment 1 ru freebsd_committer freebsd_triage 2004-03-16 13:41:50 UTC
On Tue, Mar 16, 2004 at 12:49:27PM +0100, Oliver Eikemeier wrote:
> 
> Try the Makefile from the patch:
> 
> VAR1:=	Assigned with${OUT} expansion
> VAR2!=	echo Assigned with${OUT} expansion
> OUT=	out
> all:
> 	@echo := - ${VAR1}
> 	@echo != - ${VAR2}
> 
> >Fix:
> 
> diff -u -r1.29.2.15 make.1
> --- make.1	17 Dec 2002 19:01:18 -0000	1.29.2.15
> +++ make.1	16 Mar 2004 11:12:09 -0000
> @@ -1266,6 +1266,17 @@
>  \&.endfor
>  .Ed
>  won't work, and should be rewritten the other way around.
> +.Pp
> +Undefined variables are not expanded when assigned with expansion.
> +This is intentional, but may lead to surprising results:
> +.Bd -literal
> +VAR1:=  Assigned with${OUT} expansion
> +VAR2!=  echo Assigned with${OUT} expansion
> +OUT=    out
> +all:
> +    @echo := - ${VAR1}
> +    @echo != - ${VAR2}
> +.Ed
>  .Sh SEE ALSO
>  .Xr mkdep 1 ,
>  .Xr make.conf 5
> 
So, expanding "Assigned with${OUT} expansion" with := when OUT is undefined
gives you the same string (have a look at the ``make -r -dv'' output), and it
becomes a value of VAR1, and when later you print it, it's expanded again (as
it still has the `$' character).  This time OUT is defined, and its value is
substituted.

Expanding "echo Assigned with${OUT} expansion" through != gives you "Assigned
with expansion", and that's what gets printed later.

Whatever, "undefined variables are not expanded" sounds like a non-sense
to me.  ;)

The following code fragment in make(1) is responsible for this, and has more
correct wording:

:     } else if (type == VAR_SUBST) {
:         /*
:          * Allow variables in the old value to be undefined, but leave their
:          * invocation alone -- this is done by forcing oldVars to be false.
:          * XXX: This can cause recursive variables, but that's not hard to do,
:          * and this allows someone to do something like
:          *
:          *  CFLAGS = $(.INCLUDES)
:          *  CFLAGS := -I.. $(CFLAGS)
:          *
:          * And not get an error.
:          */
:         Boolean   oldOldVars = oldVars;
: 
:         oldVars = FALSE;

Perhaps you could convert it to fit the manpage?  Definitely we shouldn't put
it in the BUGS section.



Cheers,
-- 
Ruslan Ermilov
FreeBSD committer
ru@FreeBSD.org
Comment 2 Oliver Eikemeier 2004-03-16 13:58:35 UTC
Ruslan Ermilov wrote:

> On Tue, Mar 16, 2004 at 12:49:27PM +0100, Oliver Eikemeier wrote:
> 
>>Try the Makefile from the patch:
>>
>>VAR1:=	Assigned with${OUT} expansion
>>VAR2!=	echo Assigned with${OUT} expansion
>>OUT=	out
>>all:
>>	@echo := - ${VAR1}
>>	@echo != - ${VAR2}
>>
>>
>>>Fix:
>>
>>diff -u -r1.29.2.15 make.1
>>--- make.1	17 Dec 2002 19:01:18 -0000	1.29.2.15
>>+++ make.1	16 Mar 2004 11:12:09 -0000
>>@@ -1266,6 +1266,17 @@
>> \&.endfor
>> .Ed
>> won't work, and should be rewritten the other way around.
>>+.Pp
>>+Undefined variables are not expanded when assigned with expansion.
>>+This is intentional, but may lead to surprising results:
>>+.Bd -literal
>>+VAR1:=  Assigned with${OUT} expansion
>>+VAR2!=  echo Assigned with${OUT} expansion
>>+OUT=    out
>>+all:
>>+    @echo := - ${VAR1}
>>+    @echo != - ${VAR2}
>>+.Ed
>> .Sh SEE ALSO
>> .Xr mkdep 1 ,
>> .Xr make.conf 5
>>
> 
> So, expanding "Assigned with${OUT} expansion" with := when OUT is undefined
> gives you the same string (have a look at the ``make -r -dv'' output), and it
> becomes a value of VAR1, and when later you print it, it's expanded again (as
> it still has the `$' character).  This time OUT is defined, and its value is
> substituted.

Of course you could move `OUT=out' after `all:'. The point is that you'll
encounter problems with that pretty often in bsd.port.mk.

> Expanding "echo Assigned with${OUT} expansion" through != gives you "Assigned
> with expansion", and that's what gets printed later.

I guess I tried my own example before posting it...

> Whatever, "undefined variables are not expanded" sounds like a non-sense
> to me.  ;)

Basically that is what it does: it leaves undefined variables there for late
expansion. If you try:

SUFFIX?=	.txt
_FILE:=		${FILE}${SUFFIX}

.if exists(${_FILE})
	....
.endif

FILE=		settings
SUFFIX=		.opt

.if exists(${_FILE})
	....
.endif

You'll test for the existence of `.txt' in the first .if, and for `settings.txt'
in the second. It took me a while to figure *that* out.

> The following code fragment in make(1) is responsible for this, and has more
> correct wording:
> 
> :     } else if (type == VAR_SUBST) {
> :         /*
> :          * Allow variables in the old value to be undefined, but leave their
> :          * invocation alone -- this is done by forcing oldVars to be false.
> :          * XXX: This can cause recursive variables, but that's not hard to do,
> :          * and this allows someone to do something like
> :          *
> :          *  CFLAGS = $(.INCLUDES)
> :          *  CFLAGS := -I.. $(CFLAGS)
> :          *
> :          * And not get an error.
> :          */
> :         Boolean   oldOldVars = oldVars;
> : 
> :         oldVars = FALSE;

I know. Thats the reason I wrote: `This is intentional' and didn't file a bug report.

> Perhaps you could convert it to fit the manpage?  Definitely we shouldn't put
> it in the BUGS section.

I though so. What should I use: HISTORY, CAVEATS, IMPLEMENTATION NOTES or something else?

How about:

Undefined variables are left untouched when assigned with expansion.
This is intentional, but may lead to surprising results:

VAR1:=  Assigned with${OUT} expansion
all:
	@echo ${VAR1}

OUT=    out

Outputs `Assigned without expansion'. You can use != and echo to avoid this effect.
Comment 3 ru freebsd_committer freebsd_triage 2004-03-16 14:11:34 UTC
On Tue, Mar 16, 2004 at 02:58:35PM +0100, Oliver Eikemeier wrote:
[...]
> >Perhaps you could convert it to fit the manpage?  Definitely we shouldn't 
> >put
> >it in the BUGS section.
> 
> I though so. What should I use: HISTORY, CAVEATS, IMPLEMENTATION NOTES or 
> something else?
> 
I prefer to just document this behavior of the := assignment.

> Undefined variables are left untouched when assigned with expansion.
> 
Undefined variables can't be "left", as they don't exist at all.  Instead,
saying something along the code comments, like "expanded variables can be
undefined, in this case their expansion is attempted later", and give an
example from the code.


Cheers,
-- 
Ruslan Ermilov
FreeBSD committer
ru@FreeBSD.org
Comment 4 Oliver Eikemeier 2004-03-16 14:35:17 UTC
Ruslan Ermilov wrote:

> On Tue, Mar 16, 2004 at 02:58:35PM +0100, Oliver Eikemeier wrote:
> [...]
> 
>>>Perhaps you could convert it to fit the manpage?  Definitely we shouldn't 
>>>put
>>>it in the BUGS section.
>>
>>I though so. What should I use: HISTORY, CAVEATS, IMPLEMENTATION NOTES or 
>>something else?
> 
> I prefer to just document this behavior of the := assignment.

VARIABLE ASSIGNMENTS then. Perhaps after

  Variable substitution occurs at two distinct times, depending on where
  the variable is being used.  Variables in dependency lines are expanded
  as the line is read.  Variables in shell commands are expanded when the
  shell command is executed.

>>Undefined variables are left untouched when assigned with expansion.
> 
> Undefined variables can't be "left", as they don't exist at all.  Instead,
> saying something along the code comments, like "expanded variables can be
> undefined, in this case their expansion is attempted later", and give an
> example from the code.

I don't think this is an good example:

- I think it doesn't illustrate the pitfall clearly enough

- I do not really want to encourage anyone to use this feature. It looks
  like this feature is a little too tricky, and AFAIK other makes don't
  behave this way, so Makefiles get highly unportable and are difficult
  to follow for people used to other behaviour.
  While I know that this is BSD make and we have to keep this because of
  compatibility reasons, I'm not really sure of the value of this
  construct, the example should be rewritten
    CFLAGS = -I.. $(.INCLUDES)
  anyway.

Could you come up with another example? I'm sorry if I am too intractable,
but I more like to show the pitfalls (like the example in the BUGS section),
than to point out a value which I can't see.

Oliver
Comment 5 Oliver Eikemeier 2004-03-17 07:29:21 UTC
Looks like a follwup to PR bin/6830:
  http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/6830
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:59:40 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 7 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:38:40 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>