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

n8n: 1.9.3 -> 1.46.0, repackage with pnpm.fetchDeps #319310

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

gepbird
Copy link
Contributor

@gepbird gepbird commented Jun 12, 2024

Since #290715 was merged, it's much easier to package and maintain pnpm projects.

Description of changes

  • replace all the node-env, node-composition stuff with pnpm.fetchDeps and pnpm.configHook
  • upgrade n8n to latest version
  • move to pkgs/by-name
  • set N8N_RELEASE_TYPE to stable
  • remove old patches and build inputs that seem no longer necessary
    • oracledb and @sap/hana-client removal on aarch64: these don't exist in node_modules anymore and aarch64 build works
    • pkgs.which: probably was unused before, there were no references to it
    • NIX_CFLAGS_COMPILE = "-DNODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT";: builds without this with node 20
  • remove update script: there's no update script for similarly packaged apps, and updating is fairly simple
  • format with nixfmt-rfc-style

Issues I encountered and solved

Error: cannot find '/build/source/node_modules/.pnpm/[email protected]/node_modules/sqlite3/build/Release/node_sqlite3.node

pnpm.configHook calls pnpm install with --ignore-scripts that skips downloading/building node-sqlite3 bindings. A solution is building it manually: (use node-gyp from nixpkgs rather than from pnpm installed node_modules to avoid other errors)

pushd node_modules/sqlite3
node-gyp rebuild
popd

turbo run build: Failed to create APIClient: Unable to set up TLS. builder error: No such file or directory

In turbo release 1.12.4, they switched to using rustls-tls-native-roots rust crate which tries to open a file from SSL_CERT_FILE environment variable, which is set to /no-cert-file.crt in a nix build environment. Adding cacert to build inputs will set this env var to a real file, fixing the issue.

ERR_PNPM_BAD_PM_VERSION This project is configured to use v9.1.4 of pnpm. Your current pnpm is v9.1.1.

Patching package.json to replace pnpm version to the current fixes it:

jq '.packageManager = "pnpm@${pnpm.version}"' package.json | sponge package.json

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot requested review from K900 and freezeboy June 12, 2024 14:08
@gepbird gepbird force-pushed the n8n-pnpm.fetchDeps branch 2 times, most recently from 83868b6 to f996fdf Compare June 14, 2024 12:18
@gepbird gepbird changed the title n8n: repackage with pnpm.fetchDeps n8n: 1.9.3 -> 1.45.1, repackage with pnpm.fetchDeps Jun 14, 2024
@gepbird gepbird marked this pull request as ready for review June 14, 2024 14:28
pkgs/by-name/n8/n8n/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/n8/n8n/package.nix Outdated Show resolved Hide resolved
@natsukium
Copy link
Member

natsukium commented Jun 23, 2024

Result of nixpkgs-review pr 319310 at 32950b6 run on x86_64-linux 1

1 package built successfully:
  • n8n

Result of nixpkgs-review pr 319310 at 32950b6 run on aarch64-darwin 1

1 package built successfully:
  • n8n

Copy link
Member

@natsukium natsukium left a comment

Choose a reason for hiding this comment

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

It works fine on x86_64-linux.
LGTM, thanks!

@gepbird gepbird marked this pull request as draft June 26, 2024 17:39
@gepbird gepbird changed the title n8n: 1.9.3 -> 1.45.1, repackage with pnpm.fetchDeps n8n: 1.9.3 -> 1.46.0, repackage with pnpm.fetchDeps Jun 26, 2024
@gepbird gepbird marked this pull request as ready for review June 26, 2024 17:50
@gepbird
Copy link
Contributor Author

gepbird commented Jun 26, 2024

In the last few hours I updated this package from 1.45.1 to 1.46.0 and removed the pnpm version patch.

Tested it on x86_64 and aarch64 linux, works as before.

Copy link
Member

@natsukium natsukium left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your work!

@natsukium natsukium merged commit 34328a9 into NixOS:master Jun 28, 2024
44 checks passed
@gepbird gepbird deleted the n8n-pnpm.fetchDeps branch June 28, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants