r-lib / revdepcheck

R package reverse dependency checking
https://revdepcheck.r-lib.org
Other
99 stars 31 forks source link

Better detect check error differences between CRAN and dev check #331

Open cderv opened 2 years ago

cderv commented 2 years ago

We had some issue detected on CRAN that we missed using revdepcheck comparison.

It happens because the comparison if made as a difference in check results (old vs new) and both case had an issue when ran in revdepcloud check.

* checking re-building of vignette outputs ... WARNING

However,

Regarding the latter, the issue was because some LaTeX packages are missing the cloud service

Error: processing vignette 'BENMMI_User_Manual.Rnw' failed with diagnostics:
Running 'texi2dvi' on 'BENMMI_User_Manual.tex' failed.
LaTeX errors:
! LaTeX Error: File `colortbl.sty' not found.

Type X to quit or <RETURN> to proceed,
or enter new name. (Default extension: sty)

! Emergency stop.
<read *> 

but error with the new version of the package was

Error: processing vignette 'BENMMI_User_Manual.Rnw' failed with diagnostics:
Running 'texi2dvi' on 'BENMMI_User_Manual.tex' failed.
LaTeX errors:
! LaTeX Error: Option clash for package xcolor.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H <return>  for immediate help.
 ...                                              
! LaTeX Error: File `sectsty.sty' not found.

Type X to quit or <RETURN> to proceed,
or enter new name. (Default extension: sty)

! Emergency stop.
<read *> 

which is a different error. This part was the one we should have detected and was raised on CRAN

LaTeX errors:
! LaTeX Error: Option clash for package xcolor.

If the CTAN packages were available on the cloud service, this would not have been an issue, but still I think improving the report to detect such differences in errors during check would help.

About revdepcloud issue with CTAN packages, see related https://github.com/rstudio/revdepcheck-cloud/issues/87

gaborcsardi commented 2 years ago

That is unfortunate, but I am not sure if can do much about it. Comparing our output to CRAN's output will probably produce a lot of false positives, I am afraid.

We could include the CRAN status of the revdeps in the report, so you can look at the ones that are failing on CRAN, maybe?

cderv commented 2 years ago

I was wondering if we could have a way to compare the error thrown during the checks.

If I make a parallel with test, this would mean have a snapshot comparaison of the R CMD Check output. This would allow to detect differences in the case

We missed one in knitr checks because the vignette building error was not exactly the same, and the new one was caused by knitr dev version.

In the current revdep check used in R Markdown ecosystem (crandalf), xfun::rev_check() is used and a HTML report is created when check.log are different using a diff command.

This allows to really see differences between "old" and "new". I don't think revdepcheck comparaison goes into the detail of the check log. But maybe I missed the correct report of this in the results

gaborcsardi commented 2 years ago
  • "old" has issues
  • "new" has issues
  • but the log of the R CMD checks does not show the same error.

revdepcheck should show this as a new error then, so this is a bug if it did not. If you still have the check results, can you please share them?

I don't think revdepcheck comparaison goes into the detail of the check log.

revdepcheck does compare the check log.

cderv commented 2 years ago

Some more context about the results we had that lead to this issues:

Package BENMMI had caused us a CRAN check issue when releasing knitr 1.39 and we failed to detect that using revdepcheck

CRAN summary file ````markdown ## revdepcheck results We checked 7041 reverse dependencies (7020 from CRAN + 21 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package. * We saw 1 new problems * We failed to check 10 packages Issues with CRAN packages are summarised below. ### New problems (This reports the first line of each new failure) * mlr3learners checking tests ... ERROR ### Failed to check * CausalImpact (NA) * ctsem (NA) * iemisc (NA) * iemiscdata (NA) * loon.ggplot (NA) * loon.shiny (NA) * loon.tourr (NA) * MarketMatching (NA) * SSVS (NA) * vivid (NA) ````
README content # Revdeps ## Failed to check (31) |package |version |error |warning |note | |:--------------|:-------|:-----|:-------|:----| |NA |? | | | | |NA |? | | | | |CausalImpact |? | | | | |NA |? | | | | |NA |? | | | | |ctsem |3.6.0 |1 | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |iemisc |? | | | | |iemiscdata |? | | | | |NA |? | | | | |loon.ggplot |? | | | | |loon.shiny |? | | | | |loon.tourr |? | | | | |NA |? | | | | |MarketMatching |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |NA |? | | | | |SSVS |? | | | | |NA |? | | | | |vivid |? | | | | ## New problems (1) |package |version |error |warning |note | |:----------------------------------------|:-------|:------|:-------|:----| |[mlr3learners](problems.md#mlr3learners) |0.5.2 |__+1__ | | |

README.md

Both file above have a lot of NA ( due to #310 I think)

If it is not reported in those summary, my understanding is that there is no issue too look into. We 7041 packages checked, so we can't really go in details for each one of them.

revdepcheck does compare the check log.

If I look at the detailed results, I can see the error.

Details for BENMMI package ````r > revdepcheck::cloud_details(id, "BENMMI") ══ Reverse dependency check ═══════════════════ BENMMI 4.3-7 ══ Status: OK ── Still failing x checking re-building of vignette outputs ... WARNING ── Before ───────────────────────────────────────────────────── > checking re-building of vignette outputs ... WARNING Error(s) in re-building vignettes: --- re-building ‘BENMMI_User_Manual.Rnw’ using knitr BENMMI version: 4.3.7 Copyright 2015-2022 by Rijkswaterstaat, the Netherlands (RWS). Type citation("BENMMI") on how to cite BENMMI in publications. For the tutorial, type: vignette("BENMMI_User_Manual") For Frequently Asked Questions, type: vignette("FAQ") Loading required package: parallel DEoptim package Differential Evolution algorithm in R Authors: D. Ardia, K. Mullen, B. Peterson and J. Ulrich Attaching package: 'dplyr' The following objects are masked from 'package:stats': filter, lag The following objects are masked from 'package:base': intersect, setdiff, setequal, union Error: processing vignette 'BENMMI_User_Manual.Rnw' failed with diagnostics: Running 'texi2dvi' on 'BENMMI_User_Manual.tex' failed. LaTeX errors: ! LaTeX Error: File `colortbl.sty' not found. Type X to quit or to proceed, or enter new name. (Default extension: sty) ! Emergency stop. l.270 \long \def\@secondoffive#1#2#3#4#5{#2}^^M ! ==> Fatal error occurred, no output PDF file produced! --- failed re-building ‘BENMMI_User_Manual.Rnw’ --- re-building ‘FAQ.Rnw’ using knitr Error: processing vignette 'FAQ.Rnw' failed with diagnostics: Running 'texi2dvi' on 'FAQ.tex' failed. LaTeX errors: ! LaTeX Error: File `fullpage.sty' not found. Type X to quit or to proceed, or enter new name. (Default extension: sty) ! Emergency stop. l.70 \usepackage [linkcolor=blue]{hyperref} % Required to create hyperlinks t... ! ==> Fatal error occurred, no output PDF file produced! --- failed re-building ‘FAQ.Rnw’ SUMMARY: processing the following files failed: ‘BENMMI_User_Manual.Rnw’ ‘FAQ.Rnw’ Error: Vignette re-building failed. Execution halted 0 errors ✓ | 1 warning x | 0 notes ✓ ── After ────────────────────────────────────────────────────── > checking re-building of vignette outputs ... WARNING Error(s) in re-building vignettes: --- re-building ‘BENMMI_User_Manual.Rnw’ using knitr BENMMI version: 4.3.7 Copyright 2015-2022 by Rijkswaterstaat, the Netherlands (RWS). Type citation("BENMMI") on how to cite BENMMI in publications. For the tutorial, type: vignette("BENMMI_User_Manual") For Frequently Asked Questions, type: vignette("FAQ") Loading required package: parallel DEoptim package Differential Evolution algorithm in R Authors: D. Ardia, K. Mullen, B. Peterson and J. Ulrich Attaching package: 'dplyr' The following objects are masked from 'package:stats': filter, lag The following objects are masked from 'package:base': intersect, setdiff, setequal, union Error: processing vignette 'BENMMI_User_Manual.Rnw' failed with diagnostics: Running 'texi2dvi' on 'BENMMI_User_Manual.tex' failed. LaTeX errors: ! LaTeX Error: Option clash for package xcolor. See the LaTeX manual or LaTeX Companion for explanation. Type H for immediate help. ... ! LaTeX Error: File `sectsty.sty' not found. Type X to quit or to proceed, or enter new name. (Default extension: sty) ! Emergency stop. l.74 \allsectionsfont {\sffamily}^^M ! ==> Fatal error occurred, no output PDF file produced! --- failed re-building ‘BENMMI_User_Manual.Rnw’ --- re-building ‘FAQ.Rnw’ using knitr Error: processing vignette 'FAQ.Rnw' failed with diagnostics: Running 'texi2dvi' on 'FAQ.tex' failed. LaTeX errors: ! LaTeX Error: File `fullpage.sty' not found. Type X to quit or to proceed, or enter new name. (Default extension: sty) ! Emergency stop. l.59 \usepackage [linkcolor=blue]{hyperref} % Required to create hyperlinks t... ! ==> Fatal error occurred, no output PDF file produced! --- failed re-building ‘FAQ.Rnw’ SUMMARY: processing the following files failed: ‘BENMMI_User_Manual.Rnw’ ‘FAQ.Rnw’ Error: Vignette re-building failed. Execution halted 0 errors ✓ | 1 warning x | 0 notes ✓ ````

This is not reported as a package with problem in problems.md because I think revdepcheck only report by default new broken packages.

So at the end, the package issues were not flagged, and we did not look into it.

CRAN check results for this package was ok - Having the information about that somehow would allow to flag this package as potential problem to look into.

Does it illustrate more the situation ?

I have the results for the whole 7000 packages for knitr 1.39 if necessary

gaborcsardi commented 2 years ago

This is not reported as a package with problem in problems.md because I think revdepcheck only report by default new broken packages.

Yes, but that is calculated by comparing the check output, if if both have one issue, but not the same issue, that's reported as -1 and +1, meaning the old error is gone, but there is a new error.

Can you please share the raw check results in private?

cderv commented 2 years ago

I'll send you the raw result on slack

that is calculated by comparing the check output, if if both have one issue, but not the same issue, that's reported as -1 and +1

I agree with that but my understanding is that it will only compare the R CMD check "status"

* checking re-building of vignette outputs ... WARNING

It won't compare the log of the error. I was not clear on that maybe. Sorry.

In the case we encountered, the problem is that this is the same check that is failing, but differently.

I got this understand from rcmdcheck

maybe not intended and this is an issue in rcmdcheck ?

Reprex base on the file I'll send you

pkg <- "BENMMI"
old <- file.path(pkg, "old", paste0(basename(pkg), ".Rcheck"), 
                 "00check.log")
new <- file.path(pkg, "new", paste0(basename(pkg), ".Rcheck"), 
                 "00check.log")
desc_path <- file.path(pkg, "DESCRIPTION")
description <- desc::desc(file = desc_path)

# getting the check result
old <- revdepcheck:::cloud_check_result(old, description, FALSE)
new <- revdepcheck:::cloud_check_result(new, description, FALSE)

# internal hash is seen has the same
rcmdcheck:::hash_check(old$warnings)
#> checkingrebuildingofvignetteoutputsWARNING 
#>         "ca87a5858a15632def0dbb61d5e60d5f"
rcmdcheck:::hash_check(new$warnings)
#> checkingrebuildingofvignetteoutputsWARNING 
#>         "ca87a5858a15632def0dbb61d5e60d5f"

# content is different
digest::digest(old$warnings)
#> [1] "27c2d5eaefa5aff6c8f6391b58773829"
digest::digest(new$warnings)
#> [1] "6054916295b97a8c08f1cde0c12117b3"

# cleancheck calculated inside hash_check()
gsub("[^a-zA-Z0-9]", "", rcmdcheck:::no_timing(rcmdcheck:::first_line(old$warnings)), 
     useBytes = TRUE)
#> [1] "checkingrebuildingofvignetteoutputsWARNING"
gsub("[^a-zA-Z0-9]", "", rcmdcheck:::no_timing(rcmdcheck:::first_line(new$warnings)), 
     useBytes = TRUE)
#> [1] "checkingrebuildingofvignetteoutputsWARNING"
gaborcsardi commented 2 years ago

It won't compare the log of the error. I was not clear on that maybe. Sorry.

Yeah, that is a bug indeed, we need to compare the whole thing.