Make "--no-packages-dir" the default.

Make "--no-packages-dir" the default.

The actual change itself is tiny. However, almost all of pub's existing
integration tests validated the correct output by looking at the
contents of the generated "packages" dirs. :(

I fixed most of those to instead validate against the .packages file.

I didn't change the Git ones because the .packages files end up
containing paths that point to specific Git commits. Getting that
working felt like more work than it was worth right now.

When we fully remove support for packages directories, we'll want to
fix that.

Fix #1450.

R=nweiz@google.com

Committed: https://github.com/dart-lang/pub/commit/3dd04bd17ba269ccdd34502a253041dd96ded3be

[Bob Nystrom, 2016-09-28 18:05:48]

rnystrom@google.com changed reviewers:
+ nweiz@google.com

[nweiz, 2016-09-28 20:01:21]

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart
File test/descriptor.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart#newcod...
test/descriptor.dart:210: var copied = new Map<String,
String>.from(dependencies);
Are these type annotations required? I thought the .from() methods had been
updated to be typed.

https://codereview.chromium.org/2377063002/diff/1/test/descriptor/packages.dart
File test/descriptor/packages.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/descriptor/packages.da...
test/descriptor/packages.dart:39: new Version.parse(version);
Consider factoring this try/catch into an _isSemver method.

https://codereview.chromium.org/2377063002/diff/1/test/downgrade/doesnt_chang...
File test/downgrade/doesnt_change_git_dependencies_test.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/downgrade/doesnt_chang...
test/downgrade/doesnt_change_git_dependencies_test.dart:21: pubGet(args:
["--packages-dir"]);
Why not package root here? Same question for the other git tests.

[Bob Nystrom, 2016-09-29 00:21:26]

Added all the TODOs.

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart
File test/descriptor.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart#newcod...
test/descriptor.dart:210: var copied = new Map<String,
String>.from(dependencies);
On 2016/09/28 20:01:21, nweiz wrote:
> Are these type annotations required? I thought the .from() methods had been
> updated to be typed.

Doesn't seem like it. Would you rather I leave it out and assume it will catch
up at some point?

https://codereview.chromium.org/2377063002/diff/1/test/descriptor/packages.dart
File test/descriptor/packages.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/descriptor/packages.da...
test/descriptor/packages.dart:39: new Version.parse(version);
On 2016/09/28 20:01:21, nweiz wrote:
> Consider factoring this try/catch into an _isSemver method.

Done.

https://codereview.chromium.org/2377063002/diff/1/test/downgrade/doesnt_chang...
File test/downgrade/doesnt_change_git_dependencies_test.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/downgrade/doesnt_chang...
test/downgrade/doesnt_change_git_dependencies_test.dart:21: pubGet(args:
["--packages-dir"]);
On 2016/09/28 20:01:21, nweiz wrote:
> Why not package root here? Same question for the other git tests.

We don't have an easy way of testing that a path into the Git cache contains
what we want it to. We'll need to support that eventually when we totally remove
support for "packages" directories, but I didn't want to go through all of that
right now for this ostensibly tiny patch.

[nweiz, 2016-09-29 00:32:41]

lgtm

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart
File test/descriptor.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart#newcod...
test/descriptor.dart:210: var copied = new Map<String,
String>.from(dependencies);
On 2016/09/29 00:21:26, Bob Nystrom wrote:
> On 2016/09/28 20:01:21, nweiz wrote:
> > Are these type annotations required? I thought the .from() methods had been
> > updated to be typed.
> 
> Doesn't seem like it. Would you rather I leave it out and assume it will catch
> up at some point?

Yeah, that's what I do.

https://codereview.chromium.org/2377063002/diff/20001/test/descriptor/package...
File test/descriptor/packages.dart (right):

https://codereview.chromium.org/2377063002/diff/20001/test/descriptor/package...
test/descriptor/packages.dart:39: new Version.parse(version);
I don't think you want to parse here anymore.

[Bob Nystrom, 2016-09-29 00:56:41]

Thanks!

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart
File test/descriptor.dart (right):

https://codereview.chromium.org/2377063002/diff/1/test/descriptor.dart#newcod...
test/descriptor.dart:210: var copied = new Map<String,
String>.from(dependencies);
On 2016/09/29 00:32:41, nweiz wrote:
> On 2016/09/29 00:21:26, Bob Nystrom wrote:
> > On 2016/09/28 20:01:21, nweiz wrote:
> > > Are these type annotations required? I thought the .from() methods had
been
> > > updated to be typed.
> > 
> > Doesn't seem like it. Would you rather I leave it out and assume it will
catch
> > up at some point?
> 
> Yeah, that's what I do.

Done.

https://codereview.chromium.org/2377063002/diff/20001/test/descriptor/package...
File test/descriptor/packages.dart (right):

https://codereview.chromium.org/2377063002/diff/20001/test/descriptor/package...
test/descriptor/packages.dart:39: new Version.parse(version);
On 2016/09/29 00:32:41, nweiz wrote:
> I don't think you want to parse here anymore.

Oops, done.

[Bob Nystrom, 2016-09-29 00:57:54]

Description was changed from

==========
Make "--no-packages-dir" the default.

The actual change itself is tiny. However, almost all of pub's existing
integration tests validated the correct output by looking at the
contents of the generated "packages" dirs. :(

I fixed most of those to instead validate against the .packages file.

I didn't change the Git ones because the .packages files end up
containing paths that point to specific Git commits. Getting that
working felt like more work than it was worth right now.

When we fully remove support for packages directories, we'll want to
fix that.

Fix #1450.
==========

to

==========
Make "--no-packages-dir" the default.

The actual change itself is tiny. However, almost all of pub's existing
integration tests validated the correct output by looking at the
contents of the generated "packages" dirs. :(

I fixed most of those to instead validate against the .packages file.

I didn't change the Git ones because the .packages files end up
containing paths that point to specific Git commits. Getting that
working felt like more work than it was worth right now.

When we fully remove support for packages directories, we'll want to
fix that.

Fix #1450.

R=nweiz@google.com

Committed:
https://github.com/dart-lang/pub/commit/3dd04bd17ba269ccdd34502a253041dd96ded3be
==========

[Bob Nystrom, 2016-09-29 00:57:56]

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