Add logging system to pub and sprinkle some logging in.

utils/pub/oauth2.dart

Index: utils/pub/oauth2.dart
diff --git a/utils/pub/oauth2.dart b/utils/pub/oauth2.dart
index 7cc5c852a81d4c7b4cfc2331d1a45913e28e94c0..30a3b0c6f052f20db0012de1cf821d1476e6285e 100644
--- a/utils/pub/oauth2.dart
+++ b/utils/pub/oauth2.dart
@@ -10,6 +10,7 @@ import 'dart:uri';
 // TODO(nweiz): Make this a "package:" URL, or something nicer than this.
 import '../../pkg/oauth2/lib/oauth2.dart';
 import 'io.dart';
+import 'log.dart' as log;
 import 'system_cache.dart';
 import 'utils.dart';
 
@@ -90,14 +91,21 @@ Future<Client> _getClient(SystemCache cache) {
 /// filesystem if possible. If the credentials can't be loaded for any reason,
 /// the returned [Future] will complete to null.
 Future<Credentials> _loadCredentials(SystemCache cache) {
+  var path = _credentialsFile(cache);
+  log.fine('Loading OAuth2 credentials from $path.');

[nweiz, 2012/12/05 23:56:54]

"from $path" is redundant, and also false if _credentials is null. I'd remove
it, and also log if we return early due to _credentials.

[Bob Nystrom, 2012/12/06 01:33:26]

Done.

   if (_credentials != null) return new Future.immediate(_credentials);
-  return fileExists(_credentialsFile(cache)).chain((credentialsExist) {
-    if (!credentialsExist) return new Future.immediate(null);
+  return fileExists(path).chain((credentialsExist) {
+    if (!credentialsExist) {
+      log.fine('No credentials found at $path.');

[nweiz, 2012/12/05 23:56:54]

I wouldn't include most of these fine log statements; they'll be clear from the
io.dart operations. Maybe add a transform at the end that mentions whether or
not re-authorization is required.

[Bob Nystrom, 2012/12/06 01:33:26]

Done.
Unless you really want, I'll leave that off for now. We can always add more
later.

+      return new Future.immediate(null);
+    }
 
+    log.fine('Found credentials at $path.');
     return readTextFile(_credentialsFile(cache)).transform((credentialsJson) {
+      log.fine('Read credentials file.');
       var credentials = new Credentials.fromJson(credentialsJson);
       if (credentials.isExpired && !credentials.canRefresh) {
-        printError("Pub's authorization to upload packages has expired and "
+        log.error("Pub's authorization to upload packages has expired and "
             "can't be automatically refreshed.");
         return null; // null means re-authorize
       }
@@ -105,8 +113,7 @@ Future<Credentials> _loadCredentials(SystemCache cache) {
       return credentials;
     });
   }).transformException((e) {
-    printError('Warning: could not load the saved OAuth2 credentials:'
-        '  $e\n'
+    log.error('Warning: could not load the saved OAuth2 credentials: $e\n'
         'Obtaining new credentials...');
     return null; // null means re-authorize
   });
@@ -116,9 +123,10 @@ Future<Credentials> _loadCredentials(SystemCache cache) {
 /// filesystem.
 Future _saveCredentials(SystemCache cache, Credentials credentials) {
   _credentials = credentials;
-  var credentialsFile = _credentialsFile(cache);
-  return ensureDir(dirname(credentialsFile)).chain((_) =>
-      writeTextFile(credentialsFile, credentials.toJson()));
+  var path = _credentialsFile(cache);
+  log.fine('Saving OAuth2 credentials to $path.');

[nweiz, 2012/12/05 23:56:54]

"to $path" is redundant.

[Bob Nystrom, 2012/12/06 01:33:26]

Done.

+  return ensureDir(dirname(path)).chain((_) =>
+      writeTextFile(path, credentials.toJson()));
 }
 
 /// The path to the file in which the user's OAuth2 credentials are stored.
@@ -151,7 +159,7 @@ Future<Client> _authorize() {
   server.addRequestHandler((request) => request.path == "/",
       (request, response) {
     chainToCompleter(new Future.immediate(null).chain((_) {
-      print('Authorization received, processing...');
+      log.message('Authorization received, processing...');
       var queryString = request.queryString;
       if (queryString == null) queryString = '';
       response.statusCode = 302;
@@ -170,13 +178,14 @@ Future<Client> _authorize() {
   var authUrl = grant.getAuthorizationUrl(
       new Uri.fromString('http://localhost:${server.port}'), scopes: _scopes);
 
-  print('Pub needs your authorization to upload packages on your behalf.\n'
-        'In a web browser, go to $authUrl\n'
-        'Then click "Allow access".\n\n'
-        'Waiting for your authorization...');
+  log.message(

[nweiz, 2012/12/05 23:56:54]

Unlike print(), passing multiline strings to log.message isn't equivalent to
calling it multiple times. Probably want to switch to the latter here?

[Bob Nystrom, 2012/12/06 01:33:26]

It's equivalent in normal verbosity, I think. In verbose mode, it isn't, but in
that case I think this is actually better because it makes it clear it's a
single message from a single place in code.
I think this might actually be slightly preferred?

[nweiz, 2012/12/06 19:40:02]

There are other places where multiple lines get printed in multiple messages. At
least we should be consistent.
I think the other way looks nicer, but that's just me.

[Bob Nystrom, 2012/12/07 21:13:44]

Per our discussion, this way is the new hotness since it will appear as a single
log entry. Syntactically, I kind of like the other way a bit more too, but
utility wins.

+      'Pub needs your authorization to upload packages on your behalf.\n'
+      'In a web browser, go to $authUrl\n'
+      'Then click "Allow access".\n\n'
+      'Waiting for your authorization...');
 
   return completer.future.transform((client) {
-    print('Successfully authorized.\n');
+    log.message('Successfully authorized.\n');
     return client;
   });
 }