Observatory build should try backup method if the SDK doesn't work.

Observatory build should try backup method if the SDK doesn't work.
Fix up SDK for mips and arm processors on demand.

BUG=
R=iposva@google.com, johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/9aeb907f9d1c0bac2dbde13c1aa8380f685085e6

[Bill Hesse, 2015-09-15 13:41:03]

whesse@google.com changed reviewers:
+ johnmccutchan@google.com, ricow@google.com

[Bill Hesse, 2015-09-15 13:41:03]

The script run_pub.py is basically useless now, since it just runs a subprocess
with similar arguments.  I have a follow-up to inline it where it is used.

[Bill Hesse, 2015-09-15 13:41:49]

On 2015/09/15 13:41:03, Bill Hesse wrote:
> The script run_pub.py is basically useless now, since it just runs a
subprocess
> with similar arguments.  I have a follow-up to inline it where it is used.

We stop trying to use the binaries in tools/testing/bin, because they are too
old to run the new packages, and are being deleted.

[Cutch, 2015-09-15 16:45:03]

lgtm

[Ivan Posva, 2015-09-15 17:01:41]

iposva@google.com changed reviewers:
+ iposva@google.com

[Ivan Posva, 2015-09-15 17:01:42]

Does not LGTM as it breaks sharing of a checkout across multiple devices. And
potentially breaks the x86 based builds in unexpected ways.

-Ivan

https://codereview.chromium.org/1343103002/diff/1/tools/run_pub.py
File tools/run_pub.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/run_pub.py#newcode41
tools/run_pub.py:41:
https://github.com/dart-lang/sdk/wiki/The-checked-in-SDK-in--tools
Why the double -- between "in" and "tools"?

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode597
tools/utils.py:597: #    
https://github.com/dart-lang/sdk/wiki/The-checked-in-SDK-in--tools
ditto: --

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode621
tools/utils.py:621: def CheckedInSdkFixExecutable():
The documentation talks about a CheckedInSdkFixedExecutable, but here we are
missing the Fixed and instead have a Fix, which reads really strangely. Please
update to the documented name.

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode635
tools/utils.py:635: executable)
Here you are overwriting the ia32 executable which is really a bad thing to do
as you now have completely disabled running on a ia32 or x64 machine. Also it
breaks the "do-not-change" your source repository rule.

[Bill Hesse, 2015-09-15 17:09:02]

https://codereview.chromium.org/1343103002/diff/1/tools/run_pub.py
File tools/run_pub.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/run_pub.py#newcode41
tools/run_pub.py:41:
https://github.com/dart-lang/sdk/wiki/The-checked-in-SDK-in--tools
On 2015/09/15 17:01:42, Ivan Posva wrote:
> Why the double -- between "in" and "tools"?

Done.

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode597
tools/utils.py:597: #    
https://github.com/dart-lang/sdk/wiki/The-checked-in-SDK-in--tools
On 2015/09/15 17:01:42, Ivan Posva wrote:
> ditto: --

This is the URL of the page.  I tracked it down, and found an extra space in the
page name.  Fixed.

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode621
tools/utils.py:621: def CheckedInSdkFixExecutable():
On 2015/09/15 17:01:42, Ivan Posva wrote:
> The documentation talks about a CheckedInSdkFixedExecutable, but here we are
> missing the Fixed and instead have a Fix, which reads really strangely. Please
> update to the documented name.

I like Fix, because it is an active verb, and this function has an effect.  But
I will change to Fixed if others agree.

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode635
tools/utils.py:635: executable)
On 2015/09/15 17:01:42, Ivan Posva wrote:
> Here you are overwriting the ia32 executable which is really a bad thing to do
> as you now have completely disabled running on a ia32 or x64 machine. Also it
> breaks the "do-not-change" your source repository rule.

We are already changing the source repository by checking out this SDK into it
and unzipping it, at gclient runhooks.  If you want, I can put this change into
the gclient runhooks, and do it at that point.  There is no way to have a single
SDK checkout work for multiple architectures at once.  If this checkout is on an
ARM, MIPS, or other machine, the SDK on it should be the one with the proper
dart executable.  Even if I had separate SDKs up in the cloud, they would
download to the same place, so "dart" would be the one for the host.

[Bill Hesse, 2015-09-15 22:37:06]

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode635
tools/utils.py:635: executable)
On 2015/09/15 17:01:42, Ivan Posva wrote:
> Here you are overwriting the ia32 executable which is really a bad thing to do
> as you now have completely disabled running on a ia32 or x64 machine. Also it
> breaks the "do-not-change" your source repository rule.

For testing purposes, I added a dart-ia32 binary to the repository, and
supported copying that to "dart" when building on ia32 or x64.  This could be
checked in, and then building would function on whatever platform the build was
currently running on.

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode635
tools/utils.py:635: executable)
On 2015/09/15 17:01:42, Ivan Posva wrote:
> Here you are overwriting the ia32 executable which is really a bad thing to do
> as you now have completely disabled running on a ia32 or x64 machine. Also it
> breaks the "do-not-change" your source repository rule.

We can avoid doing this, if you absolutely don't want to change the "dart" file,
by running pub always with an explicitly given dart executable, and the snapshot
from the SDK, rather than from the runnable pub command.  We would still want to
test that this command works, and fall back to the bootstrap executable if pub
doesn't run this way.

[Ivan Posva, 2015-09-15 23:32:59]

Per offline discussion either of the two recoverable options is acceptable, with
the pick the right executable being preferred.

With those changes it LGTM.

Thanks,
-ivan

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode635
tools/utils.py:635: executable)
On 2015/09/15 22:37:05, Bill Hesse wrote:
> On 2015/09/15 17:01:42, Ivan Posva wrote:
> > Here you are overwriting the ia32 executable which is really a bad thing to
do
> > as you now have completely disabled running on a ia32 or x64 machine. Also
it
> > breaks the "do-not-change" your source repository rule.
> 
> We can avoid doing this, if you absolutely don't want to change the "dart"
file,
> by running pub always with an explicitly given dart executable, and the
snapshot
> from the SDK, rather than from the runnable pub command.  We would still want
to
> test that this command works, and fall back to the bootstrap executable if pub
> doesn't run this way.

Summarizing offline discussion here:
Specifying a dart executable is preferred over the copying the currently valid
executable over the dart binary. Thinking about it a bit further, we already
have a function which gives us the CheckedInSdkExecutable so the smarts of
figuring out the currect architecture can be added right there. No?

[Bill Hesse, 2015-09-16 11:13:37]

Committed patchset #3 (id:40001) manually as
9aeb907f9d1c0bac2dbde13c1aa8380f685085e6 (presubmit successful).

[Bill Hesse, 2015-09-21 10:05:06]

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode621
tools/utils.py:621: def CheckedInSdkFixExecutable():
On 2015/09/15 17:09:02, Bill Hesse wrote:
> On 2015/09/15 17:01:42, Ivan Posva wrote:
> > The documentation talks about a CheckedInSdkFixedExecutable, but here we are
> > missing the Fixed and instead have a Fix, which reads really strangely.
Please
> > update to the documented name.
> 
> I like Fix, because it is an active verb, and this function has an effect. 
But
> I will change to Fixed if others agree.

Changed to CheckedInSdkCheckExecutable, since it now only checks whether
CheckedInSdkExecutable works or not.

https://codereview.chromium.org/1343103002/diff/1/tools/utils.py#newcode635
tools/utils.py:635: executable)
On 2015/09/15 23:32:59, Ivan Posva wrote:
> On 2015/09/15 22:37:05, Bill Hesse wrote:
> > On 2015/09/15 17:01:42, Ivan Posva wrote:
> > > Here you are overwriting the ia32 executable which is really a bad thing
to
> do
> > > as you now have completely disabled running on a ia32 or x64 machine. Also
> it
> > > breaks the "do-not-change" your source repository rule.
> > 
> > We can avoid doing this, if you absolutely don't want to change the "dart"
> file,
> > by running pub always with an explicitly given dart executable, and the
> snapshot
> > from the SDK, rather than from the runnable pub command.  We would still
want
> to
> > test that this command works, and fall back to the bootstrap executable if
pub
> > doesn't run this way.
> 
> Summarizing offline discussion here:
> Specifying a dart executable is preferred over the copying the currently valid
> executable over the dart binary. Thinking about it a bit further, we already
> have a function which gives us the CheckedInSdkExecutable so the smarts of
> figuring out the currect architecture can be added right there. No?

Done.