add dev_compiler to DEPS

add dev_compiler to DEPS

R=paulberry@google.com, ricow@google.com

Committed: https://github.com/dart-lang/sdk/commit/70366fe64299bd49bb2f13097c92fca17e495ef8

[Jennifer Messerly, 2015-06-01 22:03:39]

jmesserly@google.com changed reviewers:
+ paulberry@google.com, ricow@google.com

[Jennifer Messerly, 2015-06-01 22:11:07]

Does this look right? How do I start the request for github mirror?

I tested locally gclient sync, not sure how to replicate the verification that
package bots do (e.g. that dependencies match up, or something like that?)

[Paul Berry, 2015-06-02 01:25:11]

I suspect that third_party/pkg_tested is better home for dev_compiler than
third_party/pkg.  Here's Bob's explanation of the difference between the two:
https://codereview.chromium.org/1100553003/#msg7

My understanding (Bob, please correctly me if I'm wrong) is that if we
eventually hope to ship dev_compiler as a plug-in to analyzer as part of the
SDK, then the version of dev_compiler that ships with the SDK will necessarily
have to link against whatever version of analyzer we ship with the SDK.  The way
to make sure that is going to work is to put DDC in third_party/pkg_tested, so
that as we develop, dev_compiler will be tested against the bleeding edge
analyzer, and therefore when we freeze the code for shipping, the dev_compiler
snapshot we ship will have been tested against the version of analyzer that we
ship.

I'm sorry to be the harbinger of dependency nightmares, but I manually checked
the dependencies (I don't know how to run the automated test that the bots run,
either) and found some problems:

- dev_compiler requires logging version >=0.9.3 <0.12.0, but the version
currently pulled in by DEPS is 0.9.2. 
- dev_compiler requires shelf version ^0.6.0, but the version currently pulled
in by DEPS is 0.5.6.
- dev_compiler requires shelf_static version ^0.2.1, but no version of
shelf_static is currently pulled in by DEPS.
- dev_compiler requires source_span version ^1.1.2, but the version currently
pulled in by DEPS is 1.0.2.

I think the upshot of this is that before you can land this CL, for each of
these four dependencies you'll either need to widen the constraint in
dev_compiler's pubspec.yaml, or modify DEPS to pull in a newer version of the
package.

[Paul Berry, 2015-06-02 02:05:11]

Whoops, forgot to CC Bob.  Bob, please see my previous message on this thread.

[ricow1, 2015-06-02 06:01:32]

+1 on all that paul said.

The actual pulling in lgtm

[Jennifer Messerly, 2015-06-02 16:23:58]

On 2015/06/02 01:25:11, Paul Berry wrote:
> I suspect that third_party/pkg_tested is better home for dev_compiler than
> third_party/pkg.  Here's Bob's explanation of the difference between the two:
> https://codereview.chromium.org/1100553003/#msg7
> 
> My understanding (Bob, please correctly me if I'm wrong) is that if we
> eventually hope to ship dev_compiler as a plug-in to analyzer as part of the
> SDK, then the version of dev_compiler that ships with the SDK will necessarily
> have to link against whatever version of analyzer we ship with the SDK.  The
way
> to make sure that is going to work is to put DDC in third_party/pkg_tested, so
> that as we develop, dev_compiler will be tested against the bleeding edge
> analyzer, and therefore when we freeze the code for shipping, the dev_compiler
> snapshot we ship will have been tested against the version of analyzer that we
> ship.

yeah, I'm super confused by this...

What would make sense in "third_party/pkg"? From your description it sounds like
nothing? In fact, almost everything is in "third_party/pkg".
"third_party/pkg_tested" is only: dart_style, analyzer_cli, and pub.

Notably, analyzer's other dependencies: args, path, plugin, watcher, and
unittest are all in "third_party/pkg" (typed_mock is an exception, its source is
in SDK repo "pkg" top level folder)

I assume what we want is package:analyzer and analyzer_cli to be tested, and
have package:dev_compiler just be a normal dependency of package:analyzer. Or
else, the other ones (args, path, plugin, watcher, and unittest) should be in
pkg_tested...?

> 
> I'm sorry to be the harbinger of dependency nightmares, but I manually checked
> the dependencies (I don't know how to run the automated test that the bots
run,
> either) and found some problems:
> 
> - dev_compiler requires logging version >=0.9.3 <0.12.0, but the version
> currently pulled in by DEPS is 0.9.2. 
> - dev_compiler requires shelf version ^0.6.0, but the version currently pulled
> in by DEPS is 0.5.6.
> - dev_compiler requires shelf_static version ^0.2.1, but no version of
> shelf_static is currently pulled in by DEPS.
> - dev_compiler requires source_span version ^1.1.2, but the version currently
> pulled in by DEPS is 1.0.2.

Thanks, that's helpful. Ironically I just updated all-the-things, thinking that
would help, but it appears the repo is pretty behind in some cases. Doh.

I wonder if we can automate this somewhow? Not that I mind using the Paul Berry
Dependency Analyzer ... the error messages are really great ... but not sure I
can afford the CPU hours :-)

> I think the upshot of this is that before you can land this CL, for each of
> these four dependencies you'll either need to widen the constraint in
> dev_compiler's pubspec.yaml, or modify DEPS to pull in a newer version of the
> package.

[Paul Berry, 2015-06-02 16:49:51]

On 2015/06/02 16:23:58, John Messerly wrote:
> On 2015/06/02 01:25:11, Paul Berry wrote:
> > I suspect that third_party/pkg_tested is better home for dev_compiler than
> > third_party/pkg.  Here's Bob's explanation of the difference between the
two:
> > https://codereview.chromium.org/1100553003/#msg7
> > 
> > My understanding (Bob, please correctly me if I'm wrong) is that if we
> > eventually hope to ship dev_compiler as a plug-in to analyzer as part of the
> > SDK, then the version of dev_compiler that ships with the SDK will
necessarily
> > have to link against whatever version of analyzer we ship with the SDK.  The
> way
> > to make sure that is going to work is to put DDC in third_party/pkg_tested,
so
> > that as we develop, dev_compiler will be tested against the bleeding edge
> > analyzer, and therefore when we freeze the code for shipping, the
dev_compiler
> > snapshot we ship will have been tested against the version of analyzer that
we
> > ship.
> 
> yeah, I'm super confused by this...
> 
> What would make sense in "third_party/pkg"? From your description it sounds
like
> nothing? In fact, almost everything is in "third_party/pkg".
> "third_party/pkg_tested" is only: dart_style, analyzer_cli, and pub.
> 
> Notably, analyzer's other dependencies: args, path, plugin, watcher, and
> unittest are all in "third_party/pkg" (typed_mock is an exception, its source
is
> in SDK repo "pkg" top level folder)

My understanding is that it's ok for these packages to be in third_party/pkg
because they don't depend on anything whose source lives in the SDK.  Hence,
when the package bots test them against the versions of their dependencies that
they specify in their pubspecs, they are being tested against the versions of
their dependencies that will eventually be shipped.  But since dev_compiler
depends on analyzer (which lives in the SDK), we need to test it against the
version of analyzer that lives in the SDK.

But I admit that I don't understand the grand plan very well.  Which is why I'm
trying to drag Bob into the conversation :)

> 
> I assume what we want is package:analyzer and analyzer_cli to be tested, and
> have package:dev_compiler just be a normal dependency of package:analyzer. Or
> else, the other ones (args, path, plugin, watcher, and unittest) should be in
> pkg_tested...?
> 
> > 
> > I'm sorry to be the harbinger of dependency nightmares, but I manually
checked
> > the dependencies (I don't know how to run the automated test that the bots
> run,
> > either) and found some problems:
> > 
> > - dev_compiler requires logging version >=0.9.3 <0.12.0, but the version
> > currently pulled in by DEPS is 0.9.2. 
> > - dev_compiler requires shelf version ^0.6.0, but the version currently
pulled
> > in by DEPS is 0.5.6.
> > - dev_compiler requires shelf_static version ^0.2.1, but no version of
> > shelf_static is currently pulled in by DEPS.
> > - dev_compiler requires source_span version ^1.1.2, but the version
currently
> > pulled in by DEPS is 1.0.2.
> 
> Thanks, that's helpful. Ironically I just updated all-the-things, thinking
that
> would help, but it appears the repo is pretty behind in some cases. Doh.
> 
> I wonder if we can automate this somewhow? Not that I mind using the Paul
Berry
> Dependency Analyzer ... the error messages are really great ... but not sure I
> can afford the CPU hours :-)

Yeah, also the Paul Berry Dependency Analyzer tends to overheat if it's used too
frequently :)

> 
> > I think the upshot of this is that before you can land this CL, for each of
> > these four dependencies you'll either need to widen the constraint in
> > dev_compiler's pubspec.yaml, or modify DEPS to pull in a newer version of
the
> > package.

[Jennifer Messerly, 2015-06-04 22:30:04]

Update, realized I was attacking this from the wrong direction ... analyzer_cli
is the thing that needs to have the dev-compiler dependency & flag. Presumably
we'll want at least a few tests for that flag too, in analyzer_cli, so we have
the end-to-end. Since analyzer_cli is in pkg_tested, those tests would give us
some basic coverage w.r.t. dev_compiler against bleeding edge analyzer.

However, we may also want to run some of dev_compiler's tests, e.g. the
checker/inference tests, and for that pkg_tested would be good.

I'll have to play around with the pkg_tested.status and see if I can come up
with a filter that will narrow what's tested to only the relevant bits.


Anyway, until the CL for analyzer_cli is ready, there's no real point in adding
the DEPs entry here, since nothing would use it. So I'm going to start fixing
the DDC code so it will work from analyzer_cli (e.g. making sure errors pipe
through correctly, etc).

[Jennifer Messerly, 2015-06-11 21:16:48]

PTAL. Changes since last rev:

* dev_compiler constraints changed to match SDK versions,
* dev_compiler 0.1.1 published to Pub and tagged,
* adds shelf_static,
* things seem to be moving to third_party/pkg, including analyzer_cli and pub
(?), so I left dev_compiler there.

[Paul Berry, 2015-06-11 23:23:43]

I'm going to trust you on the dependency checking this time.  You'll get a
buildbot failure if you got it wrong :)

As to the question of whether it belongs in pkg or pkg_tested, note that Phil
moved analyzer_cli from pkg_tested to pkg as a temporary workaround for a
dependency problem he was having.  I think it's his intention to move it back to
pkg_tested once that problem is sorted out.  Adding him to the CC in case he can
provide some insight.

I still *think* pkg_tested is the right place for dev_compiler but I'm really
murky on the distinction, and both Rico and Bob are already on this thread so
I'll defer to their opinions.  Assuming this question gets answered, lgtm.

[Jennifer Messerly, 2015-06-16 17:07:11]

Committed patchset #4 (id:60001) manually as
70366fe64299bd49bb2f13097c92fca17e495ef8 (presubmit successful).

[Jennifer Messerly, 2015-06-16 17:56:31]

have y'all ever seen a failure like:

=== BUILD AGGREGATE TARGET pkg_files_stamp OF PROJECT pkg_files WITH
CONFIGURATION ReleaseIA32 ===


** BUILD FAILED **

BUILD FAILED
Running
/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/python
./tools/build.py --mode=release --arch=ia32,x64 create_sdk

from
http://build.chromium.org/p/client.dart/builders/dart-sdk-mac-be/builds/688/s...

... when I run that Python command locally on a mac, I don't see any problems,
even clearing "xcodebuild" and "gclient sync -f" first

pkg_files_stamp doesn't even seem to be doing anything interesting, except
enumerating files and getting their timestamps. If it was Windows, I'd think
maybe too long of a path name somewhere in there. Total shot in the dark is
maybe this file:
https://github.com/dart-lang/dev_compiler/blob/master/test/codegen/8invalid-c...
... but even so, I'd expect a failure from running the same build.py command
locally.

[Jennifer Messerly, 2015-06-16 18:23:58]

On 2015/06/16 17:56:31, John Messerly wrote:
> have y'all ever seen a failure like:
> 
> === BUILD AGGREGATE TARGET pkg_files_stamp OF PROJECT pkg_files WITH
> CONFIGURATION ReleaseIA32 ===
> 
> 
> ** BUILD FAILED **
> 
> BUILD FAILED
> Running
>
/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/python
> ./tools/build.py --mode=release --arch=ia32,x64 create_sdk
> 
> from
>
http://build.chromium.org/p/client.dart/builders/dart-sdk-mac-be/builds/688/s...
> 
> ... when I run that Python command locally on a mac, I don't see any problems,
> even clearing "xcodebuild" and "gclient sync -f" first
> 
> pkg_files_stamp doesn't even seem to be doing anything interesting, except
> enumerating files and getting their timestamps. If it was Windows, I'd think
> maybe too long of a path name somewhere in there. Total shot in the dark is
> maybe this file:
>
https://github.com/dart-lang/dev_compiler/blob/master/test/codegen/8invalid-c...
> ... but even so, I'd expect a failure from running the same build.py command
> locally.

It does seem all the breaks caused by my change were on Mac builders too. I'm on
10.10.3 though, so maybe too new of an OS X version to reproduce?