Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated rustBeta & rustNightly; renamed rustUnstable to rustNightly #20780

Closed
wants to merge 1 commit into from
Closed

updated rustBeta & rustNightly; renamed rustUnstable to rustNightly #20780

wants to merge 1 commit into from

Conversation

qknight
Copy link
Member

@qknight qknight commented Nov 29, 2016

Motivation for this change
  • updated rust nightly to newer revision
  • updated rust beta to newer revision
  • renamed rustUnstable to rustNightly to follow upstream naming
  • renamed various files to follow upstream naming
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@qknight, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dvc94ch, @the-kenny and @sjmackenzie to be potential reviewers.

@Ericson2314
Copy link
Member

Looks good to me, but your comparability error is breaking Travis. Surely somebody else has made such a thing, so there must be a way to get around it.

Otherwise, just delete it, or make it an alias with a warning in top-level/aliases.nix.

@@ -97,11 +97,11 @@ stdenv.mkDerivation {
rm -vr src/test/run-make/linker-output-non-utf8 || true

# Remove test targeted at LLVM 3.9 - https://github.com/rust-lang/rust/issues/36835
rm -vr src/test/run-pass/issue-36023.rs || true
rm -fvr src/test/run-pass/issue-36023.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance we can get the || true version? To me it feels more explicit like "Try to remove it but we don't care if it fails" which is what these lines are supposed to do. Same in line 104.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel free to revert these to lines but either way it is still a hack since we are using rustc.nix with 3 different revisions of rust.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I know. Maybe I get around writing a patch to remove these files (or better: add the # ignore line). Do you know which file needs to be removed for which compiler version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC: i think the two changes (the places we added the rm are only required for stable) and made beta and nightly fail.

@the-kenny
Copy link
Contributor

I'll try the builds with sandboxing enabled in about half an hour. Will report back if everything's fine :-)

@the-kenny
Copy link
Contributor

the-kenny commented Nov 29, 2016

Note that the patch doesn't apply cleanly to master (all-packaes:5217 fails). This is caused by f67061c.

@@ -48,12 +48,6 @@ rec {
--set-interpreter $(cat $NIX_CC/nix-support/dynamic-linker) \
"$out/bin/rustc"
''}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i removed it as you requested

@the-kenny
Copy link
Contributor

LGTM :) Just waiting for my build to succeed.

@qknight
Copy link
Member Author

qknight commented Nov 29, 2016

i've also started a sandboxed build but with a slightly older revision. let's wait until both succeed and then we might press the 'merge' button ;-)

@qknight
Copy link
Member Author

qknight commented Nov 29, 2016

my sandbox build succeeded successfully but the ./print-hashes.sh script does not work:

./print-hashes.sh 2014-08-07
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message><Key>dist/rust-2014-08-07-i686-unknown-linux-gnu.tar.gz.sha256</Key><RequestId>177742A282651660</RequestId><HostId>Lvrwv8LcgiIRzYtsA4fzptEVGcVB5EBFpCMyUl0zqhbQMWRHNlJY2ptbpCdVcBqYCixpJuaN6yo=</HostId></Error><?xml version="1.0" encoding="UTF-8"?>
<Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message><Key>dist/rust-2014-08-07-x86_64-unknown-linux-gnu.tar.gz.sha256</Key><RequestId>611E3E9AC855F4BA</RequestId><HostId>1V6hexxeHwAm3/MP5feZ7s5erMX0HHWZheowmafZYGatPb8FGdB3duafkEUSiLGzZV4BDF61uyM=</HostId></Error><?xml version="1.0" encoding="UTF-8"?>
<Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message><Key>dist/rust-2014-08-07-i686-apple-darwin.tar.gz.sha256</Key><RequestId>33A2A8681F0F0A90</RequestId><HostId>52QI2A0IsdVSqEG3xKXt3htw8VcWaiLESfUMHEzWtjnV5NXS5TSHztm5Ft4+sriBwbmwqtISAec=</HostId></Error><?xml version="1.0" encoding="UTF-8"?>
<Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message><Key>dist/rust-2014-08-07-x86_64-apple-darwin.tar.gz.sha256</Key><RequestId>E31D6EBAFFDFB41F</RequestId><HostId>ytjJGJa7kqx+2+tCtvTbErqkLZ2it/lYFkZHBrl63xP1igjtTY30wIDtIkrolht9CWwxlO3bzzs=</HostId></Error>

@the-kenny

don't know what to expect. can you advice? we also need the depsSha256 which might be different and which requires me to build the expression. so how to build it with i686 as platform?

@the-kenny
Copy link
Contributor

You can easily build it for i686 by using pkgs.i686Linux. I'll have a look at the script in a minute.

@the-kenny
Copy link
Contributor

Uh, why do you need to run the script for this PR anyway? As far as I see everything's fine. Or are you trying to update rustcNightlyBin?

@the-kenny
Copy link
Contributor

the-kenny commented Nov 29, 2016

Merged with minor changes as 8806344 🍻

@the-kenny the-kenny closed this Nov 29, 2016
Copy link
Member Author

@qknight qknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked all the points we were talking about:

  • added i686 support on rustNightlyBin
  • reverted 'rm -f' to rm || true'
  • removed hardcoded 'channel'

@qknight
Copy link
Member Author

qknight commented Nov 29, 2016

you were too fast, here is the code you wanted to have:

  platform = 
    if stdenv.system == "i686-linux"
      then "i686-unknown-linux-gnu"
    else if stdenv.system == "x86_64-linux" 
      then "x86_64-unknown-linux-gnu"
    else abort "missing boostrap url for platform ${stdenv.system}";

  bootstrapHash =
    if stdenv.system == "i686-linux"
      then "0c1ym0pr6j9ymskdqhsy0c3llky3s5azs9wls1h6spcisjfn9qc3"
    else if stdenv.system == "x86_64-linux"
      then "1hsvf1vj18fqxkqw8jhnwahhk2q5xcl5396czr034fphmp5n4haw"
    else throw "missing boostrap hash for platform ${stdenv.system}";

@qknight
Copy link
Member Author

qknight commented Nov 29, 2016

thanks for merging!

@sjmackenzie
Copy link
Contributor

Great stuff! Very good to see!

@the-kenny
Copy link
Contributor

the-kenny commented Nov 29, 2016 via email

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 29, 2016

Wait, was rustcBinary added in a previous commit? I am less sure about that part, if only because its missing many of the features of https://github.com/solson/rust-nightly-nix

@qknight
Copy link
Member Author

qknight commented Nov 29, 2016

@Ericson2314
i didn't understand quite what rust-nightly-nix of yours does. i only need rustc nightly but if your script fits into nixpkgs it might be interesting.

@sjmackenzie what do you think?

@the-kenny
Copy link
Contributor

@Ericson2314 rust-nightly-nix uses builtins.fetchurl to fetch todays version of rust-nightly whic is inherently impure. Therefore I don't think it belongs into nixpkgs. The version from @qknight just provides a binary for a fixed binary redistributable of the compiler, which is pure.

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 29, 2016

@the-kenny That's true. Though I believe using pre-built binaries except for bootstrapping is also discouraged.

My original idea was to share code between just the patchelf part of rust-nightly-nix and nixpkgs. In my WIP branch https://github.com/solson/rust-nightly-nix/tree/stable, you can see that patchBin, bootstrap.nix, and now nightlyBin.nix are all basically the same. I'd want to replace patchBin with something like import <nixpkgs/pkgs/development/compilers/rust/patchBin.nix>

@the-kenny
Copy link
Contributor

@Ericson2314 that sounds good to me :) The less duplicated code the better. 👍

@sjmackenzie
Copy link
Contributor

sjmackenzie commented Nov 30, 2016

@sjmackenzie what do you think?

@qknight That's some pretty neat nix code by @Ericson2314, and I learned a thing or two. Ultimately anything that provides an easy to maintain (optionally hydra compiled) binary for rust nightly is my solution, ideally whatever is created is of such a caliber that @Ericson2314 and everyone else doesn't have to write out-of-nixpkgs nix scripts for rust nightly, so many folks are doing this and it's a mess, rust nightly is a popular enough target to be supported in nixpkgs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants