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

code-server: init at 3.4.1 #87258

Merged
merged 1 commit into from Jun 22, 2020
Merged

Conversation

offlinehacker
Copy link
Contributor

Motivation for this change

Package code-server and build it from source.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/vscode-remote-development-on-nixos/3119/36

@offlinehacker
Copy link
Contributor Author

I am unsure why it fails on aarch64-linux, i will remove it from supported platforms for now

@offlinehacker
Copy link
Contributor Author

@GrahamcOfBorg build code-server

@Ma27 Ma27 self-requested a review May 10, 2020 22:20
@Ma27 Ma27 requested a review from elseym May 11, 2020 17:19
@offlinehacker offlinehacker requested a review from Ma27 May 12, 2020 05:05
@offlinehacker
Copy link
Contributor Author

I am using this package for some time, without any issues. Are we ready to merge it?

@Mic92
Copy link
Member

Mic92 commented May 27, 2020

Works on my machine (tm). There is apparently also an upgrade available.

@Mic92
Copy link
Member

Mic92 commented May 27, 2020

Maybe also patch out references to /sbin/

info  code-server 3.2.0 fd36a99a4c78669970ebc4eb05768293b657716f
info  HTTP server listening on http://127.0.0.1:8080
info    - Password is 7b68bf65db126f0c3a8e5bcc
info      - To use your own password set the PASSWORD environment variable
info      - To disable use `--auth none`
info    - Not serving HTTPS
info  Automatic updates are enabled
1warn  vscode Unable to retrieve mac address (Error: Command failed: /sbin/ifconfig -a || /sbin/ip link
/bin/sh: /sbin/ifconfig: No such file or directory
/bin/sh: /sbin/ip: No such file or directory

and disable automatic updates if possible.

@FRidh FRidh mentioned this pull request May 29, 2020
pkgs/servers/code-server/default.nix Outdated Show resolved Hide resolved
pkgs/servers/code-server/default.nix Outdated Show resolved Hide resolved
pkgs/servers/code-server/default.nix Outdated Show resolved Hide resolved
@lilyball
Copy link
Member

code-server 3.4.0 is out now.

BTW when updating to 3.4.0, the lib/vscode/build/builtInExtensions.json file is gone and the data was migrated into lib/vscode/product.json. You’ll likely need to use something like jq to modify the file as needed, if you don’t want to maintain a patch.

@breakds
Copy link
Contributor

breakds commented Jun 1, 2020

Thanks for this!

I tried it on my current 20.03 NixOS box, the client-side gets disconnected and asks for reconnection every 5 seconds or so. Anyone ran into the same problem?

@offlinehacker
Copy link
Contributor Author

offlinehacker commented Jun 7, 2020

I did following changes:

  • Update to latest 3.4.1 version
  • Explicitly set nodejs version to 12 and python version to 3
  • Use stdenv.lib.fakeSha256

Missing changes (planning to implement):

  • Disable notifications for automatic updates.

Please review, it can be merged without disabled automatic updates.

@offlinehacker offlinehacker changed the title code-server: init at 3.4.0 code-server: init at 3.4.1 Jun 7, 2020
@Mic92
Copy link
Member

Mic92 commented Jun 8, 2020

Not a blocker, but do you know what this iproute2 calls do? #87258 (comment)

@offlinehacker
Copy link
Contributor Author

@Mic92 I have to check and possibly also fix that yes

@offlinehacker
Copy link
Contributor Author

Ahh I see that I forgot to patch shebangs while fetching yarn cache. For some reason it worked on my machine, as /bin/sh exists for me and it apparently is not isolated if running in static output hash derivation, wierd.

@lilyball
Copy link
Member

lilyball commented Jun 8, 2020

/bin/sh exists because POSIX requires it.

On Linux in the sandbox /bin/sh is mapped to a minimal busybox shell (this is done as a configure argument defined in the nixpkgs.nix derivation). On macOS the sandbox explicitly allows access to system /bin/sh since it doesn't support remapping paths (system /bin/sh here is Bash 3.2.57 running in sh compatibility mode).

@offlinehacker
Copy link
Contributor Author

offlinehacker commented Jun 9, 2020

@lilyball It does not seem to exist, when ofborg is building it: https://github.com/NixOS/nixpkgs/pull/87258/checks?check_run_id=750458903 which I think is right behavior.

@offlinehacker offlinehacker force-pushed the pkgs/code-server/init branch 2 times, most recently from 038342c to 88a8fe8 Compare June 9, 2020 05:40
@offlinehacker
Copy link
Contributor Author

@Mic92 I fixed path to ifconfig. vscode tries to get id of machine and it uses mac address (hashed) to do so, see here: https://github.com/microsoft/vscode/blob/c6c6969c3c1d5a975ed213d58a586ec1f1007c33/src/vs/base/node/id.ts#L95

Path is fixed now, so this will work.

@offlinehacker
Copy link
Contributor Author

@GrahamcOfBorg build code-server

@offlinehacker
Copy link
Contributor Author

Ok, I think this is now ready to be merged, it starts without any issues, does not bother with updates and looks good in general. I will deploy it on my dev environments and do some more testing, so i can confirm this version works ok.

@wucke13
Copy link
Contributor

wucke13 commented Jun 9, 2020

One small remark: The current state of this works for me, but git. Instead of the browser displaying code-server, the terminal in which code-server is started receives the dialog for my ssh key. Is this intended?

@kamaln7
Copy link

kamaln7 commented Jun 11, 2020

I appreciate your work on this! I just set it up on my machine and it works perfectly 👍 Would be happy to see it merged 😄

I ended up installing it like this:

{ config, pkgs, ... }:
let
  xtruder = import
    (builtins.fetchTarball https://github.com/xtruder/nixpkgs/tarball/88ce5a75cbaaaf1830006501c85a6fba0c0d05c2)
    { config = config.nixpkgs.config; };
in {
  environment.systemPackages = with pkgs; [
    xtruder.code-server
  ];

  systemd.services.code-server = {
    enable = true;
    description = "Remote VSCode Server";
    after = ["network.target"];
    wantedBy = ["multi-user.target"];
    path = [ pkgs.go pkgs.git pkgs.direnv ];

    serviceConfig = {
      Type = "simple";
      ExecStart = "${xtruder.code-server}/bin/code-server";
      WorkingDirectory = "/home/kamal";
      NoNewPrivileges = true;
      User = "kamal";
      Group = "nogroup";
    };
  };
}

@offlinehacker
Copy link
Contributor Author

@wucke13 this must be issue with code-server itself?

@wucke13
Copy link
Contributor

wucke13 commented Jun 12, 2020

Might be the case, yes

Edit: This is indeed a VSCode problem.

@offlinehacker offlinehacker force-pushed the pkgs/code-server/init branch 5 times, most recently from a117fef to 2ee80c6 Compare June 15, 2020 09:23
@offlinehacker
Copy link
Contributor Author

One thing i noticed minify step is memory intensive. I tried to disable it, but had more issues, so i will leave it on.

@offlinehacker
Copy link
Contributor Author

@GrahamcOfBorg build code-server

@offlinehacker
Copy link
Contributor Author

I implemented all suggestions, improved syntax, removed unnecessary dependencies and improved comments.

Can you please check for last time if you see something that needs changing, or I think this is now really read to be merged. @lilyball @FRidh

homepage = "https://github.com/cdr/code-server";
license = licenses.mit;
maintainers = with maintainers; [ offline ];
platforms = ["x86_64-linux"];
Copy link
Member

Choose a reason for hiding this comment

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

Since this is marked as x86_64 we can remove the aarch64 checksum above.

@FRidh FRidh merged commit 9e3dd22 into NixOS:master Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants