RFC: Support local extension downloading if remote downloading fails #298
RFC: Support local extension downloading if remote downloading fails #298p12tic wants to merge 4 commits into
Conversation
| }, | ||
| "remote.SSH.localServerDownload": { | ||
| "type": "string", | ||
| "description": "Controls whether the VS Code server binary is downloaded locally and transferred to the remote host via SFTP. 'auto' tries downloading on the remote first, then falls back to downloading locally and transferring via SFTP. 'always' always downloads locally and transfers via SFTP. 'never' only downloads directly on the remote machine.", |
There was a problem hiding this comment.
s/VS Code server binary/server binaries/ to make it more general and not directly referencing VS code?
| import * as fs from 'fs'; | ||
| import * as os from 'os'; | ||
| import * as path from 'path'; |
There was a problem hiding this comment.
Do we need this much imports or isn't it reasonable to get it down to the actually used ones?
| let arch: string; | ||
| if (platform === 'windows') { | ||
| arch = 'x64'; | ||
| } else { | ||
| const unameResult = await conn.exec('uname -m'); | ||
| const remoteArch = unameResult.stdout.trim(); | ||
| switch (remoteArch) { | ||
| case 'x86_64': | ||
| case 'amd64': | ||
| arch = 'x64'; | ||
| break; | ||
| case 'armv7l': | ||
| case 'armv8l': | ||
| arch = 'armhf'; | ||
| break; | ||
| case 'arm64': | ||
| case 'aarch64': | ||
| arch = 'arm64'; | ||
| break; | ||
| case 'ppc64le': | ||
| arch = 'ppc64le'; | ||
| break; | ||
| case 'riscv64': | ||
| arch = 'riscv64'; | ||
| break; | ||
| case 'loongarch64': | ||
| arch = 'loong64'; | ||
| break; | ||
| case 's390x': | ||
| arch = 's390x'; | ||
| break; | ||
| default: | ||
| arch = remoteArch; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return { platform: platform || 'linux', arch, shell }; | ||
| } |
There was a problem hiding this comment.
This one seems to duplicate part of the code at
open-remote-ssh/src/serverSetup.ts
Line 341 in 22b4519
There was a problem hiding this comment.
Yes, #287 need to came first. I will work on it in few days.
| curl --retry 3 --connect-timeout 10 --location --show-error --silent --output vscode-server.tar.gz $SERVER_DOWNLOAD_URL | ||
| elif command -v fetch >/dev/null 2>&1; then | ||
| fetch --retry --timeout=10 --quiet --output=vscode-server.tar.gz $SERVER_DOWNLOAD_URL | ||
| if [[ -f vscode-server.tar.gz ]]; then |
There was a problem hiding this comment.
That would also use pre-located files there in general, no?
I think it is better to pass another arg (or as long as it is a contained script a replacement var) to decide which path to be used.
| # Check if server script is already installed | ||
| if(!(Test-Path $SERVER_SCRIPT)) { | ||
| del vscode-server.tar.gz | ||
| if(Test-Path "vscode-server.tar.gz") { |
There was a problem hiding this comment.
same applies as with the non-win32 script
|
Thank you for fast review. I will wait until #287 lands and then get back to this PR. |
|
@p12tic The scripts have been extracted ;) |
|
@p12tic +1 |
3 participants
Currently downloading extension remotely fails for any reason the remote workspace will fail to start up. In real VS Code this is implemented by local downloading fallback.
This PR implements a prototype in this extension. Currently tested on Linux only. Error handling is still to be improved. Best to be reviewed commit by commit.
Please let me know if this PR could be merged in principle. If yes, I will polish it and prepare for proper review.
Fixes: #38