RFR: JDK-8189095; Import JMC from artifactory using Jib and main makefiles
erik.joelsson at oracle.com
Thu Oct 12 14:52:57 UTC 2017
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.)
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/
> 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.
>> 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/
More information about the build-dev