luaPackages.vicious: Missing Dependencies

[AmeerTaweel]

Describe the bug

Vicious is a widget library, and some of its widgets have dependencies on other programs to work. None of these dependencies are specified in the current package definition. Personally, I use the volume widget, which depends on alsa-utils. The widget, of course, does not work because alsa-utils is not specified as a runtime dependency.

Steps To Reproduce

1. Add luaPackages.vicious as a Lua module for AwesomeWM:
   xsession.windowManager.awesome.luaModules = [ pkgs.luaPackages.vicious ]
2. Try to use the volume widget in your AwesomeWM config.
3. It will not work.

Expected behavior

The volume widget (and other widgets) should have their dependencies declared, and work properly.

Notify maintainers

@makefu
@Mic92

Metadata

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 5.10.94, NixOS, 22.05 (Quokka)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.7.0pre20220124_0a70b37`
 - channels(root): `"nixos-21.11"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

[makefu]

thanks for your bug report, is adding alsa-utils to either system or user packages enough for the widget to work properly?

[AmeerTaweel]

@makefu before adding alsa-utils in my config, the widget did not appear in the first place. Now that I added it, it does appear, which is a step forward. However, I get a nil value for the volume.

I'm using the latest NixOS, and there is a problem currently with amixer. You can read about the issue here. The pull request was merged to master, but still did not land in nixpkgs, as the merge happened only 3 days ago.

However, I understand that you cannot just add the change without verifying that it works.

I can wait until I get the fix for amixer, and test it and notify you.

[yipengsun]

I feel vicious probably should not include such runtime dependencies. For example, I'm using vicious with pulseaudio, so if alsa-utils is declared a dependency, I'd get a copy of alsa-utils which is not very useful to me.

In this case maybe just list alsa-utils to your config?

[AmeerTaweel]

@yipengsun I was trying to explain why we need to add all the dependencies, but I just noticed that you are a contributor! I cannot believe that a contributor to nixpkgs says such a thing.

Also, you don't have to get an extra copy, but even if you do, it's not a big deal, it's not like alsa-utils is 3 gigabytes.

[yipengsun]

OK, I guess maybe I should say, alsa-utils can be treated as is an optional dependency that maybe default to true, and if someone doesn't need it, then he/she can disable that in an overlay.

@AmeerTaweel Is that a reasonable compromise?

Edit: Not can be, is

[yipengsun]

Well, pulseaudio actually depends on alsa-lib, so I already got my copy of alsa-utils. Then I have no more objection, and indeed, alsa-utils should be a dependency, not an optional one.

[yipengsun]

Question: vicious has other "runtime" dependencies like hddtemp (that I don't use). How to add it as a dependency such that for users don't use it, they can disable it in an overlay without overriding the whole package?

[AmeerTaweel]

@yipengsun I started using Nix/NixOS just two weeks ago, so I have no idea how to write package definitions in such a way that makes them customizable by the user. So I don't know how it's done technically, but I kinda agree with you.

Most users don't use all of the widgets offered by vicious, so it makes sense to have all of them available by default, but still being able to turn them off if you want.

[yipengsun]

If vicious is not a lua package, we can do something like this:

{ pkgs, ... }:

# pretend viciousFake is just a regular package

let
  viciousFakeWrapped = pkgs.symlinkJoin {
    name = "vicious-wrapped";
    paths = [ pkgs.awesome pkgs.viciousFake pkgs.alsa-utils ];
    buildInputs = [ pkgs.makeWrapper ];
    postBuild = ''
      for prog in awesome; do
        wrapProgram "$out/bin/$prog" --set PATH "$out/bin"
      done
    '';
  };
in

{
  environment.systemPackages = [ viciousFakeWrapped ];
}

But I don't think that works in this case (you can always wrap awesome
itself, but then vicious would still have missing runtime dependencies).

For me, I also use lain, which requires lua 5.3, so I have to add it
in an overlay:

final: prev: {
  lua5_3 = prev.lua5_3.override {
    packageOverrides = luafinal: luaprev: {
      lain = luaprev.toLuaModule (prev.stdenv.mkDerivation {
        pname = "lain";
        version = "unstable-20210925";

        src = prev.fetchFromGitHub {
          owner = "lcpz";
          repo = "lain";
          rev = "4933d6";
          sha256 = "NPXsgKcOGp4yDvbv/vouCpDhrEcmXsM2I1IUkDadgjw=";
        };

        buildInputs = [ luaprev.lua ];

        installPhase = ''
          mkdir -p $out/lib/lua/${luaprev.lua.luaversion}/
          cp -r . $out/lib/lua/${luaprev.lua.luaversion}/lain/
          printf "package.path = '$out/lib/lua/${luaprev.lua.luaversion}/?/init.lua;' ..  package.path\nreturn require((...) .. '.init')\n" > $out/lib/lua/${luaprev.lua.luaversion}/lain.lua
        '';
      });
    };
  };
}

But I'm sure how to add these optional dependencies (like hddtemp) to
vicious and make them user-customizable.

(The idea is: I want to get updates from nixpkgs so I don't need
to update them myself, but I also want to turn off some of the optional
features that the vicious may provide easily in an overlay).

[yipengsun]

Actually, toLuaModule is defined as the following:

  toLuaModule = drv:
    drv.overrideAttrs( oldAttrs: {
      # Use passthru in order to prevent rebuilds when possible.
      passthru = (oldAttrs.passthru or {}) // {
        luaModule = lua;
        requiredLuaModules = requiredLuaModules drv.propagatedBuildInputs;
      };
    });

So can we extend that to something like:

toLuaModule = drv: attrs: ? {} {
  # ...
}

So we can optionally override the inputs to lua packages themselves?

[McSinyx]

Most users don't use all of the widgets offered by vicious, so it makes sense to have all of them available by default

This reasoning isn't sound, it only makes sense if most people use most of the widgets. E.g. cmus and notmuch would be unnecessarily pulled. Since Vicious is a Lua module, build time is marginal, I strongly suggest disabling optional dependencies by default.

[yipengsun]

For now, I think the problem is: We can't specify optional dependencies for lua packages such that they can be turned on/off easily in an overlay, at least to my knowledge.

By easy, I mean something like this:

final: prev: {
  lua5_3 = prev.lua5_3.override {
    packageOverrides = luafinal: luaprev: {
      vicious = luaprev.toLuaModule prev.lua5_3.packages.vicious {
        enableAlsaUtils = false;
        enableCmus = true;
        enableNotmuch = true;
      }; 
    };
  };
}

In this case I'd say some of the optional dependencies can be on by default, like alsa-utils (If you use pulseaudio, you probably already have it), some like cmus and notmuch can default to off indeed.

[AmeerTaweel]

Most users don't use all of the widgets offered by vicious, so it makes sense to have all of them available by default

This reasoning isn't sound, it only makes sense if most people use most of the widgets. E.g. cmus and notmuch would be unnecessarily pulled. Since Vicious is a Lua module, build time is marginal, I strongly suggest disabling optional dependencies by default.

Totally makes sense. I also agree.

[AmeerTaweel]

@makefu now that the fix has been merged, I updated my flake, and indeed adding alsa-utils to my config solves the problem, and the volume widget works.