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

add vscodeWithConfiguration, vscodeExts2nix, vscodeEnv #76624

Closed
wants to merge 3 commits into from

Conversation

@countoren
Copy link
Contributor

@countoren countoren commented Dec 28, 2019

move mktplcExtRefToFetchArgs to file in order to be shared with the new derivations(privately)

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • [ X ] macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • [ X ] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • [ X ] Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • [ X ] Fits CONTRIBUTING.md.
Notify maintainers

cc @jonringer @zimbatm
fix based on @zimbatm comments in #76427

countoren added 3 commits Dec 24, 2019
…nd vscodeEnv

move mktplcExtRefToFetchArgs to file in order to be shared with the new derivations(privately)
… vscodeExts2nix, vscodeEnv

change usage of toPath with toString
@eadwu
Copy link
Member

@eadwu eadwu commented Dec 29, 2019

Just a rough look so far but this looks like a vscode-with-extensions but just with mutable directories so that extensions can work without "heavy" patching instead of a vscodeWithConfiguration (which refers to the configuration file settings.json at first glance to me).

@countoren
Copy link
Contributor Author

@countoren countoren commented Dec 29, 2019

@eadwu I might not completely understand your comment(feel free to correct me) but in general:
vscode with extension installs the extensions in nix store, sometimes the extensions needs mutate files in the extension folder. base on the nix philosophy you don't want directories in the nix store to be altered. but you want to have to ability declaratively declare extensions that wont change or alter too. the problem here that vscode have in general 1 target extensions folder so in order to get the best of the both worlds(and to be able to use vscodeExts2nix which will get you the extensions that were chosen from vscode interface into a nix expression) you need to have an extensions folder that vscode could install new/update extensions into it.

@@ -0,0 +1,40 @@
#use vscodeWithConfiguration and vscodeExts2nix to create vscode exetuable that when exits(vscode) will update the mutable extension file, which is imported when getting evaluated by nix.
{ pkgs ? import <nixpkgs> {}

This comment has been minimized.

@eadwu

eadwu Dec 29, 2019
Member

Don't really see the point of re-importing nixpkgs here, most of the things here are already given (besides vscode-utils.extensionsFromVscodeMarketplace) which could be easily added.

This comment has been minimized.

@countoren

countoren Dec 30, 2019
Author Contributor

@eadwu sorry this was the wrong branch look at PR #76427 nixpkgs was removed there.


#removed not defined extensions
rmExtensions = lib.optionalString (nixExtensions++mutableExtensions != []) ''
find ${vscodeExtsFolderName} -mindepth 1 -maxdepth 1 ${lib.concatMapStringsSep " " (e : ''! -iname ${e.publisher}.${e.name}'') (nixExtensions++mutableExtensions)} -exec sudo rm -rf {} \;

This comment has been minimized.

@eadwu

eadwu Dec 29, 2019
Member

I don't really know how I feel about the fact sudo has to be used here.

This comment has been minimized.

@countoren

countoren Dec 30, 2019
Author Contributor

yes I don't like it too, any better idea for syncing the extensions folder (we need to be consistent with the nix expression when vscode starts) ?

cpExtensions = ''
${lib.concatMapStringsSep "\n" (e : ''ln -sfn ${e}/share/vscode/extensions/* ${vscodeExtsFolderName}/'') nixExtsDrvs}
${lib.concatMapStringsSep "\n" (e : ''
cp -a ${e}/share/vscode/extensions/${e.vscodeExtUniqueId} ${vscodeExtsFolderName}/${e.vscodeExtUniqueId}-${(lib.findSingle (ext: ''${ext.publisher}.${ext.name}'' == e.vscodeExtUniqueId) "" "m" mutableExtensions ).version}

This comment has been minimized.

@eadwu

eadwu Dec 29, 2019
Member

cp -a keeps the directory permissions the same, so the extension directory wouldn't be mutable no? The extension directory permissions for me are 555 (dr-xr-xr-x).

This comment has been minimized.

@countoren

countoren Dec 30, 2019
Author Contributor

for me on drawin it solved the permission problem on the csharp extension. but maybe it is not the same on a different OS. will changing permissions make sense?

@countoren
Copy link
Contributor Author

@countoren countoren commented Jan 6, 2020

outdated look at PR #76427

@countoren countoren closed this Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.