Skip to content

misc/vscode-extensions: improve scripts for updating VSC extensions#113280

Closed
cideM wants to merge 1 commit intoNixOS:masterfrom
cideM:improve-vscode-scripts
Closed

misc/vscode-extensions: improve scripts for updating VSC extensions#113280
cideM wants to merge 1 commit intoNixOS:masterfrom
cideM:improve-vscode-scripts

Conversation

@cideM
Copy link
Contributor

@cideM cideM commented Feb 16, 2021

This PR is a bit out of the blue but I've added my motivation for it under the appropriate heading. It was a fairly spontaneous thing that came up when I was migrating from controlling VSC extensions with VSC to Nix.

Motivation for this change

The current update_installed_exts.sh script does several things. It tries to find the Visual Studio Code (VSC) binary (although it misses -insiders), and then it lists the extensions and prints a Nix object for each extensions which contains the updated version and sha. The idea is that it makes it easy to migrate to having extensions controlled with Nix.

Unfortunately this makes it hard to reuse the functionality in that script in other settings. What if I just want to update a single extension?

I thought it would be nice to just separate the two concerns. As such I've created two separate scripts which are used in the update_installed_exts.sh script, so that it's 100% backwards compatible. The new update_exts.sh script makes it easier to e.g., pipe the name of an extension to it.

# Examples:
#
# $ echo 'ms-toolsai.jupyter' | ./update_exts.sh
#   {
#     name = "jupyter";
#     publisher = "ms-toolsai";
#     version = "2020.12.414227025";
#     sha256 = "1zv5p37qsmp2ycdaizb987b3jw45604vakasrggqk36wkhb4bn1v";
#   }
#
# $ ./list_installed_exts.sh | ./update_exts.sh
# ...lots of output as shown above
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • 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
  • Fits CONTRIBUTING.md.

@cideM cideM requested a review from jonringer as a code owner February 16, 2021 09:06
@cideM cideM force-pushed the improve-vscode-scripts branch from 25c6047 to 19ec7b1 Compare February 16, 2021 09:08
@cideM cideM changed the title Improve scripts for updating VSC extensions misc/vscode-extensions: improve scripts for updating VSC extensions Feb 16, 2021
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 16, 2021
@cideM cideM force-pushed the improve-vscode-scripts branch from 19ec7b1 to 3d41ca2 Compare February 16, 2021 10:35
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Feb 16, 2021
@cideM
Copy link
Contributor Author

cideM commented Feb 19, 2021

I also need to make sure that the other shell scripts are called relative to the first file, not relative to the working directory. Or maybe it's OK to use relative paths here as long as it's mentioned in the Wiki and the script itself

@SuperSandro2000
Copy link
Member

I also need to make sure that the other shell scripts are called relative to the first file, not relative to the working directory. Or maybe it's OK to use relative paths here as long as it's mentioned in the Wiki and the script itself

That should be documented in here https://github.com/NixOS/nixpkgs/tree/master/doc/languages-frameworks

@cideM
Copy link
Contributor Author

cideM commented Feb 21, 2021

Haven't forgotten was just busy over the weekend, will do the necessary changes over the next couple of days

@cideM cideM force-pushed the improve-vscode-scripts branch from 311bef0 to 26aac62 Compare February 26, 2021 22:20
@cideM
Copy link
Contributor Author

cideM commented Feb 26, 2021

I had a change of heart and decided to remove the existing script which lists installed extensions. After extracting the other bit it became apparent that listing extensions is a bit of bash when the user can just call code --list-extensions. I don't think abstracting that away helps.

I also added documentation, as was asked. I took some cues from the existing info in the Wiki:

Please let me know if that's in the right direction 🙏

@ofborg ofborg bot added the 8.has: documentation This PR adds or changes documentation label Feb 26, 2021
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples are quite long. Maybe they could be reduced to the parts that matter for that section?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking lines will likely break in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm yeah that's a good point I'll take that out

@SuperSandro2000
Copy link
Member

Not sure why the manual fails to build.

@cideM cideM force-pushed the improve-vscode-scripts branch from 26aac62 to 4f66b75 Compare March 8, 2021 18:13
@cideM
Copy link
Contributor Author

cideM commented Mar 8, 2021

@SuperSandro2000

I updated the PR. I removed all line links. I also overhauled the documentation so it has less superfluous code snippets and gets to the point a bit quicker.

Hopefully fixed doc build by giving the file a valid name

@cideM cideM force-pushed the improve-vscode-scripts branch 6 times, most recently from c4736e5 to df4e075 Compare March 8, 2021 20:53
@SuperSandro2000
Copy link
Member

Hopefully fixed doc build by giving the file a valid name

It did.

@SuperSandro2000
Copy link
Member

Please also add an entry to https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/index.xml and yourself as a codeowner for doc/languages-frameworks/vscode.section.md.

@cideM cideM force-pushed the improve-vscode-scripts branch from df4e075 to 12360bc Compare March 17, 2021 12:52
@cideM cideM requested a review from edolstra as a code owner March 17, 2021 12:52
@cideM
Copy link
Contributor Author

cideM commented Mar 17, 2021

  • Added myself to CODEOWNERS for vscode docs
  • Fixed link
  • Added index entry

@ofborg ofborg bot added the 6.topic: policy discussion Discuss policies to work in and around Nixpkgs label Mar 17, 2021
@cideM cideM closed this Apr 27, 2021
@cideM cideM deleted the improve-vscode-scripts branch April 27, 2021 10:04
@cideM cideM restored the improve-vscode-scripts branch April 27, 2021 10:05
@cideM cideM reopened this Apr 27, 2021
@cideM
Copy link
Contributor Author

cideM commented Apr 27, 2021

Accidentally removed the branch from my fork

@SuperSandro2000
Copy link
Member

@jonringer can you take a look at this?

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise looks mostly good to moe

@ShamrockLee
Copy link
Contributor

@cideM @jonringer I'll integrate #121583 with this commit after this PR is merged.

Please take a look at that PR then.

The current update_installed_exts.sh always assumes that you're trying
update all installed extensions. That makes it hard to use the
get_vsixpkg function in a different setting.

This commit removes listing extensions, since it's trivial for a user to
run `code --list-extensions` but keeps and modifies the update script,
so it's more flexible.

Documentation was added to explain how the script works and explain VSC
in Nix(OS) in general.

The actual code is mostly the same.
@cideM cideM force-pushed the improve-vscode-scripts branch from 12360bc to c58e9d3 Compare May 27, 2021 16:12
@cideM
Copy link
Contributor Author

cideM commented May 27, 2021

@jonringer I renamed the attribute that was sort of shadowing the package and I also added a paragraph about your FHS changes. It's somewhat duplicated from the Wiki but I'm in general unsure what goes where with regards to Wiki and the official documentation.

i also rearranged some headings to make the different modes of operation a bit more clear. You can ultimately use Nix or the VSC UI and I think that's gonna be the most important question people have, so it should be easy to find.

@cideM
Copy link
Contributor Author

cideM commented Jun 30, 2021

ping @jonringer (or should I ping someone else?)

@jonringer
Copy link
Contributor

I think this now conflicts with #127741.

We should probably have merged this long ago, sorry

@cideM
Copy link
Contributor Author

cideM commented Jul 1, 2021

Ah no worries, I'll just close this one then and see if the documentation changes can help with the other PR at least

@cideM cideM closed this Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: policy discussion Discuss policies to work in and around Nixpkgs 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants