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

nix-prefetch-vscode-marketplace, nix-prefetch-openvsx: init at 0.1.0 #161369

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Feb 22, 2022

Motivation for this change

This patch introduces the prefetchers for Visual Studio Code extensions from the Marketplace as well as the Open VSX Registry.

This is originally part of #121583 and is part of the tooling to bootstrap and update the extensions from different registries. Considering that the multi-registry support still has a way to go and that these two prefetchers are also useful under the current extension framework, it would be great to have them merged first.

These packages were initially based upon https://github.com/NixOS/nixpkgs/blob/master/pkgs/misc/vscode-extensions/update_installed_exts.sh

Features:

  • JSON output
  • Ability to run multiple instances at the same time (through mktemp -t)
  • Option to disable hash output (-H) or the meta-related outputs (-M).
    Thanks to the Open VSX API, specifying -H means that the extension VSIX file will not be downloaded for nix-prefetch-openvsx, which greatly shorten the time for query for people who only want to check the latest version or the metadata information. This doesn't apply for nix-prefetch-vscode-marketplace due to VSCode Marketplace's lack of public documentation about their API and prohibition to third-party use from the EULA. Nevertheless, options M does prevent the downloaded VSIX from being decompressed.

Missing feature:

  • Shell completion
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.

@ShamrockLee ShamrockLee changed the title nix-prefetch-vscode-marketplace, nix-prefetch-openvsx: init at 0.1.0 nix-prefetch-vscode-marketplace, nix-prefetch-openvsx: init at 0.2.0 Mar 28, 2022
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/in-tree-or-out-of-tree/18389/1

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.

Please share the duplicated parts of the code. Maybe we can even combine the scripts?

@ShamrockLee ShamrockLee changed the title nix-prefetch-vscode-marketplace, nix-prefetch-openvsx: init at 0.2.0 nix-prefetch-vscode-marketplace, nix-prefetch-openvsx: init at 0.1.0 Jun 17, 2022
@ShamrockLee ShamrockLee force-pushed the prefetch-vscode-ext branch 2 times, most recently from 586c2e9 to be53d2c Compare June 17, 2022 17:56
@ShamrockLee
Copy link
Contributor Author

Please share the duplicated parts of the code. Maybe we can even combine the scripts?

The slight logical difference between the two fetchers makes the merge a bit trickier than just "combining the shared commands".

nix-prefetch-vscode-marketplace is closer to a general prefetcher, since there's no additional API legally available, and all the information about the extension can be extracted only after downloading the VSIX file. As for nix-prefetch-openvsx, a main selling point of these registries are a set of well-documented APIs that provides the meta info of the extensions. The files contained in each extensions can even be downloaded separately from Open VSX Registry instances.

I managed to combine the same part by creating a common library for the two scripts to source, in which each actions are separated into bash functions to simulate Nix-like overridable and lazily-evaluated "stages". Not sure if such approach is over-engineering.

@ShamrockLee
Copy link
Contributor Author

@SuperSandro2000 Would it be too over-engineered to merge the two scripts this way? If so, I'll change them back into two imperative scripts.

@ShamrockLee ShamrockLee force-pushed the prefetch-vscode-ext branch 3 times, most recently from 60f5cee to e6ade89 Compare October 29, 2022 03:39
@SuperSandro2000
Copy link
Member

@SuperSandro2000 Would it be too over-engineered to merge the two scripts this way? If so, I'll change them back into two imperative scripts.

If they share a lot of code then that is an option but I didn't take a deeper look at the scripts.

@ShamrockLee
Copy link
Contributor Author

Cc: @AmeerTaweel @schuelermine

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1507

@AmeerTaweel
Copy link
Contributor

@ShamrockLee
Did you mention me here intentionally or by mistake?
If it was intentional. Can you please tell me why? I've been busy with university lately and I may have forgotten some stuff...

@ShamrockLee
Copy link
Contributor Author

@AmeerTaweel Sorry for bothering you for minor stuff like this.

Just want to mention that the license information and other metadata of an extension can be extracted from the manifest (package.json) packed inside the VSIX file. The file can be extracted with unzip without decompressing the whole extension. This would be an alternative solution when the querying API is not available.

@AmeerTaweel
Copy link
Contributor

@ShamrockLee No no I did not mean that at all. I was just confused, not bothered.

Thanks for showing me this. It will be helpful with my VSCode extensions flake.

It's been a while since I worked on it because of university. But I will have more time in the semester break.

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Mar 13, 2023

  1. Mild refactor and prepare for the upcoming hash-query feature of OpenVSX. See [Feature] Get the checksum of the VSIX file from the API eclipse/openvsx#678.
  2. Bump to the top of the master branch.
  3. Suppress the false-positive ShellCheck information (SC2028). See SC2028 false positive (using the correct model snippet) koalaman/shellcheck#2486.

@ShamrockLee
Copy link
Contributor Author

Actually, I'm a bit frustrated about the Bash behavior to drop set -e property for functions / sub-commands used after if or before && or ||. This disables the query_vsix_hash part in query_vsix_hash || compute_vsix_hash from failing as early as possible and brings a series of error messages when hash querying doesn't work on the Open-VSX instance before printing the result.

This Bash behavior is documented here: https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html
The maintainer said in the mailing list that they cannot change it without breaking backward compatibility.

It seems that we can only solve it by switching to another programming language or framework (Makfile or Justfile), or by trying to return as quickly as possible when failed or even avoid this sort of error handling as a whole.

@ShamrockLee
Copy link
Contributor Author

Anyway, I work around the above issue with the LBYL strategy by checking whether there's an attribute named sha256 in the manifest (package.json).

The hash query implementation is here:
ShamrockLee/nixpkgs@prefetch-vscode-ext...ShamrockLee:nixpkgs:prefetch-vscode-ext-query

It can be included in the future when eclipse/openvsx#678 is merged.

@ShamrockLee
Copy link
Contributor Author

I have finally found the relevant URL that VSCode use to download extension VSIX files from the Marketplace:

The relevant downloadUrl

From https://github.com/microsoft/vscode/blob/1.76.2/src/vs/workbench/contrib/extensions/browser/extensionsActions.ts#L166-L197

When trying to download the extension redhat.java (publisher: redhat, extension name: java) version 1.17.2023032504, the download URL will be

https://marketplace.visualstudio.com/_apis/public/gallery/publishers/redhat/vsextensions/java/1.17.2023032504/vspackage

when targetPlatform is universal,

or

https://marketplace.visualstudio.com/_apis/public/gallery/publishers/redhat/vsextensions/java/1.17.2023032504/vspackage?targetPlatform=linux-x64

when targetPlatform is linux-x64.

Here, the serviceUrl for the official Visual Studio Marketplace is https://marketplace.visualstudio.com/_apis/public/gallery.

Comparison to the legacy download URL

The legacy download URL in the current implementation used in the current vscode.extensions in Nixpkgs and the current nix-prefetch-vscode-marketplace here is

https://redhat.gallery.vsassets.io/_apis/public/gallery/publisher/redhat/extension/java/1.17.2023032504/assetbyname/Microsoft.VisualStudio.Services.VSIXPackage

The key difference is that, the legacy download URL doesn't support targetPlatform other than universal, but it supports specifying the version as latest instead of the exact version. For the new download URL, you need to get the exact version first.

Status of the legacy download URL

Aside from the targetPratform issue, there are already packages that is only available through the new download URL. One example is vscjava.vscode-gradle. See #199157 (comment)

Issues to overcome before adopting the new download URL

So, there are currently two obstacles preventing me from switching to the relevant (new) download URL

  • curl always get blocked when trying to download the VSIX file through the new URL.

    {
      "$id": "1",
      "innerException": null,
      "message": "Request was blocked due to exceeding usage of resource Count in namespace AnonymousId. For more information on why your request was blocked, see the topic \"Rate limits\" on the Microsoft Web site (https://go.microsoft.com/fwlink/?LinkId=823950).",
      "typeName": "Microsoft.TeamFoundation.Framework.Server.RequestBlockedException, Microsoft.TeamFoundation.Framework.Server",
      "typeKey": "RequestBlockedException",
      "errorCode": 0,
      "eventId": 3000
    }

    Some potential solutions can be found in the answers to the SO question https://stackoverflow.com/questions/2440729/how-can-i-emulate-a-get-request-exactly-like-a-web-browser , one of them is by setting the "User Agent" with the -A flag.

  • Need to find a way to fetch the latest version prior to downloading the VSIX file.

    It would also reduce the overhead greatly if we could find the way VSCode get those metadata without downloading the VSIX file.

@superherointj superherointj removed their assignment Mar 30, 2023
@superherointj
Copy link
Contributor

@ShamrockLee I'm sorry to let you down again. I was trying to get to this but I just couldn't. I suggest you add someone else to review this.

@ShamrockLee
Copy link
Contributor Author

@ShamrockLee I'm sorry to let you down again. I was trying to get to this but I just couldn't. I suggest you add someone else to review this.

Sure.

@ShamrockLee
Copy link
Contributor Author

As the sha256-querying feature is released from the OpenVSX upstrem, I added the implementation as the second commit.

The rebasing of the first commit is only to reset the author name from Shmrock Lee to Yueh-Shun Li (my legal name).

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
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.

None yet

6 participants