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

Provide an API for extensions to know when they're running "remotely" #74188

Closed
DanTup opened this issue May 23, 2019 · 24 comments
Closed

Provide an API for extensions to know when they're running "remotely" #74188

DanTup opened this issue May 23, 2019 · 24 comments
Assignees
Labels
api api-finalization api-proposal feature-request Request for new features or functionality remote Remote system operations issues
Milestone

Comments

@DanTup
Copy link
Contributor

DanTup commented May 23, 2019

There are some things that don't make sense when an extension is running on a remote machine/container/etc. For example in Flutter when a user tries to run an application, if there is no connected device we will show them a list of local emulators that they can start (or given them an option to create a new one):

Screenshot 2019-05-23 at 9 19 33 am

It doesn't make sense for us to do this for a remote session because if we created/launched an emulator, the user wouldn't be able to see it.

Similarly, it doesn't make sense for us to offer to launch other interactive tools like this:

Screenshot 2019-05-23 at 9 22 02 am

As far as I can tell, there's no current API to tell if your extension is running on the "local machine" where it's safe to spawn processes for the user to interactive with, but I think there's a good need for one.

@jrieken
Copy link
Member

jrieken commented May 23, 2019

@jrieken jrieken added api api-finalization api-proposal remote Remote system operations issues labels May 23, 2019
@jrieken jrieken added this to the May 2019 milestone May 23, 2019
@jrieken jrieken added the feature-request Request for new features or functionality label May 23, 2019
@DanTup
Copy link
Contributor Author

DanTup commented May 23, 2019

Oh sweet, I should've checked in there 🙈

Thanks! 👍

@jrieken
Copy link
Member

jrieken commented May 27, 2019

We aren't sold on the current proposal anymore and we have pushed/proposed a simple vscode.env.remoteAuthority-api:

		/**
		 * The [authority](#Uri.authority)-component. When `undefined`, extensions are
		 * executed in the same context (e.g. operating system or machine) in which the UI
		 * of the editor is executed. If defined, extensions are running on a different
		 * context, for instance a remote machine.
		 *
		 * *Note* that no assumptions about the actual value should be made as it is defined
		 * by extensions and not by the editor.
		 */
		export const remoteAuthority: string | undefined;

https://github.com/Microsoft/vscode/blob/6a1574e21ed6b449aea8f09f83d29c62a231c548/src/vs/vscode.d.ts#L6044-L6053

jrieken added a commit that referenced this issue May 27, 2019
@DanTup
Copy link
Contributor Author

DanTup commented May 27, 2019

That still works for my needs, though out of curiosity, why the change? If an extension can't make any assumption about the string, doesn't it amount to just being a boolean (since all we can really do is remoteAuthority === undefined)?

@jrieken
Copy link
Member

jrieken commented May 27, 2019

If an extension can't make any assumption about the string

It's likely to have a meaning once the resolver API is out ;-)

@DanTup
Copy link
Contributor Author

DanTup commented May 27, 2019

Got it :) Btw, the change above was in the main vscode definition, not proposed - is it likely that'll ship in the upcoming release? Thanks!

@jrieken
Copy link
Member

jrieken commented May 28, 2019

@aeschli discussed this in the API sync and the idea is to bring more structure into this. Often URIs are like this: vscode-remote://foo+barcafebarblablub/some/path and to extensions only the part before the + is meaningful or stable. So instead of having the plain string we could have an object that exposes the prefix (foo) to begin with.

 // vscode-remote://ssh+jssjsjsj+sjsj/
		 
export const remoteAuthority: { prefix: string } | undefined;

cc @roblourens

@jrieken
Copy link
Member

jrieken commented May 28, 2019

latest and greatest:

vscode/src/vs/vscode.d.ts

Lines 6044 to 6057 in a429fd1

/**
* Information about running remotely. When `undefined`, extensions are executed in the same
* context (e.g. operating system or machine) in which the UI of the editor is executed.
* If defined, extensions are running in a different context, for instance a remote machine.
*
* A remote [uri](#Uri) adheres to this format: `remote_scheme://auth_prefix+auth_rest/path` where
* the `remote_scheme` is defined by the editor, `auth_prefix` and `auth_rest` are defined by extensions,
* and `path` is defined by the user.
*
* Extensions contributing a remote are encouraged to use a stable `auth_prefix`-value,
* e.g `remote_scheme://ssh+23/` and `remote_scheme://ssh+42`. Such prefix is exposed
* in the `prefix`-property of the returned object.
*/
export const remoteAuthority: { prefix: string } | undefined;

Someone please review/fix the doc comment for clarity and correctness

jrieken added a commit that referenced this issue May 28, 2019
@jrieken jrieken closed this as completed May 29, 2019
@jrieken jrieken added the verification-needed Verification of issue is requested label May 29, 2019
@jrieken jrieken reopened this May 29, 2019
@jrieken jrieken removed the verification-needed Verification of issue is requested label May 29, 2019
@jrieken jrieken modified the milestones: May 2019, June 2019 May 29, 2019
@jrieken
Copy link
Member

jrieken commented May 29, 2019

We are unsure about this, we will not finalise this for May. The current evolution of ideas/proposals:

// just a string, e.g. ssh+machineId. concern: only ssh makes sense
export const remoteAuthority: string | undefined;

// more structure, e.g prefix is ssh, the rest is left out
export const remoteAuthority: { prefix: string } | undefined;

// don't mention authority as it doesn't explain itself, use 'ssh', et al as type
export const remoteContext: { type: string } | undefined;

// don't mention remote at all, mix this with other potential contexts
export const executionContext: { kind: string | undefined };

@DanTup
Copy link
Contributor Author

DanTup commented May 29, 2019

@jrieken if just knowing "if we're remote" is likely a common use case (it's what I need - I need to make sure I don't show options that will spawn local interactive processes/tools like emulators), why not also expose a simple boolean. You could ship that easily, and make it back onto this once it's finalised (keeping the API simple for those with simple requirements).

@jrieken
Copy link
Member

jrieken commented May 29, 2019

The problem isn't about exposing this information but how to call it... Internally we use remoteAuthority because remote-uris look like this vscode-remote://ssh+b3fce7b3-533b/some/path. The authority portion of that uri (ssh+b3fce7b3-533b) is handed over to an extension to "resolve" that into a host, port pair.

However, that uri is completely opaque, never surfaced, and the fact that this is the authority component isn't important. That's how we got away from the "authority" name towards something more generic like "remote".

Then, this API should answer two questions: (1) is this extension host "remote" and (2) what kind of remote is this, e.g. wsl vs ssh. The latter isn't a finite list and depends on resolver extensions. Still, we can document some well known values, like wsl, ssh, or container (that's similar to language ids or certain uri schemes). There is value in knowing those values, e.g. running in wsl allows you to launch an android emulator installed on the windows side...

Last, the question came up if this is remote specific or not. Does this compare to nodejs's process.platform property? That leads towards vscode.env.executionContent.kind but raises the question what the default is? Also, should this be designed to accommodate future JavaScript contexts, e.g. expressing that an extension host runs inside a web worker or a nodejs worker?

So, more questions than answers. We are in endgame this week so little flex for API experiments... I keep this issue updated

@DanTup
Copy link
Contributor Author

DanTup commented May 29, 2019

There is value in knowing those values, e.g. running in wsl allows you to launch an android emulator installed on the windows side

Oh, I hadn't considered that. This makes more sense now, it seems like it doesn't squash to a simple boolean even for my needs after all :-)

(whether or not I could get WSL to "see" the emulator I launched in Windows, I'm not sure, but I guess that's for another day :-))

@aeschli
Copy link
Contributor

aeschli commented May 29, 2019

Ideally you could query your environment to find out whether it makes sense to launch a certain tool.
E.g. check whether the DISPLAY variable is set. Or whether you can find a certain executable.

@DanTup
Copy link
Contributor Author

DanTup commented May 29, 2019

Checking for executables isn't good enough (for ex. you might connect to a container that has the Android SDK installed, so we can list and launch emulators, but you won't see them), though I hadn't considered DISPLAY, that might work for some; I'll do some testing. Thanks! :-)

@jrieken
Copy link
Member

jrieken commented May 29, 2019

So following @chrmarti's and @dbaeumer's idea of making this totally independent of remote, I think the logical consequence is to not have this inside a wrapper object but straight on the env object (which already represents the environment in which vs code and/or the extension host runs). That would result in something like vscode.env.kind: string | undefined, alternative to kind could be name or others

@jrieken
Copy link
Member

jrieken commented May 29, 2019

Tho, on a second thought maybe vscode.env isn't the right place at all. Currently all its values refer to the environment in which the editor runs, e.g. vscode.env.clipboard is the clipboard to which the UI has access, not the clipboard of the remote side. Same for openExternal, machineId etc.

That means we might wanna go back to having something on ExtensionContext, similar to what we initially proposed:

//#region Joh - ExecutionContext
export enum ExtensionExecutionContext {
Local = 1,
Remote = 2
}
export interface ExtensionContext {
/**
* Describes the context in which this extension is executed, e.g.
* a Node.js-context on the same machine or on a remote machine
*/
executionContext: ExtensionExecutionContext;
}
//#endregion

@DanTup
Copy link
Contributor Author

DanTup commented Jun 20, 2019

@jrieken is the current proposed API (d5534b8#diff-4878e8b25d41f2ab7c2715852b1b97aa) a reasonable bet now, or is this all still up in the air? Thanks!

@jrieken
Copy link
Member

jrieken commented Jun 20, 2019

Latest and greatest proposal

	/**
	 * In a remote window the extension kind describes if an extension
	 * runs where the UI (window) runs or if an extension runs remotely.
	 */
	export enum ExtensionKind {

		/**
		 * Extension runs where the UI runs.
		 */
		UI = 1,

		/**
		 * Extension runs where the remote extension host runs.
		 */
		Workspace = 2
	}

	export interface Extension<T> {

		/**
		 * The extension kind describes if an extension runs where the UI runs
		 * or if an extension runs where the remote extension host runs. The extension kind
		 * if defined in the `package.json` file of extensions but can also be refined
		 * via the the `remote.extensionKind`-setting. When no remote extension host exists,
		 * the value is [`ExtensionKind.UI`](#ExtensionKind.UI).
		 */
		extensionKind: ExtensionKind;
	}

	export namespace env {
		/**
		 * The name of a remote. Defined by extensions, popular samples are `wsl` for the Windows
		 * Subsystem for Linux or `ssh` for remotes using a secure shell.
		 *
		 * *Note* that the value is `undefined` when there is no remote extension host but that the
		 * value is defined in all extension hosts (local and remote) in case a remote extension host
		 * exists. Use [`ExtensionContext#extensionKind`](#ExtensionContext.extensionKind) to know if
		 * a specific extension runs remote or not.
		 */
		export const remoteName: string | undefined;
	}

@DanTup
Copy link
Contributor Author

DanTup commented Jun 20, 2019

@jrieken great, thanks! And is there a reasonable chance this work will ship in the next release?

@jrieken
Copy link
Member

jrieken commented Jun 20, 2019

Yes - the plan is to ship this in June as stable API

@roblourens
Copy link
Member

Where does remoteName come from, the authority?

@roblourens
Copy link
Member

I see the code, the example should be ssh-remote for ssh. What will it be for web?

jrieken added a commit that referenced this issue Jun 21, 2019
@jrieken
Copy link
Member

jrieken commented Jun 21, 2019

What will it be for web?

IDK, whatever we define it to be. Debugging shows 127.0.0.1:9888 but I don't know if that the final design or some pragmatic solution. The point is that you shouldn't care too much unless you know for sure what environment a remote describes, e.g wsl

@jrieken
Copy link
Member

jrieken commented Jun 24, 2019

small update: the extensionKind is now a property of Extension<T> and not of the ExtensionContext anymore

@jrieken jrieken closed this as completed Jun 24, 2019
jrieken added a commit that referenced this issue Jun 24, 2019
kiku-jw added a commit to kiku-jw/vscode that referenced this issue Jun 26, 2019
* beautify macos keyboard layout label

* Open folders and workspaces in new windows

* Basic file opening via Open File command

* Update auto detect layout info.

* Respect openFoldersInNewWindow setting for folders/workspaces

* Make openWindow function resolve at right time

* keyboard layout status bar item tooltip

* Move workspace menu and action to fileActions.contribution

* Add clarifying comment on instance service request events

* Fullscreen change event.

* Remove unneeded margin on settings editor scrollbar
Fix microsoft#75724

* fix: microsoft#72626

* Remove extra register of automatic tasks

Fixes microsoft#75758

* remove trailing '/' from repo url for baseFolderName

* handle style-attribute modifications, cache requests in addition to results, microsoft#75061

* fix microsoft#75818

* fix bad tree guide indentation

* remove TODO

* update eslint

* update distro

fixes microsoft#73872

* Revert "Revert "Merge pull request microsoft#75695 from orange4glace/master""

This reverts commit a05e05c.

* Revert "Revert "explorero: file actions disablment no longer needed""

This reverts commit b634152.

* more code insets API tweaks, microsoft#66418

* Alpine build

* Update distro hash

* Remove duplicate cp

* shellscript: Add folding markers

* fixes microsoft#75829

* show setting on windows only

* add ExtensionKind and remoteName propsed APIs, microsoft#74188

* debt - use file service based configuration file service

* fix tests

* debt create configuration file service inside configuration service

* First cut of file service based user data service

* Use user data service for reading settings

* Update distro hash

* add diagnostic tool for git file event issues

* 💄

* Update distro hash

* introduce VSCODE_STEP_ON_IT

* remove env scripts

fixes microsoft#74792

* Update xterm.css

Fixes microsoft#75827

* check if file exists

* remove alert, aria-live will read the content even with no focus

fixes microsoft#41356

* win code.sh fix

* 🧀 Fix microsoft#75831

* Add proposed api check for shell API

Part of microsoft#75091

* launch ext host window internally

* EH debugging: support multiple files and folders

* Update distro

* [email protected]

Diff: xtermjs/xterm.js@846a189...96eafd3

Changes:

- Publish improvements
- Layering/strict updates

* Fire onDidChangeMaximumDimension when dimensions are set

Fixes microsoft#73496

* Fix potential race

* Delete cached service worker entries after a short timeout

* Fix webview developer command not being registered

* Re-queue canceled geterr requests before remaining buffers

We should give higher priority to files that have previously had geterr triggered on them but did not have their request completed

* Remove log uploader

Fixes microsoft#75748

* Use localized name for macOS keyboard layout

* fixes microsoft#75856

* User keyboard layout

* simplify common keymap layer

* load user keyboard layout after initialization

* US Standard keyboard info

* better score for layout

* fast return keyboard layout if 48-keymap matches

* a single keyboard event can be a keymap

* switch to user selected keyboard layout

* Have `.get` return promise directly

* Make sure we wait until service worker is ready before creating content

* Add version check to service worker

Try to make sure our page is talking to the expected version of the service worker

* Don't use clone as much

* Move host javascript to own file

* Update distro

* Remove icon explorations before shipping stable

* Move listener to window service.

* Minimap: Render find match decorations, fixes microsoft#75216

* Fix `navigator.serviceWorker.ready` is a Promise, not a function

* Use update instead of manually tring to re-register

* Extract ITypeScript server interface

* extract server error to own file

* Extract server spanwer to own file

* Renames

* Move getQueueingType into class

* Add experimental dual TS server

Fixes microsoft#75866

* Enable "typescript.experimental.useSeparateSyntaxServer" for VS Code workspace

* Remove trailing comma

* Include server id in TS server errors

* Make execute command a configuration object

* Also include format in the syntax commands

* Fix method name

* Renames

* Better encapsulate logic of spawning different server kinds

* some fixes for mac web

* New test runner API for microsoft#74555

* update doc, microsoft#74188

* build: release only iff all builds succeed, introduce VSCODE_RELEASE env

* first version of vscode.workspace.fs

* 💄

* Tasks registration + the local ext host now has an autority

Part of microsoft/vscode-remote-release#757

* Add platform override to getDefaultShellAndArgs in terminal

Part of microsoft/vscode-remote-release#757

* Ensure no trailing path separtor on URIs from file picker

Part of microsoft#75847

* data tree view state should store scrollTop, microsoft#74410

* fix microsoft#75564

* Change promise structure of creating terminal in tasks

Potential fix for microsoft#75774

* do not allow additionalProperties

microsoft#75887

* explorer: roots forget children on new file system provider registration

microsoft#75720

* Update max tokenization limit without reload

* Use interfaces for keyboard layout registration

* Separate keyboard layout loading logic for testing

* Test browser keymapper

* unused standard keyboard event.

* Make sure we dismiss the zoom status bar entry when switching editors

* Reduce state

* Added strictly typed telemetry function (microsoft#75915)

* Added strictly typed telemetry function

* cleanup publicLog2 signature

* Extract port mapping helper function

* Re-use extractLocalHostUriMetaDataForPortMapping for openUri

* Also map 127.0.0.1 in webviews and forward it for openExternal

Fixes microsoft/vscode-remote-release#108

* use empty model when content is empty

* 💄

* Update keyboard layout file comments

* Delete breadcrumbs.filterOnType unused setting. Fixes microsoft#75969

* Add quick open/input color registrations (fixes microsoft#65153)

* Update API

* implements ExtHostEditorInsetsShape

* use divs for tree indent guides

fixes microsoft#75779

* comment out more (for microsoft#74898)

* Quick Open > Quick Input (microsoft#65153)

* build - enable language server tests again (for microsoft#74898)

* use polish for wsl1

* move extension kind to Extension-interface

* init log level of remote log service

* Open/Save local commands should not show in the command palette

Fixes microsoft#75737

* chockidar: use polling

* fix build conditions

* xterm fixes for cglicenses

* oss 1.36.0

* workaround for microsoft#75830

* update distro commit

* electron - still call setBounds() as workaround for first window

* fixes microsoft#75753

* [email protected]

* remove user data service

* use posix.join

* update doc

* Add -1 tab index to status bar entries

This keeps them out of the tab order, but allows them to be read with a screen reader

Fixes microsoft#41406

* empty view polish labels for remote case

microsoft/vscode-remote-release#511

* send remote watcher error to file service (fixes microsoft/vscode-remote-release#329)

* update distro

* better error handling in case of loader error in tests

* fix win 32 bits unit tests

* [email protected] (microsoft#76020)

* Code-insiders started from WSL doesn't return to console/ doesn't connect. Fixes microsoft/vscode-remote-release#780

* Group decorations by line before rendering

* disable support for simple fullscreen (microsoft#75054)

* telemetry - add window.nativeFullScreen

* move API to stable,  microsoft#74188

* build - add and use --disable-inspect for integration tests (microsoft#74898)

* 💄

* bump distro

* Report workspace stats in shared process

* Make return undefined explicit

* Add missing return

* Use explicit window.createWebviewManager

* gdpr comments

* webkit fullscreen detection

* Fix file name spelling

* update distro

* add logging

* disabling installing extension from gallery when not enabled

* status.workbench.keyboardLayout

* Move Inspect Keyboard Layout JSON to workbench

* return local extension after install

* install deps and packs while installing from gallery

* Fix default shell selector outside of Windows

Fixes microsoft#76040

* Add explicit win32 gheck for using user specific temp folder

* Always use settings UI when querying online services, fixes microsoft#75542

* Disable conpty in terminal when accessibility mode is on

Fixes microsoft#76043

* Move the webviewResourceRoot property to be set on each webview instead of as a global property

For microsoft#72155

This allows  us to potentially change the resource root per webview

* Make RelativeWorkspacePathResolver  a static class

* Use openExternal

* Auto restart when changing  typescript.experimental.useSeparateSyntaxServer

* Fix regular expression for rewriting iframe webview html replacing quotes

* Telemetry Command (microsoft#76029)

* Added telemetry command

* Initial Build support

* Added build logic for telemetry

* Linux Builds

* Windows builds sort of work

* Remove arm telemetry extraction

* Remove alpine telemetry extraction

* Remove accidental s

* More try catch

* Use full resource uri for transforming webview resources

This ensures we still work even if there is no base uri set

* Use outerHtml to make sure we write `<html>` element from extensions too

* Use a regexp that works across browsers

* Implement reload on iframe based webview Elements

* fix various nls issues

* 💄

* add debug output (microsoft#76024)

* Fix tasks platform for quoting

Fixes microsoft#75774

* fix hockeyapp symbols and report errors (fix microsoft#76024)

* update distro

* fix bad watch

* update distro

* Fix drive letter casing on typescript tasks

Occurs when opening by double clicking on workspace file. Fixes microsoft#75084

* update distro

* update distro

* Test remoteName and extensionKind (for microsoft#76028)

* MainThreadFileSystem does not await

* Fix microsoft#76096

* Rename runInBackground to hideFromUser

See microsoft#75278

* Update distro

* Fix minimap decoration rendering on horizontal scroll, fixes microsoft#76128

* Handle windows paths correctly when loading webvie resources

* Fix standard link handler for iframe based webviews

* Mark extensions.all as readonly

This iteration, we marked a few other arrays as readonly. We should do the same for extensions.all

* Fix microsoft#75927.

* Register mouse down on container dom node.

* Make sure we never cancel a request to just one of the ts servers

Fixes microsoft#76143

* Show document link tooltip first and put click instructions in parens

Fixes microsoft#76077

This change also update our standard link hovers to follow this format

* reset listener once users choose a dedicated keyboard layout

* switch to a new layout only when the score is higher.

* Fix kb unit test

* fix microsoft#76149

* web - document some API

* 💄 workbench API

* disable arm and alpine for stable

fixes microsoft#76159

* Fix extra auto complete on fast delete (microsoft#74675)

Fixes #vscode-remote-release/4

* use yarn --frozen-lockfile for builds

* remove `update.enableWindowsBackgroundUpdates` from online settings

* fix microsoft#76076

* revert the change

* prevent product.json containing gallery

* fix microsoft#76074

* fixes microsoft#54084

* Fix microsoft#76105

* fix microsoft#75904

* workaround for  microsoft#74934
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization api-proposal feature-request Request for new features or functionality remote Remote system operations issues
Projects
None yet
Development

No branches or pull requests

4 participants