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

Tasks to support CRAN #55

Open
hturner opened this issue Aug 24, 2023 Discussed in #54 · 11 comments
Open

Tasks to support CRAN #55

hturner opened this issue Aug 24, 2023 Discussed in #54 · 11 comments
Assignees
Labels

Comments

@hturner
Copy link
Member

hturner commented Aug 24, 2023

Discussed in #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
Copy link
Member

llrs commented Aug 24, 2023

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
Copy link
Member Author

hturner commented Aug 24, 2023

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
Copy link
Member

llrs commented Aug 24, 2023

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)

@aitap aitap self-assigned this Aug 25, 2023
@agila5
Copy link

agila5 commented Aug 27, 2023

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

@agila5 agila5 self-assigned this Aug 27, 2023
@villegar villegar self-assigned this Aug 30, 2023
@malcolmbarrett malcolmbarrett self-assigned this Aug 30, 2023
@malcolmbarrett
Copy link

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

@agila5
Copy link

agila5 commented Aug 30, 2023

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

@agila5
Copy link

agila5 commented Aug 31, 2023

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
Copy link
Member

llrs commented Aug 31, 2023

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
Copy link
Contributor

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
Copy link
Member

llrs commented Aug 31, 2023

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:

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
Copy link
Contributor

aitap commented Oct 17, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants