Skip to content

Conversation

yzhou216
Copy link
Contributor

Resolves #433006

cc @matthiasbeyer for testing

Things done


Add a 👍 reaction to pull requests you find important.

@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 17, 2025
@yzhou216
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 434339

Logs: https://github.com/yzhou216/nixpkgs-review-gha/actions/runs/17014400811

Download packages from cache:
  • x86_64-linux
    nix-store -r --add-root nixpkgs-pr-434339-x86_64-linux \
      --option binary-caches 'https://cache.nixos.org/ https://yzhou216.cachix.org' \
      --option trusted-public-keys '
      cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
      yzhou216.cachix.org-1:O7nGwQnLGGBGE8zYFMipVzPVN956FcV57y9SqwEP+O8=
      ' \
      /nix/store/y84mrnvz1nlqzldxgn6brld6p3k1znca-rhai-lsp-0-unstable-2022-10-25
  • aarch64-linux
    nix-store -r --add-root nixpkgs-pr-434339-aarch64-linux \
      --option binary-caches 'https://cache.nixos.org/ https://yzhou216.cachix.org' \
      --option trusted-public-keys '
      cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
      yzhou216.cachix.org-1:O7nGwQnLGGBGE8zYFMipVzPVN956FcV57y9SqwEP+O8=
      ' \
      /nix/store/pv64hnbk6z8q02i9wjm2i89fq8dd8qy4-rhai-lsp-0-unstable-2022-10-25
  • x86_64-darwin
    nix-store -r --add-root nixpkgs-pr-434339-x86_64-darwin \
      --option binary-caches 'https://cache.nixos.org/ https://yzhou216.cachix.org' \
      --option trusted-public-keys '
      cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
      yzhou216.cachix.org-1:O7nGwQnLGGBGE8zYFMipVzPVN956FcV57y9SqwEP+O8=
      ' \
      /nix/store/0vx78ysp6z1lwk7y9bckzaajbf18v47a-rhai-lsp-0-unstable-2022-10-25
  • aarch64-darwin
    nix-store -r --add-root nixpkgs-pr-434339-aarch64-darwin \
      --option binary-caches 'https://cache.nixos.org/ https://yzhou216.cachix.org' \
      --option trusted-public-keys '
      cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
      yzhou216.cachix.org-1:O7nGwQnLGGBGE8zYFMipVzPVN956FcV57y9SqwEP+O8=
      ' \
      /nix/store/cav4gjx9dk45a4n369lgxvpcspvlaz2h-rhai-lsp-0-unstable-2022-10-25

x86_64-linux

✅ 1 package built:
  • rhai-lsp

aarch64-linux

✅ 1 package built:
  • rhai-lsp

x86_64-darwin (sandbox = true)

✅ 1 package built:
  • rhai-lsp

aarch64-darwin (sandbox = true)

✅ 1 package built:
  • rhai-lsp

@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Aug 17, 2025
@matthiasbeyer
Copy link
Contributor

Awesome! I think we should PR the Cargo.lock file to upstream, what do you think?

@matthiasbeyer
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 434339
Commit: b9f8691e0adeb09a1e253de7caf104ecc470a58d


x86_64-linux

✅ 1 package built:
  • rhai-lsp

Copy link
Contributor

@matthiasbeyer matthiasbeyer 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.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. and removed 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Aug 17, 2025
@yzhou216
Copy link
Contributor Author

Awesome! I think we should PR the Cargo.lock file to upstream, what do you think?

The upstream hasn't updated for 3 years now, I'm not sure if this is worth it.

@matthiasbeyer
Copy link
Contributor

Okay, then please replace the lockfile here with the patch from rhaiscript/lsp#108

@yzhou216
Copy link
Contributor Author

Okay, then please replace the lockfile here with the patch from rhaiscript/lsp#108

What's different from adding a Cargo.lock along with the derivation? Is there a preferred way to do this in Nixpkgs?

@matthiasbeyer
Copy link
Contributor

I think patch is preferred, because it does not check in thousand of lines into nixpkgs.

@yzhou216
Copy link
Contributor Author

I think patch is preferred, because it does not check in thousand of lines into nixpkgs.

  patches = [
    (fetchpatch {
      name = "add-cargo-lock.patch";
      url = "https://github.com/rhaiscript/lsp/commit/77bd9052297d74320d84c7fcaa3fe75ddc211543.patch";
      sha256 = "";
    })
  ];

I just tried to use fetchpatch. I don't think it works because cargoHash has to be used with Cargo.lock and patchPhase happens after that. Any idea how to get around that?

@matthiasbeyer
Copy link
Contributor

Meeeeh I forgot about that. Maybe we should just wait for rhaiscript/lsp#108

@matthiasbeyer
Copy link
Contributor

Upstream refuses to include a lockfile, so I would say we rather not include the package than committing thousand of lines to nixpkgs. What do you think?

@yzhou216
Copy link
Contributor Author

Upstream refuses to include a lockfile, so I would say we rather not include the package than committing thousand of lines to nixpkgs. What do you think?

Seems like that's the only choice.

Wouldn't the lock file be regenerated upon build if it is not included?

I didn't see this one coming, tho, lol. Don't know what to say tbh... I guess people are just not into build systems and don't understand how they work?

@matthiasbeyer
Copy link
Contributor

I guess people are just not into build systems and don't understand how they work?

Yep.

I suggest we close this then, upstream is not interested in anyone using their software, it seems. 😢

@yzhou216
Copy link
Contributor Author

I guess people are just not into build systems and don't understand how they work?

Yep.

I suggest we close this then, upstream is not interested in anyone using their software, it seems. 😢

I agree. Closing.

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

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Package request: rhai LSP

2 participants