search

r-lib / pkgbuild

Find tools needed to build R packages
https://pkgbuild.r-lib.org
Other
65 stars 33 forks source link

building a package deletes inst/doc #128

Closed GeoBosh closed 1 year ago

GeoBosh commented 2 years ago

I know that this was raised in #58 but there the discussion was about specific user cases. devtools::check() also uses pkgbuild to build a package and thus deletes inst/doc, as well.

The fact is that inst/doc is not designated for automatically generated files only. In that sense, deleting it is analogous to the former roxygen2 approach to overwrite, for example, NAMESPACE (which it does not do any more).

WRE, section 1.4, explicitly provides for the possibility to have other files and subdirectories in inst/doc:

"In addition to the help files in Rd format, R packages allow the 
 inclusion of documents in arbitrary other formats. The standard 
 location for these is subdirectory inst/doc of a source package, 
 the contents will be copied to subdirectory doc when the package
 is installed. 
 ...
 At install time an HTML index for all vignettes in the package is 
 automatically created from the \VignetteIndexEntry statements 
 unless a file index.html exists in directory inst/doc. This index is 
 linked from the HTML help index for the package. If you do 
 supply a inst/doc/index.html file it should contain relative links 
 only to files under the installed doc directory, ..."

A compromise would be to delete files in inst/doc except for inst/doc/index.html and not touch subdirectories of inst/doc.

gaborcsardi commented 2 years ago

The rcmdcheck package already uses switch in DESCIRPTION to avoid cleaning up inst/doc (https://github.com/r-lib/rcmdcheck/blob/a38200c997f58478c8d3e2c4d2dd5e6244dabcba/R/options.R#L87). . If we had the same in pkgbuild, would that work for you?

GeoBosh commented 2 years ago

That would be great. I assume that it would be honoured by devtools::check() as well.

jennybc commented 1 year ago

FYI I mentioned the prospect of this in the vignettes chapter of R Packages like so:

You can prevent the cleaning of inst/doc/ with pkgbuild::build(clean_doc =). You can put Config/build/clean-inst-doc: FALSE in DESCRIPTION to prevent rcmdcheck from cleaning inst/doc/ and something similar may be implemented for pkgbuild (https://github.com/r-lib/pkgbuild/issues/128).

The general stance in devtools and the book is to not keep anything in inst/doc/ and, instead, to let the official build machinery have full control. But we acknowledge that there can be good reasons to do otherwise, so I try to explain how to prevent the devtools-verse from constantly clobbering inst/doc/.

GeoBosh commented 1 year ago

Doesn't an option to prevent wiping off inst/doc (rather then the other way out) look like a very strong expression of stance, while remaining friendly?

Not a big deal here but similar stance on pdf's by pkgdown (which ignores them) and the GHA check workflows are really painful (the latter don't include LaTeX examples and the incorporation in a workflow of the separately provided examples is tricky and changes as the GHA evolve).

gaborcsardi commented 1 year ago

I don't think it is very important to wipe out inst/doc on the CI, where it should not exist for HTML vignettes, and if it does then we probably should not wipe it out in the first place.

OTOH, it is easy to package up outdated files on your local computer when you are submitting a package, so cleaning inst/doc makes sense there. Also, I am not sure if we can change the default at this point.

In any case, when we implement support for Config/build/clean-inst-doc: FALSE it will be a simple one time operation that you need to add to your package, and then pkgbuild will not clean up inst/doc.

We'll think about whether the default could be different on the CI.