Add support for XZ-compressed packages #1100

ranma42 · May 8, 2017

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 · May 8, 2017

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 · May 8, 2017

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 · May 8, 2017

brson reviewed May 8, 2017

View changes

brson · May 8, 2017

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 · May 8, 2017

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

ranma42 · May 14, 2017

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 · May 14, 2017

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 · May 16, 2017

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

Diggsey · May 17, 2017

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

ranma42 · May 19, 2017

@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 · May 20, 2017

@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 · May 20, 2017

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 · May 21, 2017

@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 · May 21, 2017

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

Diggsey · May 21, 2017

alexcrichton/xz2-rs#7

bors · May 23, 2017

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

ranma42 · May 23, 2017

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

Diggsey · May 23, 2017

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

@bors r+

bors · May 23, 2017

📌 Commit f3aeab3 has been approved by Diggsey

bors · May 23, 2017

⌛️ Testing commit f3aeab3 with merge d12fd95...

bors · May 24, 2017

💔 Test failed - status-travis

Diggsey · May 24, 2017

@bors retry

bors · May 24, 2017

⌛️ Testing commit f3aeab3 with merge ed56939...

bors · May 24, 2017

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

brson referenced this pull request May 8, 2017
Merged
Allow rustup to handle unavailable packages #1063

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

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

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

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

ranma42 referenced this pull request May 24, 2017
Closed
Tarballs we deliver should be compressed better #21724

brson referenced this pull request Jun 8, 2017
Closed
Updated releases notes for 1.19 #42503

ghost referenced this pull request Jan 24, 2018
Closed
rust-overlay: prefer xz archives? #77

rust-lang/rustup.rs

Join GitHub today

Add support for XZ-compressed packages #1100

Conversation

ranma42 commented May 8, 2017

This comment has been minimized.

brson commented May 8, 2017

This comment has been minimized.

ranma42 commented May 8, 2017

brson reviewed May 8, 2017 View changes

This comment has been minimized.

brson May 8, 2017

This comment has been minimized.

ranma42 May 9, 2017

This comment has been minimized.

brson May 9, 2017

This comment has been minimized.

brson commented May 8, 2017

brson referenced this pull request May 8, 2017

Allow rustup to handle unavailable packages #1063

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

This comment has been minimized.

ranma42 commented May 14, 2017

This comment has been minimized.

alexcrichton commented May 14, 2017

This comment has been minimized.

ranma42 commented May 16, 2017

This comment has been minimized.

Diggsey commented May 17, 2017

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

This comment has been minimized.

ranma42 commented May 19, 2017

This comment has been minimized.

Diggsey commented May 20, 2017

This comment has been minimized.

carols10cents commented May 20, 2017

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

This comment has been minimized.

ranma42 commented May 21, 2017

This comment has been minimized.

Diggsey commented May 21, 2017

This comment has been minimized.

Diggsey commented May 21, 2017

This comment has been minimized.

bors commented May 23, 2017

ranma42 added some commits May 8, 2017

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

This comment has been minimized.

ranma42 commented May 23, 2017 • edited

This comment has been minimized.

Diggsey commented May 23, 2017

This comment has been minimized.

bors commented May 23, 2017

This comment has been minimized.

bors commented May 23, 2017

bors added a commit that referenced this pull request May 23, 2017

This comment has been minimized.

bors commented May 24, 2017

This comment has been minimized.

Diggsey commented May 24, 2017

This comment has been minimized.

bors commented May 24, 2017

bors added a commit that referenced this pull request May 24, 2017

This comment has been minimized.

bors commented May 24, 2017

Hide details View details bors merged commit f3aeab3 into rust-lang:master May 24, 2017 1 of 3 checks passed

1 of 3 checks passed

ranma42 referenced this pull request May 24, 2017

Tarballs we deliver should be compressed better #21724

brson referenced this pull request Jun 8, 2017

Updated releases notes for 1.19 #42503

ghost referenced this pull request Jan 24, 2018

rust-overlay: prefer xz archives? #77

brson reviewed May 8, 2017

View changes

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

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

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

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

ranma42 commented May 23, 2017 •

edited

bors merged commit `f3aeab3` into rust-lang:master May 24, 2017
1 of 3 checks passed