Skip to content

nixos/xmonad: rename NIX_GHC env var to XMONAD_GHC#184784

Merged
sternenseemann merged 2 commits intoNixOS:masterfrom
sternenseemann:xmonad-new-env-var
Aug 4, 2022
Merged

nixos/xmonad: rename NIX_GHC env var to XMONAD_GHC#184784
sternenseemann merged 2 commits intoNixOS:masterfrom
sternenseemann:xmonad-new-env-var

Conversation

@sternenseemann
Copy link
Member

Upstream XMonad was using our xmonad patch file for their flake build to
support our nixos module. This would of course break the build upstream
if the version we patched and their master branch diverged. We
discussed that it'd make sense to upstream the environment var code.
In the process it seemed sensible to rename the NIX_GHC variable as
well, since it isn't really Nix-specific – it's just a way to set the
GHC binary to execute. This change has been implemented upstream in an
unreleased version of xmonad now – meaning we'll be able to drop the
xmonad patch soon!

This also clarifies the situation in nixpkgs a bit: NIX_GHC is easy to
confuse with the environment variable used in the ghcWithPackages
wrapper where it is used to set an alternative prefix for a GHC-wrapper
for applications trying to discover it via e.g. ghc-paths. It is an
implementation detail in this context, as it is in the case of the
xmonad module. Since they are different implementations doing different
things, different names also make sense.

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.

Upstream XMonad was using our xmonad patch file for their flake build to
support our nixos module. This would of course break the build upstream
if the version we patched and their master branch diverged. We
[discussed] that it'd make sense to upstream the environment var code.
In the process it seemed sensible to rename the NIX_GHC variable as
well, since it isn't really Nix-specific – it's just a way to set the
GHC binary to execute. This change has been [implemented] upstream in an
unreleased version of xmonad now – meaning we'll be able to drop the
xmonad patch soon!

This also clarifies the situation in nixpkgs a bit: NIX_GHC is easy to
confuse with the environment variable used in the ghcWithPackages
wrapper where it is used to set an alternative prefix for a GHC-wrapper
for applications trying to discover it via e.g. ghc-paths. It is an
implementation detail in this context, as it is in the case of the
xmonad module. Since they are different implementations doing different
things, different names also make sense.

[discussed]: NixOS@36d5761
[implemented]: xmonad/xmonad@23f36d7
@sternenseemann
Copy link
Member Author

@ofborg test xmonad

@github-actions github-actions bot added 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 2, 2022
@sternenseemann sternenseemann mentioned this pull request Aug 2, 2022
4 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 2, 2022
Copy link
Member

@ivanbrennan ivanbrennan left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍 🎉

@sternenseemann
Copy link
Member Author

@ofborg test xmonad

@bobby285271 bobby285271 added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Aug 3, 2022
@sternenseemann sternenseemann merged commit 53b33ee into NixOS:master Aug 4, 2022
@sternenseemann sternenseemann deleted the xmonad-new-env-var branch August 4, 2022 12:29
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Sep 14, 2022
We had a long standing downstream patch for XMonad allowing us to
override the ghc and xmessage binaries used. That has been upstreamed
and released in 0.17.1 and we can drop it!

* patch upstreamed in xmonad/xmonad#409
* downstream patch adjusted in NixOS#184784
sternenseemann added a commit that referenced this pull request Sep 14, 2022
We had a long standing downstream patch for XMonad allowing us to
override the ghc and xmessage binaries used. That has been upstreamed
and released in 0.17.1 and we can drop it!

* patch upstreamed in xmonad/xmonad#409
* downstream patch adjusted in #184784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants