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

llvm7: replace patch with official upstream commit #54998

Merged
merged 1 commit into from Jan 31, 2019

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Jan 31, 2019

Hopefully this also addresses any problems enountered
with the patch used previously.

Motivation for this change

cc #54370
(although this may not fix everything, should fix Optional error)

Same goal as #54122, but uses upstream commit instead of patch from
Debian (which I believe matches upstream's first attempt to fix this as
well).

Basically this was tricky to get right on all compilers,
thankfully it seems to have been sorted out.

Fingers crossed :).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Hopefully this also addresses any problems enountered
with the patch used previously.
@dtzWill
Copy link
Member Author

dtzWill commented Jan 31, 2019

@GrahamcOfBorg build llvm_7

@hedning
Copy link
Contributor

hedning commented Jan 31, 2019

Should probably go straight to staging-next?

@dtzWill
Copy link
Member Author

dtzWill commented Jan 31, 2019

I'm happy to target staging-next, but I'm not sure I understand why? Shouldn't it go to staging, staging merges into staging-next? Anyway if it's useful there that works for me, and feel free to pick the commit as needed...

@dtzWill
Copy link
Member Author

dtzWill commented Jan 31, 2019

Aww Darwin builder didn't get anywhere near LLVM :( haha.

@vcunat
Copy link
Member

vcunat commented Jan 31, 2019

If a breakage is considered a blocker for merging staging-next to master, it's usually better to first stabilize staging-next before merging in more of other staging changes. EDIT: I don't really know how severe this particular problem is.

@dtzWill
Copy link
Member Author

dtzWill commented Jan 31, 2019

Alright, I'm not sure either. I'll merge to staging and we can pick it into staging-next if there's a need.

@dtzWill dtzWill merged commit 442a74b into NixOS:staging Jan 31, 2019
@dtzWill dtzWill deleted the fix/llvm7-optional-round-2 branch January 31, 2019 19:44
@hedning
Copy link
Contributor

hedning commented Feb 1, 2019

Rustc depends on llvm-7, so the breakage is pretty big. In particular the unstable job won't pass due to inkscape depending on rustc.

vcunat pushed a commit that referenced this pull request Feb 1, 2019
@vcunat
Copy link
Member

vcunat commented Feb 1, 2019

Picked to staging-next. LLVM 7 still isn't even a large rebuild:

Estimating rebuild amount by counting changed Hydra jobs.
    172 x86_64-darwin
   1058 x86_64-linux

@dtzWill
Copy link
Member Author

dtzWill commented Feb 12, 2019

http://lists.llvm.org/pipermail/llvm-dev/2019-February/129985.html <-- 7.1.0 will include this :).

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

Successfully merging this pull request may close these issues.

None yet

5 participants