JanMarvin / openxlsx2

openxlsx2 - read, write and modify xlsx files
https://janmarvin.github.io/openxlsx2/
Other
121 stars 11 forks source link

Post release CRAN issues #1073

Closed JanMarvin closed 2 months ago

JanMarvin commented 4 months ago

Our CRAN page is a little noisier than usual.

https://cran.r-project.org/web/checks/check_results_openxlsx2.html

Not sure what the ggplot2 warning is supposed to mean and I don’t remember any changes to it. Any ideas @olivroy ? Maybe it’s related to my attempt at rewriting the Basic Vignette. The gcc 12 warning is (at least to my understanding) bogus. The compiler complains, but everything’s fine. I tried with a R 4.1 windows release, but wasn’t able to recreate the issue. It should be gone, once R 4.5 drops.

STRING_PTR note

Related to Rcpp, they are working on an update, but it wont ship with their current release. Nothing we can do.

  File ‘openxlsx2/libs/openxlsx2.so’:
    Found non-API call to R: ‘STRING_PTR’

  Compiled code should not call non-API entry points in R.

  See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual,
  and section ‘Moving into C API compliance’ for issues with the use of
  non-API entry points.

Vignette error

No idea what is going on

  Errors in running code in vignettes:
  when running code in ‘openxlsx2_charts_manual.Rmd’
    ...

  > if (requireNamespace("ggplot2")) {
  +     library(ggplot2)
  +     p <- ggplot(mtcars, aes(x = mpg, fill = as.factor(gear))) + 
  +         ggtitle("Dist ..." ... [TRUNCATED] 
  Loading required namespace: ggplot2

    When sourcing ‘openxlsx2_charts_manual.R’:
  Error: file.exists(fileName) is not TRUE
  Execution halted

    ‘Update-from-openxlsx.Rmd’ using ‘UTF-8’... [1s/1s] OK
    ‘conditional-formatting.Rmd’ using ‘UTF-8’... [1s/3s] OK
    ‘openxlsx2.Rmd’ using ‘UTF-8’... [1s/1s] OK
    ‘openxlsx2_charts_manual.Rmd’ using ‘UTF-8’... failed
    ‘openxlsx2_formulas_manual.Rmd’ using ‘UTF-8’... [1s/1s] OK
    ‘openxlsx2_read_to_df.Rmd’ using ‘UTF-8’... [1s/1s] OK
    ‘openxlsx2_style_manual.Rmd’ using ‘UTF-8’... [3s/3s] OK

GCC 12.3.0 warning

We've seen this before, I doubt that it is valid

  Found the following significant warnings:
    xlsb_funs.h:587:88: warning: array subscript 'FRTVersionFields[0]' is partly outside array bounds of 'uint16_t [1]' {aka 'short unsigned int [1]'} [-Warray-bounds]
  See 'd:/Rcompile/CRANpkg/local/4.3/openxlsx2.Rcheck/00install.out' for details.
  * used C++ compiler: 'g++.exe (GCC) 12.3.0'

Caused by this: https://github.com/JanMarvin/openxlsx2/blob/fa80047180c386d52a3c676819c41c3694ee9e64/src/xlsb_funs.h#L577-L588

and this https://github.com/JanMarvin/openxlsx2/blob/fa80047180c386d52a3c676819c41c3694ee9e64/src/xlsb_defines.h#L197-L200

According to my understanding that's a false positive in GCC. flags is a uint16_t, we read the first 15 bytes and the last one. Looks fine to me. Though we still could comment the entire conversion, we do not use it, it is only to print some debug information that might be useful in the future.

JanMarvin commented 4 months ago

So far all issues are oldrel related and I’m not losing sleep, but sometimes it’s better to be prepared.

Edit: And it builds fine with 4.1 (please don't ask why I know and or test this) and probably 4.3 too, unless there is something else going on.

olivroy commented 4 months ago

This looks like perfectly fine ggplot2 code? https://janmarvin.github.io/openxlsx2/articles/openxlsx2_charts_manual.html#add-ggplot2-plot-to-workbook. I don't know and have never seen this

JanMarvin commented 4 months ago

Now that you mention it, the issue might be from here. The tempfile was somehow not created, but I'm going to ignore this for now. Might be something strange on that specific CRAN machine. Binaries for 4.3 are available via r-universe

https://github.com/JanMarvin/openxlsx2/blob/87cc58c11589c2a1fc423038ef6273a69b765fd7/R/class-workbook.R#L5846 https://github.com/JanMarvin/openxlsx2/blob/87cc58c11589c2a1fc423038ef6273a69b765fd7/R/class-workbook.R#L5865

JanMarvin commented 2 months ago

Because the next release is coming up, I’ve reached out to CRAN, asking for guidance, how to handle the oldrel MacOS errors.

JanMarvin commented 2 months ago

Finally progress. While CRAN still hasn't responded to my mail (:disappointed:) I'm now able to reproduce the error.

In a terminal:

export _R_CHECK_VIGNETTES_SKIP_RUN_MAYBE_=false
R CMD build openxlsx2
R CMD check --as-cran openxlsx2_1.8.0.9000.tar.gz # works fine
R CMD check openxlsx2_1.8.0.9000.tar.gz # bails with the same error as seen on CRAN: https://www.r-project.org/nosvn/R.check/r-oldrel-macos-arm64/openxlsx2-00check.html