Skip to content

wasmtime: don't propagate binaries for C API & library#438391

Merged
pyrox0 merged 3 commits intoNixOS:masterfrom
nekowinston:wasmtime/dont-propagate-binaries
Sep 5, 2025
Merged

wasmtime: don't propagate binaries for C API & library#438391
pyrox0 merged 3 commits intoNixOS:masterfrom
nekowinston:wasmtime/dont-propagate-binaries

Conversation

@nekowinston
Copy link
Member

Reduces the closure size if just the library/C API is used.

According to nix-tree, running on ./result-dev:
before:
NAR Size: 142.75 MiB | Closure Size: 233.31 MiB | Added Size: 233.31 MiB
after:
NAR Size: 142.92 MiB | Closure Size: 183.94 MiB | Added Size: 183.94 MiB

I've also noticed that there's a preexisting function (moveToOutput) doing what's done manually; it just doesn't seem to be documented anywhere.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nekowinston nekowinston added the 0.kind: enhancement Add something new or improve an existing system. label Aug 29, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Aug 29, 2025
@nix-owners nix-owners bot requested review from ereslibre and matthewbauer August 29, 2025 21:21
@nekowinston
Copy link
Member Author

Since I've contributed the C API and done a bit of continuous troubleshooting/optimizing on it, I'd be glad to be added as a maintainer if the current maintainers are happy with that.

@ereslibre
Copy link
Member

I'd be glad to be added as a maintainer if the current maintainers are happy with that.

Absolutely! Feel free to add yourself in this PR!

Copy link
Member

@ereslibre ereslibre left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, it's great to have smaller closures!

@ereslibre
Copy link
Member

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 438391
Commit: 6c867d22dee9c36a93a1874697da46702fc99059


x86_64-linux

✅ 2 packages built:
  • wasmtime
  • wasmtime.dev

Copy link
Member

@ereslibre ereslibre left a comment

Choose a reason for hiding this comment

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

Approved automatically following the successful run of nixpkgs-review.

@ereslibre
Copy link
Member

@NixOS/nixpkgs-merge-bot merge

@nixpkgs-merge-bot
Copy link
Contributor

@ereslibre merge not permitted (#305350):
R-Ryantm Maintainer merge: pr author is not r-ryantm
CommitterPR: pr author is not committer

@nixpkgs-ci nixpkgs-ci bot 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 Aug 29, 2025
@nekowinston
Copy link
Member Author

nekowinston commented Aug 30, 2025

Oh, I just realized that if we turned building the static library into an opt-in, we could get rid of another 117M:
NAR Size: 31.58 MiB | Closure Size: 72.6 MiB | Added Size: 72.6 MiB

The reason I'm obsessing about this a bit is that #394870 would ask people to pull wasmtime.dev into their tree-sitter and thus neovim closure. I'd wager that the smaller that is, the higher the chances people will be okay with that being the default.

--- a/pkgs/by-name/wa/wasmtime/package.nix
+++ b/pkgs/by-name/wa/wasmtime/package.nix
@@ -6,6 +6,7 @@
   cmake,
   versionCheckHook,
   nix-update-script,
+  enableStatic ? stdenv.hostPlatform.isStatic,
 }:
 rustPlatform.buildRustPackage (finalAttrs: {
   pname = "wasmtime";
@@ -63,6 +64,9 @@ rustPlatform.buildRustPackage (finalAttrs: {
       # https://github.com/rust-lang/cargo/issues/9661
       cp -r target/${cargoShortTarget}/release/build/wasmtime-c-api-impl-*/out/include $dev/include
     ''
+    + lib.optionalString (!enableStatic) ''
+      rm $dev/lib/*.a
+    ''
     + lib.optionalString stdenv.hostPlatform.isDarwin ''
       install_name_tool -id \
         $dev/lib/libwasmtime.dylib \

There's some prior art for removing the static libraries like that in tree-sitter itself:

postInstall = ''
PREFIX=$out make install
${lib.optionalString (!enableShared) "rm $out/lib/*.so{,.*}"}
${lib.optionalString (!enableStatic) "rm $out/lib/*.a"}
''

@ereslibre
Copy link
Member

ereslibre commented Aug 30, 2025

@nekowinston thanks for the context. I think it would make sense to change that as another PR, and I'd be fine approving that. I didn't have the real need to optimize that much, but I think it makes sense if it helps for the neovim case you mentioned.

@nekowinston
Copy link
Member Author

Alright, I'll open a follow-up PR for that 🙂

@pyrox0 pyrox0 merged commit 3f405f0 into NixOS:master Sep 5, 2025
32 of 33 checks passed
@nekowinston nekowinston deleted the wasmtime/dont-propagate-binaries branch September 5, 2025 15:53
@samestep
Copy link
Contributor

samestep commented Oct 7, 2025

Hey @nekowinston! I hit an issue that was caused by this PR, and wanted to check whether it's intentional or just a bug: wasmtime no longer appears on the PATH for nix-shell or nix develop. You can test this by making a flake and putting wasmtime in the dev shell, or more simply by just running the following command in the Nixpkgs repo:

nix-shell --pure -I nixpkgs=$PWD -p wasmtime --run "wasmtime --version"

As of 3f405f0 and later, that command fails and prints wasmtime: command not found, whereas on its parent commits 4a9c6a9 and 6c867d2, it succeeds and prints wasmtime 33.0.0.

If it was intentional to remove wasmtime from the PATH for the wasmtime package, is there a workaround I can use downstream? Or if not, do you happen to know what the right fix is?

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

Labels

0.kind: enhancement Add something new or improve an existing system. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants