From 58b1cc508b7ff9398452d226b0caf91ee95e716b Mon Sep 17 00:00:00 2001
From: "rnystrom@google.com" <rnystrom@google.com>
Date: Tue, 29 Jan 2013 00:56:07 +0000
Subject: [PATCH] Handle parsing the "version" file better.

Review URL: https://codereview.chromium.org//12087008

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@17740 260f80e4-7a28-3924-810f-c04153c831b5
---
 lib/src/sdk.dart              |  8 +++----
 test/command_line_config.dart | 40 ++++++++++-------------------------
 test/pub_test.dart            | 30 +++++++++++++++++++++-----
 test/real_version_test.dart   |  2 +-
 test/test_pub.dart            | 15 ++++++++-----
 5 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/lib/src/sdk.dart b/lib/src/sdk.dart
index 0e47acd8..98007b5a 100644
--- a/lib/src/sdk.dart
+++ b/lib/src/sdk.dart
@@ -14,7 +14,7 @@ import 'version.dart';
 /// Matches an Eclipse-style SDK version number. This is four dotted numbers
 /// (major, minor, patch, build) with an optional suffix attached to the build
 /// number.
-final _versionPattern = new RegExp(r'^(\d+)\.(\d+)\.(\d+)\.(\d+)(.*)$');
+final _versionPattern = new RegExp(r'^(\d+)\.(\d+)\.(\d+)\.(\d+.*)$');
 
 /// Gets the path to the root directory of the SDK.
 String get rootDirectory {
@@ -41,15 +41,15 @@ Version _getVersion() {
   var version = new File(revisionPath).readAsStringSync().trim();
 
   // Given a version file like: 0.1.2.0_r17495
-  // We create a semver like:   0.1.2+0._r17495
+  // We create a semver like:   0.1.2+0.r17495
   var match = _versionPattern.firstMatch(version);
   if (match == null) {
     throw new FormatException("The Dart SDK's 'version' file was not in a "
         "format pub could recognize. Found: $version");
   }
 
-  var build = match[4];
-  if (match[5].length > 0) build = '$build.${match[5]}';
+  // Semantic versions cannot use "_".
+  var build = match[4].replaceAll('_', '.');
 
   return new Version(
       int.parse(match[1]), int.parse(match[2]), int.parse(match[3]),
diff --git a/test/command_line_config.dart b/test/command_line_config.dart
index 1d4f7bf6..790e91ec 100644
--- a/test/command_line_config.dart
+++ b/test/command_line_config.dart
@@ -6,6 +6,7 @@ library command_line_config;
 
 import 'dart:io';
 
+import '../../../pkg/path/lib/path.dart' as path;
 import '../../../pkg/unittest/lib/unittest.dart';
 import '../../pub/utils.dart';
 
@@ -70,6 +71,8 @@ class CommandLineConfiguration extends Configuration {
   void _printStackTrace(String stackTrace) {
     if (stackTrace == null || stackTrace == '') return;
 
+    print('');
+
     // Parse out each stack entry.
     var stack = [];
     for (var line in stackTrace.split('\n')) {
@@ -79,33 +82,6 @@ class CommandLineConfiguration extends Configuration {
 
     if (stack.length == 0) return;
 
-    // Find the common prefixes of the paths.
-    var common = 0;
-    while (true) {
-      var matching = true;
-      var c;
-      for (var frame in stack) {
-        if (frame.isCore) continue;
-        if (c == null) c = frame.library[common];
-
-        if (frame.library.length <= common || frame.library[common] != c) {
-          matching = false;
-          break;
-        }
-      }
-
-      if (!matching) break;
-      common++;
-    }
-
-    // Remove them.
-    if (common > 0) {
-      for (var frame in stack) {
-        if (frame.isCore) continue;
-        frame.library = frame.library.substring(common);
-      }
-    }
-
     // Figure out the longest path so we know how much to pad.
     int longest = stack.mappedBy((frame) => frame.location.length).max();
 
@@ -138,7 +114,7 @@ class CommandLineConfiguration extends Configuration {
 
 class _StackFrame {
   static final fileRegExp = new RegExp(
-      r'#\d+\s+(.*) \((file:///.+):(\d+):(\d+)\)');
+      r'#\d+\s+(.*) \(file://(/.+):(\d+):(\d+)\)');
   static final coreRegExp = new RegExp(r'#\d+\s+(.*) \((.+):(\d+):(\d+)\)');
 
   /// If `true`, then this stack frame is for a library built into Dart and
@@ -172,7 +148,13 @@ class _StackFrame {
       isCore = true;
     }
 
+    var library = match[2];
+    if (!isCore) {
+      // Make the library path relative to the entrypoint.
+      library = path.relative(library);
+    }
+
     var member = match[1].replaceAll("<anonymous closure>", _LAMBDA);
-    return new _StackFrame._(isCore, match[2], match[3], match[4], member);
+    return new _StackFrame._(isCore, library, match[3], match[4], member);
   }
 }
\ No newline at end of file
diff --git a/test/pub_test.dart b/test/pub_test.dart
index 13426f19..6de4dbcc 100644
--- a/test/pub_test.dart
+++ b/test/pub_test.dart
@@ -123,11 +123,31 @@ main() {
 
   });
 
-  integration('displays the current version', () {
-    dir(sdkPath, [
-      file('version', '0.1.2.3'),
-    ]).scheduleCreate();
+  group('version', () {
+    integration('displays the current version', () {
+      dir(sdkPath, [
+        file('version', '0.1.2.3'),
+      ]).scheduleCreate();
+
+      schedulePub(args: ['version'], output: VERSION_STRING);
+    });
 
-    schedulePub(args: ['version'], output: VERSION_STRING);
+    integration('parses a release-style version', () {
+      dir(sdkPath, [
+        file('version', '0.1.2.0_r17645'),
+      ]).scheduleCreate();
+
+      schedulePub(args: ['version'], output: "Pub 0.1.2+0.r17645\n");
+    });
+
+    integration('parses a dev-only style version', () {
+      // The "version" file generated on developer builds is a little funky and
+      // we need to make sure we don't choke on it.
+      dir(sdkPath, [
+        file('version', '0.1.2.0_r16279_bobross'),
+      ]).scheduleCreate();
+
+      schedulePub(args: ['version'], output: "Pub 0.1.2+0.r16279.bobross\n");
+    });
   });
 }
diff --git a/test/real_version_test.dart b/test/real_version_test.dart
index 1688fa7f..b0329a66 100644
--- a/test/real_version_test.dart
+++ b/test/real_version_test.dart
@@ -25,7 +25,7 @@ main() {
   // Note that this test expects to be invoked from a Dart executable that is
   // in the built SDK's "bin" directory. Note also that this invokes pub from
   // the built SDK directory, and not the live pub code directly in the repo.
-  test('Pub can parse the real SDK "version" file', () {
+  test('parse the real SDK "version" file', () {
     // Get the path to the pub binary in the SDK.
     var dartPath = new Options().executable;
     var pubPath = path.join(path.dirname(dartPath), "pub");
diff --git a/test/test_pub.dart b/test/test_pub.dart
index 03f0f660..81bc9e9c 100644
--- a/test/test_pub.dart
+++ b/test/test_pub.dart
@@ -808,7 +808,9 @@ abstract class Descriptor {
     if (name is String) {
       var path = join(dir, name);
       return exists(path).then((exists) {
-        if (!exists) Expect.fail('File $name in $dir not found.');
+        if (!exists) {
+          throw new ExpectException('File $name in $dir not found.');
+        }
         return validate(path);
       });
     }
@@ -824,7 +826,7 @@ abstract class Descriptor {
     return listDir(dir).then((files) {
       var matches = files.where((file) => endsWithPattern(file, name)).toList();
       if (matches.isEmpty) {
-        Expect.fail('No files in $dir match pattern $name.');
+        throw new ExpectException('No files in $dir match pattern $name.');
       }
       if (matches.length == 1) return validate(matches[0]);
 
@@ -888,8 +890,9 @@ class FileDescriptor extends Descriptor {
       return readTextFile(file).then((text) {
         if (text == contents) return null;
 
-        Expect.fail('File $file should contain:\n\n$contents\n\n'
-                    'but contained:\n\n$text');
+        throw new ExpectException(
+            'File $file should contain:\n\n$contents\n\n'
+            'but contained:\n\n$text');
       });
     });
   }
@@ -1133,7 +1136,9 @@ class NothingDescriptor extends Descriptor {
 
   Future validate(String dir) {
     return exists(join(dir, name)).then((exists) {
-      if (exists) Expect.fail('File $name in $dir should not exist.');
+      if (exists) {
+        throw new ExpectException('File $name in $dir should not exist.');
+      }
     });
   }
 
-- 
GitLab