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

llvmPackages_{11,12,13,14}: remove stdenv override #184408

Merged
merged 1 commit into from
Nov 27, 2022

Conversation

fabianhjr
Copy link
Member

@fabianhjr fabianhjr commented Aug 1, 2022

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
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@vcunat
Copy link
Member

vcunat commented Nov 18, 2022

Can you expand on how this affects x86_64? It looks conditioned by .isi686.

@fabianhjr
Copy link
Member Author

Due to i686 packages, eg via steam which includes some i686 stuff

@vcunat
Copy link
Member

vcunat commented Nov 18, 2022

Eh, OK, the statements confused me. Especially "requires testing on i686", as that would be exactly the same binaries.

@fabianhjr
Copy link
Member Author

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.

@fabianhjr
Copy link
Member Author

These are the changed packages in my system:

these 58 derivations will be built:
  /nix/store/bv5iwlwas3rwj0qaq8q1rsp96nm0wvv7-nvidia-x11-520.56.06-6.0.9.drv
  /nix/store/90lqfjgxnh42ssyxk7fdypmh7ldb5sxn-nvidia-settings-520.56.06.drv
  /nix/store/c1f3sdqnkgrgjjjn6hhpp2bai1pnlykm-steam-run-usr-target.drv
  /nix/store/cmw4p2736xd6sj7xplgk9878dvxzz234-nvidia-vaapi-driver-0.0.7.drv
  /nix/store/klppflhnwa16jnh9h4jd1z81wjzlhfrz-steam-run-usr-multi.drv
  /nix/store/qydrkhp485j0iql3njq6ifia7q69k7i4-steam-run-fhs.drv
  /nix/store/vwnzsnqf3ai3pg4sp9xk05bgfjw3wfvx-steam-run.drv
  /nix/store/h8dr5m1wsgdhvp6s7y15cwjnx9krqy6g-steam-run.drv
  /nix/store/qa67zyl18f5d5cq56z2612f1499axh28-steam-usr-multi.drv
  /nix/store/xh5ycqjhr2p4a5bqava4bqi3lxb7qha1-steam-usr-target.drv
  /nix/store/3g6fl5ffhyyr7cjd5hxjli7klp3sblqd-steam-fhs.drv
  /nix/store/5cm1zihc0kcxf1jhjf43ygvmq17wv0a4-steam.drv
  /nix/store/v2f3qdg03z31ybhk7ahkvafbmcxbj7j4-steam.drv
  /nix/store/zvjgnclm7jqlh1pqp2snszw8rysqia0c-nixos-version.drv
  /nix/store/yi6dk0gl48s5y6fvb5k607nkm7r2xb97-system-path.drv
  /nix/store/v6dbhj4zwqlpp222v1lrjg8vji4i8q5z-set-environment.drv
  /nix/store/cvypp2pk3hf2sxbcrywm8mkza6zb7f3d-etc-profile.drv
  /nix/store/hl68bycvijbprmy8myszn5bdwcwl430b-abstractions-bash.drv
  /nix/store/fkyrwgrn9wdrv4idvv2rgamhi8n6kf10-apparmor.d.drv
  /nix/store/0l4zn7wpx1i18gm7yc32bdm2p6cq3x91-unit-apparmor.service.drv
  /nix/store/1czdqnbzmkvq2lz33hkaxb0sa32by480-dbus-1.drv
  /nix/store/pzrq808vs34d11j505m7xczblnxlf58r-linux-6.0.9-modules.drv
  /nix/store/dhj0lqi1kbry883lacngy1f057a4vah5-linux-6.0.9-modules-shrunk.drv
  /nix/store/1vk6r1p2sy770fcs9shjigldajr94a2q-stage-1-init.sh.drv
  /nix/store/1yvpjjzvqy5svd4jam5zf8ksfj0by8rk-nixos-version_fish-completions.drv
  /nix/store/76c2fhvykx5zz58vnmv7369wngm6z1qg-opengl-drivers-32bit.drv
  /nix/store/dwm3vzqpmwwbj43zn6c0pija5wsd086q-opengl-drivers.drv
  /nix/store/npinrsgv7ncvsbs1vcibfsifyyh33c1l-nixos-tmpfiles.d.drv
  /nix/store/62jxy3cziwjmkim5wqqpqfdgz2vn3km2-tmpfiles.d.drv
  /nix/store/ghnl1a57gdnljd9fn18658qpw0kwl71c-man-paths.drv
  /nix/store/791vgdbpkajm1a36l6iwih8vfkbkmw8z-man-cache.drv
  /nix/store/g0b5akg4c1pa9bmkkkhkq27vrlzwpr60-kernel-modules.drv
  /nix/store/xl3zvir76wv3aaqf25akhvh86c6n6w90-initrd-linux-6.0.9.drv
  /nix/store/9772hlx0c2bqfdw198yhy7r5db090289-etc-nixos-env-preinit.fish.drv
  /nix/store/b2hn8sr2z6ys2yxz18gjpx9jd0frc6b7-system-generators.drv
  /nix/store/f3c25iys8ag83fdkivghdci1hd4n09pp-etc-man_db.conf.drv
  /nix/store/gda7rgh1dlqrqqfksg5gakh88ghbnqw4-etc-49-pipewire-modules.conf.drv
  /nix/store/f84bh3z1pfh867pjksv8l7xih5iasrjs-unit-dbus.service.drv
  /nix/store/icl23v5xvgs3innwss9yni1r81229rqg-user-units.drv
  /nix/store/k8yrwsgm14dny077xx3sxfy11is9yq8s-etc-environment.drv
  /nix/store/lqvr5bxhvsfpm3gq3sr2xkv7cckpqgcv-steam_fish-completions.drv
  /nix/store/skn7njfq5ib8n4lhwrfmi7fihnxkl5ak-nvidia-settings-520.56.06_fish-completions.drv
  /nix/store/zfail4b1zpsjp8im107823nl611pl1d7-nvidia-x11-520.56.06-6.0.9_fish-completions.drv
  /nix/store/zngrdsqw708076rjbv9633bkz8hs5bs0-steam-run_fish-completions.drv
  /nix/store/nq4im3hpwv32089gr72m688n0pybdv1s-system_fish-completions.drv
  /nix/store/rv2qs5h00qb0zhi2i1n7s3js4c1jil7d-system-shutdown.drv
  /nix/store/9aibawsrpb3fc8g8crx7rb0vy04x7ga0-unit-nvidia-resume.service.drv
  /nix/store/ivprvjcwimlxydgvdjl3k442cl4farcq-unit-dbus.service.drv
  /nix/store/jr2s7qjz7arbg8psc38axszyn4vkpf3v-xserver.conf.drv
  /nix/store/izrd86nlyq26qn8wvq3ydbzmy7aka9y5-unit-display-manager.service.drv
  /nix/store/lppxk6vzwrq72xvbkqvgd34gc5y32w4l-unit-nvidia-hibernate.service.drv
  /nix/store/ncqqwvhnpxllbi14b03g5rsq6lfcqbyr-unit-polkit.service.drv
  /nix/store/ql41a84mh11nkm5wz1kf07w7im5fh9xr-unit-nvidia-suspend.service.drv
  /nix/store/yspb0i7k7j3v0c26w5w42956aj0q4i3h-unit-accounts-daemon.service.drv
  /nix/store/yy9f9rzg0smg3bsp027a9va77v6b3s0v-unit-systemd-fsck-.service.drv
  /nix/store/z5qcacv3zdr84p5cvwvl1x8ykkp8iq8c-system-units.drv
  /nix/store/zdqi6ins8yyq86dylqpicn71bifb7scn-etc.drv
  /nix/store/82h93jmq5fzcb6bsbph37b0f7xy1frb5-nixos-system-fabian-tower-22.11.git.422a341fc11.drv
these 41 paths will be fetched (196.47 MiB download, 877.79 MiB unpacked):
  /nix/store/19iscd971ysi022ds6897n2d3wjj1dsl-cups-2.4.2-man
  /nix/store/1a1xg1nnqbv3p9x11i06mnzfvamayhs7-libdecor-0.1.0
  /nix/store/1cn1cfn3daj2rws8zz2r9d0hyr4m8y62-libappindicator-gtk2-12.10.1+20.10.20200706.1
  /nix/store/2wayhi6yh311c5iisls0g3imbjxvazf0-opencv-4.6.0
  /nix/store/37dqlqs1w5m6q9daznfsglyvd2s0160j-tracker-3.4.1
  /nix/store/3bz8nr928l2qqfq7r88xa30kb78fisqx-qrencode-4.1.1
  /nix/store/4jylnrv7zghi5lj49k2zzrhqajis9ajs-libdbusmenu-gtk2-16.04.0
  /nix/store/6b0iq1vwclpxbsl0c4zc8s38bsxbiqmj-llvm-14.0.6-lib
  /nix/store/7plxmy7fx7xj4l3pra9xv0pfzbl1q40q-SDL2_ttf-2.20.1
  /nix/store/7w69vrzqkkgwn8lw9rmw4jp8y6p3kgg9-cups-2.4.2
  /nix/store/7x7xrnkgl3hvb3vvsncrhi3ny1klvgrq-pipewire-0.3.60
  /nix/store/84bi9ca6v09vnf95j94jzdbary2cnvig-llvm-14.0.6
  /nix/store/avxxy2cg8czyl6drxpjrqhbsdybmsgq7-systemd-251.7-man
  /nix/store/b4w7gln78nwna48k12yzlylrpjq4j2l2-ffmpeg-4.4.2
  /nix/store/bay9lzpz2y16j0gh4im01xf93qs2867c-gst-plugins-bad-1.20.3
  /nix/store/cajn6yq8hqf0g7zvnlk04z46qicfv6z1-ffmpeg-4.4.2-man
  /nix/store/cygzk2asw2icksmwaxkjppd5yphcx9bl-smpeg-0.4.5
  /nix/store/f46nssmj5piqq05b08h4lc3rixny9k8w-libudev0-shim-1
  /nix/store/f9bif7hiv9al1w4rypkxj5flaj4vcb3z-gtk+3-3.24.34
  /nix/store/fm8s7pppncjz9jgzpwvgl01650hgffil-ffmpeg-4.4.2-bin
  /nix/store/fryb8ccfqzk23mqqmgkr7b3g0lchbzgq-libcamera-0.0.1
  /nix/store/ghw7clw76lrfmgvcwhs0dn36i731zxbc-mesa-22.2.3-drivers
  /nix/store/hl2n3q8ryqni64lkivgr0p01arw2qfvq-libindicator-gtk2-12.10.1
  /nix/store/i6pfspjpcwrphh19xm2ygrax1z6q7qj6-SDL2-2.24.2
  /nix/store/jnk0c05s11xpjnjj2xpwjr436zm7zjx6-SDL2-2.24.2
  /nix/store/jrjr3a0q5y1253493rf78vid63rhjg4f-openal-soft-1.22.2
  /nix/store/kjmxc0sqjj0p4hb5gfh6bzh4ls15df4y-gtk+-2.24.33
  /nix/store/lamfjmxcgsrk0nci94p6a55b2qkdsfdn-bluez-5.65
  /nix/store/lvqb3bpqf8czka7vyhw88s908pdp0ifd-pipewire-0.3.60-lib
  /nix/store/ma8lq57h072agmbjxcq67x5604m3mzki-SDL2_mixer-2.0.4
  /nix/store/mm41gfv5dcgcz36livapmsq9pv3l98iw-librsvg-2.55.1
  /nix/store/pdy5mp35p1fq91y0ks443qpi8m8m5ybh-cups-2.4.2-lib
  /nix/store/pw12ssf2fj4dhia8nxai4n9mkdxi96g3-SDL2_image-2.0.5
  /nix/store/qb20lbdy2fabrlpj2a1dg5f5rqy24pmx-chromaprint-1.5.1
  /nix/store/qrxcg8m7kmm8l4hky9bz18daf578fqrf-ffmpeg-4.4.2
  /nix/store/rigrh24vwdj5asgfx24kksfsihmramzm-gcc-7.5.0-lib
  /nix/store/rpy4rj6nwch99rg8n9ka96zvcf5h8yni-SDL_mixer-1.2.12
  /nix/store/sbmicz7p04npb7p1fg0maw9y2fpj84hw-gst-plugins-bad-1.20.3-dev
  /nix/store/w3h4ykhnjli7s2m80a8d755iy9hypdcn-libgudev-237
  /nix/store/wi8yizwrk1j6qfimiqaxggwpf0rq3wh3-mesa-22.2.3

@fabianhjr
Copy link
Member Author

And the diff-closure:

extra: ∅ → ε, +25632.2 KiB
gcc: 7.5.0 → ∅, -5664.7 KiB
initrd: ∅ → ε
initrd-kmod-blacklist: ∅ → ε
keymap: ∅ → ε
link: ∅ → ε
llvm: +5358.2 KiB
man: -8.0 KiB
mdadm.conf: ∅ → ε
stage: ∅ → 1-init.sh, +26.9 KiB

Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

I see the override introduced in 0a8334d without any comment on that; maybe @ggreif or @primeos recall why.

I checked some i686 builds and found no issues. (wine steam lutris-free caused many i686 builds of various versions of llvm and clang)

@fabianhjr
Copy link
Member Author

Will rebase

@primeos
Copy link
Member

primeos commented Nov 27, 2022

I see the override introduced in 0a8334d without any comment on that; maybe @ggreif or @primeos recall why.

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 stdenv on i686 was older and that GCC 7 was required to avoid a compilation error but as of 0a8334d the default GCC was 9 on both architectures. I guess dropping it should be fine then. Seems like I also dropped if for llvmPackages_git (#120780) / didn't copy it over.
Edit: It's of course also possible that compilation failed with GCC 9 but testing that is probably not worth it.

Thanks for the overdue cleanup :)

Copy link
Member

@primeos primeos left a 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.

@fabianhjr
Copy link
Member Author

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)
@vcunat
Copy link
Member

vcunat commented Nov 27, 2022

OK, I think we've got enough feedback.

@vcunat vcunat merged commit 94a3d6e into NixOS:master Nov 27, 2022
@fabianhjr fabianhjr deleted the llvm-remove-gcc-override branch November 27, 2022 17:05
@misuzu misuzu mentioned this pull request Jan 8, 2023
13 tasks
ElvishJerricco pushed a commit to ElvishJerricco/nixpkgs that referenced this pull request Jan 8, 2023
github-actions bot pushed a commit that referenced this pull request Jan 28, 2023
K900 pushed a commit that referenced this pull request Jan 28, 2023
@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 0 10.rebuild-linux: 101-500
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants