Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bash-language-server: use pnpm.fetchDeps instead of node2nix #317954

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Jun 7, 2024

Motivation for this change

CC @malob per 7594254 .

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@NyCodeGHG
Copy link
Member

maybe add an alias to pkgs/development/node-packages/aliases.nix?

@doronbehar
Copy link
Contributor Author

maybe add an alias to pkgs/development/node-packages/aliases.nix?

Good idea! I didn't know about it (nix search doesn't show packages from nodePackages, because we don't recurseIntoAttrs nodejs.pkgs...).

@NyCodeGHG
Copy link
Member

Result of nixpkgs-review pr 317954 run on x86_64-linux 1

1 package failed to build:
  • bash-language-server

@NyCodeGHG
Copy link
Member

this does not build on my machine

@nix { "action": "setPhase", "phase": "buildPhase" }
Running phase: buildPhase
Executing npmBuildHook

> compile
> tsc -b

Finished npmBuildHook
@nix { "action": "setPhase", "phase": "installPhase" }
Running phase: installPhase
Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: /build/tmp.j6SpMvrkRZ/v3
  Virtual store is at:             ../../nix/store/yhg3chw7ih4scbl2nqlvx4qrra0vc4px-bash-language-server-5.3.3/lib/bash-language-server/node_modules/.pnpm
Progress: resolved 0, reused 1, downloaded 0, added 0
Progress: resolved 236, reused 236, downloaded 0, added 0
 ERR_PNPM_NO_OFFLINE_META  Failed to resolve @babel/[email protected] in package mirror /build/tmp.xNVHfbiuCT/.cache/pnpm/metadata/registry.npmjs.org/@babel/core.json

This error happened while installing a direct dependency of /build/source
Progress: resolved 236, reused 236, downloaded 0, added 0

@NyCodeGHG
Copy link
Member

@ofborg build bash-language-server

@doronbehar
Copy link
Contributor Author

Aish weird! It didn't fail like that when I opened the PR. Apparently this was a symptom of using the wrong pnpm version to generate pnpmDeps. Now I tested it locally, rebased onto latest master, and it worked for me.

@NyCodeGHG
Copy link
Member

Result of nixpkgs-review pr 317954 run on x86_64-linux 1

1 package built:
  • bash-language-server

@doronbehar doronbehar merged commit 3f84a27 into NixOS:master Jun 13, 2024
23 of 25 checks passed
@andrevmatos
Copy link
Member

This seems to have broken nixvim:

       error: Alias bash-language-server is still in node-packages.nix

@gkze
Copy link
Contributor

gkze commented Jun 14, 2024

Does bashls need to be removed from node-packages.nix?

@NyCodeGHG
Copy link
Member

Does bashls need to be removed from node-packages.nix?

it does, sorry for the breakage.

NyCodeGHG added a commit to NyCodeGHG/nixpkgs that referenced this pull request Jun 14, 2024
bash-language-server has been packaged outside of nodePackages in NixOS#317954, but not correctly removed from nodePackages.
NyCodeGHG added a commit to NyCodeGHG/nixpkgs that referenced this pull request Jun 14, 2024
bash-language-server has been packaged outside of nodePackages in NixOS#317954, but not correctly removed from nodePackages.
NyCodeGHG added a commit to NyCodeGHG/nixpkgs that referenced this pull request Jun 14, 2024
bash-language-server has been packaged outside of nodePackages in NixOS#317954, but not correctly removed from nodePackages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants