Re: pkglint and R package MASTER_SITES

[Brook Milligan <brook%nmsu.edu@localhost>]

> On Aug 9, 2020, at 2:03 PM, Roland Illig <roland.illig%gmx.de@localhost> wrote:
> 
> On 09.08.2020 21:38, Brook Milligan wrote:
>> I think the algorithm is:
>> 
>> - does a package include math/R/Makefile.extension and does it
>> override MASTER_SITES? if yes, then warn (or warn if
>> PKG_DEVELOPER=yes).
> 
> Pkglint cannot know whether PKG_DEVELOPER is set or not.  Furthermore,
> PKG_DEVELOPER is a misnamed variable.  It should rather be named
> RUN_VERY_BASIC_SANITY_CHECKS and thus should be enabled by everyone, not
> only pkgsrc developers.

Fair enough.

> What about this rule instead: Warn if an included file defines
> MASTER_SITES and the including file overrides it with only a subset of
> the default MASTER_SITES.
> 
> As far as I understood you, this would have caught the R packages.  The
> rule sounds general enough to be useful for all other packages as well.

Yes, that would have caught the immediate problem.  

That does not really address ongoing maintenance, but perhaps that is too much for now.  For example, one of the issues I ran into while fixing these overrides was that repositories using https were not listed in MASTER_SITES_R_CRAN, but archives seem to be there and not on the http analogues.  Adding https sites to the default was easy and fixed all the cases except those that overrode MASTER_SITES.  Under the suggested solution, that discrepancy would not be discovered until pkglint was run on such a package, potentially much later.  I'm ok with that for now, though, even though it seems that overriding with the full set of defaults is stylistically wrong.

>> If this had been in place before, pkglint would have caught early all
>> the cases I have had to fix and would result in ignorable warnings
>> for 2 of the 300+ R packages.
> 
> I don't like "ignorable warnings".

I generally agree, but an automated tool cannot differentiate all cases perfectly.  I guess you are favoring missing issues than triggering on false ones.  That is a tradeoff.

> There are several places where the package author can suppress a pkglint
> warning by writing a rationale comment near the location of the warning.
> For this scenario I am not sure whether there are any special keywords
> to qualify as a rationale, therefore I would just regard any comment as
> being a rationale.

Is this documented?  I don't see anything about this in the pkglint man page.

Cheers,
Brook