Let docgen detect a dart-sdk directory that's differently named, and do it correctly on bleeding ed…

Let docgen detect a dart-sdk directory that's differently named, and do it correctly on bleeding edge



BUG=
R=ricow@google.com

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

[Alan Knight, 2014-11-19 19:09:48]

alanknight@google.com changed reviewers:
+ ricow@google.com, sethladd@google.com

[Alan Knight, 2014-11-19 19:09:48]

Let's try that again, but we look only at the portion before "edge" if it's
present. I think that this general mechanism of checking for a matching version
should be ok for anything on dev or stable.

[ricow1, 2014-11-20 07:38:30]

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
File pkg/docgen/lib/src/package_helpers.dart (right):

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
pkg/docgen/lib/src/package_helpers.dart:14: /// Helper accessor to determine the
full pathname of the root of the dart
this comment seems out of date, we are not necessarily finding the root of the
dart checkout are we?

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
pkg/docgen/lib/src/package_helpers.dart:27: // When we find a version file or
sdk we are one level below the root.
comment don't match the case where we hit pkg?

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
pkg/docgen/lib/src/package_helpers.dart:28: while (!found && base != 'sdk' &&
base != 'pkg') {
looking at the 3 examples in the function comment we would never hit sdk

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
pkg/docgen/lib/src/package_helpers.dart:33: // the same version as we are
running.
could you elaborate on _why_ it is important for the versions to match?

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
pkg/docgen/lib/src/package_helpers.dart:62: version.substring(0,
edgePosition(version));
remove edgePosition below and just do:
Sting withoutEdge(String version) => version.split('-edge').first;

[ricow1, 2014-11-20 07:55:00]

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
File pkg/docgen/lib/src/package_helpers.dart (right):

https://codereview.chromium.org/737033002/diff/1/pkg/docgen/lib/src/package_h...
pkg/docgen/lib/src/package_helpers.dart:62: version.substring(0,
edgePosition(version));
On 2014/11/20 07:38:30, ricow1 wrote:
> remove edgePosition below and just do:
> Sting withoutEdge(String version) => version.split('-edge').first;

s/Sting/String :-)

[Alan Knight, 2014-11-20 18:49:10]

OK, disregard the first patch set. I actually looked in more detail at what's
going on here and there are really only two scenarios

1) We are running from the dartdocgen script, either in a built SDK or a
distribution, either downloaded, homebrew, or whatever. The script always passes
in a --sdk parameter, which the program is using to find the location of other
executables (dart, pub) but otherwise ignoring. This parameter looks like it's
reliable in all scenarios, so the reverse engineering that the program is doing
is just a waste of time. Changed it to just use that value if it's present, much
simpler.

2) We are running directly from source. In this case we do need to find the
root, in order to locate the SDK, but it's in a known position directly above
pkg.

2a) There's maybe an odd edge case where you run the snapshot not from the
script and don't pass --sdk, in which case you might be under dart-sdk. I left
that logic in there just in case. I took out the check for 'sdk', which I think
was only there to handle the case where we were looking up from the dart/sdk/bin
directory, but I don't think that ever happens, we always start from the
snapshot or docgen/bin directory.

[Alan Knight, 2014-12-03 18:14:18]

Ping

[ricow1, 2014-12-04 07:18:09]

lgtm, much better, and sorry for delay

[Alan Knight, 2014-12-04 17:10:27]

Committed patchset #2 (id:20001) manually as 42112 (presubmit successful).