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 
> <christopherke...@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-boun...@r-project.org> on behalf of 
> Simon Urbanek <simon.urba...@r-project.org>
> Sent: Saturday, November 18, 2023 6:14 PM
> To: Adam <asebsadow...@gmail.com>
> Cc: r-package-devel@r-project.org <r-package-devel@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 <asebsadow...@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.urba...@r-project.org 
>> <mailto:simon.urba...@r-project.org>> wrote:
>> Adam,
>> 
>> 
>>> On Nov 19, 2023, at 9:39 AM, Adam <asebsadow...@gmail.com 
>>> <mailto:asebsadow...@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@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-package-devel

______________________________________________
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel

Reply via email to