Nashorn uses this mark to identify generated files. Erik may have assumed other languages might do the same and was planning ahead. If I had known I would have changed the marker. In Nashorn's case, since $ only affects class names, a) should be an easy change.
Sent from my iPhone 4 On 2013-03-04, at 12:59 AM, David Holmes <[email protected]> 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. >> >> http://cr.openjdk.java.net/~erikj/nashorn-build/webrev.01/ > > 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: > > sun/awt/motif/X11GB2312\$$$$Decoder.class > > 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: > > sun/awt/motif/X11GB2312\$$Decoder.class > > 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. > > > David > ----- > > > > >> -- Jim >> >> >> >> >> On 2013-02-27, at 6:15 AM, Alan Bateman <[email protected]> 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 \$$$$ substitution. >>>> # But note that if you echo these values they will NOT display as expected. >>>> class_list = $(patsubst $(JDK_OUTPUTDIR)/classes/%,%,\ >>>> $(foreach i,$(1), $(subst $$,\$$$$, $(wildcard >>>> $(JDK_OUTPUTDIR)/classes/$i)))) >>>> >>>> >>>> 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). >>> >>> -Alan >>
