divnix / digga

A flake utility library to craft shell-, home-, and hosts- environments.
https://digga.divnix.com
MIT License
1k stars 108 forks source link

`emacs-overlay` cause errors #464

Closed a12l closed 2 years ago

a12l commented 2 years ago

Expected Behavior

When you add inputs.emacs-overlay.url = "github:nix-community/emacs-overlay/e47ffb5d60d8e8271d412945c685dbeac2fca7a4"; to your flake.nix file and rebuild your configuration, Emacs is downloaded and added to your host's /nix/store.

This behavior is demonstrated when you add inputs.emacs-overlay.url = "github:nix-community/emacs-overlay/334ba8c610cf5e41dfe130507030e5587e3551b4"; to your flake.nix file instead and rebuild your configuration. This is the last working commit.

Current Behavior

When you add inputs.emacs-overlay.url = "github:nix-community/emacs-overlay/e47ffb5d60d8e8271d412945c685dbeac2fca7a4"; to your flake.nix file you get the following error when you try to build the configuration

$ nixos-rebuild build --flake .#
building the system configuration...
error: attribute 'optionalAttrs' missing

       at /nix/store/4rkachpqaswh6bkbcvc02sv72i2nsl4a-source/default.nix:180:6:

          179|
          180| } // super.lib.optionalAttrs (super.config.allowAliases or true) {
             |      ^
          181|   emacsGcc = builtins.trace "emacsGcc has been renamed to emacsNativeComp, please update your expression." emacsNativeComp;
(use '--show-trace' to show detailed location information)

And if you try to install Emacs, e.g. by creating a emacs.nix profile with the following lines; adding that profile to one of your suits; and create an empty init.el file in the same directory as the profile

{pkgs, ...}: {
  home.packages = [
    (pkgs.emacsWithPackagesFromUsePackage { config = ./init.el; })
  ];
}

you get this error

$ nixos-rebuild build --flake .#
error: value is null while a set was expected

       at /nix/store/a5kpkzkipwzqpxadnlpxbbkq9zgk4r8a-source/default.nix:180:6:

          179|
          180| } // super.lib.optionalAttrs (super.config.allowAliases or true) {
             |      ^
          181|   emacsGcc = builtins.trace "emacsGcc has been renamed to emacsNativeComp, please update your expression." emacsNativeComp;
(use '--show-trace' to show detailed location information)

Possible Solution

Steps to Reproduce

  1. Add inputs.emacs-overlay.url = "github:nix-community/emacs-overlay/e47ffb5d60d8e8271d412945c685dbeac2fca7a4"; to your flake.nix.

Context

@adisbladis gave this example demonstrating that no bug was introduced with e47ffb5d60d8e8271d412945c685dbeac2fca7a4.

{
  description = "Foo";
  inputs = {
    emacs-overlay.url = "github:nix-community/emacs-overlay/e47ffb5d60d8e8271d412945c685dbeac2fca7a4";
    emacs-overlay.inputs.nixpkgs.follows = "nixpkgs";
    nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
  };
  outputs = { self, nixpkgs, emacs-overlay }: let
    pkgs = import nixpkgs {
      system = "x86_64-linux";
      overlays = [
        emacs-overlay.overlay
      ];
    };
  in {
    packages.x86_64-linux.default = pkgs.emacs;
  };
}

This downloads and installs Emacs without any errors for me.

Your Environment

Pacman99 commented 2 years ago

@blaggacao @gytis-ivaskevicius Because of issues like these, I'm starting to think we should just stop auto-exporting overlays. Its a really finicky process that doesn't seem to work well. Specifically for digga, but related to fup, we could switch to a similar exporting model as modules, exportOverlays and overlays. And then for packages output, we can force exportOverlays to follow a specific format.

Pacman99 commented 2 years ago

And for general context, this issue happens because fup's overlay exporting function has to read what the attributes are in the overlay to figure out how to export them. This causes issues when overlays have any code on the "outside" of the attrset like prev.lib.optionalAttrs, because we can't evaluate attribute names.

ParetoOptimalDev commented 2 years ago

I'm affected by this one as well.

Worth noting that I had no idea what this error meant:

error: attribute 'optionalAttrs' missing

       at /nix/store/8kc9an5l1pcgw6g10z1c9fwb82nc8nyj-source/default.nix:180:6:

          179|
          180| } // super.lib.optionalAttrs (super.config.allowAliases or true) {
             |      ^
          181|   emacsGcc = builtins.trace "emacsGcc has been renamed to emacsNativeC
omp, please update your expression." emacsNativeComp;
(use '--show-trace' to show detailed location information)

Which is a symptom of upstream issue NixOS/nixpkgs#167893 and linking this downstream example of that bad error message might be useful.

bandithedoge commented 2 years ago

I am also affected by this issue, although it does not come from including overlays but from adding an input. For example, adding github:taffybar/taffybar as an input, even without adding its overlay to my config, a rebuild throws this error:

error: value is a list while a set was expected

       at /nix/store/xa034labcji67siyx0w1aq70xbrhsf7g-source/lib/exportOverlays.nix:84:101:

           83|       inputOverlays = mapAttrs
           84|         (_: v: (if isOverlay (v.overlay or null) then [ v.overlay ] else [ ]) ++ (filter isOverlay (attrValues (v.overlays or { }))))
             |                                                                                                     ^
           85|         (removeAttrs inputs [ "self" ]);
(use '--show-trace' to show detailed location information)

Somehow this happens whenever I add any new input, without even using it anywhere. Due to how ridiculous this sounds I'm pretty sure there is an issue with my config, but maybe someone can suggest a fix. My flake.nix can be found here.

btobolaski commented 2 years ago

This is probably not the right spot for this but, can someone tell me how to get emacs-overlay working with digga?

linyinfeng commented 2 years ago

This is probably not the right spot for this but, can someone tell me how to get emacs-overlay working with digga?

If your configuration using emacs-overlay is broken because of this issue, you could try my workaround: wrap the overlay in a very simple "protector". https://github.com/linyinfeng/dotfiles/blob/d92f5ddd6b7bad676e331c2157d625ad7d5e81ac/flake.nix#L113-L121 Here is the definition of the "protector": https://github.com/linyinfeng/dotfiles/blob/d92f5ddd6b7bad676e331c2157d625ad7d5e81ac/lib/overlay-null-protector.nix

montchr commented 2 years ago

As @GTrunSec pointed out in https://github.com/divnix/digga/pull/469#issuecomment-1142433850 the cause is most likely the way we try to automagically detect whether an overlay should include the additional channels argument. The change to emacs-overlay demonstrates that this approach doesn’t hold up in its current form.

I took a crack at a fix (not yet pushed) but it would require deprecating the current approach and adding a new arbitrarily-named externalOverlays option to add overlays the “normal” way. That doesn’t feel quite right, like another workaround to support multiple possible shapes of an overlay. So I thought about about alternatives to make the approach to overlays less fragile.

My current thinking is that we should do away with the “optional” channels argument to overlays altogether, and instead move towards selecting packages as needed from other channel inputs by way of legacyPackages without the use of overlays. This would of course be a breaking change, but I think it would be beneficial in the long run by making things less confusing.

I’ll put together a new PR today to test this out.

Pacman99 commented 2 years ago

Yeah I'm for anything that generally moves us away from overlays to accomplish things. I think overlays are a bad design decision from nixpkgs itself, not that nixpkgs has a choice.

We should generally push to using input packages directly instead of loading them into your channel.

This bug has surfaced some problematic design decisions originally made within digga.

a12l commented 2 years ago

We should generally push to using input packages directly instead of loading them into your channel.

Could you expand on what that means?

Pacman99 commented 2 years ago

For example if you want to add agenix to your environment, I think this would be better than adding agenix to your overlay.


{ pkgs, inputs, ... }: {
environment.systemPackages = [ inputs.agenix.packages.${pkgs.system} ];
}
montchr commented 2 years ago

@a12l @ParetoOptimalDev @bandithedoge @btobolaski @linyinfeng

With #483 merged, this issue should be fixed. If you revert any workarounds you've added to your configs and then run nix flake lock --update-input digga, are you able to rebuild your config with emacs-overlay included?