RFR: JDK-8146002: Need to support mirrors for bootstrapping Jib

[Erik Joelsson]

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.joelsson at 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.joelsson at 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
>>>
>>