Add dartdoc to the generated utils snapshot.

Add dartdoc to the generated utils snapshot.

This is the same as r23575 (14135007), but fixed.

R=dgrove@google.com

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

[dgrove, 2013-06-04 22:51:41]

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
File sdk/lib/_internal/dartdoc/lib/dartdoc.dart (right):

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:802: String get tmpPath =>
pathos.join(normalizedDartdocPath, 'tmp');
How will this work in the SDK when dart-sdk/lib/_internal/dartdoc goes away
(which will happen right after this patch lands)?  I don't think we should be
writing files in the SDK at all.

[nweiz, 2013-06-04 22:54:00]

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
File sdk/lib/_internal/dartdoc/bin/dartdoc.dart (right):

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/bin/dartdoc.dart:39: mainWithOptions(Options options)
{
Why not just have the snapshot call main()? "new Options()" will still get the
right value.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/bin/dartdoc.dart:199: dartdoc.dartdocPath =
libPath.append('lib/_internal/dartdoc');
It's weird that this is separate from what's essentially the same operation
below.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/bin/dartdoc.dart:251: .then((_) =>
copyDirectory(libPath.append('lib/_internal/dartdoc/static'),
libPath and scriptPath seem very brittle. It's not clear from reading the code
where libPath will point, let alone that it will correctly point to that
location when run from the repo vs the sdk.

At the very least, scriptDir and libPath both need documentation that describes
their locations when run from the repo or from the sdk. If possible, use a
single variable that refers to a well-defined path from which the relative paths
work in both the SDK and the repo.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
File sdk/lib/_internal/dartdoc/lib/dartdoc.dart (right):

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:789: '.snapshot';
This overflow is kind of ugly. Just put the entire body on the second line.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:793: return
pathos.dirname(pathos.dirname(new Options().executable));
Style nit: use "=>"

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:797: /// places.
What is "the dartdoc directory"? The directory containing dartdoc's source? How
does that make sense in the SDK where it doesn't have any source files?

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:798: String get normalizedDartdocPath
=> runningFromSdk ?
Why aren't you using this in bin/dartdoc to get at the static directory?

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:802: String get tmpPath =>
pathos.join(normalizedDartdocPath, 'tmp');
Document this.

Also, why is your temp path in your source path/in the SDK? You should never be
writing to the SDK. Why aren't you creating a directory in /tmp? See withTempDir
in pub's io.dart.

[Andrei Mouravski, 2013-06-05 00:18:49]

This is just to check this in. I will fix this with a real temporary directory
tomorrow.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
File sdk/lib/_internal/dartdoc/bin/dartdoc.dart (right):

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/bin/dartdoc.dart:39: mainWithOptions(Options options)
{
On 2013/06/04 22:54:00, nweiz wrote:
> Why not just have the snapshot call main()? "new Options()" will still get the
> right value.

Because the snapshot actually calls this with different options.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/bin/dartdoc.dart:199: dartdoc.dartdocPath =
libPath.append('lib/_internal/dartdoc');
On 2013/06/04 22:54:00, nweiz wrote:
> It's weird that this is separate from what's essentially the same operation
> below.

Yes, but I don't want to try and unstuck this. That'll come in a new CL.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/bin/dartdoc.dart:251: .then((_) =>
copyDirectory(libPath.append('lib/_internal/dartdoc/static'),
On 2013/06/04 22:54:00, nweiz wrote:
> libPath and scriptPath seem very brittle. It's not clear from reading the code
> where libPath will point, let alone that it will correctly point to that
> location when run from the repo vs the sdk.
> 
> At the very least, scriptDir and libPath both need documentation that
describes
> their locations when run from the repo or from the sdk. If possible, use a
> single variable that refers to a well-defined path from which the relative
paths
> work in both the SDK and the repo.

I agree, and this will be fixed in a new CL.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
File sdk/lib/_internal/dartdoc/lib/dartdoc.dart (right):

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:789: '.snapshot';
On 2013/06/04 22:54:00, nweiz wrote:
> This overflow is kind of ugly. Just put the entire body on the second line.

Done.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:793: return
pathos.dirname(pathos.dirname(new Options().executable));
On 2013/06/04 22:54:00, nweiz wrote:
> Style nit: use "=>"

Done.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:797: /// places.
On 2013/06/04 22:54:00, nweiz wrote:
> What is "the dartdoc directory"? The directory containing dartdoc's source?
How
> does that make sense in the SDK where it doesn't have any source files?

Done.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:798: String get normalizedDartdocPath
=> runningFromSdk ?
On 2013/06/04 22:54:00, nweiz wrote:
> Why aren't you using this in bin/dartdoc to get at the static directory?

Done.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:802: String get tmpPath =>
pathos.join(normalizedDartdocPath, 'tmp');
On 2013/06/04 22:51:42, dgrove wrote:
> How will this work in the SDK when dart-sdk/lib/_internal/dartdoc goes away
> (which will happen right after this patch lands)?  I don't think we should be
> writing files in the SDK at all.

Will be addressed in next CL.

https://codereview.chromium.org/16364004/diff/2001/sdk/lib/_internal/dartdoc/...
sdk/lib/_internal/dartdoc/lib/dartdoc.dart:802: String get tmpPath =>
pathos.join(normalizedDartdocPath, 'tmp');
On 2013/06/04 22:54:00, nweiz wrote:
> Document this.
> 
> Also, why is your temp path in your source path/in the SDK? You should never
be
> writing to the SDK. Why aren't you creating a directory in /tmp? See
withTempDir
> in pub's io.dart.

Done.

[dgrove, 2013-06-05 00:21:44]

This LGTM, but definitely needs the followup CL ASAP.

[Andrei Mouravski, 2013-06-05 00:32:54]

Committed patchset #3 manually as r23622 (presubmit successful).