8000 installGithub.r not downloading dependencies correctly in docker build · Issue #227 · rocker-org/rocker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

installGithub.r not downloading dependencies correctly in docker build #227

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

Closed
stephlocke opened this issue Apr 4, 2017 · 16 comments
Closed

Comments

@stephlocke
Copy link
stephlocke commented Apr 4, 2017

Using a Dockerfile like

FROM rocker/tidyverse
RUN installGithub.r lockedata/TextAnalysis

When this goes through a build step on Docker Hub, I'm getting errors like:

�[91mDownloading GitHub repo lockedata/TextAnalysis@master
�[0m
�[91mInstalling 9 packages: curl, DBI, janeaustenr, psych, SnowballC, stringi, tibble, tidytext, tokenizers
Installing packages into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
�[0m
�[91mError in download.file(url, destfile, method, mode = "wb", ...) : 
  unused argument (update = TRUE)
�[0m
�[91mWarning in download.packages(pkgs, destdir = tmpd, available = available,  :
  download of package ‘curl’ failed

I'm assuming problem is PICNIC but the examples I've seen of installGithub look similar to the above

Any guidance on how to resolve?

@eddelbuettel
Copy link
Member

Once a *verse error filed at rocker (which is not where that code is). Will close. Can you repo in the repo containing the Dockerfile you reference?

@cboettig
Copy link
Member
cboettig commented Apr 4, 2017

@stephlocke Whoops, sorry about that, looks like the argument in the littler package script now needs to be patched to match the name of the argument in remotes::install_github(). @eddelbuettel just submitted a PR for this at
eddelbuettel/littler@master...cboettig:patch-1 if that's ok.

@eddelbuettel
Copy link
Member

Oh, I see. Line 45 definitely has merit. Wonder how/why that worked before -- I guess I never use the upgrade facility myself. But I think we can have a more minimal change -- no need to change the existing cmdline arg. Unless I am under-caffeinated and missing something...

Will release a new littler in due course. Have some other example scripts, but at least one needs R 3.4.0.

@eddelbuettel
Copy link
Member

@cboettig I honestly still don't understand the issue. remotes::install_github appears to have neither argument. In typical Gabor fashion I now followed this five functions down the rabbit hole and I still don;t see. Can you wave the magic clue bat at me?

I presume you use the -u flag somewhere?

@cboettig
Copy link
Member
cboettig commented Apr 4, 2017

@eddelbuettel I feel your pain. I believe install_github gets the upgrade argument from install_deps : https://github.com/r-pkgs/remotes/blob/master/R/install.R#L49

I haven't been able to track down if/when the argument changed from update to upgrade; it's possible it was always upgrade and this was my mistake, but seems unlikely since the command used to run fine on docker containers (possibly only on docker containers that had older littler with the installGithub.r before I added the -u option?)

I don't actually call it with -u most of the time, I mostly omit that and let it do the default, but either way it currently throws an error because the argument has the wrong name.

8000

@eddelbuettel
Copy link
Member

Jinx. I was also following it down to install_deps and just found it here.

I am somewhat suspicion that the wold population of -u argument users to installGithub.r is ... just you. Do we need the switch at all? Is it meant to updated existing 'from github' installation? All? Just the package we're working on and its depends?

@eddelbuettel
Copy link
Member

git blame to the rescue; this appears to be a recent-ish change of Nov 2016 in eddelbuettel/littler#46 which is, coincidentally, the last PR to have gotten in.

@cboettig
Copy link
Member
cboettig commented Apr 4, 2017

You're probably correct that few people^[1] use -u, so we could just delete that argument entirely. But note that in it's current form the installGithub.r script cannot run regardless of what arguments you do or don't include. So some patch is needed at some point.

I know it was my PR that added the -u argument to littler, what I couldn't find out is if something changed in remotes that has now caused remotes::install_github() to error when it is given an argument called update (perhaps it used to just be swallowed silently by the ...).

^[1]: though I think I'm not the only one who finds the default behavior of upgrading all the dependencies automatically to be annoying, e..g r-lib/remotes#53

@eddelbuettel
Copy link
Member

Exactly what is broken in installGithub.r? Quick session from r-base:

~$ docker run --rm -ti r-base /bin/bash
root@9144b964ca7d:/# installGithub.r eddelbuettel/digest
Error in library(remotes) : there is no package called ‘remotes’
root@9144b964ca7d:/# install.r remotes
trying URL 'https://cran.rstudio.com/src/contrib/remotes_1.0.0.tar.gz'
Content type 'application/x-gzip' length 37958 bytes (37 KB)
==================================================
downloaded 37 KB

* installing *source* package ‘remotes’ ...
** package ‘remotes’ successfully unpacked and MD5 sums checked
** R
** inst
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded
* DONE (remotes)

The downloaded source packages are in
        ‘/tmp/downloaded_packages’
root@9144b964ca7d:/# installGithub.r eddelbuettel/digest
Downloading GitHub repo eddelbuettel/digest@master
Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
* installing *source* package ‘digest’ ...
** libs
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c aes.c -o aes.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c crc32.c -o crc32.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c digest.c -o digest.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c init.c -o init.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c md5.c -o md5.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c pmurhash.c -o pmurhash.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c raes.c -o raes.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c sha1.c -o sha1.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c sha2.c -o sha2.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c sha256.c -o sha256.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG      -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c xxhash.c -o xxhash.o
gcc -std=gnu99 -shared -L/usr/lib/R/lib -Wl,-z,relro -o digest.so aes.o crc32.o digest.o init.o md5.o pmurhash.o raes.o sha1.o sha2.o sha256.o xxhash.o -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/digest/libs
** R
** inst
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded
* DONE (digest)
root@9144b964ca7d:/# 

@eddelbuettel
Copy link
Member

Also, I tend to

  • keep my .libPath() very current and
  • close to never ever install from GitHub [1]

so I have never been bitten.

[1] drat and all that. You know my view on package repositories as opposed to Russian roulette with random snapshots of code repositories.

@cboettig
Copy link
Member
cboettig commented Apr 4, 2017

@eddelbuettel Correct, the script is only broken if the requested package has dependencies which need to be installed from CRAN. e.g. on r-base you can reproduce @stephlocke 's error above using installGithub.r. So if littler is to keep installGithub.r I still think it should be patched.

You know I'm with you re: drat and .libPath() ;-)

@eddelbuettel
Copy link
Member

I am not trying to be difficult here but I still don't get it -- Depends: which are missing need to be installed whether the argument is called upgrade or update. How/why does that lead to breakage?

(And yes, installing from git is the devil incarnate, doing it recursively with further depends is just batshit insane and asking for trouble. But hey, if it is important to you I am sure you will fix the littler script accordingly ;-)

I may do it too but I still, umpteen issue ticket messages later, need a minimal repr.ex.

@cboettig
Copy link
Member
cboettig commented Apr 4, 2017

@eddelbuettel Not sure I can completely debug remotes functions as to precisely which line causes remotes errors on the call:

> remotes::install_github("lockedata/TextAnalysis", update = TRUE)

but not error on the call:

> remotes::install_github("eddelbuettel/digest", update = TRUE)

but it is relatively clear that the function calls a subroutine if it detects that the package has some other R packages in the Depends list of the DESCRIPTION file (which digest doesn't, but TextAnalysis does), and that this subroutine gets upset by seeing the unrecognized argument update (since it wants the argument to be called upgrade).

I think you can confirm though that with the exception of packages like digest that have no listed dependencies, that installGithub.r script will throw an error and that the error could be fixed by the single change I proposed in littler, right?

@eddelbuettel
Copy link
Member

(Sorry, was out bidding farewell to a friend leaving for your Wrong Coast...)

Wouldn't that analysis put the error with remotes rather than littler?

@cboettig
Copy link
Member
cboettig commented Apr 5, 2017

No, the problem is that I've given the wrong name for the argument in the installGithub.r script, it should have been upgrade and not update. Switch it to upgrade and it works. I don't see how that's a problem with the code in remotes.

@eddelbuettel
Copy link
Member

Ok, then we're back to my initial comment on your commit that we only need to change line 45 and nothing else. I'm ok with that.

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

No branches or pull requests

3 participants
0