Start implementing pub object model. Rudimentary package cache class, and a simple pub command to l…

Start implementing pub object model. Rudimentary package cache class, and a simple pub command to list its contents.

Committed: https://code.google.com/p/dart/source/detail?r=6779

[Bob Nystrom, 2012-04-16 22:06:15]

It begins!

[Bob Nystrom, 2012-04-16 23:47:15]

Fixed the ill-conceived notion that packages should load their dependencies
before you can poke at them. Should be ready to review now. PTAL!

[nweiz, 2012-04-17 20:08:46]

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart
File utils/pub/cache.dart (right):

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:66: final pubspecPath = join(rootDir, name, 'pubspec');
Unused variable

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:69: _parsePubspec(name).then((dependencies) {
I think using transform here rather than manually instantiating a Completer
makes the async control flow clearer. It's also less code.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:95: readFuture.then((pubspec) {
I feel like it's a flaw in our Future API that you can't use transform here
because of the error-handling above. There should be a way of mixing transform
and handleException so you can do something like ".transformException(() =>
<String>[])".

The way it works in Closure is that onSuccess and onError both function like
#transform/#chain by default.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:99: .map((name) => name.trim())
I thought the style was putting the . on the previous line in the vain hope that
one day semicolons become optional.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/command_lis...
File utils/pub/command_list.dart (right):

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/command_lis...
utils/pub/command_list.dart:10: cache.listAll().then((packages) {
How consistent is the output ordering of listDir? If it's not consistent, it's
probably worth sorting here to maintain the printed ordering.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart
File utils/pub/io.dart (right):

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:26: if (piece == '..' && parts.length > 0 &&
Unless you have a good reason to try and clean up the paths, I'd just let the
underlying APIs sort out ".." and friends. It just adds needless complexity
here.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:58: completer.complete(text);
I don't understand your heuristic for when you use "() =>" and when not to.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:144: dir.onDone  = (done) {
nit: extra space after "Done".

What are the semantics of the "done" parameter?

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:158: if (basename(file) == '.DS_Store') return;
The documentation implies that this is going to filter out all hidden files, not
just a pre-defined blacklist.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:175: var exitCode;
Explicitly type this because it's not being initialized here

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:176: var error;
Unused variable

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:185: bool processDone = false;
var or final

[Bob Nystrom, 2012-04-18 18:09:18]

Thanks!

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart
File utils/pub/cache.dart (right):

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:66: final pubspecPath = join(rootDir, name, 'pubspec');
On 2012/04/17 20:08:46, nweiz wrote:
> Unused variable

Done.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:69: _parsePubspec(name).then((dependencies) {
On 2012/04/17 20:08:46, nweiz wrote:
> I think using transform here rather than manually instantiating a Completer
> makes the async control flow clearer. It's also less code.

Great idea. Still learning my way around transform() and chain().

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:95: readFuture.then((pubspec) {
On 2012/04/17 20:08:46, nweiz wrote:
> I feel like it's a flaw in our Future API that you can't use transform here
> because of the error-handling above. There should be a way of mixing transform
> and handleException so you can do something like ".transformException(() =>
> <String>[])".
> 
> The way it works in Closure is that onSuccess and onError both function like
> #transform/#chain by default.

Done.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/cache.dart#...
utils/pub/cache.dart:99: .map((name) => name.trim())
On 2012/04/17 20:08:46, nweiz wrote:
> I thought the style was putting the . on the previous line in the vain hope
that
> one day semicolons become optional.

Done.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/command_lis...
File utils/pub/command_list.dart (right):

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/command_lis...
utils/pub/command_list.dart:10: cache.listAll().then((packages) {
On 2012/04/17 20:08:46, nweiz wrote:
> How consistent is the output ordering of listDir? If it's not consistent, it's
> probably worth sorting here to maintain the printed ordering.

Added a TODO in listDir that it may need to sort. Right now, the results do seem
to be sorted and tests will fail if that changes, so I hate to add unneeded
code.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart
File utils/pub/io.dart (right):

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:26: if (piece == '..' && parts.length > 0 &&
On 2012/04/17 20:08:46, nweiz wrote:
> Unless you have a good reason to try and clean up the paths, I'd just let the
> underlying APIs sort out ".." and friends. It just adds needless complexity
> here.

This is mostly copied from file_system and I have run into cases where the
underlying API barfs if it gets a path that contains ../ for some reason. I
agree this is a little hairy, but I like that the hair is at least contained
here.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:58: completer.complete(text);
On 2012/04/17 20:08:46, nweiz wrote:
> I don't understand your heuristic for when you use "() =>" and when not to.

It was apparently based on astrology. Made less crazy.

I think the original process was:
1. Try to use () =>
2. But if there's a nearby lambda (like onError and onFile next to each other)
that can't (because it contains a statement) make them all use () {}.
Somehow this got added:
3. Roll 2d6. If < 8, ignore rule 2.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:144: dir.onDone  = (done) {
On 2012/04/17 20:08:46, nweiz wrote:
> nit: extra space after "Done".

Done.

> 
> What are the semantics of the "done" parameter?

I think onDone will always be called and it will be passed false if onError was
called beforehand? I'm not sure.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:158: if (basename(file) == '.DS_Store') return;
On 2012/04/17 20:08:46, nweiz wrote:
> The documentation implies that this is going to filter out all hidden files,
not
> just a pre-defined blacklist.

Clarified docs.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:175: var exitCode;
On 2012/04/17 20:08:46, nweiz wrote:
> Explicitly type this because it's not being initialized here

Done.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:176: var error;
On 2012/04/17 20:08:46, nweiz wrote:
> Unused variable

Done.

https://chromiumcodereview.appspot.com/10091014/diff/18/utils/pub/io.dart#new...
utils/pub/io.dart:185: bool processDone = false;
On 2012/04/17 20:08:46, nweiz wrote:
> var or final

Unused.

[nweiz, 2012-04-19 23:13:01]

lgtm