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

Make build_package() more robust #28

Closed
wants to merge 1 commit into from
Closed

Make build_package() more robust #28

wants to merge 1 commit into from

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Jul 10, 2017

against .tar.gz file names that contain the platform or other arbitrary text. This was necessary to make revdepcheck work on my Linux system. I do wonder why crancache stores platform-specific files in src/contrib .

Happy to add a test for the sanitize...() function.

against .tar.gz file names that contain the platform or other arbitrary text
@codecov-io
Copy link

codecov-io commented Jul 10, 2017

Codecov Report

Merging #28 into master will decrease coverage by 0.59%.
The diff coverage is 43.75%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #28     +/-   ##
=========================================
- Coverage   41.87%   41.28%   -0.6%     
=========================================
  Files          12       12             
  Lines         597      608     +11     
=========================================
+ Hits          250      251      +1     
- Misses        347      357     +10
Impacted Files Coverage Δ
R/build.R 61.53% <43.75%> (-38.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c12106...c18ed06. Read the comment docs.

@gaborcsardi
Copy link
Member

This is all good, thanks.

I wonder why this is needed for revdepcheck, though. You are not supposed to be checking binary packages, only source.

I do wonder why crancache stores platform-specific files in src/contrib .

There is no good standard place to put them, and if we put them there, then the standard install.packages, etc. functions find them.

But you are right, that this is awkward, and maybe during revdepcheck they are found instead of the proper source packages, which is bad.

@krlmlr
Copy link
Member Author

krlmlr commented Jul 10, 2017

It looks like the binary packages overwrite their source counterparts indeed. This is for hms:

setwd("~/git/R/hms")
revdepcheck:::revdep_results()
#> ✔ bigrquery 0.4.1                        ── E: 0     | W: 0     | N: 0    
#> ✔ DBI 0.7                                ── E: 1     | W: 0     | N: 0    
#> ✔ DBItest 1.5                            ── E: 1     | W: 0     | N: 0    
#> ✔ dplyr 0.7.1                            ── E: 1     | W: 0     | N: 0    
#> ✔ feather 0.3.1                          ── E: 0     | W: 0     | N: 0    
#> ✔ haven 1.1.0                            ── E: 0     | W: 0     | N: 2    
#> ✔ odbc 1.1.1                             ── E: 0     | W: 0     | N: 1    
#> ✔ readr 1.1.1                            ── E: 1     | W: 0     | N: 0    
#> ✔ scales 0.4.1                           ── E: 1     | W: 0     | N: 0    
#> ✔ tidyverse 1.1.1                        ── E: 0     | W: 0     | N: 0
revdepcheck:::revdep_results()[[4]]$new
#> 
#> ── R CMD check results ─────────────────────────────────── dplyr 0.7.1 ────
#> 
#> ❯ checking if this is a source package ... ERROR
#>   Only *source* packages can be checked.
#> 
#> ── 1 error ✖ | 0 warnings ✔ | 0 notes ✔

I'll look into choosing a different path for the binaries, will ping you otherwise.

@gaborcsardi
Copy link
Member

I think the binaries should go in a different repo, and crancache should only add this repo to the list of repos, unless source packages are requested.

(I don't know how much of this is happening currently.)

@krlmlr
Copy link
Member Author

krlmlr commented Jul 11, 2017

I guess we can scrap this.

@krlmlr krlmlr closed this Jul 11, 2017
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

Successfully merging this pull request may close these issues.

None yet

3 participants