-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
llvmPackages_{11,12,13,14}: remove stdenv override #184408
llvmPackages_{11,12,13,14}: remove stdenv override #184408
Conversation
Can you expand on how this affects x86_64? It looks conditioned by |
Due to i686 packages, eg via steam which includes some i686 stuff |
Eh, OK, the statements confused me. Especially "requires testing on i686", as that would be exactly the same binaries. |
Yeah, meant more of i686 native platform (or an i686 install) since I only tested building and using on x86_64 with i686 libs for compat. |
These are the changed packages in my system:
|
And the diff-closure:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will rebase |
5fa43ba
to
25554a5
Compare
Good question, that should've been documented - sorry... :o Seems like I manually merged #94204 but it isn't documented there either. I assumed that the default Thanks for the overdue cleanup :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me - thanks for the cleanup.
I'd improve the commit message though as it's confusing (IMO).
A minimal version could look like this (we could also update it when merging):
llvmPackages_{11,12,13,14}: remove old stdenv override for i686
This doesn't seem required anymore.
Something more verbose would of course also be nice (e.g., mention that it affects x86_64 because some packages like Steam depend on i686 packages, that it's unclear if it was ever required, etc.) but the is affecting NixOS x86_64
is confusing without context.
Will reword, thanks for the suggestion. |
This doesn't seem required anymore. (Present in NixOS x86_64 via i686 compat libs with things like wine/steam)
25554a5
to
eca87f7
Compare
OK, I think we've got enough feedback. |
See NixOS#194634 for details. PRs: - NixOS#191372 - NixOS#190936 - NixOS#82131 - NixOS#199844 - NixOS#197674 - NixOS#184408 - NixOS#193004
See NixOS#194634 for details. PRs: - NixOS#191372 - NixOS#190936 - NixOS#82131 - NixOS#199844 - NixOS#197674 - NixOS#184408 - NixOS#193004
See NixOS#194634 for details. PRs: - NixOS#191372 - NixOS#190936 - NixOS#82131 - NixOS#199844 - NixOS#197674 - NixOS#184408 - NixOS#193004
See NixOS#194634 for details. PRs: - NixOS#191372 - NixOS#190936 - NixOS#82131 - NixOS#199844 - NixOS#197674 - NixOS#184408 - NixOS#193004
See NixOS#194634 for details. PRs: - NixOS#191372 - NixOS#190936 - NixOS#82131 - NixOS#199844 - NixOS#197674 - NixOS#184408 - NixOS#193004
This doesn't seem required anymore. (Present in NixOS x86_64 via i686 compat libs with things like wine/steam)
Description of changes
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes