Overriding a nested attribute

How can I override the webapp attribute in this package: nixpkgs/pkgs/by-name/ma/mattermost/package.nix at bffc22eb12172e6db3c5dde9e3e5628f8e3e7912 · NixOS/nixpkgs · GitHub?

Since it’s nested under the main package derivation, I’m unsure how to modify it. For example, I’d like to add patches to it. I’d appreciate your insight.

Here’s what I’ve already tried and seems not working:

pkgs.mattermost.overrideAttrs (oldAttrs: {
  webapp = oldAttrs.webapp.overrideAttrs (webAttrs: {
    patches = [
      ./custom.patch
    ];
  });
});

pkgs.mattermost.webapp.overrideAttrs ( ... )

Thanks! However, using pkgs.mattermost.webapp.overrideAttrs would only build the mattermost-webapp package on its own. My goal is to override the webapp attribute while still building the complete mattermost package, so the changes are incorporated into the entire application.

Using something like:

mattermost = pkgs.mattermost.webapp.overrideAttrs ( ... )

would result in just the mattermost-webapp package, not the full mattermost app.

There’s nothing like virtual members or dynamic mocking in the Nix language, so if you want to change how something internal to a derivation is defined, there’s no easier way than overriding any parts of the derivation that use that thing to use the replacement instead.

Looking at mattermost specifically, it seems that webapp is only used in the definition of the postInstall hook, which suggests to me that something like this could work:

let
  webapp-replacement = pkgs.mattermost.webapp.overrideAttrs (oldAttrs: {
    patches = oldAttrs.patches or [ ] ++ [ ./custom.patch ];
  });
in
pkgs.mattermost.overrideAttrs (oldAttrs: {
  postInstall = builtins.replaceStrings [ pkgs.mattermost.webapp.outPath ] [ "${webapp-replacement}" ] oldAttrs.postInstall;
})

(Ideally you’d be removing the original mattermost.webapp from postInstall’s string context as well, in order to prevent it from being a build dependency, but I don’t know of an easy way to do that without discarding the entire context.)

1 Like

Did you try:

mattermost.overrideAttrs (prev: { webapp = prev.webapp.overrideAttrs (prevWebapp: {...}); })

I believe this is essentially the same as what I tried earlier, except I used different parameters (oldAttrs and webAttrs). Another idea could be to have webapp inherit the patches from the top-level src, allowing me to override mattermost directly and have the changes propagate to webapp. However, I’m unsure how to implement that.

Oh, my bad, that didn’t register :sweat_smile:

Something like that should work. Are you putting it in an overlay or similar? Alternatively, you can set services.mattermost.package. We should support this use case.

1 Like

I think @rhendric’s suggestion is correct. Since webapp is referenced in postInstall, it also needs to be overridden. Someone also pointed out that using finalAttrs instead of rec in the original derivation could potentially avoid this issue and make the earlier attempts work, although I haven’t had a chance to test that yet.

I must have missed this in the code review:

cp -r ${webapp}/* $out/client/ and the use of ${src} below should just read cp -r $webapp/* $out/client/ - that is, without the curlies. We should probably fix that.

Could you clarify what difference it would make to remove the curlies in this case?

It would use the value of the attribute at build time (i.e. the value of the environment variable), rather than substituting it into the script source using Nix. The attrset containing the webapp is a rec for other reasons, so the latter is possible but it tends to break overriding when packages do this.

If that line of code is being revised, why not use a symlink?

1 Like

We probably should just symlinkJoin them together :slight_smile:

Edit: Oh, you’re referring to just symlinking it in there, which makes sense too.

When using symlinks instead of cp, would we still need to override postInstall? While that approach avoids string context issues, I’m wondering if there’s a cleaner way to handle this, like using finalAttrs as suggested earlier? Could this help make my original attempt work?

Yes, if we used a non-curly $webapp variable as @numinit suggested here, I believe your original attempt would have worked.

Apologies for missing that - the intent was certainly to have the webapp be easily overridable. I’ll see if I can fix that after some rest, if the PR is made before I get to it tag me in it :slight_smile:

@simpleboy0 @rhendric Done, and added a test: mattermost: allow overriding the webapp easily by numinit · Pull Request #373085 · NixOS/nixpkgs · GitHub

Thanks for the report.

2 Likes

It’s merged. Give your channel or flake an update and try your original suggestion again.

(Edit: you may have to wait for https://status.nixos.org/ to kick over)

1 Like