RFR 8210318: idea.sh script doesn't work on Mac
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
The change looks fine to me Maurizio. Maybe you could append a .$$ to the temporary file name to make it less likely to overwrite something that already exists in /tmp - Michael. On 03/09/2018, 13: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
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
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
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
Ah - thanks for the tip, I'll give that a try 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
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
On 2018-09-05 13:42, Maurizio Cimadamore wrote:
Here's the modified webrev - as suggested, I replaced /tmp/replacement with $1.tmp
Looks good to me. /Magnus
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
Looks good, thanks! /Erik On 2018-09-05 04:42, Maurizio Cimadamore wrote:
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
participants (4)
-
Erik Joelsson
-
Magnus Ihse Bursie
-
Maurizio Cimadamore
-
Michael McMahon