search

microsoft / vscode-remote-repositories-github

Quickly browse, search, edit, and commit to any GitHub or Azure Repos repository directly from within Visual Studio Code.
Creative Commons Attribution 4.0 International
205 stars 107 forks source link

Adopt new mechanism for publishing extension insiders builds #91

Closed joyceerhl closed 2 years ago

joyceerhl commented 2 years ago

See https://github.com/microsoft/vscode/issues/15756

isidorn commented 2 years ago

@joyceerhl if you need help feel free to reach out. I will write some docs later this week to help with this.

isidorn commented 2 years ago

Here are the docs https://github.com/microsoft/vscode-docs/blob/vnext/api/working-with-extensions/publishing-extension.md#pre-release-extensions Soon they will be on the website.

joyceerhl commented 2 years ago

Problem

Who is using RemoteHub Insiders

Case study: GHPRI

Other migrations we have done in the past

Corner cases to consider

Proposed user migration experience

Open questions

joyceerhl commented 2 years ago

Another thought: today we bundle the insiders version of RemoteHub in insiders.vscode.dev through the builtinExtensions workbench construction option but I don't imagine we can do that via the workbench construction options once RemoteHub adopts the prerelease publishing flow. We could just let the user manually switch to the pre-release version even in insiders.vscode.dev. We would need to ensure that the prerelease extension flow is smooth for extensions which are bundled as builtin in the workbench embedder, cc @joaomoreno

joaomoreno commented 2 years ago

Another thought: today we bundle the insiders version of RemoteHub in insiders.vscode.dev through the builtinExtensions workbench construction option but I don't imagine we can do that via the workbench construction options once RemoteHub adopts the prerelease publishing flow. We could just let the user manually switch to the pre-release version even in insiders.vscode.dev. We would need to ensure that the prerelease extension flow is smooth for extensions which are bundled as builtin in the workbench embedder, cc @joaomoreno

What do you think we should do, @sandy081?

joaomoreno commented 2 years ago

Oh I guess we should just follow the same pattern as for GHPR, right @sandy081?

sandy081 commented 2 years ago

Yes. Same as GHPRI extension.

/**
 * The identifier of an extension in the format: `PUBLISHER.NAME`.
 * For example: `vscode.csharp`
 */
type ExtensionId = string;

type MarketplaceExtension = ExtensionId | { readonly id: ExtensionId, preRelease?: boolean };
/**
 * Additional builtin extensions those cannot be uninstalled but only be disabled.
 * It can be one of the following:
 *  - an extension in the Marketplace
 *  - location of the extension where it is hosted.
 */
readonly additionalBuiltinExtensions?: readonly (MarketplaceExtension | UriComponents)[];

Please let me know if there is any help I can do or if you need any info.

joyceerhl commented 2 years ago

@sandy081 does the automatic migration touch extension storage at all? I'd prefer for RemoteHub to display some kind of dialog or user confirmation before starting the migration.

sandy081 commented 2 years ago

Automatic migration does only uninstalling the nightly extension and install the pre-release extension. Storage is not migrated. It is mentioned in the Note here https://github.com/microsoft/vscode-docs/blob/vnext/api/working-with-extensions/publishing-extension.md#pre-release-extensions

isidorn commented 2 years ago

We have internal commands to install pre-release extension and to uninstall extensions, so if you would like to customise this experience you could do it from the Remote-Repositories extension. For example show a dialog, notification and only once user agrees you execute those commands...

sandy081 commented 2 years ago

This might not work for GitHub Repositories extension in insiders.vscode.dev because it is an additional builtin extension contributed by vscode.dev. So even though extension uninstalls the nightly extension, the nightly extension will come back on reload.

joyceerhl commented 2 years ago

After talking to Sandeep yesterday about a related problem, I now believe that RemoteHub can adopt pre-releases the same way that GHPRI did, i.e.:

vscode.dev

On vscode.dev, RemoteHub and VS Code qualities are tightly coupled--RemoteHub Insiders is pre-installed in insiders.vscode.dev, and RemoteHub Stable is pre-installed in vscode.dev. There's no way to uninstall the bundled RemoteHub extension, and users will get a warning if they try to install RemoteHub Insiders in vscode.dev or RemoteHub Stable in insiders.vscode.dev. I learned from Sandeep that insiders.vscode.dev and vscode.dev have separate IndexDB instances, so we won't be overwriting any changes by copying github.remotehub-insiders extension storage to github.remotehub extension storage on vscode.dev.

image

github.dev

On github.dev, RemoteHub Stable is always pre-installed, regardless of VS Code quality. Any usage of RemoteHub Insiders results in an error notification. There shouldn't be any cases where RemoteHub Insiders is used. (Note however that it looks like github.dev has the same shared IndexDB instance across VS Code qualities)

Desktop

On desktop, a user may have installed:

In this situation, VS Code core cannot simply overwrite github.remotehub extension storage with github.remotehub-insiders extension storage. At minimum we would need to check which one the user would prefer before blowing away user data. In this scenario I think it might be better to simply mark the extension as deprecated but not uninstall it, and on extension activation, display a warning that says "This extension has been deprecated in favor of pre-release versions of the github.remotehub extension. Please commit all your uncommitted changes to avoid losing work."

image

@sandy081 let me know if I have this right. If so, then I believe we can proceed with the following action items:

  1. Publish pre-release builds of RemoteHub

    • [x] Require VS Code >1.63 (we already do this due to adopting enabledApiProposals)
    • [x] Change Publish insiders workflow to publish with --pre-release flag instead of tacking on -insider to extension id
    • [x] After bumping version number to publish stable builds, also bump version number in main branch so that VS Code picks up pre-releases over stable releases if the user has opted into pre-releases
    • [ ] Remove the error message that we display when github.remotehub-insiders and github.remotehub are installed together, since that will be redundant

  2. Publish one final RemoteHub Insiders which displays a modal dialog warning that it has been deprecated in favor of pre-releases

  3. Add RemoteHub to the migration list, migrate extension storage in web, and mark extension as deprecated

  4. Update vscode.dev to use pre-releases by default

    • [x] Bundle the pre-release version of RemoteHub instead of the deprecated RemoteHub Insiders extension
sandy081 commented 2 years ago

@joyceerhl Above makes sense to me.

Please correct me if I understand correctly:

joyceerhl commented 2 years ago

On desktop we will have a notification indicating that github-remotehub-insiders is deprecated and asking the user to ensure they have committed any changes that they need, with two action buttons to

  1. Install Pre-Release
  2. Don't Show Again

The Install Pre-Release option will install RemoteHub pre-releases and then uninstall RemoteHub Insiders.

Lad convinced me today that we should not use a modal dialog on desktop because it's up to the user if they choose to continue using the deprecated RemoteHub Insiders extension. We will also explicitly call out in the RemoteHub Insiders README that RemoteHub Insiders is deprecated and unsupported.

joyceerhl commented 2 years ago

Here's the Insiders deprecation flow on desktop:

  1. Deprecation warning notification displayed on extension activation, and README also contains deprecation notice image
  2. Select the action button to install image
  3. Insiders extension gets uninstalled, stable extension gets installed image

The pre-release version of the extension will also include a notice saying that it works best with VS Code Insiders, which is what we had for RemoteHub Insiders builds:

image

joyceerhl commented 2 years ago

While testing migrating uncommitted changes from github.remotehub-insiders to the pre-release of github.remotehub, I discovered that RemoteHub actually stores uncommitted changes in two separate IndexedDB databases:

  1. It writes metadata representing which files have uncommitted changes to the vscode-web-state-db-global database. This metadata is stored under the extension ID, i.e. github.remotehub-insiders.
  2. The actual changed file contents are stored in the vscode-web-db database via vscode-userdata URIs (this is abstracted away from RemoteHub, we store these using the ExtensionContext.globalStorage URI and workspace.fs.writeFile). If we don't migrate this data as well, the SCM view will say that files have changed, but RemoteHub won't be able to resolve the actual changed contents, so for example opening the diff editor will show that there are no uncommitted changes.

    image

So to completely migrate uncommitted changes and ensure that they continue to be available in web, we would need to:

  1. Copy the github.remotehub-insiders value in the vscode-web-state-db-global table to a new key, github.remotehub. I simulated this by running the following snippet in devtools:
var connection = indexedDB.open('vscode-web-state-db-global', 1);
connection.onsuccess = (e) => {
    var database = e.target.result;
    var transaction = database.transaction('ItemTable', 'readwrite');

    var objectStore = transaction.objectStore('ItemTable');

    var cursor = objectStore.openCursor();

    const data = new Promise((resolve, reject) => {
        cursor.onsuccess = () => {
            if (cursor === undefined || cursor.result.key === 'github.remotehub-insiders') {
                resolve(cursor.result);
            } else {
                cursor.result.continue();
            }
        }        
    });

    data.then((d) => {
        objectStore.put(d.value, 'github.remotehub');
    });
}
  1. Copy any values stored under a key that begins with /User/globalStorage/github.remotehub-insiders/ in the vscode-web-db table to a key that begins with /User/globalStorage/github.remotehub/. I simulated this in RemoteHub by doing a string replace on the globalStorage uri path that RemoteHub prefixes the URIs for all files with uncommitted changes with.

Extensions can't access other extensions' memento storage, so we definitely need workbench help with part 1. RemoteHub cannot enumerate the keys in the vscode-web-db table since that is abstracted away behind the vscode-userdata URI, so RemoteHub can't cleanly attempt part 2--we would have to do the migration as each file read request comes in. @sandy081, Is this something that the workbench can help to migrate as well?

eamodio commented 2 years ago

@joyceerhl in your comment above (https://github.com/microsoft/vscode-remote-repositories-github/issues/91#issuecomment-991201718) you mention that the Insiders version of RemoteHub can't be used on github.dev -- but it certainly can as I have been using it that way for a while (might require switching to the Insiders version of VS Code though).

And as for the desktop migration (or lack of) plan -- will it be clear to users what "data" would be lost? Because it could certainly be spread across many repositories (not just the currently opened one).

joyceerhl commented 2 years ago

@eamodio How did you get RemoteHub Insiders installed in github.dev with Insiders VS Code? With VS Code Insiders in github.dev, if I uninstall the stable build of RemoteHub and install the insiders build of RemoteHub, the stable build of RemoteHub comes back on a reload and I get the error notification from RemoteHub about the side by side stable/insiders installs not being supported. The only way this seems possible is if you disable the stable build of RemoteHub and install the insiders build of RemoteHub, and in that case it seems unlikely the user would have stable changes to retrieve. (And there's no option to disable the stable build of RemoteHub in vscode.dev--not sure why)

will it be clear to users what "data" would be lost? Because it could certainly be spread across many repositories (not just the currently opened one)

We should definitely clearly call that out in the message we display to users, and in the extension README. Was there any language from the previous extension ID rename that we can reuse? We should also call out how users can retrieve their changes the way we did for ms-vscode.remotehub:

Migrating from the previous (deprecated) version

If you were using the previous version of Remote Repositories, and have uncommitted changes that you want to retrieve, please install the Remote Repositories (Deprecated) extension. Then open the desired repository and commit your changes.

eamodio commented 2 years ago

@joyceerhl Yeah, you need to disable the Stable version and install the Insiders version. I agree, that if you are in that mode you likely don't have stable data that you care about, but we'd need to migrate the Insiders version of data -- so wanted to make sure that would happen.

sandy081 commented 2 years ago

Is this something that the workbench can help to migrate as well?

Currently I am working on migrating all extension storages from insiders extension to pre-release extension, this includes

sandy081 commented 2 years ago

@joyceerhl Summarizing my understandings. Please correct me if wrong

https://github.com/microsoft/vscode/blob/5e630c145f53e9a9a2975806bf8530e64ba43ae5/src/vs/workbench/workbench.web.api.ts#L35

So, I would recommend following:

Let me know if it covers your major scenarios?

joyceerhl commented 2 years ago

Yes, to my knowledge that should cover RemoteHub's needs for migration. To signal the workbench through API, would we have something like the following?

type MarketplaceExtension = ExtensionId | { readonly id: ExtensionId, preRelease?: boolean, migrateStorage?: boolean };
sandy081 commented 2 years ago

Exactly ( you read my mind 😄 ), but with a small change 

type MarketplaceExtension = ExtensionId | { readonly id: ExtensionId, preRelease?: boolean, migrateStorageFrom?: ExtensionId };
joyceerhl commented 2 years ago

Yes that makes sense to me 😊 I have a PR to adopt the preRelease option for RemoteHub and GHPRI in the vscode.dev embedder, so when the migrateStorageFrom option is available in Insiders, I will update that PR to signal storage migration for RemoteHub and test to ensure that the migration works as expected.

sandy081 commented 2 years ago

@joyceerhl Added above API - https://github.com/microsoft/vscode/issues/140088 - this will be available in tomorrow's insiders. Please test it out and adopt. Thanks.

joyceerhl commented 2 years ago

Tested API in Code-OSS + web, migration works like a charm 🙏Will test again with VS Code Insiders + web tomorrow before merging my vscode.dev PR. Then I'll publish a final Insiders update with the deprecation notice so that desktop users will be notified, and remove our RemoteHub Insiders publishing workflow.

joyceerhl commented 2 years ago

API looks good in Insiders too with some more testing this morning--now merging and deploying embedder adoption of preRelease and migrateStorageFrom flags for additionalBuiltinExtensions.

joyceerhl commented 2 years ago

There seems to be an issue where the global storage contents get wiped. I've reverted my change and redeployed vscode.dev for now so that more insiders.vscode.dev users do not run into this. From debugging with Sandeep we saw this in the window logs:

[2022-01-12 09:59:26.699] [window] [info] Migrating workspace extension storage from GitHub.remotehub-insiders to GitHub.remotehub...
[2022-01-12 09:59:26.837] [window] [info] Migrated workspace extension storage from GitHub.remotehub-insiders to GitHub.remotehub
[2022-01-12 09:59:26.957] [window] [info] Migrating global extension storage from github.remotehub-insiders to GitHub.remotehub...
[2022-01-12 09:59:27.021] [window] [info] Migrated global extension storage from github.remotehub-insiders to GitHub.remotehub
[2022-01-12 09:59:27.086] [window] [info] Migrating workspace extension storage from github.remotehub-insiders to GitHub.remotehub...
[2022-01-12 09:59:27.150] [window] [info] Migrated workspace extension storage from github.remotehub-insiders to GitHub.remotehub
joyceerhl commented 2 years ago
rchiodo commented 2 years ago

/verified

Tried the deprecated version. It told me to update:

image

Prerelease is default in insiders:

image