NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.21k stars 14.2k forks source link

Discourse package update script can't find gem #228426

Closed IreneKnapp closed 1 year ago

IreneKnapp commented 1 year ago

Describe the bug

I am attempting to follow the directions in <nixpkgs/pkgs/servers/web-apps/discourse/how_to_update.md> to prepare an update of Discourse from 3.1.0b2 to 3.1.0b4, which is the live in-production version upstream. They do not work; specifically I get a build failure pertaining to the Ruby gem sass-embedded, as detailed below.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Use update.py print-diffs and manually tweak the backend settings in the module definition to track upstream changes.
  2. Use update.py update
  3. Run discourse.tests as directed by the documentation.

Observe that the package does not build.

The gem to blame can be determined either by bisecting the diff on gemset.nix, or by tracing through the evaluation of the nix build functions for Ruby. I did the latter.

Expected behavior

The build should complete; there is no way to know whether the tests should additionally pass, but it would be nice.

Screenshots

$ nix build -L -f ../../../../ discourse.tests
error: attribute 'source' missing

       at /home/irenes/ops/nixpkgs/pkgs/development/ruby-modules/bundled-common/functions.nix:88:14:

           87|     inherit ruby;
           88|     inherit (attrs.source) type;
             |              ^
           89|     source = removeAttrs attrs.source ["type"];
(use '--show-trace' to show detailed location information)

Additional context

These upgrade directions are new, as is the script they rely on, update.py, which itself relies on the Ruby Bundler package and the nix-specific tool bundix. Based on the commit history, I appear to be the first person other than the maintainer to attempt to do this.

What I have found is that update.py produces a gemset.nix which is missing the sources attribute for one specific dependency. This causes the build to fail. That dependency is sass-embedded, and from my debugging it appears that the reason is that no x64_64-linux version of it was ever uploaded to rubygems.org.

This looks like it might be a bundix bug rather than an update.py bug, but unfortunately I am not sufficiently fluent in Ruby to know whether this is expected behavior. What I do know is that it leads to a build failure; if the determination is that this should be filed against bundix, I am happy to take that one. I am somewhat confused why there are platform-specific versions of these uploads at all, as I had understood Ruby to be an interpreted language.

Notify maintainers

@talyz any help would be lovely and much appreciated <3

Metadata

 - system: `"x86_64-linux"`
 - host os: `Linux 6.1.12, NixOS, 22.11 (Raccoon), 22.11.20230222.c95bf18`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.11.1`
 - channels(root): `"nixos-22.11"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

(but nix-info lies; I'm using flakes, so the channel is irrelevant. I'm on 22.11 though)

emilylange commented 1 year ago

https://github.com/discourse/discourse/commit/6e522e4aad6814fae79bcb46ffe73449394101f8 added sass-embedded as dependency, which in turn depends on dart-sass-embedded.

dart-sass-embedded isn't in nixpkgs yet, but there is a PR for it:

https://github.com/ntkme/sass-embedded-host-ruby/blob/f6ca1c9fe5b635979fff3888e3ec2d19ca9a06fc/ext/sass/Rakefile#L20-L24 is a bit unfortunate. SASS_EMBEDDED can be used to specify a path of dart-sass-embedded instead of downloading it from GitHub releases, but it has to be a tar.gz to unpack.

Might be worth patching, not sure. For now, I fixed just git apply #227286 and used dart-sass-embedded.src. This will break as soon as dart-sass-embedded is built from source (instead of relying on release blobs):

diff --git a/pkgs/servers/web-apps/discourse/default.nix b/pkgs/servers/web-apps/discourse/default.nix
index 504a96d3614..d851054698f 100644
--- a/pkgs/servers/web-apps/discourse/default.nix
+++ b/pkgs/servers/web-apps/discourse/default.nix
@@ -38,6 +38,7 @@
 , fixup_yarn_lock
 , nodePackages
 , nodejs_16
+, dart-sass-embedded

 , plugins ? []
 }@args:
@@ -186,6 +187,11 @@ let
               cp $(readlink -f ${libpsl}/lib/libpsl.so) vendor/libpsl.x86_64.so
             '';
           };
+          sass-embedded = gems.sass-embedded // {
+            postPatch = ''
+              export SASS_EMBEDDED=${dart-sass-embedded.src}
+            '';
+          };
         };

     groups = [

But there are two more issues:

https://github.com/discourse/discourse/commit/c320c286f92cc610a27f853a37cdc276227c44b1 added a postinstall hook to app/assets/javascripts/discourse/package.json which tries to call ../node_modules/.bin/patch-package, but errors out because at this point in time this hasn't been patchShebangs-ed yet.

Fixed with:

diff --git a/pkgs/servers/web-apps/discourse/asserts_patch-package_from_path.patch b/pkgs/servers/web-apps/discourse/asserts_patch-package_from_path.patch
new file mode 100644
index 00000000000..9f7d98b069f
--- /dev/null
+++ b/pkgs/servers/web-apps/discourse/asserts_patch-package_from_path.patch
@@ -0,0 +1,13 @@
+diff --git a/app/assets/javascripts/discourse/package.json b/app/assets/javascripts/discourse/package.json
+index 9e4533d2..e57f8a5f 100644
+--- a/app/assets/javascripts/discourse/package.json
++++ b/app/assets/javascripts/discourse/package.json
+@@ -14,7 +14,7 @@
+     "build": "ember build",
+     "start": "ember serve",
+     "test": "ember test",
+-    "postinstall": "yarn --silent --cwd .. patch-package"
++    "postinstall": "patch-package"
+   },
+   "dependencies": {
+     "@babel/core": "^7.21.4",
diff --git a/pkgs/servers/web-apps/discourse/default.nix b/pkgs/servers/web-apps/discourse/default.nix
index 504a96d3614..0a394e66ecb 100644
--- a/pkgs/servers/web-apps/discourse/default.nix
+++ b/pkgs/servers/web-apps/discourse/default.nix
@@ -199,7 +199,7 @@ let

     yarnOfflineCache = fetchYarnDeps {
       yarnLock = src + "/app/assets/javascripts/yarn.lock";
-      sha256 = "0ryc4p5s35mzg1p71z98x5fvr5fpldmgghdi1viha4ckbpv153lw";
+      sha256 = "0a20kns4irdpzzx2dvdjbi0m3s754gp737q08z5nlcnffxqvykrk";
     };

     nativeBuildInputs = runtimeDeps ++ [
@@ -207,6 +207,7 @@ let
       redis
       nodePackages.uglify-js
       nodePackages.terser
+      nodePackages.patch-package
       yarn
       nodejs_16
     ];
@@ -226,6 +227,12 @@ let
       # Fix the rake command used to recursively execute itself in the
       # assets precompilation task.
       ./assets_rake_command.patch
+
+      # `app/assets/javascripts/discourse/package.json`'s postinstall
+      # hook tries to call `../node_modules/.bin/patch-package`, which
+      # hasn't been `patchShebangs`-ed yet. So instead we just use
+      # `patch-package` from `nativeBuildInputs`.
+      ./asserts_patch-package_from_path.patch
     ];

     # We have to set up an environment that is close enough to

And finally, discourse bumped its minimal ruby version to 3.2:

diff --git a/pkgs/servers/web-apps/discourse/default.nix b/pkgs/servers/web-apps/discourse/default.nix
index 504a96d3614..fd0e3fc5a2a 100644
--- a/pkgs/servers/web-apps/discourse/default.nix
+++ b/pkgs/servers/web-apps/discourse/default.nix
@@ -8,7 +8,7 @@
 , bundlerEnv
 , callPackage

-, ruby_3_1
+, ruby_3_2
 , replace
 , gzip
 , gnutar
@@ -52,7 +52,7 @@ let
     sha256 = "sha256-wkNTm5/QyujPcMUrnc6eWmjhrRQAthhmejmjpy6zmbE=";
   };

-  ruby = ruby_3_1;
+  ruby = ruby_3_2;

   runtimeDeps = [
     # For backups, themes and assets