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

Support more recent VSCode Server versions #78

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

Ten0
Copy link
Contributor

@Ten0 Ten0 commented Mar 21, 2024

Resolves #67

Following this message:
#67 (comment)

It took me some time to free up some time to understand how the script worked and propose this implementation - it's cleaner than that of #68 and seems more stable.
Closes #68

Notably this changes the directory structure a bit, leaving patched files in their respective directories, which feels simpler and cleaner because I don't need to construct global filenames whose construction method depends on the VSCode version. (I'm using \$(dirname "\$0") in the intermediate script that replaces the node binary so that it keeps working after the directory is moved).

I've tested that this works:

  • On old VSCode remote versions
  • On new VSCode remote versions
  • Migrating from previous directory structure to new directory structure (so updating the flake won't break existing users)

@willgorman
Copy link

This fixed vscode-server for me on vscode 1.87.2 with remote ssh extension 0.109.0, thanks!

@rockboynton
Copy link

It fixed the issue for me too on nixos-unstable. The actual issue I was facing was this reported by VS Code when trying to connect to my remote NixOS machine:

Failed to connect to the remote extension host server (Error: WrappedError(WrappedError { message: "error checking server integrity", original: "failed to run command \"/home/rboynton/.vscode-server/cli/servers/Stable-5c3e652f63e798a5ac2f31ffd0d863669328dc4c.staging/server/bin/code-server --version\" (code 127): Could not start dynamically linked executable: /home/rboynton/.vscode-server/cli/servers/Stable-5c3e652f63e798a5ac2f31ffd0d863669328dc4c.staging/server/node

Thanks for the fix @Ten0, this should be merged into mainline

Copy link

@samuela samuela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together @Ten0 !)


if [[ -e $patched_file ]]; then
return 0
fi

# Backwards compatibility with previous versions of nixos-vscode-server.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose noting specifically which "previous versions" of VSCode this applies to along with the current date. Presumably we'll want to remove this at some future point, and having that info here could help to make the context more readily apparent.

Copy link
Contributor Author

@Ten0 Ten0 Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's of nixos-vscode-server (this repo). Anything prior to the commit that will ultimately introduce this comment will be relevant, so it seems to be the job of git to maintain this information, reasonably accessible via git blame.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah oops, misread! Fair enough... when do you think we should remove this?

Copy link
Contributor Author

@Ten0 Ten0 Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say a few months after this is merged, so that everyone had a chance to update this repo and/or their vscode client version.

@@ -144,7 +144,7 @@ The installation path for VS Code server is configurable and the default can dif

```nix
{
services.vscode-server.installPath = "~/.vscode-server-oss";
services.vscode-server.installPath = "$HOME/.vscode-server-oss";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooc why make this change? is it necessary to fix #79?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem related but was necessary to have something functional.

@MinerSebas
Copy link
Contributor

This also fixed the issue for me with code 1.88 and remote ssh v0.111.

@lorenzleutgeb
Copy link

lorenzleutgeb commented Apr 9, 2024

Works for me.

$ jq '.nodes."vscode-server"' < flake.lock
{
  "inputs": {
    "flake-utils": [
      "utils"
    ],
    "nixpkgs": [
      "nixpkgs"
    ]
  },
  "locked": {
    "lastModified": 1711042850,
    "narHash": "sha256-8PDNi/dgoI2kyM7uSiU4eoLBqUKoA+3TXuz+VWmuCOc=",
    "owner": "Ten0",
    "repo": "nixos-vscode-server",
    "rev": "b02b3cceaae22fb66c00f03f1aff705e9711956e",
    "type": "github"
  },
  "original": {
    "owner": "Ten0",
    "ref": "support_new_vscode_versions",
    "repo": "nixos-vscode-server",
    "type": "github"
  }
}

Client

Via the GUI:

Version: 1.88.0 (user setup)
Commit: 5c3e652f63e798a5ac2f31ffd0d863669328dc4c
Date: 2024-04-03T13:26:18.741Z (5 days ago)
Electron: 28.2.8
ElectronBuildld: 27744544
Chromium: 120.0.6099.291
Node.js: 18.18.2
V8: 12.0.267.19-electron.0
OS: Windows_NT x64 10.0.22621

Server

$ code --version
1.85.2
8b3775030ed1a69b13e4f4c628c612102e30a681
x64
$ code --list-extensions --show-versions | grep ms-vscode
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]

@Ten0
Copy link
Contributor Author

Ten0 commented Apr 9, 2024

@msteen Would you consider following up on your proposal here?

I wouldn't like to mute this PR but I'm starting to get annoyed by all the notifications here 😅

@Dzieni
Copy link

Dzieni commented Apr 24, 2024

I can confirm that it helped.

Is there anything blocking the merge?

@Ten0
Copy link
Contributor Author

Ten0 commented Apr 24, 2024

Is there anything blocking the merge?

Last bit of time from @msteen

@msteen msteen merged commit fc900c1 into nix-community:master Apr 24, 2024
@msteen
Copy link
Collaborator

msteen commented Apr 24, 2024

I lack a setup to properly test this at the moment. Given multiple confirmations I am just going to merge for now.

@steinerkelvin
Copy link

steinerkelvin commented Apr 25, 2024

I've got the following error:

error: builder for '/nix/store/9akfv0g37cswz2kga9kpj417vblhfmj4-auto-fix-vscode-server.drv' failed with exit code 1;
       last 10 log lines:
       >     old_patched_file="~/.vscode-server/.${old_patched_file%%.*}.patched"
       >                       ^----------------^ SC2088 (warning): Tilde does not expand in quotes. Use $HOME.
       >
       >
       > In /nix/store/c9vbs0m695ppp8i1707xyspwjai27jk5-auto-fix-vscode-server/bin/auto-fix-vscode-server line 26:
       >     old_patched_file="~/.vscode-server/.${old_patched_file%%-*}.patched"
       >                       ^----------------^ SC2088 (warning): Tilde does not expand in quotes. Use $HOME.
       >
       > For more information:
       >   https://www.shellcheck.net/wiki/SC2088 -- Tilde does not expand in quotes. ...
       For full logs, run 'nix log /nix/store/9akfv0g37cswz2kga9kpj417vblhfmj4-auto-fix-vscode-server.drv'.
{
 "locked": {
   "lastModified": 1713958148,
   "narHash": "sha256-8PDNi/dgoI2kyM7uSiU4eoLBqUKoA+3TXuz+VWmuCOc=",
   "owner": "nix-community",
   "repo": "nixos-vscode-server",
   "rev": "fc900c16efc6a5ed972fb6be87df018bcf3035bc",
   "type": "github"
 }
}

I had this in my config:

    services.vscode-server = {
     enable = true;
     installPath = "~/.vscode-server";
     # installPath = "~/.vscode-server-insiders";
   };

Replacing the ~ solved it:

    services.vscode-server = {
      enable = true;
      installPath = "$HOME/.vscode-server";
      # installPath = "$HOME/.vscode-server-insiders";
    };

@Ten0
Copy link
Contributor Author

Ten0 commented Apr 26, 2024

Yeah not sure why ~ used to work but it generally would fail now so I've replaced them all within the PR but of course if they are reintroduced by your config that may break things.

@msteen
Copy link
Collaborator

msteen commented Apr 26, 2024

Should we maybe error or autofix this in the implementation? Noting ~/ no longer works, or replacing it automatically with $HOME/, as I can imagine many users will run into this problem?

@Ten0
Copy link
Contributor Author

Ten0 commented Apr 26, 2024

Looks like the install path that was specified here is the default anyway, so there's probably little amount of people who are affected. In addition they managed to fix it pretty quickly. So overall I think I'd prefer to let users migrate to the writing that works than add maintenance complexity by setting a standard that we are fixing the paths.

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 this pull request may close these issues.

VSCode 1.82 and Remote 0.106.1 (or earlier?) compatibility
9 participants