Re: About the last patch for private variables

[Paul Smith]

On Thu, 2009-04-09 at 18:00 +0200, Ramón García wrote:
> I have sent a patch, which I believe to be complete, for the
> implementation of private variables. I have not received any news
> about whether it was committed or not.

Hi Ramon.  Over the long weekend I found some time to work on this, and
was able to get it committed.  Please try it out and make sure I didn't
break anything: I had to make a few changes as the patch did more than
just implement this (there were extra changes like removing the page
feeds (^L) etc.)

Also, the code to allow multiple modifiers (the while loop) caused some
problems, and using a modifier like "private" means that variable and
prerequisite names "private" didn't work properly which is not good
(backward-compatibility problems).  I noticed we had the same problem
previously with other modifiers like "export" and "override", and also
some limitations (you couldn't use both "export" and "override" on the
same variable assignment).  Also there was some code duplication in the
parser.

As a result it seem like time to fix the variable assignment parser to
resolve these issues, so that's what took the extra time.  You can now
say things like:

        export override foo = bar
        
        export = foo
        
        private override export foo = bar
        
        override export define foo
        endef

etc.  Note this also extends the capability of "private" to apply to
globally-scoped variables as well.  What does this mean?  It means that
global variables marked private are available in other global contexts
but NOT available inside a target context (when expanding a recipe).  I
don't know if this is very useful but it's consistent which is good.

The changes required two small backward-compatibility issues: first,
using backslash to escape "=" in prerequisite lists doesn't work
anymore.  That was a bad decision on my part, since that syntax never
worked for normal variables.  If you want to create a target (or
variable) with an "=" in the name you have to hide it behind a variable:
        E = =
        foo: bar$(E)baz

That always worked.  Second, you can no longer create a variable name
with whitespace in it just by declaring it like this:
        foo bar = baz
This is now a syntax error.  You can do it with a variable, if you must,
although in the future we may disallow variable names containing
whitespace altogether.

Also everyone please test this and make sure I didn't break anything.
Also I think that the new algorithm is not significantly slower than the
previous one but it's probably worth testing on any massive makefiles
you happen to have lying around (esp. on makefiles with lots of
variables assignments).

This _almost_ allows for target-specific define/endef variables.  In
order to make this work there are a few things that need to be fixed:
first, the do_define() and record_target_var() functions need to be
brought into sync so the former can be called in the context of the
latter (or whatever).  This is not hard.  But the big problem is the
second one: handling ignored target-specific defines in a conditional.

We have to know when we're in a define even when we're ignoring content,
because otherwise things like this break:

        ifeq (1,0)
          define foo
            ifeq (1,1)
              $(info here)
            endif
          endef
        endif

If we don't parse enough of the ignored content of the if-condition to
realize that we're inside a define/endef, then the parser chooses the
FIRST endif as the end of the condition.  There is code currently to
handle this in the "normal" variable assignment parser; even in ignored
sections we parse enough to know if it's a define and if so we set a
flag in_ignored_define so we ignore EVERYTHING until we see an endef.

We would need to handle this somehow for target-specific defines to
work; however, this is much harder since today if we're ignoring we skip
to the next line long before we realize this is a target that might have
a target-specific variable attached to it.

I think the answer here is two-fold: first we should change the regular
behavior so that rather than keeping in_ignored_define, we ALWAYS invoke
do_define() and just pass it an argument to say whether we're ignoring
or not.

Second we have to modify the way we skip lines in a false conditional so
we can recognize a target-specific define and handle it properly.

I'm not suggesting anyone needs to work on this right now; just thinking
out loud...

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <address@hidden>          Find some GNU make tips at:
 http://www.gnu.org                      http://make.mad-scientist.us
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist