Add logging system to pub and sprinkle some logging in.

utils/pub/oauth2.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library oauth2;
 
 import 'dart:io';
 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';
 
 export '../../pkg/oauth2/lib/oauth2.dart';
 
 /// The pub client's OAuth2 identifier.
 final _identifier = '818368855108-8grd2eg9tj9f38os6f1urbcvsq399u8n.apps.'
     'googleusercontent.com';
 
 /// The pub client's OAuth2 secret. This isn't actually meant to be kept a
@@ skipping 60 matching lines @@
         _identifier, _secret, credentials, httpClient: curlClient));
   }).chain((client) {
     return _saveCredentials(cache, client.credentials).transform((_) => client);
   });
 }
 
 /// Loads the user's OAuth2 credentials from the in-memory cache or the
 /// 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
       }
 
       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
   });
 }
 
 /// Save the user's OAuth2 credentials to the in-memory cache and the
 /// 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.
 String _credentialsFile(SystemCache cache) =>
   join(cache.rootDir, 'credentials.json');
 
 /// Gets the user to authorize pub as a client of pub.dartlang.org via oauth2.
 /// Returns a Future that will complete to a fully-authorized [Client].
 Future<Client> _authorize() {
   // Allow the tests to inject their own token endpoint URL.
@@ skipping 12 matching lines @@
       httpClient: curlClient);
 
   // Spin up a one-shot HTTP server to receive the authorization code from the
   // Google OAuth2 server via redirect. This server will close itself as soon as
   // the code is received.
   var completer = new Completer();
   var server = new HttpServer();
   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;
       response.headers.set('location', 'http://pub.dartlang.org/authorized');
       return Futures.wait([
         closeHttpResponse(request, response),
         grant.handleAuthorizationResponse(queryToMap(queryString))
       ]);
     }).transform((results) {
       server.close();
       return results[1];
     }), completer);
   });
   server.listen('127.0.0.1', 0);
 
   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;
   });
 }