Add support for XZ-compressed packages #1100

ranma42 · 2017-05-08T12:20:47Z

When XZ-compressed packages are available, prefer them in place of the
GZip-compressed ones as they can provide significant savings interms
of download size.

This should be the last step towards fixing rust-lang/rust#21724

brson · 2017-05-08T20:48:02Z

This looks great @ranma42. Thank you for pushing this through all the tooling.

I do think there should be tests for xz compression in rustup.

Unfortunately rustup's testing is fairly cumbersome. I think the simplest thing here is to just add some unit tests that the installation code works correctly with xz. The basic way to do this is to duplicate this test case, add a new 'xz' boolean to MockDistServer::write that causes the mock dist server to generate both the 'gz' and 'xz' tarballs and the appropriate keys in the manifest, and thread that call into the new test case. That will at least provide a smoke test that the xz fields work.

ranma42 · 2017-05-08T20:52:01Z

Ok, I will try and add a test as suggested.
I am not very happy with the code that computes c_u_h? Is there any better way to write that?

brson · 2017-05-08T21:07:25Z

src/rustup-dist/src/manifestation.rs

+                    xz = try!(TarXzPackage::new_file(&installer_file, temp_cfg));
+                    &xz
+                }
+            };


This is quite a fascinating pattern, leaving one local uninitialized. I did not know that was possible.

rustc does not complain, but I guess it might be regarded as bad practice.
It is easy to transform this in a single local that is always assigned by wrapping the two cases in an enum, if it is preferred.

It's good as is, thanks.

brson · 2017-05-08T21:08:08Z

@ranma42 I don't have a better suggestion for that offhand.

ranma42 · 2017-05-14T06:49:44Z

I updated the lzma-sys package (to fix some linking/crosscompiling issues) and added a test.
I am not very happy with the test, because even though it actually creates and uses the xz tarball it does not explicitly check for it: if the mock dist server was modified to send a gz-only manifest, the test would still pass. What would be the best way to check which tarball is being used?

alexcrichton · 2017-05-14T19:40:12Z

For that one specific test could the xz tarball have different files than the gz tarball, and then you assert that the xz-specific files exist?

ranma42 · 2017-05-16T07:00:47Z

That is apparently not so easy, because only the contents listed in the manifest.in file are installed.

Diggsey · 2017-05-17T15:38:37Z

@ranma42 what about running rustup with the --verbose option, and asserting that it fetches the xz file?

ranma42 · 2017-05-19T11:51:51Z

@Diggsey I tried to implement your suggestion and it seems to work locally, but somehow my new commit fails badly on the CI services
How can I debug the Cargo.lock issue?

Diggsey · 2017-05-20T14:52:22Z

@ranma42 I'm not sure, but while trying to reproduce locally, I ran into this other issue:
alexcrichton/xz2-rs#6

I'm not keen on making rustup (even) harder to build on windows...

carols10cents · 2017-05-20T19:10:47Z

Taking a look at the travis logs, it says:

error: failed to parse lock file at: /buildslave/Cargo.lock

Caused by:

  package `libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)` is specified as a dependency, but is missing from the package list

I was able to check out this PR's branch locally and attempt to build, since I'm not on windows and not hitting the issue @Diggsey is.

I'm indeed able to reproduce the error happening on travis. I reset Cargo.lock, then did cargo build (which worked), then compared my Cargo.lock to the one on this branch, and these are the changes I see:

diff --git a/Cargo.lock b/Cargo.lock
index 0602c7b..324ff53 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -345,12 +345,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"

 [[package]]
 name = "lzma-sys"
-version = "0.1.3"
+version = "0.1.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "filetime 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "gcc 0.3.45 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

 [[package]]
@@ -997,7 +997,7 @@ name = "xz2"
 version = "0.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "lzma-sys 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lzma-sys 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

 [metadata]
@@ -1038,7 +1038,7 @@ dependencies = [
 "checksum libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)" = "babb8281da88cba992fa1f4ddec7d63ed96280a1a53ec9b919fd37b53d71e502"
 "checksum libz-sys 1.0.8 (registry+https://github.com/rust-lang/crates.io-index)" = "c18b5826abbfafb0160b37e1991e2d327c1fe38c955e496ea306f72c06d7570c"
 "checksum log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "ab83497bf8bf4ed2a74259c1c802351fcd67a65baa86394b6ba73c36f4838054"
-"checksum lzma-sys 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c5eaaa53b35fa17482ee2c001b04242827b47ae0faba72663fee3dee32366248"
+"checksum lzma-sys 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "fedff6a5cbb24494ec6ee4784e9ac5c187161fede04c7767d49bf87544013afa"
 "checksum markdown 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bdb7e864aa1dccbebb05751e899bc84c639df47490c0c24caf4b1a77770b6566"
 "checksum matches 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "15305656809ce5a4805b1ff2946892810992197ce1270ff79baded852187942e"
 "checksum memchr 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "d8b629fb514376c675b98c1421e80b151d3817ac42d7c667717d282761418d20"

@ranma42 I'm guessing there was a merge conflict to Cargo.lock at some point? Can you try checking out the changes your branch makes to Cargo.lock, then building, then checking in Cargo.lock again?

ranma42 · 2017-05-21T08:00:16Z

@carols10cents it was not a merge conflict in the sense of git (it would have been marked as such by github), but in a sense that was the problem. Thank you for helping out diagnosing the problem!

@Diggsey what is the best way forward regarding alexcrichton/xz2-rs#6 ?
Should we try and get that fixed in lzma-sys or upstream (in liblzma)?

Diggsey · 2017-05-21T10:13:20Z

@ranma42 I'm working on a fix for the xz2 issue, then we can get this merged.

Diggsey · 2017-05-21T12:37:25Z

alexcrichton/xz2-rs#7

bors · 2017-05-23T02:36:16Z

☔ The latest upstream changes (presumably #1131) made this pull request unmergeable. Please resolve the merge conflicts.

When XZ-compressed packages are available, prefer them in place of the GZip-compressed ones as they can provide significant savings interms of download size.

ranma42 · 2017-05-23T06:07:23Z

Why does AppVeyor report the branch as non-mergeable? (I rebased, upgraded lzma-sys and force-pushed)

Diggsey · 2017-05-23T23:17:19Z

No idea :/ Let's give bors a try.

@bors r+

bors · 2017-05-23T23:17:19Z

📌 Commit f3aeab3 has been approved by Diggsey

bors · 2017-05-23T23:17:27Z

⌛ Testing commit f3aeab3 with merge d12fd95...

Add support for XZ-compressed packages When XZ-compressed packages are available, prefer them in place of the GZip-compressed ones as they can provide significant savings interms of download size. This should be the last step towards fixing rust-lang/rust#21724

bors · 2017-05-24T00:32:10Z

💔 Test failed - status-travis

Diggsey · 2017-05-24T00:40:14Z

@bors retry

bors · 2017-05-24T00:40:21Z

⌛ Testing commit f3aeab3 with merge ed56939...

Add support for XZ-compressed packages When XZ-compressed packages are available, prefer them in place of the GZip-compressed ones as they can provide significant savings interms of download size. This should be the last step towards fixing rust-lang/rust#21724

bors · 2017-05-24T01:56:48Z

☀️ Test successful - status-appveyor, status-travis
Approved by: Diggsey
Pushing ed56939 to master...

brson reviewed May 8, 2017

View reviewed changes

brson mentioned this pull request May 8, 2017

Allow rustup to handle unavailable packages #1063

Merged

ranma42 force-pushed the package-xz branch from b614e1f to bdd89e3 Compare May 14, 2017 05:56

ranma42 force-pushed the package-xz branch from bdd89e3 to 7232a56 Compare May 19, 2017 10:12

ranma42 force-pushed the package-xz branch from 7232a56 to c180842 Compare May 21, 2017 05:43

ranma42 added 2 commits May 23, 2017 07:17

Add support for XZ-compressed packages

462d719

When XZ-compressed packages are available, prefer them in place of the GZip-compressed ones as they can provide significant savings interms of download size.

Add test for xz tarballs

f3aeab3

ranma42 force-pushed the package-xz branch from c180842 to f3aeab3 Compare May 23, 2017 06:04

bors merged commit f3aeab3 into rust-lang:master May 24, 2017

ranma42 mentioned this pull request May 24, 2017

Tarballs we deliver should be compressed better rust-lang/rust#21724

Closed

brson mentioned this pull request Jun 8, 2017

Updated releases notes for 1.19 rust-lang/rust#42503

Closed

ghost mentioned this pull request Jan 24, 2018

rust-overlay: prefer xz archives? mozilla/nixpkgs-mozilla#77

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for XZ-compressed packages #1100

Add support for XZ-compressed packages #1100

ranma42 commented May 8, 2017

brson commented May 8, 2017

ranma42 commented May 8, 2017

brson May 8, 2017

ranma42 May 9, 2017

brson May 9, 2017

brson commented May 8, 2017

ranma42 commented May 14, 2017

alexcrichton commented May 14, 2017

ranma42 commented May 16, 2017

Diggsey commented May 17, 2017

ranma42 commented May 19, 2017

Diggsey commented May 20, 2017

carols10cents commented May 20, 2017

ranma42 commented May 21, 2017

Diggsey commented May 21, 2017

Diggsey commented May 21, 2017

bors commented May 23, 2017

ranma42 commented May 23, 2017 •

edited

Loading

Diggsey commented May 23, 2017

bors commented May 23, 2017

bors commented May 23, 2017

bors commented May 24, 2017

Diggsey commented May 24, 2017

bors commented May 24, 2017

bors commented May 24, 2017

Add support for XZ-compressed packages #1100

Add support for XZ-compressed packages #1100

Conversation

ranma42 commented May 8, 2017

brson commented May 8, 2017

ranma42 commented May 8, 2017

brson May 8, 2017

Choose a reason for hiding this comment

ranma42 May 9, 2017

Choose a reason for hiding this comment

brson May 9, 2017

Choose a reason for hiding this comment

brson commented May 8, 2017

ranma42 commented May 14, 2017

alexcrichton commented May 14, 2017

ranma42 commented May 16, 2017

Diggsey commented May 17, 2017

ranma42 commented May 19, 2017

Diggsey commented May 20, 2017

carols10cents commented May 20, 2017

ranma42 commented May 21, 2017

Diggsey commented May 21, 2017

Diggsey commented May 21, 2017

bors commented May 23, 2017

ranma42 commented May 23, 2017 • edited Loading

Diggsey commented May 23, 2017

bors commented May 23, 2017

bors commented May 23, 2017

bors commented May 24, 2017

Diggsey commented May 24, 2017

bors commented May 24, 2017

bors commented May 24, 2017

ranma42 commented May 23, 2017 •

edited

Loading