Dollar ($) expansion still needs attention
erik.joelsson at oracle.com
Mon Mar 4 12:42:27 UTC 2013
This certainly needs to be looked at. Thanks for bringing it up. Your
assessment of the situation looks correct to me.
On 2013-03-04 05:59, David Holmes wrote:
> On 27/02/2013 10:14 PM, Jim Laskey (Oracle) wrote:
>> I wanted to double check and trace the origins of the MakeBase.gmk
>> patch before I responded. It was part of the original set Erik sent
>> me, but looking thru the other parts of the patch it's not clear why
>> it was necessary.
> So I got to the bottom of this. Basically if a make variable expands
> to contain a filename for a nested class (ie one with $ in it) then
> you need to escape the $ so that make doesn't treat it as a
> meta-character. And because $ is also the shell meta-character you end
> up needing a double escape of the form \$$$$, depending on how that
> variables gets used in targets/pre-reqs/recipes.
> Prior to the nashorn change ListPathsSafely didn't do any $ escaping
> and any explicit reference to a nested class file, such as:
> in the makefile, had to use the \$$$$. And everything worked fine.
> What nashorn introduced was .java files with $ in their name:
> > dir ../nashorn/src/jdk/nashorn/internal/scripts/
> total 16
> drwxr-xr-x 2 daholme uucp 4096 Feb 26 01:26 .
> drwxr-xr-x 8 daholme uucp 4096 Feb 26 01:26 ..
> -rw-r--r-- 1 daholme uucp 2027 Feb 26 01:26 JO$.java
> -rw-r--r-- 1 daholme uucp 1322 Feb 26 01:26 JS$.java
> so when ListPathsSafely was used together with a src directory to
> expand into a set of source files, there was no $ escaping and so the
> $ got dropped from the name, and so the nashorn build failed.
> So the escaping was added to ListPathsSafely. But not the full \$$$$
> as the need for that depends upon exactly how the make variable is
> used. For Java compilation commands ListPathSafely only had to do a
> simpler $ -> $$ escape sequence.
> But this means that anything explicitly escaped with \$$$$ and passed
> to ListPathsSafely was now broken as there were too many escapes. This
> is what broke the profiles builds. So to fix that we had to reduce the
> number of escapes in the explicitly named files ie:
> so that after processing by ListPathsSafely it was back to the full
> \$$$$ form.
> That would have been the end of it, except, as I pointed out, not all
> uses of nested class file names get passed through ListFilesSafely.
> Those that do not still need the \$$$$ escape, while those that do
> need only \$$. Hence we have the problem that when writing the name of
> such a file you have to know how it is going to be used - which is not
> a very maintainable solution.
> I propose that we regress the change to ListpathsSafely so that it
> doesn't do the $ escape, and instead we either:
> a) rename the nashorn .java files with $ in their name; or else
> b) add the $ escape at a different point in the src list expansion so
> that it only affects src list expansion
> Pragmatically I prefer (a) because having $ in filenames is really a
> PITA when it is both the make meta-character and the shell
> meta-character. It's too late for .class file names but if we can
> avoid adding it to .java file names then we will simplify our lives. :)
> Otherwise (b) should be done somewhere in the bowels of
> JavaCompilation.gmk, probably by adjusting $1_ALL_SRCS.
>> -- Jim
>> On 2013-02-27, at 6:15 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>> On 27/02/2013 02:47, David Holmes wrote:
>>>> So the same file names are listed once with \$$ and once with
>>>> \$$$$, and they both have to be that way to work!
>>>> This is untenable. There should only be one way to write the name
>>>> of a nested class file inside the makefile.
>>>> FYI in Profiles.gmk when expanding foo/*.class I already had to do
>>>> a similar substitution as is now in ListPathsSafely:
>>>> # Function to expand foo/*.class into the set of classes
>>>> # NOTE: Classfiles with $ in their name are problematic as that is the
>>>> # meta-character for both make and the shell! Hence the \$$$$
>>>> # But note that if you echo these values they will NOT display as
>>>> class_list = $(patsubst $(JDK_OUTPUTDIR)/classes/%,%,\
>>>> $(foreach i,$(1), $(subst $$,\$$$$, $(wildcard
>>>> So I'd like to understand why the nashorn change was made so that
>>>> we can determine how to get back to only having one way to specify
>>>> file names containing $
>>> I completely agree, it's very difficult to maintain. Also the two
>>> patches yesterday were just to get the build working again (Erik is
>>> out this week so it wasn't possible to establish why MakeBase.gmk
>>> was changed, also I cannot find the review thread to know if it came
>>> up during the review).
More information about the build-dev