att / rcloud.rcap

RCloud powered dashboards and websites
Other
8 stars 13 forks source link

don't send builds to /data/rcloud #4

Closed gordonwoodhull closed 8 years ago

gordonwoodhull commented 8 years ago

This is a known problem, just entering it here because it's one of the blockers for a public release of RCAP.

Since I have to build packages as myself and then install them as someone else, I've been patching the gruntfile to build the .tar.gz (R CMD build step) to my home directory.

Then I install it manually (R CMD INSTALL step).

I'm not sure what the idiomatic grunt thing is. Maybe creating an output directory within the project directory would be simplest.

There is no particular reason to build to rcloud.packages, nothing special about that directory. There also isn't much magic in build_packages.sh except that it pulls the version out of the DESCRIPTION file so that it knows what file to R CMD INSTALL.

So we could have two targets, one which just builds the package and one which also installs it.

gordonwoodhull commented 8 years ago

Sorry if this is a little incoherent. I'm definitely willing to help with this, since I'm mostly responsible for mystifying build_package.sh - it's something which doesn't have to be.

shaneporter commented 8 years ago

I like the idea of creating an output directory within the project directory.

So, two targets: I guess the default grunt task would be build and install. Another, build_package, would just build it.

The gruntfile also needs some general tidying up, so it would be a good opportunity to do that at the same time.

shaneporter commented 8 years ago

Let me know your thoughts on the change I made. Happy to adapt it if required.

gordonwoodhull commented 8 years ago

Yes, that looks good. I would suggest "build" instead of "buildpackage".

A bigger issue is that this is not really the R way to do things - really R CMD build should trigger grunt rather than the other way around. If we put R in charge then it can be submitted to rforge.net, CRAN, etc. Unfortunately I don't know any more about this.

gaborcsardi commented 8 years ago

We talked about this (putting R in charge) with Doug and we can certainly do it.

On the other hand, R probably should not run grunt at install time, but the package that is submitted to rforge/cran should already have all the JS included.

So maybe something like this would work:

gordonwoodhull commented 8 years ago

Right, that's why I was suggesting running grunt during R CMD build, not R CMD INSTALL. So there would be no need to check all that stuff into the GitHub repo, but they would still get added to the tarball.

But I might not be understanding how this works, or there may be another inconsistency between the Simonverse and the Hadleyverse.

gaborcsardi commented 8 years ago

It is not the end of the world if devtools::install_github() does not work for our package. But then you need to download the package, build it (for which you need to have bower, grunt, npm, etc.), all by hand. It is much more cumbersome for the user.

I experimented a bit with having two branches, and automatically build the second from the first with Travis, and this works well for the users usually. It is still somewhat cumbersome for potential contributors, because they are not used to this setup.

gordonwoodhull commented 8 years ago

So it seems that our options are either

  1. distribute a tarball with everything through rforge (and conceivably CRAN), and not support devtools::install_github
  2. check all dependencies into github

I doubt I can draw @s-u into the conversation on a private repo, but if I understand the trade-off, I'll ask him next time I see him.

gaborcsardi commented 8 years ago

@gordonwoodhull I was wondering why devtools::install_github() does not run R CMD build on the downloaded repo, and as it turns out it has an option called local, and setting that to FALSE will run R CMD build after all.

E.g. devtools::install_github("s-u/rcleaflet", local = FALSE)

Unfortunately the default is local = TRUE.

gaborcsardi commented 8 years ago

I asked them to change the default to local = FALSE. https://github.com/hadley/devtools/issues/1126 FYI.

gaborcsardi commented 8 years ago

I am happy to fix this, once we decide on how. :)

gordonwoodhull commented 8 years ago

My understanding is that we just want a DESCRIPTION in the root directory and we want either R CMD mkdist or build to work. This will make the package compatible with rforge.net.

It sounds like it works well enough with install_github; the objective is to have it work with install.packages too. And rforge.net is good enough, we don't really need CRAN.

Does this make sense?

gaborcsardi commented 8 years ago

Yes.

install.packages won't work on the GH repo, though. It will probably work on the releases.

Do we want to put the package on rforge.net?

gordonwoodhull commented 8 years ago

Yes, we generally use rforge as our distribution mechanism for RCloud-specific packages.

gaborcsardi commented 8 years ago

Have a PR for this in #124

gaborcsardi commented 8 years ago

PR #124 merged.

gordonwoodhull commented 7 years ago

@gaborcsardi, am I correct that we still don't have a way to do an automated build of rcloud.rcap - only the github releases work? Thanks.

gaborcsardi commented 7 years ago

No, there is a way, see the README: https://github.com/att/rcloud.rcap#installation

devtools::install_github("att/rcloud.rcap", local = FALSE)

Obviously, you need a bunch of tools for this, because it will build locally. E.g. node.js, npm, bower, grunt and GNU make.

gordonwoodhull commented 7 years ago

I tried that but it failed earlier than the tools:

Downloading GitHub repo att/rcloud.rcap@master
Error: Does not appear to be an R package (no DESCRIPTION)
gaborcsardi commented 7 years ago

Indeed, because we use the develop branch, I am not sure why, actually. I'll fix the README in a sec:

devtools::install_github("att/rcloud.rcap@develop", local = FALSE)

It takes a while to run, and the R installer does not allow any output, so wait a minute when you see * running ‘cleanup’

gordonwoodhull commented 7 years ago

Cool. We'd still like to support R CMD verb installation of some sort so that we can put the package on rforge and also use some automated scripts which we have for installing packages on the Research lake.

gordonwoodhull commented 7 years ago

When I installed it using the above method, and tried to load the RCAP Designer, I got

Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: pubsub
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/requirejs-text/text.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: rcap/bower_components/requirejs-text/text
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/require-css/css.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: rcap/bower_components/require-css/css
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/parsleyjs/dist/parsley.min.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: parsley
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/datatables.net-buttons/js/buttons.html5.min.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: datatablesbuttons/buttons.html5.min
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/datatables.net/js/jquery.dataTables.min.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: datatables/jquery.dataTables.min
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/jquery.sparkline/dist/jquery.sparkline.min.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: jquery.sparkline/jquery.sparkline.min
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/select2/dist/js/select2.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: select2/js/select2
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/ionrangeslider/js/ion.rangeSlider.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: ionrangeslider/js/ion.rangeSlider
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/spectrum/spectrum.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: spectrum/spectrum
http://requirejs.org/docs/errors.html#scripterror
https://rcloud-2.research.att.com/shared.R/rcloud.rcap/bower_components/wysiwyg.js/dist/standalone.js Failed to load resource: the server responded with a status of 404 (Code 404)
require-common.js:65 Uncaught Error: Script error for: wysiwyg/standalone
http://requirejs.org/docs/errors.html#scripterror
gaborcsardi commented 7 years ago

R CMD build does not care about the build script failing and you end up with a broken package that is missing some files.

This is hard (impossible?) to fix without changing R CMD build. Maybe the R code or a configure file could check if everything is there with some checksums.

gordonwoodhull commented 7 years ago

I see - thanks for the explanation. I guess our needs are satisfied by #144 for now.

s-u commented 7 years ago

The reason here is that the build step is done in cleanup which is run silently, i.e., it is allowed to fail. I guess the best way to fix this would be to add pre-flight script support to build in R.

gaborcsardi commented 7 years ago

Yes, this is what we discussed above, AFAIR.

We could have the JS artifacts in the repo, and then we have no problems installing the package with standard R tools. That's fine with me, too.

But if you want to

  1. avoid having bower_components in the repository, and
  2. want to build & install with R CMD build or devtools, then AFAIK the best you can do is the cleanup script.

Or some kind of hack that checks if everything included, at install time. At install time, we can run whatever we want. This could be tricky, too, because a simple checksum might not be enough, since e.g. the SASS tools build slightly different css on different platforms and they are all "correct".