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

[Erik Joelsson]

On 2017-10-12 16:52, Erik Joelsson wrote:
> 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.)
>
I meant trailing slash!

/Erik
> /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
>>>>
>>>
>>
>