emacs: fix default values of withFoo flags after overrideAttrs #365784
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the normal way to override src and version led to wrong
default values of withFoo flags.
Here is an minimal reproducible example for withXwidgets which should
default to false when version >= 30.
```console
$ nix eval --include nixpkgs=$PWD --impure --expr '
let pkgs = import <nixpkgs> { config = { }; overlays = [ ]; };
in (pkgs.emacs29-pgtk.overrideAttrs { version = "30"; }).withXwidgets
'
true
```
This fix keeps backward compatibility for the .override interface.
Fixes: nix-community/emacs-overlay#455
nix-owners
bot
requested review from
adisbladis,
AndersonTorres,
matthewbauer and
ttuegel
jian-lin
commented
Dec 17, 2024
| withGlibNetworking ? withPgtk || withGTK3 || (withX && withXwidgets), | ||
| withGpm ? stdenv.hostPlatform.isLinux, | ||
| withImageMagick ? lib.versionOlder version "27" && (withX || withNS), | ||
| withNativeCompilation ? throw "dummy value for backward compatibility, should never be evaled", |
There was a problem hiding this comment.
Probably we need a better way to handle this.
The indention of keeping this optional flags is to keep backward compatibility for the .override interface: emacs29-pgtk.override { withXwidgets = false; }.
Comment on lines
+544
to
+558
| assertions = | ||
| assert | ||
| (finalAttrs.withGTK3 && !finalAttrs.withNS && variant != "macport") | ||
| -> finalAttrs.withX || finalAttrs.withPgtk; | ||
| assert | ||
| finalAttrs.noGui | ||
| -> !(finalAttrs.withX || finalAttrs.withGTK3 || finalAttrs.withNS || variant == "macport"); | ||
| assert finalAttrs.withAcl -> stdenv.hostPlatform.isLinux; | ||
| assert finalAttrs.withAlsaLib -> stdenv.hostPlatform.isLinux; | ||
| assert finalAttrs.withGpm -> stdenv.hostPlatform.isLinux; | ||
| assert | ||
| finalAttrs.withNS -> stdenv.hostPlatform.isDarwin && !(finalAttrs.withX || variant == "macport"); | ||
| assert finalAttrs.withPgtk -> finalAttrs.withGTK3 && !finalAttrs.withX; | ||
| assert finalAttrs.withXwidgets -> !finalAttrs.noGui && (finalAttrs.withGTK3 || finalAttrs.withPgtk); | ||
| true; # dummy value to make syntax right |
There was a problem hiding this comment.
This works but maybe we can improve it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the normal way to override src and version led to wrong default values of withFoo flags.
Here is an minimal reproducible example for withXwidgets which should default to false when version >= 30.
This fix keeps backward compatibility for the .override interface.
Fixes: nix-community/emacs-overlay#455
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.