[R-pkg-devel] Cryptic error on Windows but not Debian

[Simon Urbanek]

Chris,

this was not a change in interpretation, but rather CRAN's tools have gotten better at detecting such bad behavior.

I would like to point out that there is absolutely no reason to mangle user's .Renviron since the package can always set any environment variables it needs from its (legally managed) configuration files (as described in the policy) on load.

Regarding the approach you suggested, personally, I think it is bad - no package should be touching personal configuration files - it's entirely unnecessary and dangerous (e.g., your code will happily remove the content of the file on write error losing all user's values - that's why you never write important files directly, but rather create a copy which you atomically move in place *after* you know the content is correct).

Cheers,
Simon



> On Nov 19, 2023, at 12:40 PM, Kenny, Christopher <christopherkenny using fas.harvard.edu> wrote:
> 
> Rather than using tools::R_user_dir(), you can also ask the user for a path where they would like to save the information to. This allows you to test it with a temporary directory file, but would allow the user to specify their .Renviron file, if they so choose. This acts as a middle ground managing a separate package-specific file and storing it as an environmental variable. This is how I approach it in a handful of packages, including `feltr` (see https://github.com/christopherkenny/feltr/blob/HEAD/R/felt_key.R) and `bskyr` (see https://github.com/christopherkenny/bskyr/blob/main/R/auth_user.R).
> 
> For what it's worth, some of this confusion may come from a relatively recent change in interpretation of the policy mentioned below by Simon (even though the text has long read that way). For years, CRAN allowed packages which had the practice of opting into writing to the default .Renviron file. That old reading comports with the example you point to in qualtRics, where the writing is controlled by the `install` argument, with a default of FALSE. Since sometime in the last year, the interpretation was updated and you are now met with a message from the volunteer which states:
> "Please ensure that your functions do not write by default or in your examples/vignettes/tests in the user's home filespace (including the package directory and getwd()). This is not allowed by CRAN policies.
> Please omit any default path in writing functions. In your examples/vignettes/tests you can write to tempdir()."
> 
> The approach used in `feltr` and other packages to explicitly require a path as an argument appears to be okay with the new reading of the policy. (At least, the CRAN volunteers seem to accept packages which use this approach.)
> 
> Best,
> Chris
> 
> 
> From: R-package-devel <r-package-devel-bounces using r-project.org> on behalf of Simon Urbanek <simon.urbanek using R-project.org>
> Sent: Saturday, November 18, 2023 6:14 PM
> To: Adam <asebsadowski using gmail.com>
> Cc: r-package-devel using r-project.org <r-package-devel using r-project.org>
> Subject: Re: [R-pkg-devel] Cryptic error on Windows but not Debian 
>  
> Adam,
> 
> no, it is your code in mm_authorize() that violates the CRAN policy, it is not about the test. You may not touch user's .Renviron and there is no reason to resort to such drastic measures. If you want to cache user's credentials, you have to do it in a file located via tools::R_user_dir().
> 
> Cheers,
> Simon
> 
> 
>> On Nov 19, 2023, at 12:07 PM, Adam <asebsadowski using gmail.com> wrote:
>> 
>> Thank you dearly, Simon, for pointing out the policy. May a test do the following?
>> 
>> 1. Save the user's original value for env var X.
>> 2. Write a new value for env var X during a test.
>> 3. Write back the original value for env var X at the end of the test.
>> 
>> An example:
>> 
>> test_that("mm_authorize() sets credentials", {
>>    skip_on_cran()
>>    key_original <- mm_key()
>>    url_original <- mm_url()
>>    withr::defer({
>>      mm_authorize(
>>        key = key_original,
>>        url = url_original,
>>        overwrite = TRUE
>>      )
>>    })
>>    mm_authorize(
>>      key = "1",
>>      url = "https://api.megamation.com/uw/joe/ <https://api.megamation.com/uw/joe/>",
>>      overwrite = TRUE
>>    )
>>    expect_false(
>>      endsWith(Sys.getenv("MEGAMATION_URL"), "/")
>>    )
>> })
>> 
>> Best,
>> Adam
>> 
>> 
>> On Sat, Nov 18, 2023 at 4:52 PM Simon Urbanek <simon.urbanek using r-project.org <mailto:simon.urbanek using r-project.org>> wrote:
>> Adam,
>> 
>> 
>>> On Nov 19, 2023, at 9:39 AM, Adam <asebsadowski using gmail.com <mailto:asebsadowski using gmail.com>> wrote:
>>> 
>>> Dear Ivan,
>>> 
>>> Thank you for explaining in such depth. I had not submitted to CRAN before.
>>> I will look into tools::R_user_dir().
>>> 
>>> - May you point me toward the policy that the package should not edit .Renviron?
>> 
>> 
>> It is the policy you have agreed to when submitting your package to CRAN:
>> 
>> "CRAN Repository Policy
>> [...]
>> The code and examples provided in a package should never do anything which might be regarded as malicious or anti-social. The following are illustrative examples from past experience.
>> [...]
>>   - Packages should not write in the user’s home filespace (including clipboards), nor anywhere else on the file system apart from the R session’s temporary directory. [...]
>>    For R version 4.0 or later (hence a version dependency is required or only conditional use is possible), packages may store user-specific data, configuration and cache files in their respective user directories obtained from tools::R_user_dir(), provided that by default sizes are kept as small as possible and the contents are actively managed (including removing outdated material).
>> "
>> 
>> Cheers,
>> Simon
>> 
> 
> 
>         [[alternative HTML version deleted]]
> 
> ______________________________________________
> R-package-devel using r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-package-devel