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
