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

Infer publish target from package manifest & allow multiple --packagePath args #461

Closed
radeksimko opened this issue May 31, 2022 · 7 comments · Fixed by #482
Closed

Infer publish target from package manifest & allow multiple --packagePath args #461

radeksimko opened this issue May 31, 2022 · 7 comments · Fixed by #482
Assignees

Comments

@radeksimko
Copy link

I'm not sure if this relates to #450 (comment) at all - i.e. the server not reflecting target at all. A quick look into the codebase would suggest it doesn't - feel free to correct me if I'm wrong though!

Currently ovsx behaviour seems to differ from vsce in that it essentially requires --target/-t for platform-specific publishing. When the flag is omitted and platform-specific vsix is provided via --packagePath, that vsix is uploaded as what appears to be universal build.


$ find . -iname 'terraform-*-2.21.0.vsix'
./terraform-darwin-arm64-2.21.0.vsix
./terraform-linux-x64-2.21.0.vsix
./terraform-win32-arm64-2.21.0.vsix
./terraform-linux-armhf-2.21.0.vsix
./terraform-darwin-x64-2.21.0.vsix
./terraform-linux-arm64-2.21.0.vsix
./terraform-win32-ia32-2.21.0.vsix
./terraform-win32-x64-2.21.0.vsix
$ npx ovsx publish --packagePath $(find . -iname 'terraform-*-2.21.0.vsix')
🚀  Published hashicorp.terraform v2.21.0
❌  Extension hashicorp.terraform version 2.21.0 is already published.
See the documentation for more information:
https://github.com/eclipse/openvsx/wiki/Publishing-Extensions
$ cd /Users/radeksimko/.vscode-oss/extensions/hashicorp.terraform-2.21.0
$ grep 'TargetPlatform' .vsixmanifest
<Identity Language="en-US" Id="terraform" Version="2.21.0" Publisher="hashicorp" TargetPlatform="win32-x64"/>
$ cat package.json | jq .__metadata
{
  "id": "4a23294b-fd16-4c51-9759-da9936474cf8",
  "publisherId": "a8dd781d-e144-400a-943c-417165981be2",
  "publisherDisplayName": null,
  "targetPlatform": "undefined",
  "updated": false,
  "isPreReleaseVersion": false,
  "preRelease": false,
  "installedTimestamp": 1653989764075
}

The mentioned publish command with $(find ...) represents IMO common workflow in CI, where targets are inferred from the manifest inside vsix, which is what vsce seems to do:
https://github.com/microsoft/vscode-vsce/blob/2ec05b137bdbfa743e10c674241396f1cd86e5aa/src/publish.ts#L47-L54

ovsx does seem to support publishing of multiple targets at a time, but AFAICT they still have to be passed explicitly to --target:

const packagePaths = options.packagePath || [undefined];
const targets = options.targets || [undefined];
for (const packagePath of packagePaths) {
for (const target of targets) {
internalPublishOptions.push({ ... options, packagePath: packagePath, target: target });
}
}
await Promise.all(internalPublishOptions.map(publishOptions => doPublish(publishOptions)));


I think that aligning the two flags' behaviour with vsce would be useful.

More importantly I think that ovsx should prevent publishing platform-specific extension as universal as that leaves it broken for everyone except for the one platform, which is especially unfortunate when such versions can't be unpublished. That is - even if you decide to not infer target and not support multiple args in packagePath, ovsx should error out in the above cases.

@amvanbaren
Copy link
Contributor

@radeksimko The main difference between ovsx and vsce is that vsce has some client-side validation before it publishes an extension. That's why vsce extracts the target from the xmlManifest. On the other hand ovsx only uses target to build a package. If packagePath points to a *.vsix package then target is not used.

// if the packagePath is a link to a vsix, don't need to package it
if (options.packagePath && options.packagePath.endsWith('.vsix')) {
options.extensionFile = options.packagePath;
delete options.packagePath;
delete options.target;
}

open-vsx.org doesn't support target platform yet. It might be useful to add a compatibility check in ovsx to check for the features the server supports. Especially since ovsx lets you configure the registry to connect to via the -r or --registryUrl option.

Do the same issues pop up when you run the master branch locally or on Gitpod?

@radeksimko
Copy link
Author

radeksimko commented Jun 8, 2022

If packagePath points to a *.vsix package then target is not used.

Oh that's good to know actually! That would be another blocker I'd hit straight away once open-vsx.org supports target platforms. 😅 Does that mean it's impossible to build the packages once and publish them both to MS Marketplace and open-vsx.org if we need platform-specific builds?

The main difference between ovsx and vsce is that vsce has some client-side validation before it publishes an extension. That's why vsce extracts the target from the xmlManifest. On the other hand ovsx only uses target to build a package.

Assuming vsix is basically just a ZIP file and the manifest is XML - both being non-proprietary files, is there a reason why ovsx can't read the vsix and infer the target from there?

It might be useful to add a compatibility check in ovsx to check for the features the server supports.

👍🏻 💯 totally - that would have saved a lot of my time and attempts earlier 😄

Do the same issues pop up when you run the master branch locally or on Gitpod?

You mean when I try to spin up my own open-vsx.org server locally? I can try that.

I don't understand your reference to Gitpod though - I thought Gitpod is just another "consumer" of extensions from open-vsx.org - where does publishing come into this?

@amvanbaren
Copy link
Contributor

Does that mean it's impossible to build the packages once and publish them both to MS Marketplace and open-vsx.org if we need platform-specific builds?

ovsx uses the createVSIX function in the vsce package to build extension packages, so packages built for the MS Marketplace (using vsce) should be the same as packages built with ovsx. The vsce version that ovsx uses is a bit older though.
Ultimately it's how the server processes the package when it's published that makes the difference.

Assuming vsix is basically just a ZIP file and the manifest is XML - both being non-proprietary files, is there a reason why ovsx can't read the vsix and infer the target from there?

It can, but for now the server reads the target from the XML manifest and performs the same type of checks server-side. As bandwith usage grows it's better to duplicate this logic to ovsx, so that the publish command fails faster without having to upload the package to the server first.
For comparison:

  • The vsce client-side validation: https://github.com/microsoft/vscode-vsce/blob/main/src/publish.ts#L143-L171
  • The openvsx server-side validation:
    var extension = repositories.findExtension(extensionName, namespace);
    if (extension == null) {
    extension = new Extension();
    extension.setName(extensionName);
    extension.setNamespace(namespace);
    extension.setPublishedDate(extVersion.getTimestamp());
    vsCodeIdService.createPublicId(extension);
    entityManager.persist(extension);
    } else {
    var existingVersion = repositories.findVersion(extVersion.getVersion(), extVersion.getTargetPlatform(), extension);
    if (existingVersion != null) {
    throw new ErrorResultException(
    "Extension " + namespace.getName() + "." + extension.getName()
    + " " + extVersion.getVersion()
    + (TargetPlatform.isUniversal(extVersion) ? "" : " (" + extVersion.getTargetPlatform() + ")")
    + " is already published"
    + (existingVersion.isActive() ? "." : ", but is currently inactive and therefore not visible."));
    }
    }

I don't understand your reference to Gitpod though - I thought Gitpod is just another "consumer" of extensions from open-vsx.org - where does publishing come into this?

Gitpod is a cloud developer tool that provides ready-to-code workspaces. And yes, they also use open-vsx.org to provision workspaces with the right extensions 😄. You can install the Gitpod browser addon or create a new workspace at https://gitpod.io/workspaces. It saves you the trouble of setting up PostgreSQL, ElasticSearch, running the webui and server, and publishing some test extensions.

@radeksimko
Copy link
Author

I have to say I am somewhat struggling to stand up the dev environment in gitpod.

I am not familiar with Java, so maybe I'm missing something obvious.

Both Postgres and ElasticSearch seem to be running
Screenshot 2022-06-10 at 10 41 17
Screenshot 2022-06-10 at 10 40 38

and the UI app as well
Screenshot 2022-06-10 at 10 45 13

but I was unable to start the server itself

Screenshot 2022-06-10 at 10 43 54

***************************
APPLICATION FAILED TO START
***************************

Description:

Failed to configure a DataSource: 'url' attribute is not specified and no embedded datasource could be configured.

Reason: Failed to determine a suitable driver class


Action:

Consider the following:
        If you want an embedded database (H2, HSQL or Derby), please put it on the classpath.
        If you have database settings to be loaded from a particular profile you may need to activate it (no profiles are currently active).

https://gist.github.com/radeksimko/b864ce47e964f921815e9847b918bb43

@amvanbaren
Copy link
Contributor

Here's how I open Open VSX in Gitpod:

  • Install the Gitpod browser addon
  • Navigate to https://github.com/eclipse/openvsx
  • Click the green Gitpod button
  • A new tab opens
  • Login with Github, if prompted
  • Let the workspace initialize (takes ~5 min)

gitpod-open-openvsx2

@radeksimko
Copy link
Author

Looks like I didn't need the extension, but this was the crucial bit 👇🏻 - Thanks!

Let the workspace initialize (takes ~5 min)

It wasn't entirely obvious that I needed to wait for anything 😅


With the dev environment running now, I was able to publish vsix for multiple platforms and the target was effectively inferred (although on the server-side, as you mentioned).

I was also able to work around the single-argument --packagePath

find . -iname 'terraform-*-2.20.0.vsix' | xargs -I{} npx ovsx publish --packagePath {}

which isn't too bad - maybe even cleaner than the previous command.

The only minor oddity was the UI, which only seems to show the platform which was first uploaded:
Screenshot 2022-06-13 at 09 25 10

I didn't try to install these locally published extensions (not sure how to point VS Codium or Gitpod to a custom registry), but the publishing process at least succeeded on the CLI, so that gives me some confidence? 😅


I guess the primary source of confusion is really mainly the old version of the server, allowing platform-specific extensions to be silently published as some kind of weird hybrid between universal and the target platform.

Do you have any update on when the new version of the server is going to be rolled out?

@amvanbaren
Copy link
Contributor

The only minor oddity was the UI, which only seems to show the platform which was first uploaded:

That's a bug in both the server and ovsx.
On the server side when during publishing an ExtensionVersion doesn't have an Extension, the server creates a new Extension instance. Now that target platforms are supported and publishing requests are handled in parallel, it can happen that the same Extension instance is created multiple times in different transactions. This causes a data integrity violation, and that's why you only see one target platform in the webui.
And ovsx exits on the first error it encounters, which makes it unclear to the end user what exactly happened.

PR #482 addresses these issues. If you want you can test it by running the server on Gitpod and building the cli locally.

Do you have any update on when the new version of the server is going to be rolled out?

There's been some progress. You can follow the current status through issue https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/1483

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants