Extraction of .zip files working.

Extraction of .zip files working.

Also, URLs with port numbers encoded in them will strip out the numbers
when saving to the local cache.

R=tapted@chromium.org

Committed: https://github.com/tapted/bleeding_edge/commit/43fefde

[tapted, 2014-01-08 04:26:02]

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/js/...
File dart/sdk/lib/_internal/pub/js/archive.js (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/js/...
dart/sdk/lib/_internal/pub/js/archive.js:1: function saveDirs(dest, dirs,
onComplete) {
Can we add a git submodule pointing pub/js/jszip/ to
https://github.com/Stuk/jszip.git ?

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/io.dart (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/io.dart:235: .then((dir) => dir.remove())
The changes in this file look orthogonal to the zip stuff -- let's do a separate
patch for them

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/source/hosted.dart (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/source/hosted.dart:81:
"$server/packages/$package/versions/$version.zip");
The `.zip` should be a string constant somewhere -- we might need to hunt down
other occurrences.

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/source/hosted.dart:89: dest = dir;
nit: outdent these 3 lines two spaces

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:8: import
'package:js/js.dart' as js;
nit: sort

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:317: //return
Link.create(symlink).then((link) => link.create(target));
is this needed? (we shouldn't commit commented out code, but you can add a
TODO/assert

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/man...
File dart/sdk/lib/_internal/pub/manifest.json (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/man...
dart/sdk/lib/_internal/pub/manifest.json:18: "http://localhost:8080/"
should `localhost` be pajamallama0.syd.corp.google.com or similar?

[pajamallama, 2014-01-08 05:15:54]

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/js/...
File dart/sdk/lib/_internal/pub/js/archive.js (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/js/...
dart/sdk/lib/_internal/pub/js/archive.js:1: function saveDirs(dest, dirs,
onComplete) {
On 2014/01/08 04:26:03, tapted wrote:
> Can we add a git submodule pointing pub/js/jszip/ to
> https://github.com/Stuk/jszip.git ?

Done.

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/io.dart (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/io.dart:235: .then((dir) => dir.remove())
On 2014/01/08 04:26:03, tapted wrote:
> The changes in this file look orthogonal to the zip stuff -- let's do a
separate
> patch for them

Done.

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/source/hosted.dart (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/source/hosted.dart:81:
"$server/packages/$package/versions/$version.zip");
On 2014/01/08 04:26:03, tapted wrote:
> The `.zip` should be a string constant somewhere -- we might need to hunt down
> other occurrences.

Done.

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/source/hosted.dart:89: dest = dir;
On 2014/01/08 04:26:03, tapted wrote:
> nit: outdent these 3 lines two spaces

Done.

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:8: import
'package:js/js.dart' as js;
On 2014/01/08 04:26:03, tapted wrote:
> nit: sort

Done.

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:317: //return
Link.create(symlink).then((link) => link.create(target));
On 2014/01/08 04:26:03, tapted wrote:
> is this needed? (we shouldn't commit commented out code, but you can add a
> TODO/assert

Done.

[pajamallama, 2014-01-08 06:05:21]

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/man...
File dart/sdk/lib/_internal/pub/manifest.json (right):

https://codereview.appspot.com/48730045/diff/1/dart/sdk/lib/_internal/pub/man...
dart/sdk/lib/_internal/pub/manifest.json:18: "http://localhost:8080/"
On 2014/01/08 04:26:03, tapted wrote:
> should `localhost` be http://pajamallama0.syd.corp.google.com or similar?

Done.

[tapted, 2014-01-08 06:50:07]

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/js/archive.js (right):

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/js/archive.js:2: * JS Helpers for interacting with
the JSZip library
nit: needs a '.' on the end

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/js/archive.js:40: function extractZipFile(data, dest,
onFinish) {
nit: add a comment for this function - really needs some context about the
argument types (I'm guessing the following, so fix, e.g, if `data` isn't
actually a String). Something like

  /**
   * Asynchronously extract an uncompressed .zip archive to |dest|.
   *
   * @param {String} data Uncompressed zip data, residing in memory.
   * @param {FileSystemEntry} dest Directory to extract the archive.
   * @param {function(boolean)} onFinish Called when finished; false on failure.
   */

More details at
https://developers.google.com/closure/compiler/docs/js-for-compiler#types

For the other functtons, something like

/** @private */

might be appropriate

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/lib/src/io.dart (right):

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/lib/src/io.dart:253: void createPackageSymlink(String
name, String target, String symlink,
looks like the rebase is messed up..

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/pub.html (right):

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/pub.html:15: <script type="text/javascript"
src="js/jszip/jszip.js"></script>
nit: I think the "text/javascript" is just noise for these

[pajamallama, 2014-01-08 23:44:54]

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/js/archive.js (right):

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/js/archive.js:2: * JS Helpers for interacting with
the JSZip library
On 2014/01/08 06:50:07, tapted wrote:
> nit: needs a '.' on the end

Done.

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/js/archive.js:40: function extractZipFile(data, dest,
onFinish) {
On 2014/01/08 06:50:07, tapted wrote:
> nit: add a comment for this function - really needs some context about the
> argument types (I'm guessing the following, so fix, e.g, if `data` isn't
> actually a String). Something like
> 
>   /**
>    * Asynchronously extract an uncompressed .zip archive to |dest|.
>    *
>    * @param {String} data Uncompressed zip data, residing in memory.
>    * @param {FileSystemEntry} dest Directory to extract the archive.
>    * @param {function(boolean)} onFinish Called when finished; false on
failure.
>    */
> 
> More details at
> https://developers.google.com/closure/compiler/docs/js-for-compiler#types
> 
> For the other functtons, something like
> 
> /** @private */
> 
> might be appropriate

Done.

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/lib/src/io.dart (right):

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/lib/src/io.dart:253: void createPackageSymlink(String
name, String target, String symlink,
On 2014/01/08 06:50:07, tapted wrote:
> looks like the rebase is messed up..

Done.

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/pub.html (right):

https://codereview.appspot.com/48730045/diff/80001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/pub.html:15: <script type="text/javascript"
src="js/jszip/jszip.js"></script>
On 2014/01/08 06:50:07, tapted wrote:
> nit: I think the "text/javascript" is just noise for these

Done.

[tapted, 2014-01-08 23:53:18]

lgtm

[pajamallama, 2014-01-08 23:58:42]

Committed patchset #7 manually as r43fefde (presubmit successful).