Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

building a package deletes inst/doc #128

Closed
GeoBosh opened this issue Dec 9, 2021 · 5 comments · Fixed by #149
Closed

building a package deletes inst/doc #128

GeoBosh opened this issue Dec 9, 2021 · 5 comments · Fixed by #149
Labels
feature a feature request or enhancement

Comments

@GeoBosh
Copy link

GeoBosh commented Dec 9, 2021

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
Copy link
Member

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
Copy link
Author

GeoBosh commented Dec 9, 2021

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

@gaborcsardi gaborcsardi added the feature a feature request or enhancement label Dec 9, 2021
@jennybc
Copy link
Member

jennybc commented Nov 18, 2022

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 (#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
Copy link
Author

GeoBosh commented Nov 21, 2022

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
Copy link
Member

gaborcsardi commented Nov 21, 2022

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.

gaborcsardi added a commit that referenced this issue Nov 23, 2022
Now `pkgbuild::build()` will not clean up `inst/doc` by default if the
`Config/build/clean-inst-doc` entry in `DESCRIPTION` is set to `FALSE`.

Closes #128.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants