Skip to content

Conversation

@JuliusFreudenberger
Copy link
Contributor

@JuliusFreudenberger JuliusFreudenberger commented Jan 25, 2025

Partially supersedes #358048, therefore pinging @techknowlogick.

https://github.com/gravitational/teleport/releases/tag/v15.4.26
https://github.com/gravitational/teleport/releases/tag/v16.4.14

No problems with teleport_15.
And teleport_16 now is also building. Previously there were errors, see below for more information.


However, teleport_16 does not build due to problems with wasm-opt.

The relevant part of the build log is the following:

[INFO]: found wasm-opt at "/nix/store/bsdyv113pcng0zdh09kf11l2jkqf70gx-binaryen-120_b/bin/wasm-opt"
[INFO]: Optimizing wasm binaries with `wasm-opt`...
[wasm-validator error in function fastpathprocessor_process\20externref\20shim] unexpected false: table.fill requires bulk-memory [--enable-bulk-memory], on 
(table.fill $1
 (local.get $8)
 (ref.null noextern)
 (i32.const 4)
)
Fatal: error validating input
Error: failed to execute `wasm-opt`: exited with exit status: 1
  full command: "/nix/store/bsdyv113pcng0zdh09kf11l2jkqf70gx-binaryen-120_b/bin/wasm-opt" "src/ironrdp/pkg/ironrdp_bg.wasm" "-o" "src/ironrdp/pkg/ironrdp_bg.wasm-opt.wasm" "-O"
To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.
Caused by: failed to execute `wasm-opt`: exited with exit status: 1
  full command: "/nix/store/bsdyv113pcng0zdh09kf11l2jkqf70gx-binaryen-120_b/bin/wasm-opt" "src/ironrdp/pkg/ironrdp_bg.wasm" "-o" "src/ironrdp/pkg/ironrdp_bg.wasm-opt.wasm" "-O"
To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.
error: builder for '/nix/store/3hxscqvd8zmrsdc7kcwn4mp5lv8iakvh-teleport-webassets-16.4.14.drv' failed with exit code 1;
       last 25 log lines:
       >    Compiling tracing-web v0.1.3
       >    Compiling sspi v0.15.0
       >    Compiling ironrdp-svc v0.1.1 (https://github.com/Devolutions/IronRDP?rev=2f57fd2de320f58fe240d88a83519255ba94cb73#2f57fd2d)
       >    Compiling ironrdp-graphics v0.1.0 (https://github.com/Devolutions/IronRDP?rev=2f57fd2de320f58fe240d88a83519255ba94cb73#2f57fd2d)
       >    Compiling ironrdp-dvc v0.1.0 (https://github.com/Devolutions/IronRDP?rev=2f57fd2de320f58fe240d88a83519255ba94cb73#2f57fd2d)
       >    Compiling ironrdp-displaycontrol v0.1.0 (https://github.com/Devolutions/IronRDP?rev=2f57fd2de320f58fe240d88a83519255ba94cb73#2f57fd2d)
       >    Compiling ironrdp-connector v0.2.1 (https://github.com/Devolutions/IronRDP?rev=2f57fd2de320f58fe240d88a83519255ba94cb73#2f57fd2d)
       >    Compiling ironrdp-session v0.2.0 (https://github.com/Devolutions/IronRDP?rev=2f57fd2de320f58fe240d88a83519255ba94cb73#2f57fd2d)
       >    Compiling ironrdp v0.1.0 (/build/source/web/packages/teleport/src/ironrdp)
       >     Finished `release` profile [optimized + debuginfo] target(s) in 2m 55s
       > [INFO]: found wasm-opt at "/nix/store/bsdyv113pcng0zdh09kf11l2jkqf70gx-binaryen-120_b/bin/wasm-opt"
       > [INFO]: Optimizing wasm binaries with `wasm-opt`...
       > [wasm-validator error in function fastpathprocessor_process\20externref\20shim] unexpected false: table.fill requires bulk-memory [--enable-bulk-memory], on
       > (table.fill $1
       >  (local.get $8)
       >  (ref.null noextern)
       >  (i32.const 4)
       > )
       > Fatal: error validating input
       > Error: failed to execute `wasm-opt`: exited with exit status: 1
       >   full command: "/nix/store/bsdyv113pcng0zdh09kf11l2jkqf70gx-binaryen-120_b/bin/wasm-opt" "src/ironrdp/pkg/ironrdp_bg.wasm" "-o" "src/ironrdp/pkg/ironrdp_bg.wasm-opt.wasm" "-O"
       > To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.
       > Caused by: failed to execute `wasm-opt`: exited with exit status: 1
       >   full command: "/nix/store/bsdyv113pcng0zdh09kf11l2jkqf70gx-binaryen-120_b/bin/wasm-opt" "src/ironrdp/pkg/ironrdp_bg.wasm" "-o" "src/ironrdp/pkg/ironrdp_bg.wasm-opt.wasm" "-O"
       > To disable `wasm-opt`, add `wasm-opt = false` to your package metadata in your `Cargo.toml`.
       For full logs, run 'nix log /nix/store/3hxscqvd8zmrsdc7kcwn4mp5lv8iakvh-teleport-webassets-16.4.14.drv'.
error: 1 dependencies of derivation '/nix/store/z3ahvccxw2cgs61hqbjwgkqh4985xy64-teleport-16.4.14.drv' failed to build

I investigated but did not come up with a working solution. The problem seems to be with upstream using wasm-pack version 0.12.1 with wasm-bindgen-cli version 0.2.95, which leads to instructions which need a separate flag --enable-bulk-memory (see wasm-bindgen/wasm-bindgen#4250) or a version bump of wasm-bindgen-cli which includes wasm-bindgen/wasm-bindgen#4237 (which upstream would have to do; however they do not have this problem, as they are still using Rust 1.81.0).

I also found the PRs gravitational/teleport#50178 and Homebrew/homebrew-core#200874 which seem to help with this problem but could not get it to work.

I might add that I am quite new to nix packaging (I wrote a few derivation for my own config but not for packages that sophisticated as this one.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Jan 25, 2025
@NixOSInfra NixOSInfra added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jan 25, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 25, 2025
@JuliusFreudenberger
Copy link
Contributor Author

I found the problem: The patch was included in the go module but was needed for the webassets derivation.

@JuliusFreudenberger JuliusFreudenberger marked this pull request as ready for review January 26, 2025 10:28
@techknowlogick
Copy link
Member

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 376749


aarch64-darwin

✅ 4 packages built:
  • teleport (teleport_16)
  • teleport.client (teleport_16.client)
  • teleport_15
  • teleport_15.client

@techknowlogick
Copy link
Member

Thanks!! 🙏

@techknowlogick techknowlogick added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jan 27, 2025
@JuliusFreudenberger JuliusFreudenberger changed the title maintainers: add juliusfreudenberger; teleport_15: 15.4.26 -> 15.4.22, teleport_16: 16.4.6 -> 16.4.14 maintainers: add juliusfreudenberger; teleport_15: 15.4.21 -> 15.4.26, teleport_16: 16.4.6 -> 16.4.14 Jan 31, 2025
@JuliusFreudenberger
Copy link
Contributor Author

Now that #377534 landed, I rebased this branch.

These releases include gravitational/teleport#50399 and gravitational/teleport#50398 respectively, which contains an update of a library with security fixes (CVE-2024-45338).
Sadly, I cannot add the security label.

@JuliusFreudenberger
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 376749


x86_64-linux

✅ 4 packages built:
  • teleport (teleport_16)
  • teleport.client (teleport_16.client)
  • teleport_15
  • teleport_15.client

Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

Awesome work. Just some minor nits that can be done now or later but everything LGTM.

@donovanglover donovanglover removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 31, 2025
@donovanglover donovanglover added 1.severity: security Issues which raise a security issue, or PRs that fix one 12.approvals: 2 This PR was reviewed and approved by two persons. labels Jan 31, 2025
Includes a patch disabling the wasm optimizing step for ironrdp.
This is needed as long as upstream requires wasm-pack 0.12.1.
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

LGTM grats on your first contribution to nixpkgs!

@donovanglover donovanglover merged commit 5104a73 into NixOS:master Jan 31, 2025
25 of 27 checks passed
@JuliusFreudenberger JuliusFreudenberger deleted the teleport_bump branch January 31, 2025 15:27
@techknowlogick
Copy link
Member

Thanks @JuliusFreudenberger and @donovanglover !!

@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Feb 2, 2025

Backport failed for release-24.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.11
git worktree add -d .worktree/backport-376749-to-release-24.11 origin/release-24.11
cd .worktree/backport-376749-to-release-24.11
git switch --create backport-376749-to-release-24.11
git cherry-pick -x 8268a3d9145fcf3fd2b07c59cf34275cb3a9bd24 a963a8727306bd274d589229e777ed2363dffb90 916add980f75e10cdc1d67fefc7b4b7d045265a6 4b9484816d0a58582eb79e790b9cb0bddcd8ecdd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.severity: security Issues which raise a security issue, or PRs that fix one 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants