Here's the modified webrev - as suggested, I replaced /tmp/replacement
with $1.tmp
http://cr.openjdk.java.net/~mcimadamore/8210318_v2/
Thanks
Maurizio
On 04/09/18 18:50, Erik Joelsson wrote:
Hello,
$TARGET was just pseudo code. In your case it's $1.tmp.
/Erik
On 2018-09-04 10:34, Maurizio Cimadamore wrote:
Hi Erik,
would $TARGET be set by make?
Maurizio
On 04/09/18 16:55, Erik Joelsson wrote:
Hello,
When choosing a temp file in the build, we avoid using /tmp whenever
possible. A common pattern is instead to write to $TARGET.tmp and
then mv that to $TARGET. Though unlikely to cause an issue,
/tmp/replacement is a global location which is potentially risky
(file permissions, concurrent execution etc).
Otherwise looks good.
/Erik
On 2018-09-03 05:39, Maurizio Cimadamore wrote:
Hi,
following the latest updates to the idea.sh script, Mac users
reported issues - mostly having to do with usage of 'sed' - more
specifically:
* sed -i option is not portable - it has different formats in Mac
vs. Linux. This patch does without -i, by moving the replaced file
onto a temporary file, then moving such file on top of the template
file in a subsequent step. This should be more robust.
* sed doesn't like newlines in replaced text in Mac. I've thus
omitted the newline from the SOURCE template - as that was mostly
cosmetic.
Thanks for Michael McMahon to report (and figure out how to deal
with) these issues, and to Alan Bateman for testing the patch.
I also fixed another minor glitch, this time in the langtools-only
template - which was still referring to the old ant file location
in the various run configuration.
Webrev:
http://cr.openjdk.java.net/~mcimadamore/8210318/
Cheers
Maurizio
