rstudio-server: Initial commit for rstudio-server and associated wrapper #95831

cfhammill · 2020-08-20T03:25:16Z

Motivation for this change

Documented in more detail at https://discourse.nixos.org/t/adding-rstudio-server-r/8644/4, but in short, I wanted to be able to run rstudio-server inside a nix-shell so that I could get plots faster than using X-forwarding.

Things done

Modified the rstudio build code to build the server version, with some more tweaks.

Coarse diff between rstudio and rstudio-server:

Nix file is under the same directory as rstudio. Called server.nix
Added to top-level as rstudio-server
Calls wrap program on each binary
Adds readline6 and pam dependencies for building
Removes reference to icons (build failed if left in)

Coarse diff between rstudioWrapper and rstudioServerWrapper

Uses the same libPath wrapping on the rsession binary
Also adds a fontconfig dependency and wrapping to fix the plot fonts issue noted here
Wraps rserver to point to the wrapped rsession

Potential To-Do's

I think the rstudio-server build could be refactored into the same nix file as the rstudio build, adding an argument to toggle between the two.
I think this could be converted into a NixOS module and made to work with pam. For now it's simple enough to write a a small script to simple authentication (this can be passed to rserver as the --auth-pam-helper argument if memory serves.)
I think that rstudioWrapper and rWrapper should be given the same fontconfig fix as rstudioServerWrapper but perhaps that's best for another PR.

CC @ehmry @changlinli @ciil

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)
- NixOS
- macOS
- [ x ] 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"
[ x ] 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
[ x ] Fits CONTRIBUTING.md.

nixos-discourse · 2020-08-20T03:28:05Z

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

https://discourse.nixos.org/t/adding-rstudio-server-r/8644/6

alexvorobiev · 2020-09-02T19:08:13Z

Thank you for adding the server! Are you planning to add a corresponding NixOS module for the service?

cfhammill · 2020-09-06T21:31:30Z

Hi @alexvorobiev, I've actually never used NixOS so I wouldn't know where to start with adding a module. My colleague @bcdarwin has offered to help creating a module if this PR goes through.

nviets · 2020-09-28T21:24:14Z

Any update on this PR?

nixos-discourse · 2020-09-28T22:06:51Z

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

https://discourse.nixos.org/t/adding-rstudio-server-r/8644/7

pkgs/development/r-modules/wrapper-rstudio-server.nix

pkgs/applications/editors/rstudio/server.nix

pkgs/development/r-modules/wrapper-rstudio-server.nix

pkgs/top-level/all-packages.nix

jonringer · 2020-09-28T23:33:29Z

* I think the `rstudio-server` build could be refactored into the same nix file as the `rstudio` build, adding an argument to toggle between the two.

If rstudio-server used mkDerivation, I would say use passthru.unwrapped to allow a reference to the underlying package, but I'm not sure if runCommand filters those attrs

SuperSandro2000

please format your code with nixpkgs-fmt

pkgs/applications/editors/rstudio/server.nix

stale · 2021-09-19T01:23:36Z

I marked this as stale due to inactivity. → More info

alexvorobiev · 2021-09-22T15:33:32Z

This is still important

nviets · 2021-09-22T15:48:29Z

What is pending for this update to be released?

pkgs/applications/editors/rstudio/server.nix

pkgs/development/r-modules/wrapper-rstudio-server.nix

billksun · 2021-11-22T22:32:24Z

@cfhammill, are you available to make the review edits? I am in need of this as well.

cfhammill · 2021-12-03T19:54:27Z

sorry for the delay, I'm back looking at this now and will hopefully have a chance to make the requested revisions soon

jbedo · 2021-12-06T02:43:58Z

It's worth revisiting the rstudio derivation as it has changed considerably with the latest version bump: `$NIX_QT5_TMP` is no-longer used and qmake is referred to as an explicit input.

cfhammill · 2021-12-06T02:54:33Z

thanks @jbedo I was wondering if redoing this PR off the updated rstudio/default.nix, this was the push I needed.

cfhammill · 2021-12-08T05:02:05Z

I have the new version of rstudio-server building based on the updated rstudio derivation. A couple of pain points

The rstudio-server service doesn't work, you have to use the rserver binary directly. This is probably the best we can do for non-nixOS systems.

rserver as far as I can tell requires a few system directories that need to be writeable. I never noticed this before because my systems had those directories around from non-nix installs of rstudio server. the directories in question are

/var/run/rstudio-server (this needs to have global write with the sticky bit set according to rserver, but ostensibly this location can be changed with the --server-data-dir flag)
/var/lib/rstudio-server (needs some write permissions, not sure if it needs to be global, but as far as I can tell this cannot be moved to another directory)

so a user wanting to run rstudio-server would run
sudo mkdir /var/run/rstudio-server && sudo chmod a+wt /var/run/rstudio-server
sudo mkdir /var/lib/rstudio-server && sudo chmod a+wt /var/lib/rstudio-server

once, and then to launch the server the user would run

rserver --auth-none 1

possibly in the background.

If anyone has any clever ideas to avoid the global directories I'd welcome a contribution. We could probably patch the CMake files or the c++ source code to override these default locations and point them elsewhere, not sure where, I think write perms within the nix store is a no-go, if we can't think of a sensible default, an upstream PR to add an argument to move the /var/lib/rstudio-server location might be possible.

jbedo · 2021-12-08T05:33:17Z

Systemd will create them for you, use RunTimeDirectory= and StateDirectory= options, along with DynamicUser. You can ripgrep nixos/modules/services for some examples.

cfhammill · 2021-12-08T05:47:53Z

@bcdarwin has offered to help turn this into a module, do you mean this would work for a non-nixOS system? If so can you give me more detail on what to do?

cfhammill · 2021-12-28T08:13:07Z

All sounds good to me, I didn't test myself, booted in to a nixos vm, tried a few things couldn't get basic stuff going and gave up, but if you both are giving thumbs up I'm sure it's fine.

Shall I squash in prep for merge or @jbedo will you squash merge?

jbedo · 2022-01-06T05:21:45Z

It would be better if you could squash this down to a couple of commits, one adding the rstudio-server expression and another adding the module + tests. There's now a conflict with the ids to resolve, just needs to be bumped.

bcdarwin · 2022-01-10T22:02:23Z

Squashed, uid/gid updated, and rebased onto master.

bcdarwin · 2022-01-11T00:39:15Z

Added the new module to the release notes as per updated contributing guidelines.

@jbedo should be ready to merge.

jonringer

otherwise diff LGTM at a glance.

Would like for a rstudio user to verify

jonringer · 2022-01-11T05:06:31Z

pkgs/applications/editors/rstudio/default.nix

+
+    desktopItems = [
+      (makeDesktopItem {
+        name = "${pname}";


Suggested change

name = "${pname}";

name = pname;

jbedo · 2022-01-11T09:21:11Z

LGTM too, I'll see if I can find an rstudio-server user to test this more but pretty confident in the basic functionality as @bcdarwin has also verified it.

bcdarwin · 2022-01-12T02:25:10Z

The example value for services.rstudio-server.package (currently pkgs.rstudioServerWrapper.override { packages = [ pkgs.rPackages.ggplot2 ]; }) does not build due to a dangling symlink issue with the wrapper:

error: not a directory: `/nix/store/svhiws2l6jn7ygl4zx0yjg10i7i9yr43-RStudio-1.4.1717-wrapper/share'

However, I don't have time to look into it this week.

jbedo · 2022-01-12T06:04:18Z

Resolved. Added a crappy test for it too.

bcdarwin · 2022-01-12T06:50:46Z

Resolved. Added a crappy test for it too.

Works now, thanks.

bcdarwin · 2022-01-15T06:02:42Z

Rebased and resolved merge conflicts again.

Co-authored-by: Justin Bedo <cu@cua0.org> Co-authored-by: Jonathan Ringer <jonringer@users.noreply.github.com> Co-authored-by: Sandro <sandro.jaeckel@gmail.com> Co-authored-by: Benjamin Darwin <bcdarwin@gmail.com>

jbedo · 2022-01-16T23:30:38Z

We can merge this once the checks have passed, hopefully before we have to deal with further conflicts. @jonringer I think further issues are likely to be more fundamental problems with the underlying R/RStudio expressions rather than the rstudio-server module, which we can deal with separately.

jonringer · 2022-01-17T03:37:18Z

I'm fine with incremental improvement. And I'm not familiar with r to really guide to the desired end state.

Thanks for reviewing it

cfhammill requested a review from peti as a code owner August 20, 2020 03:25

ofborg bot added the 8.has: package (new) label Aug 20, 2020

ofborg bot requested review from ciil, ehmry and changlinli August 20, 2020 13:57

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Aug 20, 2020

jonringer reviewed Sep 28, 2020

View reviewed changes

cfhammill force-pushed the cfh/add-rstudio-server branch from ffff180 to c57d0c6 Compare February 22, 2021 19:41

SuperSandro2000 requested changes Mar 21, 2021

View reviewed changes

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 19, 2021

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 22, 2021

SuperSandro2000 requested changes Sep 22, 2021

View reviewed changes

cfhammill requested a review from jbedo as a code owner December 8, 2021 06:02

bcdarwin force-pushed the cfh/add-rstudio-server branch from ba879c1 to c1deac9 Compare January 10, 2022 22:01

bcdarwin force-pushed the cfh/add-rstudio-server branch from c1deac9 to a627b3a Compare January 10, 2022 22:25

github-actions bot added 8.has: changelog 8.has: documentation labels Jan 10, 2022

bcdarwin force-pushed the cfh/add-rstudio-server branch from 0eba6cf to 544bd86 Compare January 10, 2022 23:18

jonringer reviewed Jan 11, 2022

View reviewed changes

bcdarwin force-pushed the cfh/add-rstudio-server branch from 544bd86 to 0b9cb84 Compare January 11, 2022 05:11

jbedo force-pushed the cfh/add-rstudio-server branch from 0b9cb84 to a6d327e Compare January 12, 2022 06:03

ofborg bot added the 2.status: merge conflict label Jan 12, 2022

bcdarwin force-pushed the cfh/add-rstudio-server branch from a6d327e to d05cda5 Compare January 15, 2022 05:58

ofborg bot removed the 2.status: merge conflict label Jan 15, 2022

CHRIS hammill and others added 4 commits January 17, 2022 10:24

add cfhammill (me) to the maintainers list

d3bc05b

rstudio-server, rstudioServerWrapper: init at rstudio.version (1.4.1717)

fd51177

Co-authored-by: Justin Bedo <cu@cua0.org> Co-authored-by: Jonathan Ringer <jonringer@users.noreply.github.com> Co-authored-by: Sandro <sandro.jaeckel@gmail.com> Co-authored-by: Benjamin Darwin <bcdarwin@gmail.com>

nixos/rstudio-server: init

0fe0153

nixos/rstudio-server: add to 22.05 release notes

43047ec

jbedo force-pushed the cfh/add-rstudio-server branch from d05cda5 to 43047ec Compare January 16, 2022 23:27

jbedo merged commit dfa93a4 into NixOS:master Jan 17, 2022

bcdarwin deleted the cfh/add-rstudio-server branch January 17, 2022 16:14

cfhammill mentioned this pull request Mar 1, 2023

rstudioServerWrapper: inherit meta from rstudio #205122

Draft

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rstudio-server: Initial commit for rstudio-server and associated wrapper #95831

rstudio-server: Initial commit for rstudio-server and associated wrapper #95831

cfhammill commented Aug 20, 2020

nixos-discourse commented Aug 20, 2020

alexvorobiev commented Sep 2, 2020

cfhammill commented Sep 6, 2020

nviets commented Sep 28, 2020

nixos-discourse commented Sep 28, 2020

jonringer commented Sep 28, 2020

SuperSandro2000 left a comment

stale bot commented Sep 19, 2021

alexvorobiev commented Sep 22, 2021

nviets commented Sep 22, 2021

billksun commented Nov 22, 2021

cfhammill commented Dec 3, 2021

jbedo commented Dec 6, 2021 via email

cfhammill commented Dec 6, 2021

cfhammill commented Dec 8, 2021 •

edited

jbedo commented Dec 8, 2021 via email

cfhammill commented Dec 8, 2021

cfhammill commented Dec 28, 2021

jbedo commented Jan 6, 2022

bcdarwin commented Jan 10, 2022 •

edited

bcdarwin commented Jan 11, 2022

jonringer left a comment

jonringer Jan 11, 2022

bcdarwin Jan 11, 2022

jbedo commented Jan 11, 2022

bcdarwin commented Jan 12, 2022 •

edited

jbedo commented Jan 12, 2022 via email

bcdarwin commented Jan 12, 2022

bcdarwin commented Jan 15, 2022

jbedo commented Jan 16, 2022

jonringer commented Jan 17, 2022

rstudio-server: Initial commit for rstudio-server and associated wrapper #95831

rstudio-server: Initial commit for rstudio-server and associated wrapper #95831

Conversation

cfhammill commented Aug 20, 2020

Motivation for this change

Things done

nixos-discourse commented Aug 20, 2020

alexvorobiev commented Sep 2, 2020

cfhammill commented Sep 6, 2020

nviets commented Sep 28, 2020

nixos-discourse commented Sep 28, 2020

jonringer commented Sep 28, 2020

SuperSandro2000 left a comment

Choose a reason for hiding this comment

stale bot commented Sep 19, 2021

alexvorobiev commented Sep 22, 2021

nviets commented Sep 22, 2021

billksun commented Nov 22, 2021

cfhammill commented Dec 3, 2021

jbedo commented Dec 6, 2021 via email

cfhammill commented Dec 6, 2021

cfhammill commented Dec 8, 2021 • edited

jbedo commented Dec 8, 2021 via email

cfhammill commented Dec 8, 2021

cfhammill commented Dec 28, 2021

jbedo commented Jan 6, 2022

bcdarwin commented Jan 10, 2022 • edited

bcdarwin commented Jan 11, 2022

jonringer left a comment

Choose a reason for hiding this comment

jonringer Jan 11, 2022

Choose a reason for hiding this comment

bcdarwin Jan 11, 2022

Choose a reason for hiding this comment

jbedo commented Jan 11, 2022

bcdarwin commented Jan 12, 2022 • edited

jbedo commented Jan 12, 2022 via email

bcdarwin commented Jan 12, 2022

bcdarwin commented Jan 15, 2022

jbedo commented Jan 16, 2022

jonringer commented Jan 17, 2022

cfhammill commented Dec 8, 2021 •

edited

bcdarwin commented Jan 10, 2022 •

edited

bcdarwin commented Jan 12, 2022 •

edited