nushellPlugins.*: automatically patch dependencies to support current nushell version#441173
Conversation
e3c9044 to
0dce073
Compare
dtomvan
left a comment
There was a problem hiding this comment.
I know it's still a draft, but here's the things I currently find hacky: hope that helps.
There was a problem hiding this comment.
prev.name is not available for some reason.
error: attribute 'name' missing
at /nix/store/i2srmbli3vkrh11g5islzy5zzm1hxybs-source/pkgs/shells/nushell/plugins/default.nix:27:36:
26| (rustPlatform.fetchCargoVendor ({
27| inherit (prev) src name;
| ^
28| hash = prev.cargoHash;
Thanks for the comments, just wanted to test if this is possible. Didn't know about cargo-edit etc. could have saved a lot of time. I will address your comments in the next days. |
|
0dce073 to
6380f3d
Compare
|
@dtomvan could you take another look? |
6380f3d to
e54a24c
Compare
|
|
cargoHash doesn't seem to be stable now. I get 3 different hashes at 3 different times. I suspect that cargo edit isn't deterministic and can lock different downstream crate versions dependent on the time they are fetched. I'm not sure if the same problem exits with my earlier approach. |
dtomvan
left a comment
There was a problem hiding this comment.
That's unfortunate. This looks much better to me though.
| postBuild = '' | ||
| if ! [ -z "''${NUSHELL_VERSION_UPGRADED}" ]; then | ||
| cp Cargo.toml $out/Cargo.toml | ||
| fi | ||
| ''; | ||
| })).overrideAttrs | ||
| (old: { | ||
| buildCommand = old.buildCommand + '' | ||
| cp $vendorStaging/Cargo.toml $out/Cargo.toml || true | ||
| ''; | ||
| }); | ||
|
|
||
| postPatch = '' | ||
| cp ${final.cargoDeps}/Cargo.lock ./Cargo.lock | ||
| cp ${final.cargoDeps}/Cargo.toml ./Cargo.toml || true | ||
| ''; |
There was a problem hiding this comment.
Why would we have to conditionally copy cargo.toml? In this case it may silently continue after failing to copy Cargo.toml (however that may happen)
| if is_compatible nu-plugin "${nushell.version}"; then | ||
| echo "nu-plugin is compatible, no patching needed" | ||
| else | ||
| cargo upgrade -p nu-plugin@${nushell.version} -p nu-protocol@${nushell.version} |
There was a problem hiding this comment.
You might be able to pin the exact version with an =-sign before the version. https://docs.rs/semver/latest/semver/enum.Op.html
Also, if you want to debug the reproducibility of this, you can probably use -v and CARGO_LOG=upgrade=trace to figure out what cargo commands it's calling under the hood and what versions it's selecting. cargo itself isn't that deterministic in and of itself though...
|
Is this patch still relevant after nushell plugins becoming merge-bot eligible through #482961? |
|
I'd say yes, since some plugins' upstreams are very, very slow with updating. I have two PRs open for 7 months now: |
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.