r-lib / devtools

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

BUG: check_win_nnn() ignores 'Maintainer' field, but that's what win-builder prioritizes #2509

Closed HenrikBengtsson closed 1 year ago

HenrikBengtsson commented 1 year ago

Issue

devtools:::check_win() uses devtools:::maintainer(), which picks up the maintainer from the Authors@R field in the DESCRIPTION file, if that exists, and then falls back to the Maintainer field. However, according to https://win-builder.r-project.org/, the win-builder service looks for the Maintainer field.

This becomes a problem if there is both an Authors@R and a Maintainer field and they don't agree. For instance, I wanted checks another maintainer's package, so I edited the Authors@R field to use my email address, but missed the 'Maintainer' field. The check results went to the original package maintainer, despite devtools saying:

> devtools::check_win_devel()
Building windows version of projpred (2.4.0-1)
ℹ Using R-devel with win-builder.r-project.org.
Email results to henrik@me.example.org?

1: Absolutely
2: I forget
3: Uhhhh... Maybe?

Verification

FWIW, I've just double-checked using:

Package: teeny
Version: 1.0
Title: A Minimal, Valid, Complete R Package
Description: A minimal, valid, complete R package that can be used as a baseline for testing and troubleshooting various components of R
 and R packages.
Authors@R: c(
   person("Henrik", "Bengtsson", role=c("aut", "cre", "cph"),
          email = "alice@example.org"))
Maintainer: Henrik Bengtsson <bob@example.org>
License: GPL (>= 3)

and, indeed, devtools::check_win_devel() says it will go to alice@example.org and win-builder sent it to bob@example.org.

HenrikBengtsson commented 1 year ago

A follow-up: I just emailed the CRAN Team to ask them to clarify this on https://win-builder.r-project.org/. While doing so, I also suggested that they might want to switch the ordering to be Authors@R first with Maintainer as a fallback, because that is the priority order of R CMD check.

So, you might want to hold back updating this.

jennybc commented 1 year ago

The documented way to achieve your goal is to use the email argument of check_win_devel():

email

An alternative email to use, default NULL uses the package Maintainer's email.

https://devtools.r-lib.org/reference/check_win.html

And yes, the devtools ecosystem also prioritizes Authors@R over a hard-wired Maintainer field, as does the base R tooling, really.

HenrikBengtsson commented 1 year ago

Why close until devtools and win-builder works the same way here? Right now devtools::check_win_devel() is misleading and risks sending the original maintainer email reports that they don't want.

HenrikBengtsson commented 1 year ago

Also, argument email seems to not work when there's a Maintainer field;

> devtools::check_win_devel(email = "alice@example.org")
Error in `devtools::check_win_devel()`:
! DESCRIPTION can't use Maintainer field when changing `email`
Run `rlang::last_trace()` to see where the error occurred.
HenrikBengtsson commented 1 year ago

email An alternative email to use, default NULL uses the package Maintainer's email.

And it looks like the help on email should be updated to refer to the 'Authors@R' field instead of 'Maintainer' field, given the current behavior.

HenrikBengtsson commented 1 year ago

Here's Uwe's clarification on the winbuilder design and how this works (with minor tweaks of mine):

  1. R CMD build generates the Maintainer field from Authors@R unless someone had added an extra Maintainer field.

  2. winbuilder uses Maintainer on purpose: If someone unpacks the package, inserts themselves as Maintainer to get feedback from winbuilder, then winbuilder sends it to the changed maintainer. The checks will contain a note that Authors@R and Maintainer disagree. So it is simpler for non maintainers to quickly check a package on winbuilder.

  3. If Authors@R is changed, then the checks will always note that the two fields disagree. R always uses the Maintainer field. The function we always use is utils::maintainer().

  4. The right procedure for a package maintainer is to change Authors@R and delete Maintainer which will then regenerated by R CMD build (but only if not present).

My conclusions:

  1. For manual edits of DESCRIPTION, devtools does not emulate (1)+(2).
  2. When using devtools::check_win_nnn(email = ...) is in line with (2) and (4), because it gives an error if a Maintainer field exists.