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

update.dev.pkg HTTP status error when options(pkgType = "binary") #3148

Closed
ProfFancyPants opened this issue Nov 16, 2018 · 6 comments
Closed
Milestone

Comments

@ProfFancyPants
Copy link

I think the intended behavior is for update.dev.pkg() to obtain updated version regardless if options(pkgType = "binary") is set. A solution might be to temporaryly set options(pkgType = "both") just before base::read.dcf within data.table:::dcf.repo.

update.dev.pkg <- function (object = "data.table", repo = "https://Rdatatable.github.io/data.table", 
  field = "Revision", ...) {
  pkg = object
  stopifnot(is.character(pkg), length(pkg) == 1L, !is.na(pkg), 
    is.character(repo), length(repo) == 1L, !is.na(repo), 
    is.character(field), length(field) == 1L, !is.na(field))
  ## POTENTIAL TEMPORARY FIX ##############################
  opts <- options(pkgType = "both"); on.exit(opts) 
  #########################################################
  una = is.na(ups <- dcf.repo(pkg, repo, field))
  upg = una | !identical(ups, dcf.lib(pkg, field))
  if (upg) utils::install.packages(pkg, repos = repo, ...)
  if (una) cat(sprintf("No commit information found in DESCRIPTION file for %s package. Unsure '%s' is correct field name in PACKAGES file in your devel repository '%s'.\n", 
      pkg, field, file.path(repo, "src", "contrib", "PACKAGES")))
  cat(sprintf("R %s package %s %s (%s)\n", pkg, 
    c("is up-to-date at", "has been updated to")[upg + 1L], 
    dcf.lib(pkg, field), 
    utils::packageVersion(pkg)))
}

Minimal reproducible example

options(pkgType = "binary")
data.table:::update.dev.pkg()

options(pkgType = "both")
debugonce(data.table:::update.dev.pkg)

The issue is comes from how base::read.dcf is constructing the url. For pkgType = "binary" it is looking for "[...]bin/windows/contrib/3.5/PACKAGES" which doesn't exist because the binary is stored in https://ci.appveyor.com/project/Rdatatable/data-table/build/job//artifacts

Within: base::read.dcf "binary"

Browse[3]> file
A connection with
description "https://Rdatatable.github.io/data.table/bin/windows/contrib/3.5/PACKAGES"
class "url-wininet"
mode "r"
text "text"
opened "closed"
can read "yes"
can write "no"

Within: base::read.dcf "both"

Browse[3]> file
A connection with
description "https://Rdatatable.github.io/data.table/src/contrib/PACKAGES"
class "url-wininet"
mode "r"
text "text"
opened "closed"
can read "yes"
can write "no"

Session info

> sessionInfo()
R version 3.5.1 (2018-07-02)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 17134)
Matrix products: default
locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252    LC_MONETARY=English_United States.1252 LC_NUMERIC=C                           LC_TIME=English_United States.1252    
attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     
loaded via a namespace (and not attached):
[1] compiler_3.5.1    tools_3.5.1       R.methodsS3_1.7.1 data.table_1.11.9 R.utils_2.7.0     packrat_0.4.9-3   R.oo_1.22.0

> packageDescription("data.table")
Package: data.table
Version: 1.11.9
Title: Extension of `data.frame`
Authors@R: c( person("Matt","Dowle", role=c("aut","cre"), email="mattjdowle@gmail.com"), person("Arun","Srinivasan", role="aut", email="arunkumar.sriniv@gmail.com"),
                    person("Jan","Gorecki", role="ctb"), person("Michael","Chirico", role="ctb"), person("Pasha","Stetsenko", role="ctb"), person("Tom","Short", role="ctb"),
                    person("Steve","Lianoglou", role="ctb"), person("Eduard","Antonyan", role="ctb"), person("Markus","Bonsch", role="ctb"), person("Hugh","Parsonage", role="ctb"),
                    person("Scott","Ritchie", role="ctb"))
Depends: R (>= 3.1.0)
Imports: methods
Suggests: bit64, curl, R.utils, knitr, xts, nanotime, zoo
Description: Fast aggregation of large data (e.g. 100GB in RAM), fast ordered joins, fast add/modify/delete of columns by group using no copies at all, list columns, friendly and
                    fast character-separated-value read/write. Offers a natural and flexible syntax, for faster development.
License: MPL-2.0 | file LICENSE
URL: http://r-datatable.com
BugReports: https://github.com/Rdatatable/data.table/issues
VignetteBuilder: knitr
ByteCompile: TRUE
Revision: 55168edde9bd08bb11d6f75545535ea3165a4119
NeedsCompilation: yes
Packaged: 2018-11-14 22:02:07 UTC; travis
Author: Matt Dowle [aut, cre], Arun Srinivasan [aut], Jan Gorecki [ctb], Michael Chirico [ctb], Pasha Stetsenko [ctb], Tom Short [ctb], Steve Lianoglou [ctb], Eduard Antonyan
                    [ctb], Markus Bonsch [ctb], Hugh Parsonage [ctb], Scott Ritchie [ctb]
Maintainer: Matt Dowle <mattjdowle@gmail.com>
Built: R 3.5.1; x86_64-w64-mingw32; 2018-11-16 18:48:24 UTC; windows

-- File: C:/Users/<username>/Documents/R/win-library/3.5/data.table/Meta/package.rds 
@jangorecki
Copy link
Member

We do build and publish windows binaries. They are just not available in default repository defined in update.dev.pkg. Call the function with repo="https://Rdatatable.gitlab.io/data.table". It might be better to just change the default value rather than changing option which might have been intentionally defined by user.

@jangorecki
Copy link
Member

jangorecki commented Nov 17, 2018

I made PR, once it will be merged the following call should work update.dev.pkg(type="both")

@ProfFancyPants
Copy link
Author

I think the optimal end result would be that even if options(pkgType = "binary") is set update.dev.pkg() would source the binary version of the newest dev build. Of course update.dev.pkg(type="binary") would as well.

I have been using my own implementation of update.dev.pkg until recently because I didn't know it existed. It would go to the "https://ci.appveyor.com/project/Rdatatable/data-table" and use a browser server to navigate to ".../build/job/va73y9dpvvv4v5it/artifacts" because the build link hash (i.e. "va73y9dpvvv4v5it") was always different. From what I could tell there was no permanent link. I did this for fun; I know it is silly.

I am wondering how problematic it would be if a copy of data.table_1.11.9.zip was automatically hosted "https://Rdatatable.github.io/data.table/bin/windows/contrib/3.5/PACKAGES" whenever the appveyor thingy updates. I am not familiar with how this stuff works.

@jangorecki
Copy link
Member

jangorecki commented Nov 18, 2018

Hardcoding option is terrible solution because it takes away R built-in flexibility from user. There might be environment where installing from binaries is forbidden, thus such option (type="source") can be set globally for all R sessions, so we should not override that.

There is permanent link, did you tried?

data.table::update.dev.pkg(repo="https://Rdatatable.gitlab.io/data.table")

Implementing integration of travis and appveyor won't reliably work. There is no reason to implement that anyway because there is good alternative that already have such integration natively, and AFAIK it works.

@ProfFancyPants
Copy link
Author

I think I made a typo above. Instead of saying "solution might be to temporarily" I meant "A temporary solution might be." I agree that it is a bad idea to hardcode this fix. What I am suggesting is that the user's pkgType option should be obeyed without a workaround like gitlab.

So data.table::update.dev.pkg(repo="https://Rdatatable.gitlab.io/data.table") does work, where as the GitHub link does not. This reason for this is because there is a file located at the "https://Rdatatable.gitlab.io/data.table/bin/windows/contrib/3.5/PACKAGES" link, and there isn't one at "https://Rdatatable.github.io/data.table/bin/windows/contrib/3.5/PACKAGES"

good alternative that already have such integration natively, and AFAIK it works.

Great! What is it? And can we do it on github as well? Or should update.dev.pkg default repo url be changed to the gitlab?

@jangorecki
Copy link
Member

jangorecki commented Nov 19, 2018

Great! What is it? And can we do it on github as well? Or should update.dev.pkg default repo url be changed to the gitlab?

It is GitLab. We cannot do it on GitHub because it does not offer CI.
We should probably change default repo url to GitLab.

jangorecki added a commit that referenced this issue Nov 19, 2018
@jangorecki jangorecki added this to the 1.12.0 milestone Nov 19, 2018
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

2 participants