RFR: JDK-8146002: Need to support mirrors for bootstrapping Jib (original) (raw)

Peter B. Kessler Peter.B.Kessler at Oracle.COM
Tue Dec 22 20:00:28 UTC 2015

I am not a reviewer, but ....

If you want the range of IFS to be short, how about

for mirror in <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>I</mi><mi>F</mi><mi>S</mi><mo>=</mo><mi mathvariant="normal">&quot;</mi><mo separator="true">,</mo><mi mathvariant="normal">&quot;</mi><mi>e</mi><mi>c</mi><mi>h</mi><mi>o</mi></mrow><annotation encoding="application/x-tex">(IFS=&quot; ,&quot; echo </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal" style="margin-right:0.05764em;">FS</span><span class="mspace" style="margin-right:0.2778em;"></span><span class="mrel">=</span><span class="mspace" style="margin-right:0.2778em;"></span></span><span class="base"><span class="strut" style="height:0.8889em;vertical-align:-0.1944em;"></span><span class="mord">&quot;</span><span class="mpunct">,</span><span class="mspace" style="margin-right:0.1667em;"></span><span class="mord">&quot;</span><span class="mord mathnormal">ec</span><span class="mord mathnormal">h</span><span class="mord mathnormal">o</span></span></span></span>{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 <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>e</mi><mi>c</mi><mi>h</mi><mi>o</mi></mrow><annotation encoding="application/x-tex">(echo </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal">ec</span><span class="mord mathnormal">h</span><span class="mord mathnormal">o</span></span></span></span>{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 jibmirrors 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

More information about the build-dev mailing list