wasmtime: add enableShared/enableStatic & shell completions#440501
wasmtime: add enableShared/enableStatic & shell completions#440501GaetanLepage merged 3 commits intoNixOS:masterfrom
Conversation
03b154f to
162020f
Compare
|
1 similar comment
|
ereslibre
left a comment
There was a problem hiding this comment.
Thank you @nekowinston!
Since we are optimizing, I'm wondering if it would make sense to add an option to skip wasmtime-cli, if a project only needs the C API.
| + lib.optionalString (stdenv.buildPlatform.canExecute stdenv.hostPlatform) '' | ||
| installShellCompletion --cmd wasmtime \ | ||
| --bash <("$out/bin/wasmtime" completion bash) \ | ||
| --zsh <("$out/bin/wasmtime" completion zsh) \ | ||
| --fish <("$out/bin/wasmtime" completion fish) | ||
| '' | ||
| + lib.optionalString (!stdenv.buildPlatform.canExecute stdenv.hostPlatform) '' | ||
| installShellCompletion --cmd wasmtime \ | ||
| --bash "${buildPackages.wasmtime}"/share/bash-completion/completions/*.bash \ | ||
| --zsh "${buildPackages.wasmtime}"/share/zsh/site-functions/* \ | ||
| --fish "${buildPackages.wasmtime}"/share/fish/*/* | ||
| ''; |
There was a problem hiding this comment.
Do you know if there's a difference between those two in practice? (Does wasmtime completion do anything other than cat the completion files?).
I'm asking because if not, we could simplify to just run the second option in both cases, when stdenv.buildPlatform.canExecute and when it cannot.
There was a problem hiding this comment.
So my understanding of this snippet (referenced from tree-sitter's nixpkgs shell completions) is that this is for pkgsCross cross compilation. First we need wasmtime completion to put the files under $out/share/{bash,fish,zsh} on our native architecture.
Then when building e.g. pkgsCross.armv7l-hf-multiplatform.wasmtime, buildPackages refers to the host architecture; the completions are copied from there to the cross-compilation target, since the build platform can't run wasmtime completion itself if it's targeting armv7.
I actually just tried this on my end with nix-build . -A pkgsCross.armv7l-hf-multiplatform.wasmtime and have noticed that our cargoShortTarget is wrong. I think it should be stdenv.hostPlatform instead of stdenv.targetPlatform, because the C API header cp fails.
There was a problem hiding this comment.
Oh, I see, thank you. I didn't know this construct; out of interest I tried to use the second option and I run into infinite recursion, so I see now. The solution on this PR is working as expected:
❯ tree result
result
├── bin
│ └── wasmtime
└── share
├── bash-completion
│ └── completions
│ └── wasmtime.bash
├── fish
│ └── vendor_completions.d
│ └── wasmtime.fish
└── zsh
└── site-functions
└── _wasmtime
@nekowinston oops, nvm, this is already possible by just building the |
Right now we're still always building both things, but most users will download just what they need from the binary cache. |
|
ereslibre
left a comment
There was a problem hiding this comment.
Thanks for the optimizations and fixing the cargoShortTarget issue!
|
|
|
Mmh, 69e1e80 broke building I'll see if I have time to figure out why and how to fix that… ([Edit:] It fails at |
The follow-up as discussed in #438391. @ereslibre
With added shell completions for the binary. 🙂
Looking at the use case I mentioned, this takes the size down to
NAR Size: 31.58 MiB | Closure Size: 72.6 MiB | Added Size: 31.58 MiB
when included with most things, since gcc and glibc are usually already included somewhere else.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.