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

rustc: add QA documentation #73296

Merged
merged 1 commit into from Dec 20, 2019
Merged

rustc: add QA documentation #73296

merged 1 commit into from Dec 20, 2019

Conversation

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Nov 12, 2019

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@Mic92 Mic92 requested a review from LnL7 as a code owner Nov 12, 2019
@Mic92
Copy link
Contributor Author

@Mic92 Mic92 commented Nov 12, 2019

@@ -1,3 +1,8 @@
# After updating check ...
# 1. if rustc can produce rust binaries on x86_64-linux, aarch64-linux and x86_64-darwin:

This comment has been minimized.

@edolstra

edolstra Nov 12, 2019
Member

Don't really agree with this. For example, I have no ability to test on aarch64 or macOS.

This comment has been minimized.

@Mic92

Mic92 Nov 12, 2019
Author Contributor

@GrahamcOfBorg usually can build this in time. For aarch64 we have a beefy community builder: https://github.com/nix-community/aarch64-build-box
which I have access to and usually try to build upgrades there and post the result back to the pr thread. For macOS alternatively I have a bunch of nixpkgs maintainer that I can ping instead. We followed this pattern for a while until you started merging rust upgrades without testing.

@@ -1,3 +1,8 @@
# After updating check ...
# 1. if rustc can produce rust binaries on x86_64-linux, aarch64-linux and x86_64-darwin:
# i.e. nix build -f . fd or @GrahamcOfBorg build fd on github

This comment has been minimized.

@edolstra

edolstra Nov 12, 2019
Member

What does fd mean? And to what directory does -f . refer?

This comment has been minimized.

@Mic92

Mic92 Nov 12, 2019
Author Contributor

fd is just a simple rust example that should build on all platforms.

# 1. if rustc can produce rust binaries on x86_64-linux, aarch64-linux and x86_64-darwin:
# i.e. nix build -f . fd or @GrahamcOfBorg build fd on github
# 2. if our LLVM version matches with rust upstream
# 3. if firefox or thunderbird still build on x86_64-linux

This comment has been minimized.

@edolstra

edolstra Nov 12, 2019
Member

What are the implications of this? E.g. if firefox fails to build, should we simply refrain from updating Rust?

This comment has been minimized.

@Mic92

Mic92 Nov 12, 2019
Author Contributor

Yes. Because this is a channel blocker. A way to fix it in that case is to to pin rust to an older version for firefox. However breaking firefox in unstable is undesired.

@Mic92 Mic92 force-pushed the Mic92:rustc branch from 9c1a503 to 7b935d5 Nov 12, 2019
@Mic92
Copy link
Contributor Author

@Mic92 Mic92 commented Nov 12, 2019

Other usual suspects in rust land: cc @symphorien @jD91mZM2 @zimbatm

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Nov 12, 2019

Is there any particular reason why rustc updates have been merged to master?

@Mic92
Copy link
Contributor Author

@Mic92 Mic92 commented Nov 12, 2019

Yeah. Maybe I should add this as well, we usually prefer to merge rustc updates to staging.

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Nov 12, 2019

Yeah. Maybe I should add this as well, we usually prefer to merge rustc updates to staging.

Thanks, we should be doing this always. I can recall at least two occasions where this has taken up my time because there were urgent failures on master because of a rustc update. When it's on staging it isn't as urgent and we can fix it during its workflow.

@Mic92 Mic92 force-pushed the Mic92:rustc branch from 7b935d5 to 610c9c7 Nov 13, 2019
@Mic92 Mic92 force-pushed the Mic92:rustc branch from 610c9c7 to d2d1c53 Nov 13, 2019
@Mic92
Copy link
Contributor Author

@Mic92 Mic92 commented Nov 13, 2019

It now mentions, that rust updates should go to staging first and that one can ask other people for help to test rust on other platforms. In my experience this testing takes less than a week depending on how fast people respond but it is worth it to avoid unnecessary breakages in master.

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Nov 13, 2019

It now mentions, that rust updates should go to staging first and that one can ask other people for help to test rust on other platforms. In my experience this testing takes less than a week depending on how fast people respond but it is worth it to avoid unnecessary breakages in master.

Oh, I know this my be rather nitpicky, but maybe just having a README.md or QA-Instructions.md in the dir would be better?

@jD91mZM2
Copy link
Member

@jD91mZM2 jD91mZM2 commented Nov 13, 2019

I think it makes sense to have it inline, as you can't possibly avoid opening that file when updating rust. I'm normally somewhat of an expert at failing to read documentation ;)

@LnL7
Copy link
Member

@LnL7 LnL7 commented Nov 13, 2019

Until recently we've only kept a single version of rust around. With 2 or more versions we could keep the default on a version compatible with firefox as well as make introducing versions much less intrusive. Similar to what we do for most other compilers/interpreters.

@Mic92 Mic92 merged commit d68c70f into NixOS:master Dec 20, 2019
15 checks passed
15 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
rustc on aarch64-linux Success
Details
rustc on x86_64-darwin Success
Details
rustc on x86_64-linux Success
Details
@Mic92 Mic92 deleted the Mic92:rustc branch Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.