These are all good suggestions, but to not delay this further, I'm going
to go with what I have. Keep the range of IFS shorter isn't actually
necessary in this case.
/Erik
On 2015-12-22 21:00, Peter B. Kessler wrote:
I am not a reviewer, but ....
If you want the range of IFS to be short, how about
for mirror in $(IFS=" ," echo ${jib_server_mirrors}); do
assuming you really only want IFS set for the expansion of the values
for mirror. Then you don't have to remember to save and restore it,
either.
Or, if you find IFS confusing, how about
for mirror in $(echo ${jib_server_mirrors} | sed -e 's/,/ /g') ; do
to say that what you mean is that commas are treated as spaces in list
of mirrors. (Unless you find "sed" confusing, too. :-) You could
pull the sed command out to where you assign jib_server_mirrors so you
only have to do it once and can't forget to do it.
... peter
On 12/22/15 11:36 AM, Tim Bell wrote:
Erik:
Looks good to me as well.
/Tim
Right, new webrev: http://cr.openjdk.java.net/~erikj/8146002/webrev.03/
/Erik
On 2015-12-22 17:43, Magnus Ihse Bursie wrote:
Nice. Just one thing more: please save and restore the old value of
IFS. Things can behave strange when modifying IFS so it's good to
have it changed for as short a period as possible.
/Magnus
22 dec. 2015 kl. 15:33 skrev Erik Joelsson
<erik.joels...@oracle.com>:
Thanks for fast review!
Added IFS and verified that it accepts any combination of space
and comma as separator.
Webrev: http://cr.openjdk.java.net/~erikj/8146002/webrev.02/
/Erik
On 2015-12-22 15:23, Magnus Ihse Bursie wrote:
If you set IFS=' ,' before looping over the jib_mirrors variable,
you will allow a comma-separated list as well as a space
separated, which makes it possible to set using environment
variables in a reasonable way. I think. :) I always have to look
up the details on IFS.
Otherwise it's looking fine.
/Magnus
22 dec. 2015 kl. 15:13 skrev Erik Joelsson
<erik.joels...@oracle.com>:
Hello,
If we are to rely on Jib, the whole process needs redundancy in
download locations. For the main downloads, this is already in
place, but the weak link is the initial bootstrapping of the
tool itself. I propose to add support in common/bin/jib.sh for
checking a list of mirrors to download the install script from.
Bug: https://bugs.openjdk.java.net/browse/JDK-8146002
Webrev: http://cr.openjdk.java.net/~erikj/8146002/webrev.01/
/Erik
