r-devel / r-project-sprint-2023

Material for the R project sprint
https://contributor.r-project.org/r-project-sprint-2023/
17 stars 3 forks source link

Tasks to support CRAN #55

Open hturner opened 1 year ago

hturner commented 1 year ago

Discussed in https://github.com/r-devel/r-project-sprint-2023/discussions/54

Originally posted by **hturner** August 24, 2023 Uwe Ligges has some ideas for small tasks that volunteers might do to support the CRAN Team. This might include tasks such as * Improving the information in notes/warnings/errors to give package authors more hints about what has to be changed * Improving the processing of incoming packages to CRAN
llrs commented 1 year ago

While I do not attend the sprint I'm interested in helping the CRAN Team. 5 minutes ago someone asked if there was a CRAN Task View for Package Development.

hturner commented 1 year ago

CRAN Task Views are a community initiative and moved to GitHub for better community engagement a couple of years ago, see: https://github.com/cran-task-views/ctv.

llrs commented 1 year ago

Yes, sorry I just wanted to point out that people are somewhat lost on how to best comply with CRAN policy. (There is a new proposal)

agila5 commented 1 year ago

Hi! I would also like to help on this issue during the sprint :)

malcolmbarrett commented 1 year ago

I missed the issue version of this, but I'm interested in the first task

agila5 commented 1 year ago

Since room 2.48 is closed, me and Neeraj are at Breakout area 2.61.

agila5 commented 1 year ago

We created a small and simple patch on point 1 and submitted it to Uwe. Happy to add more details during the wrapup session!

llrs commented 1 year ago

I recently saw a comment about a check and I thought that a similar, but simpler, comment to what we did yesterday could be added to R.

Here is the patch:

Index: src/library/tools/R/check.R
===================================================================
--- src/library/tools/R/check.R (revision 85037)
+++ src/library/tools/R/check.R (working copy)
@@ -1918,7 +1918,8 @@
             wrapLog("Portable packages must use only ASCII",
                     "characters in their R code,\n",
                     "except perhaps in comments.\n",
-                    "Use \\uxxxx escapes for other characters.\n")
+                    "Use \\uxxxx escapes for other characters.\n",
+                    "Find them with tools::showNonASCIIfile(<filename>).\n")
         } else resultLog(Log, "OK")

         checkingLog(Log, "R files for syntax errors")

I'll try to work now on checking the license with the templates provided by CRAN. I started looking into it and I found that the changes should happen in src/library/tools/R/QC.R in the .check_package_license function. I'm not sure if a change there would affect only on R CMD check --as-cran or all checks via R CMD check affecting also Bioconductor and other repositories. I'll explore more and post back here.

I also found, what I think is a typo in setting the environmental variables for --as-cran. I didn't found a comment and it is documented to be set as warn for --as-cran:

Index: src/library/tools/R/check.R
===================================================================
--- src/library/tools/R/check.R (revision 85037)
+++ src/library/tools/R/check.R (working copy)
@@ -6825,7 +6825,7 @@
         Sys.setenv("_R_CHECK_NO_STOP_ON_TEST_ERROR_" = "TRUE")
         Sys.setenv("_R_CHECK_PRAGMAS_" = "TRUE")
         Sys.setenv("_R_CHECK_COMPILATION_FLAGS_" = "TRUE")
-        Sys.setenv1("_R_CHECK_R_DEPENDS_", "warn")
+        Sys.setenv("_R_CHECK_R_DEPENDS_", "warn")
         ## until this is tested on Windows
         Sys.setenv("_R_CHECK_R_ON_PATH_" = if(WINDOWS) "FALSE" else "TRUE")
         Sys.setenv("_R_CHECK_PACKAGES_USED_IN_TESTS_USE_SUBDIRS_" = "TRUE")
mmaechler commented 1 year ago

Thank you for the good idea and the patch.

To your 2nd part: No, that is not a typo, and your change would (almost surely) introduce a bug by doing something slightly differently than intended.

llrs commented 1 year ago

Thanks Martin for checking my first patch.

Thanks, now I see it. I didn't check the context carefully enough, sorry. Now I see that `Sys.setenv1` is defined in src/library/tools/R/utils.R with the following comment: "Sys.setenv() *one* variable unless it's set (to non-empty) already - export/move to base?". I didn't found it used anywhere else but I found some code that seems to be used for the same purpose on some lines above: ```r prev <- Sys.getenv("_R_CHECK_SCREEN_DEVICE_", NA_character_) if(is.na(prev)) Sys.setenv("_R_CHECK_SCREEN_DEVICE_" = "stop") ``` But I don't understand enough to see if `.Internal` makes a difference there.

Getting back to the MIT, BSD2 and BSD3 and the template. The idea was to check if the file LICENSE has all the appropriate fields for the license. I don't feel confident to propose such a change without checking interactively before. I could fabricate some tests and add them to the source code.

Would it be fine if I propose some tests for external or internal function of tools? My idea would be to have some DESCRIPTION and LICENSE files to check (I might need to create a folder as some functions depend on it). I hope this way it will be quicker an easier to catch the major problems before proposing a patch. Would they go under the test/ or the src/library/tools/tests/ folder?

aitap commented 1 year ago

There are ideas for _R_CHECK_DEPENDS_ONLY_ when .Library contains user packages due to being writeable, but a different implementation (an environment variable telling R to pretend certain packages don't exist) may be needed in the end for PR18584.

Another R CMD check message improvement implemented after the Sprint.