rocker-org / rocker-versioned

Run current & prior versions of R using docker
https://hub.docker.com/r/rocker/r-ver
GNU General Public License v2.0
297 stars 169 forks source link

Add RStudio default settings for handling history (#79) #80

Closed timtrice closed 6 years ago

timtrice commented 6 years ago

Added to Dockerfiles:

Originally, this was added to all Dockerfiles in subdir rstudio. However, on tidyverse builds these settings were overwritten somehow (user-settings file existed but contents were slim and not equal).

Added similar section to all tidyverse Dockerfiles.

Sample test builds of rstudio and tidyverse images verify settings work as expected.

cboettig commented 6 years ago

@timtrice thanks!

Um, weird that this file should be overwritten in the tidyverse image, would be nice to track down why that happens. (e.g. will that also happen to the verse image?) Seems like we should be able to get this to propagate without manually adding it all the way up the stack, yes?

One thing to consider is having a script create this at runtime by adding said script to the init list (like we do with the userconf script in the RStudio image; basically put the shell script somewhere in /etc/services.d/). Might avoid the config being overwritten.

timtrice commented 6 years ago

Ok, the propagation issue was an error on my end; I didn't change the FROM value in the tidyverse Dockerfiles (don't tell anyone).

I did not revert any commits as I figured it best to keep the history. Not sure how the best way to handle this would have been.

Nonetheless, the tidyverse Dockerfiles have been put back as previous.

I added user-settings into each rstudio version directory and used COPY in the rstudio/Dockerfiles to bring them into the images. Then I needed to chown the .rstudio directory to avoid an error.

A sample test on a couple of images seems to work as expected. Is there an ideal way to build all of the images to look for errors (Travis)?

cboettig commented 6 years ago

Cool, great that this propagates as expected then.

So https://github.com/rocker-org/rocker-versioned/blob/master/circle.yml builds all the images sequentially and then does a few (mostly trivial) tests; guess one could add a test there to see, e.g. if the config file is there; but not sure if it is worth it.

Just curious, why did you switch to COPY instead of the bash echo? It doesn't really matter since COPY comes at the end and we already have some COPY calls for userconf, so this is fine; just that COPY breaks the docker build caching so I otherwise avoid it (but still COPY better than ADD or a RUN wget here)

timtrice commented 6 years ago

Maybe I misunderstood your previous comment about adding a script. I interpreted it as having the settings in their own file (thus adding the user-settings files) then using COPY similar to what was done for userconf.sh.

cboettig commented 6 years ago

No problem. Anyway, this looks good to me!

cboettig commented 6 years ago

@timtrice I think this may be interfering with RStudio's detection of git? In the current images, if terminal confirms that git is installed, but RStudio doesn't detect git (i.e. no git pane, new project from git fails). Can you take a look and confirm?

cboettig commented 6 years ago

@timtrice never mind, digging a bit more but looks like an error from a stale config on my end and not in a fresh pull