vscode-with-system-extensions: WIP

[jraygauthier]

This is a work in progress / draft, an idea that I had for quite some time and I finally got some time to implement.

This works really well (on both stable and unstable) and leave room for you to install extensions dynamically while profiting from our nixpkgs integrated extensions set (which is sometimes required).

However, major thing (in my opinion) that remain is that I wasn't able to avoid the big ~126MiB electron copy. Also some cleanup required depending on the chosen solution. Note that this will get reclaimed back on nix-store --optimise (hardlinking both copies), however I would prefer to avoid this.

I wanted to get some fresh ideas how to get rid of this copy. I tried some LD_PRELOAD patch but this was a failure, the programs fails to load. The ideal would be that upstream do something about it (see [Feature Request]: Add an option to specify the resource directory · Issue #31121 · electron/electron), however that might not happen soon unless we feed them with some well crafted PR (which I wasn't yet able to do). Patching our copy is not yet an option as we're not yet building electron ourselves from sources (see #17073).

To make it work on stable, you need to add the following lines as well (after the code electron binary copy):

# For some reason the following is required on stable 21.09 but not unstable
# as of 2021/12/01:
symlink_reinstall_as_real_copy "$out/lib/vscode/resources/app/node_modules.asar"

Here's a ref to the issue comment that gave me this idea in the first place:

• Allow extensions to be installed for all users · Issue #56614 · microsoft/vscode

  • Dampfwalze commented on Sep 23

    In the install directory of VSCode is an extensions folder in
    \Microsoft VS Code\resources\app\extensions. Simply copy all
    necessary extensions in here and you are good to go!

Motivation for this change

Allow users to dynamically install extension while still being backed by our fine well integrated extension set.

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/)
• 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.

Extra system / builtin extensions are all displayed in vscode's extensions pane (ctrl+shift+x) alongside other builtins when typing @builtin in the search box. Extensions all seem to work fine (I am currently working with a huge set of extra builtin extensions > 20).

[turion]

Phew. First of all, I really appreciate that you're putting effort into making vs code extensions better. It's a complicated topic with no optimal solution in the past yet, I believe. But if this PR is the way to go, I'll have to remove myself from the maintainers list.

Why:

Mainly because there is a lot of C code in there which I don't understand. Which means that if it breaks, I don't know who will fix it. Maybe you, but who else if you're not available? Upstream vscode development is impossible to predict, so I assume that breakage is likely when vscode changes.

Why do I not understand this C code? This is probably to a large extent because I haven't done much C programming. Is it really necessary to write this in C, or is there maybe a shell workaround?

But I think there are some other aspects that might improve the situation:

• You've basically written a small C library that solves some specific problem. Tbh I don't understand what your solution is, and it's not clear from just reading through the code. How about making it a separate library. In the process, improve the following:
• Is your library specific to this problem, or does it solve a general problem?
• Can you document?
• Can you test?
• Can you explain the general motivation and solution sketch in a readme?

But honestly, someone with much more C knowledge than me has to comment.

[Synthetica9]

@turion very well put I think. My €0.02:

• Is this also possible with VSCodium?
• If so; we should probably also test this with a NixOS test.

[jraygauthier]

Hi @turion and @Synthetica9 and thanks for taking some time looking at this.

@turion :

The C code is only for pkgs/build-support/libredirect-self which represents the failed attempt at avoiding the big electron copy working around the current impossibility to parameterize electron with its resource directory (see electron/electron#31121). This is not currently used (commented out code marked as "TODO") and in the current state of things, we simply accept the big 126MiB electron copy. So if ultimately not needed, I will be happy to get rid of it ;). Note however that this ld preload lib is already tested (see its test.c module).

This is currently a draft, my intention was to get some help making this work without have to create an electron copy (i.e: reuse the existing copy).

A much simpler (less efficient but most likely more robust) approach would be to simply integrate this to the existing vscode derivation adding a extraBuiltInExtensions parameter right there and merge these with existing builtin extensions under $out/lib/vscode/resources/app/extensions. However, the downside would be that every time the set of builtin / system extensions would be changed, a new copy of the entire vscode derivation would be created in the nix store.

@Synthetica9:

Is this also possible with VSCodium?

Most likely, I assume it uses the same extension mechanism and electron.

If so; we should probably also test this with a NixOS test.

That would be nice once converging once a final acceptable solution if picked. Remember, this is merely a draft. I would be interested in the approach you would use to automate the check that vscode still works and that the extensions were effectively loaded (vscode being a gui).

[jraygauthier]

Some refs to the electron copy I would like to avoid:

nixpkgs/pkgs/applications/editors/vscode/with-system-extensions.nix

Line 124 in 461dd5d

# TODO: Avoid this copy by using a 'LD_PRELOAD' trick. Launching

And the commented out failed attempt at using an LD_PRELOAD hack to trick the electron process into thinking its executable is the wrapper (so that it uses our modified $out/lib/vscode/resources and not the original) by changing the returned value for /proc/self/exe (as in $ readlink /proc/self/exe) :

nixpkgs/pkgs/applications/editors/vscode/with-system-extensions.nix

Line 132 in 461dd5d

# TODO: Investigate. Causes trouble.

[Synthetica9]

I would be interested in the approach you would use to automate the check that vscode still works and that the extensions were effectively loaded (vscode being a gui).

Probably something like:

• Install an extension using the mechanism proposed here
• Verify that that works (By checking that it is enabled in the extensions view?)
• Install an extension from the store
• Verify that that works (dito)

We'd test this the same way we currently test gui applications, including vscodium: by just simulating keypresses, see https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/vscodium.nix

[jraygauthier]

@Synthetica9 : Thanks for the example, I was wondering how you would handle getting a feedback post you key-presses. Using the "save as" side effect was quite crafty. I will have to think about something like this to validate extensions functionality. Clearly some extensions change the editor text so I would be able to re-use the same save trick.

[mohe2015]

While I like the idea I'm not sure if we should do this as long as you can easily download extensions from the website / use extensionsFromVscodeMarketplace and declaratively use them. Of course some users don't want to configure everything declaratively but I think this is a drawback you sometimes need to make with nix.

[pasqui23]

@mohe2015 I think that this PR allows vscode to both install extensions imperatively and declaratively

[jraygauthier]

@pasqui23 / @mohe2015 : Exactly, it allows both and that is the exact reason why I think it would be the best approach if we find a way solve the above mentioned size issue (it might even be deemed quite acceptable without solving this issue given what it brings to the table).

However, I think I tried too early to attempt fixing this size issue and it just lead to unwarranted complexities. These are apparently getting in the way of showcasing this new approach.

When I have some more spare time, I think I will just open separate cleaner non-draft PR with the simpler vscode derivation extraBuiltInExtensions approach described above and go with it as a first step. If this new PR is merged, it will provide people with an opportunity of experimenting with the approach and might just make the point (the remaining size issue) of the current issue more tangible and might lead more people contributing ideas how to fix / workaround it.