r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.37k stars 753 forks source link

Alternative to `cleanup = FALSE` #2448

Closed fweber144 closed 1 year ago

fweber144 commented 1 year ago

It seems like argument cleanup of devtools::check() has been defunct in devtools v2.4.4. I realized this after updating devtools to v2.4.4 and then running R CMD check via RStudio's keyboard shortcut Ctrl + Shift + E on one of my packages. I have deselected option "Cleanup output after successful R CMD check" in RStudio's global options. My question is now what the recommended alternative to cleanup = FALSE is. I couldn't find anything about that in the documentation and neither by searching the internet.

jennybc commented 1 year ago

This hasn't been touched recently and, in particular, not on my watch.

In the NEWS file, I find this, indicating the deprecation happened 6 years ago:

https://github.com/r-lib/devtools/blame/8ef16cfc09394db710740d25e48cfc958e804f15/NEWS.md#L789-L790

The cleanup argument to check() is deprecated: it now always returns the path to the check directory.

So even though it sounds like you thought you were relying on this argument to get some behaviour, I don't think it has actually been doing anything for you. If you're noticing some change in your life, I think there's likely some other explanation.

fweber144 commented 1 year ago

Yes, the deprecation already happened some time ago. But now in v2.4.4 it was defunct, i.e., using cleanup = FALSE leads to an error now, not only a warning. Reprex:

  1. In RStudio, create a new package via "File" → "New project" → "New directory" → "R package".
  2. To avoid an R CMD check warning, specify the License: field in the DESCRIPTION file of the newly created R package (e.g., setting it to GPL-3).
  3. Ensure that RStudio's global option "Cleanup output after successful R CMD check" (under "Tools" → "Global Options" → "Packages" → "Development") is deselected.
  4. Run R CMD check via RStudio's keyboard shortcut Ctrl + Shift + E while being in the R project corresponding to the newly created R package.
  5. With devtools v2.4.3, I get Warning: `cleanup` is deprecated at the beginning of the output from the "Build" pane, but the check runs through and as desired, the check directory (at ../<package_name>.Rcheck) is not deleted. In contrast, with devtools v2.4.4, the check aborts directly at the beginning with the error message

    Error: The `cleanup` argument of `check()` was deprecated in lifecycle 1.11.0 and is now defunct.
    Execution halted
    
    Exited with status 1.
  6. Now select RStudio's global option "Cleanup output after successful R CMD check" (under "Tools" → "Global Options" → "Packages" → "Development").
  7. With devtools v2.4.3, I do not get Warning: `cleanup` is deprecated at the beginning of the output from the "Build" pane anymore and as before, the check runs through, but the check directory (at ../<package_name>.Rcheck) is deleted (as expected). With devtools v2.4.4, the behavior is the same as with v2.4.3: The check runs through, but the check directory (at ../<package_name>.Rcheck) is deleted (as expected).

So, in summary, there is no way to keep the check directory at ../<package_name>.Rcheck with devtools v2.4.4. That's the reason for my question above: the recommended alternative to cleanup = FALSE with devtools v2.4.4.

My RStudio version is:

RStudio 2022.07.0+548 "Spotted Wakerobin" Release (34ea3031089fa4e38738a9256d6fa6d70629c822, 2022-07-06) for Ubuntu Jammy

fweber144 commented 1 year ago

Given my comment above, may I ask @jennybc to re-open this? Sorry for not including a reprex in the first place.

fweber144 commented 1 year ago

I conducted some experiments and have found out the following:

  1. In the reprex given above, running devtools::check(document = FALSE, check_dir = "..", args = "--output=.") or devtools::check(document = FALSE, check_dir = "..") at the RStudio console keeps the <package_name>.Rcheck directory after the check (and also the <package_name>_<package_version>.tar.gz file). This holds for devtools 2.4.3 as well as 2.4.4, so this could be the desired alternative. However, this requires to run devtools::check() at the console. Running it in RStudio's build pane via RStudio's keyboard shortcut Ctrl + Shift + E is more convenient, I think. This is what point 2 below is about.
  2. With devtools 2.4.4 (which requires to select the global option "Cleanup output after successful R CMD check" under "Tools" → "Global Options" → "Packages" → "Development"), when specifying --output=.. in the additional R CMD check arguments of the Project Options (in RStudio) and then running the check in RStudio's build pane via Ctrl + Shift + E, the <package_name>.Rcheck directory is kept. However, that <package_name>.Rcheck directory is then not at the same directory level as the folder containing the RStudio project (i.e., <package_name>), but one level up. This is definitely a downside. Strangely, specifying --output=. in the additional R CMD check arguments of the Project Options causes the <package_name>.Rcheck directory to be deleted after the check. Also note that the <package_name>_<package_version>.tar.gz file is not kept (it seems to be put to a temporary directory which does not exist anymore after the check). With devtools 2.4.3, the behavior is the same when selecting the global option "Cleanup output after successful R CMD check", but it slightly differs when deselecting that global option: In that case, the <package_name>_<package_version>.tar.gz file is kept at the same level as the folder containing the RStudio project (i.e., <package_name>).

Thus, I guess the problem is mainly with RStudio. Currently, I think there are the following RStudio-related problems, apart from the downside I mentioned in point 2 above (the check directory being one level up):

Because of these RStudio-related issues, I will create a new RStudio issue. However, there are also some problems with devtools, I think:

kevinushey commented 1 year ago

For reference, this is where the IDE was relying on cleanup = FALSE for devtools::check():

https://github.com/rstudio/rstudio/blob/175f031b4f193f6a971163f088227c68d5fc569e/src/cpp/session/modules/build/SessionBuild.cpp#L1080-L1081

I'm guessing most users left the default preferences here, hence why this is the first time we're seeing this now.

This is where Hadley made the change:

https://github.com/r-lib/devtools/commit/7cc2bdf74cbae90233fa6c84f221631d1876598d#diff-580bc730038e74accf0289eb2f9bfae59d84a9d79a9b096546f57bafaf85f9a7R79

Was that change really intentional? If so, it seems like it should be accompanied by a NEWS entry?

It seems like, as far as the IDE is concerned, we can just stop using cleanup = FALSE and rely on our own code which manually deletes the .Rcheck directory (if appropriate) after calling devtools::check().

jennybc commented 1 year ago

I'm confident the deprecation was intentional, so I'm implementing suggestions from @fweber144 to make the situation more clear for any user who bangs their shins against this.

fweber144 commented 1 year ago

Thank you, @jennybc and @kevinushey, for spending your time on this. Your suggestions sound reasonable to me.