vscode-extensions.rust-lang.rust-analyzer: 0.3.1059 -> 0.3.1426

[Garmelon]

Description of changes

Changelog: Changelog #130 to Changelog #168

Supersedes #213850. Only after doing all the updating did I realize somebody else had already opened a PR, but the RA release cycle marches on relentlessly.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 23.05 Release Notes (or backporting 22.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
• Fits CONTRIBUTING.md.

[superherointj]

Please fix the conflict:
pkgs/development/node-packages/node-packages.nix

[Garmelon]

Done, the node-packages.nix on master is more recent so there's no need to include it in this PR any more.

[superherointj]

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

1 package failed to build:

• vscode-extensions.matklad.rust-analyzer

[superherointj]

@Garmelon Please check for failure on build.

[Garmelon]

I noticed as well, sorry. Apparently, the new node-packages.nix is missing the anser package so I do need to do a full regeneration, which will take a few hours. I could bump vscode-extensions.rust-lang.rust-analyzer while I'm at it but it probably makes sense to open a new PR for that instead.

[superherointj]

I noticed as well, sorry. Apparently, the new node-packages.nix is missing the anser package so I do need to do a full regeneration, which will take a few hours. I could bump vscode-extensions.rust-lang.rust-analyzer while I'm at it but it probably makes sense to open a new PR for that instead.

I agree ideally should be a discrete PR for node. When merged, rebase, and I can merge here, no problem.

[Garmelon]

So I should open a new PR just for node-packages? I think that would fail to pick up the new rust-analyzer dependency because pkgs/applications/editors/vscode/extensions/rust-analyzer/build-deps/package.json won't include the new dependency yet. On the other hand, I can't update the rust-analyzer plugin without updating node-packages.nix because then the build will fail due to a missing dependency (the current situation).

As far as I understand, the node-packages.nix update needs to happen at the same time as the rust-analyzer vscode plugin update. Once the node package update finishes, I'll rebase it into PR again if that's fine.

[superherointj]

node-packages.nix

The node-packages.nix bump is global, right? Affecting all node/JS packages?
If it is, I think ideally it should be not part of this PR.
Then, once that lands, just rebase this PR, and it will build fine.

[Garmelon]

I think when updating node-packages.nix, node2nix uses the contents of pkgs/applications/editors/vscode/extensions/rust-analyzer/build-deps/package.json (among other files). This means that I can't update node-packages.nix before I update the rust-analyzer extension because its package.json won't include the newly added anser dependency yet, so it won't be included in node-packages.nix.

I also can't update it after I update the extension because then the extension won't build until node-packages.nix is updated again.

This means that it has to be updated in the same PR as the extension because the extension dependencies changed (unless I'm missing something obvious).

[superherointj]

I think when updating node-packages.nix, node2nix uses the contents of pkgs/applications/editors/vscode/extensions/rust-analyzer/build-deps/package.json (among other files). This means that I can't update node-packages.nix before I update the rust-analyzer extension because its package.json won't include the anser dependency yet, so it won't be included in node-packages.nix.

I also can't update it after I update the extension because then the extension won't build until node-packages.nix is updated again.

This means that it has to be updated in the same PR as the extension because the extension dependencies changed (unless I'm missing something obvious).

Right. It is local lock to this extension then? I was thinking it was a general bump of JS packages (global). (I'm still trying to check this here.)

[Garmelon]

node-packages.nix is global, yes

[superherointj]

node-packages.nix is global, yes

If it is global, make it a discrete PR.

Then you can cherry-pick commit for tests. And rebase once is merged.

[Garmelon]

I can't, unless I'm missing something.

I could maybe update node-packages.nix by overwriting rust-analyzer's package.json without committing that change, but that runs the risk of any new node packages being removed again by another update by another person before this PR is merged again.

In previous rust-analyzer extension PRs, node-packages.nix was updated in the same PR.

[Garmelon]

Looking through the history, node-packages.nix seems to commonly be updated along with other applications, usually in the same commit.

Though looking through those commits, sometimes packages are added manually to node-packages.json. Maybe that's what I should do in this case? node-packages' generate.sh previously picked up on the new rust-analyzer dependency without me touching that file though, and most of rust-analyzer's dependencies are not in node-packages.json.

[superherointj]

Though looking through those commits, sometimes packages are added manually to node-packages.json. Maybe that's what I should do in this case?

That is how I would do it. A discrete PR for JS, one commit for adding package, one for bumping JS packages globally.

node-packages' generate.sh previously picked up on the new rust-analyzer dependency without me touching that file though.

I don't know what happened. Sorry.

[superherointj]

@Garmelon Coupling a global JS bump + package addition to a VSCode extension bump is not ideal. If you had already decoupled JS changes, the JS part would have been accepted/merged already. Seems to me there are less people willing to review VSCode extensions than JS bumps. Also the VSCode reviewer wouldn't have to review the JS bumps. Win-win.

[Garmelon]

Okay, I will try to separate the JS change into a new PR. Do you think I should add all of rust-analyzer's new dependencies while I'm at it? Most of them are not part of node-packages.json yet. In any case, hopefully the people maintaining the node packages will shout at me if I do it wrong :D

This section on Javascript packages inside nixpkgs seems to suggest I should. It doesn't appear to be necessary for most dependencies though.

As a rule of thumb, the package set should only provide end user software packages, such as command-line utilities. Libraries should only be added to the package set if there is a non-NPM package that requires it.

Thanks for your time so far.

[superherointj]

Maybe node2nix could be used to make dependencies local to this package? So you don't have to bother about changing global packages? Then, being local, makes sense to include here in this PR.

[Garmelon]

So far, I've been using the extension's update.sh script, which always updates node-packages.nix (either via sed directly or via node2nix if the dependencies changed). Maybe that script should be adjusted as well if the workflow of packaging the extension should change, but I don't know enough to do that.

The more I look into this, the less I understand it. I don't know whether the update script does the right thing or is horribly outdated.

[superherointj]

Asked on Matrix:

Update:

Update 2:

[Garmelon]

Maybe node2nix could be used to make dependencies local to this package?

The extension seems to have originally been packaged that way, then moved to the global node-packages.nix in 8318c36, which was a part of #77752

[superherointj]

Let's do a global update then.
Just update PR here with changes...

[Garmelon]

In that case, the question remains whether I should bump this PR to the latest extension version as well

[superherointj]

In that case, the question remains whether I should bump this PR to the latest extension version as well

Yes, bump to latest version.

Just noticed that upstream included package-lock.json so buildNpmPackage can be used. It would make upgrades more straight forward.

[Garmelon]

I don't know enough about the hacks and other things this derivation is doing to make that switch. For example, the update.sh script combines dependencies and devDependencies for some reason. Even if I did, it would probably be worth a separate PR.

I'll push a normal update in a few hours once the update script finishes.

[superherointj]

My concern of global bumps is silently breaking something else. (As raitobezarius just pointed out.)
As upstream provides package-lock.json, the lockfile doesn't have to be included in nixpkgs, so buildNpmPackage can be used here. So dependencies to this package are local. And it is less trouble on next bumps.

[superherointj]

in a few hours once the update script finishes.

That doesn't sound right... Bumping this extension shouldn't require that much work...
Humm... maybe we should just ignore this update script and package this extension using buildNpmPackage...

[Garmelon]

Updating node-packages.nix takes a long time.

Global node-packages bumps happen from time to time, things will break either way.

If you manage to build it using buildNpmPackage, that sounds like the better solution. I don't think I'll be able to. It would be nice to update the extension soon because the current version is almost a year out of date.

[superherointj]

pkgs/development/node-packages/node-packages.nix

Needs to be a discrete commit.

[superherointj]

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

2 packages failed to build:

• openmoji-black
• openmoji-color

52 packages built:

• balanceofsatoshis
• bibtex-tidy
• bitwarden-cli
• castnow
• code-server
• commitlint
• create-cycle-app
• discourse
• discourseAllPlugins
• emanote
• epgstation
• fast-cli
• flexoptix-app
• google-clasp
• grafana-image-renderer
• gtop
• joplin
• lighthouse
• mastodon-bot
• morgen
• node2nix
• pm2
• postcss-cli
• pyright
• redoc-cli
• reveal-md
• slack
• teams
• theLoungePlugins.plugins.closepms
• theLoungePlugins.plugins.giphy
• theLoungePlugins.themes.flat-blue
• theLoungePlugins.themes.flat-dark
• thelounge
• uppy-companion
• vimPlugins.coc-explorer
• vimPlugins.coc-go
• vimPlugins.coc-metals
• vimPlugins.coc-pyright
• vimPlugins.coc-sh
• vimPlugins.coc-stylelint
• vimPlugins.coc-vetur
• vimPlugins.markdown-preview-nvim
• vscode
• vscode-extensions.matklad.rust-analyzer
• vscode-extensions.ms-python.vscode-pylance
• vscode-fhs
• vscode-with-extensions
• vscodium
• vscodium-fhs
• wrangler
• yaml-language-server
• zx

[superherointj]

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

2 packages failed to build:

• openmoji-black
• openmoji-color

52 packages built:

• balanceofsatoshis
• bibtex-tidy
• bitwarden-cli
• castnow
• code-server
• commitlint
• create-cycle-app
• discourse
• discourseAllPlugins
• emanote
• epgstation
• fast-cli
• flexoptix-app
• google-clasp
• grafana-image-renderer
• gtop
• joplin
• lighthouse
• mastodon-bot
• morgen
• node2nix
• pm2
• postcss-cli
• pyright
• redoc-cli
• reveal-md
• slack
• teams
• theLoungePlugins.plugins.closepms
• theLoungePlugins.plugins.giphy
• theLoungePlugins.themes.flat-blue
• theLoungePlugins.themes.flat-dark
• thelounge
• uppy-companion
• vimPlugins.coc-explorer
• vimPlugins.coc-go
• vimPlugins.coc-metals
• vimPlugins.coc-pyright
• vimPlugins.coc-sh
• vimPlugins.coc-stylelint
• vimPlugins.coc-vetur
• vimPlugins.markdown-preview-nvim
• vscode
• vscode-extensions.matklad.rust-analyzer
• vscode-extensions.ms-python.vscode-pylance
• vscode-fhs
• vscode-with-extensions
• vscodium
• vscodium-fhs
• wrangler
• yaml-language-server
• zx

[Garmelon]

During building of openmoji, I get the error

/build/source/helpers/generate-font-glyphs.js:13
const svgFiles = glob('./color/svg/*.svg');
                 ^
TypeError: glob is not a function

This seems to be the same error as #219202 which was caused by #218736, so at least this PR didn't break anything that wasn't already broken.