RFR: JDK-8189095; Import JMC from artifactory using Jib and main makefiles

[Erik Joelsson]

Hello,

http://cr.openjdk.java.net/~erikj/8189095/webrev.03/

Renamed TestCommon.gmk to UtilsForTests.gmk.

Renamed the macros to EncodeSpace and DecodeSpace.

Fixed a compile problem on Solaris. ($(@D) and $(dir $@) don't behave 
exactly alike. I believe the former does not leave a trailing space, so 
mixing them does not allow for string comparisons.)

/Erik


On 2017-10-12 16:23, Magnus Ihse Bursie wrote:
> On 2017-10-12 15:40, Erik Joelsson wrote:
>> Unfortunately, it didn't stay as easy as that. After hitting snag 
>> after snag, I finally decided to implement some kind of general 
>> support for file names with spaces in them, with support in CacheFind 
>> and SetupCopyFiles, as well as the various install-file variants. 
>> This got a little bit more messy than I would have liked, but at 
>> least I added a test for SetupCopyFiles to verify the functionality. 
>> Also note that this only works fully with gnu make 4.0 or later. With 
>> 3.81, I only get spaces to work in the leaf file and not in any 
>> directory. We certainly don't want to encourage anyone to use file 
>> names with spaces anywhere however, so even limited support is ok IMO.
>>
>> While working on the tests I found some problems with the current 
>> tests that I also fixed to get a clean baseline for these changes.
>>
>> Webrev: http://cr.openjdk.java.net/~erikj/8189095/webrev.02/
>
> Hm.
>
> It's not really pretty. (Apart from MacBundles.gmk, that one got 
> really cleaner.). It might be the best solution to a hairy problem, 
> though. :-&
>
> I'm not sure I'm so fond of the S2Q and Q2S names. I realize you 
> wanted something short, but it looks really cryptic. Spelling out 
> "SpaceToQuestionMark" is not the answer either. How about 
> "EncodeSpaces" and "DecodeSpaces", or something like that?
>
> It's very nice that you added the test (and fixed the test suite!). 
> Kudos! However, it took me some time to realize why you needed a 
> TestCommon.gmk when we already had a TestMakeBase.gmk. Maybe drop the 
> Test prefix from the utilities, more like CommonUtils.gmk? The fact 
> that it resides in test/make/ makes it clear that it is for testing, 
> but having no Test- prefix doesn't implicate that we're actually 
> *testing* something.
>
> /Magnus
>
>> /Erik
>>
>>
>> On 2017-10-11 18:44, Erik Joelsson wrote:
>>> Please review this small fix for the mac-bundles target. The current 
>>> solution does not handle files with spaces in them. By changing this 
>>> to a single rule with a recursive copy, any filenames that includes 
>>> spaces will be handled correctly.
>>>
>>> Webrev: http://cr.openjdk.java.net/~erikj/8189095/webrev.01/
>>>
>>> /Erik
>>>
>>
>