Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: mattermost: split and build webapp from source #126629

Closed
wants to merge 1 commit into from

Conversation

ryantm
Copy link
Member

@ryantm ryantm commented Jun 11, 2021

Motivation for this change

I want to build mattermost webapp from source so I can patch it.

Things done

Trying to split mattermost up into server and webapp and build the webapp from source. Unfortunately, when I do this, the npm run build does not output anything to dist, so I'm clearly doing something wrong.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 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
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Jun 11, 2021
@ofborg ofborg bot requested review from kalbasit and fpletz June 11, 2021 22:10
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jun 11, 2021
@happysalada
Copy link
Contributor

happysalada commented Jun 12, 2021

Just a little bit more context.
It appears the version is from feb 18 https://github.com/mattermost/mattermost-server/releases?after=v5.31.3
They recently upgraded from node 12 to node 16 https://github.com/mattermost/mattermost-webapp/commits/master/package-lock.json

The build failure is most likely a dependency problem. (yarn2nix not translating the dependencies just like they are in the package-lock.json). Unfortunately I tried to use node2nix to generate the dependencies. It seems to work fine, up until I try to build it, then I get a segfault (from nix). Maybe it's just on my system.

Here are things that can be tried.

  • try node2nix on your system. (you have to run node2nix --development -l package-lock.json to generate the files. Then create a derivation for the dependencies just like in the readme. Very similar to what you are doing with yarn2nix). If it segfaults on your system, then it's really a bug in nix.
  • Update the mattermost version first then try again? Like I was saying, there has been heavy changes on the js side since that release. You might get a better chance with the dependencies. I tried on the latest master to install dependencies and build with npm and didn't run in any problems. So if the yarn2nix fails it's a dependency problem.
  • Go the Fixed Output Derivation way. We have to remember to actually use node 12. It might not be the prettiest way, but in my experience it will work every time.

You can try disabling the source-map, but if there really is an error in the dependencies generated then you have no idea what you will get after building.

@happysalada
Copy link
Contributor

I have a branch here, just adding one commit to show what the build process with node2nix would be like
https://github.com/happysalada/nixpkgs/tree/mattermost

For me running this results in a segfault. If it works on your machine, then you should be able to continue with it.

@happysalada
Copy link
Contributor

Just a small update on this.
I've tried with version 5.36.2 of mattermost. Even on local (so completely impure way of doing things) installing with yarn and building I get a failure. I get the same failure in the nix build.
Your approach is the right one here. The problem likely lies on using yarn vs npm.
The problem is that node2nix can't be used since upstream explicitely requires node16 to build and node2nix does not support node16 (I've had problems with it in the past).

It seems to me the only way here would be to go FOD.

Comment on lines +16 to +19
# buildFlagsArray = ''
# -ldflags=
# -X ${goPackagePath}/model.BuildNumber=nixpkgs-${version}
# '';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# buildFlagsArray = ''
# -ldflags=
# -X ${goPackagePath}/model.BuildNumber=nixpkgs-${version}
# '';
# ldflags = [
# "-X ${goPackagePath}/model.BuildNumber=nixpkgs-${version}"
# ];

Comment on lines +28 to +31
platforms = platforms.unix;
};

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
platforms = platforms.unix;
};
}
};
}

@happysalada
Copy link
Contributor

Ok, last update, I gave a shot to the FOD with the following expression

  node_modules = stdenv.mkDerivation {
    pname = "${pname}-modules";
    inherit version src;
    buildInputs = [ nodejs-16_x ];
    configurePhase = ''
      export HOME=$(mktemp -d)
    '';
    buildPhase = ''
      npm install
    '';
    installPhase = ''
      cp -r node_modules $out
    '';
    impureEnvVars = lib.fetchers.proxyImpureEnvVars;
    outputHashAlgo = "sha256";
    outputHashMode = "recursive";
    outputHash = "sha256-ID7sStzMln4K+PLplAedC0DOK0q1abqzL6j1u7wDyVI";

    postBuild = ''
      cp -r $out/deps/mattermost-webapp-modules/node_modules/* $out/node_modules/
    '';
  };

the output hash changes, it's not reproducible.

I think this should just be retried on future updates of mattermost, if the yarn vs npm behavior difference persists, perhaps something like npmlock2nix will solve it.

@ryantm ryantm closed this Oct 18, 2021
@wmertens
Copy link
Contributor

wmertens commented Jun 8, 2022

@happysalada do you know what was the source of the hash changes? If you have two different builds and compare them, maybe it turns out that it's something simple to patch?

@happysalada
Copy link
Contributor

I don't unfortunately, I never investigated further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants