Move the markdown package from Imports to Suggests
yihui opened this issue · 86 comments
For package authors who use R Markdown vignettes based on the vignette engine knitr::knitr
(or knitr::docco_linear
, knitr::docco_classic
, etc.), you should declare the (soft) dependency on markdown, e.g., in the package DESCRIPTION:
Suggests: markdown
VignetteBuilder: knitr
Previously they didn't need to do this because markdown has been a hard dependency of knitr (in Imports
), so the availability of markdown is guaranteed by knitr, but I want to make markdown a soft dependency of knitr in the future.
I think most packages use the vignette engine knitr::rmarkdown
to write vignettes now, but there may still be a few vignettes using the knitr::knitr
engine, which depends on markdown. If I move markdown to Suggests
in knitr, users have to make sure markdown is available to R CMD check
. To achieve that, they have to add markdown to Suggests
to their DESCRIPTION
files.
Similarly, if your vignette is based on the vignette engine knitr::rmarkdown
, you have to declare the dependency on rmarkdown, e.g.,
Suggests: rmarkdown
VignetteBuilder: knitr
The error message when building the source package via R CMD build
should tell you if you need to add markdown or rmarkdown to Suggests
, which looks like this:
* checking for file ‘PKG/DESCRIPTION’ ... OK
* preparing ‘PKG’:
* checking DESCRIPTION meta-information ... OK
* installing the package to build vignettes
* creating vignettes ... ERROR
--- re-building ‘doc.Rmd’ using rmarkdown_notangle
Error: processing vignette 'doc.Rmd' failed with diagnostics:
The 'rmarkdown' package should be declared as a dependency of
the 'PKG' package (e.g., in the 'Suggests' field of DESCRIPTION),
because the latter contains vignette(s) built with the 'rmarkdown'
package. Please see https://github.com/yihui/knitr/issues/1864
for more information.
--- failed re-building ‘doc.Rmd’
SUMMARY: processing the following file failed:
‘doc.Rmd’
Error: Vignette re-building failed.
Execution halted
If you are a package author affected by this issue, but not clear about what you need to do, please reply below and I'll be glad to help. I recommend that you skip the replies below. Thanks!
hi I got this from win-builder:
* checking re-building of vignette outputs ... [1s] WARNING
Error(s) in re-building vignettes:
--- re-building 'examples.Rmd' using knitr
Error: processing vignette 'examples.Rmd' failed with diagnostics:
The 'markdown' package should be declared as a dependency of the 'directlabels' package (e.g., in the 'Suggests' field of DESCRIPTION), because it contains vignette(s) built with the 'markdown' package. Please see https://github.com/yihui/knitr/issues/1864 for more information.
--- failed re-building 'examples.Rmd'
SUMMARY: processing the following file failed:
'examples.Rmd'
Error: Vignette re-building failed.
Execution halted
my vignette uses
%\VignetteEngine{knitr::knitr}
which I think does not require markdown (even in suggests), if I understand your discussion above correctly. so it seems strange that I would need to add markdown in suggests. do I really have to?
Yes, you have to.
ok
I think I'm getting a false positive warning on the CRAN Winbuilder. I've got all my vignettes set to
%\VignetteEngine{knitr::rmarkdown}
If I have not set in DESCRIPTION
that
Suggests:
rmarkdown
I'm getting no warnings locally (R=4.0.2
, rmarkdown=2.2
, knitr=1.28
). On the CRAN Winbuilder, I'm getting:
Error: processing vignette 'quickstart.Rmd' failed with diagnostics:
The 'markdown' package should be declared as a dependency of the 'clustermq' package (e.g., in the 'Suggests' field of DESCRIPTION), because it contains vignette(s) built with the 'markdown' package. Please see #1864 for more information.
The missing package, however, is Suggests: rmarkdown
and not markdown
.
@mschubert I need a reproducible example. Could you provide a link to your package (presumably on Github?)? Thanks!
Package was clustermq:3d223af
and you can find the Winbuilder output here. I won't have time to track this down further unfortunately (since adding Suggests: rmarkdown
solves the issue & a warning for the right package is there), sorry.
@mschubert In your case, you are using the vignette engine knitr::rmarkdown
, so you have to add rmarkdown
to Suggests
(which you did mschubert/clustermq@02e06f8): https://yihui.org/knitr/demo/vignette/ Because you didn't do that, you saw the message about markdown
, and that was because knitr fell back to using markdown to build your vignettes. Neither rmarkdown
nor markdown
was specified in your Suggests
, and that's why you saw the error.
Yes, that's what I tried to say - sorry if that was not clear. My comment was that the error message of "missing markdown
package in Suggests
" is confusing when the vignette states that it uses rmarkdown
(and the markdown
package should never be required).
Perhaps a better error message would be to add whatever markdown builder you use to Suggests
? Or, maybe do nothing, but I thought it was useful to document that this message is also displayed when rmarkdown
is missing (I guess people will google the issue and find this).
@mschubert Yes, I definitely agree the messages were confusing in this case. I'll think more about it and try to make it clearer. Thanks a lot for your feedback!
I agree with @mschubert. I had the same situation. I was about to add 'markdown' to suggests because of the message, when I really should have added 'rmarkdown' to suggests, because I use knitr::rmarkdown
(if I understood correctly).
With knitr 1.30 (the current latest version on CRAN), the message should tell you to add rmarkdown
instead of markdown
to Suggests when the vignette engine knitr::rmarkdown
is used.
Any idea when the actual moving to suggests may happen?
@jangorecki It may happen relatively soon. I'm talking to CRAN and if they could forgive me for breaking (potentially a small number of) packages that didn't declare markdown in Suggests
, I'll move markdown to Suggests
in knitr in the next CRAN release. There might be some BioC packages that will be affected, and I'm not sure how many will still have this problem after six months.
Is there a way to get around this issue? This error is preventing us from submitting two packages to Bioconductor...
@jangorecki I talked to CRAN maintainers last month and about 470 CRAN packages will be broken if I moved markdown to Suggests
. This is not a trivial number, and I definitely don't want these packages to be killed just because of this minor issue. The next step for me is to write to these package maintainers and see how many would be willing to make this small change and update their packages on CRAN.
With knitr 1.30 (the current latest version on CRAN), the message should tell you to add
rmarkdown
instead ofmarkdown
to Suggests when the vignette engineknitr::rmarkdown
is used.
@yihui Looks like I am still getting the markdown
error even though I already have rmarkdown
in DESCRIPTION file: https://github.com/terrytangyuan/autoplotly/blob/d5bfe555fe6261964f2e79512f73cf215e142dff/DESCRIPTION#L30
The CI is using knitr==1.32
. Please see the build log for more details: https://travis-ci.org/github/terrytangyuan/autoplotly/builds/767305693
Is there anything I missed?
@terrytangyuan I am also seeing something similar. I use rmarkdown
in my vignette, and have rmarkdown
in suggest, but the error persists:
The 'markdown' package should be declared as a dependency of the 'loadr' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package. Please see #1864 for more information.
If I add markdown
instead of rmarkdown
in the Suggests, the error disappears.
> sessionInfo()
R Under development (unstable) (2021-04-08 r80148)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.2 LTS
Matrix products: default
BLAS/LAPACK: /usr/lib/x86_64-linux-gnu/openblas-pthread/libopenblasp-r0.3.8.so
locale:
[1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C
[3] LC_TIME=en_US.UTF-8 LC_COLLATE=en_US.UTF-8
[5] LC_MONETARY=en_US.UTF-8 LC_MESSAGES=C
[7] LC_PAPER=en_US.UTF-8 LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C
attached base packages:
[1] stats graphics grDevices utils datasets methods base
other attached packages:
[1] loadr_0.1.3 testthat_3.0.2
loaded via a namespace (and not attached):
[1] Rcpp_1.0.6 compiler_4.1.0 pillar_1.5.1 prettyunits_1.1.1
[5] remotes_2.3.0 tools_4.1.0 digest_0.6.27 pkgbuild_1.2.0
[9] pkgload_1.2.1 memoise_2.0.0 lifecycle_1.0.0 tibble_3.1.0
[13] pkgconfig_2.0.3 rlang_0.4.10 cli_2.4.0 rstudioapi_0.13
[17] xopen_1.0.0 xfun_0.22 fastmap_1.1.0 withr_2.4.1
[21] stringr_1.4.0 roxygen2_7.1.1 knitr_1.32 xml2_1.3.2
[25] desc_1.3.0 fs_1.5.0 vctrs_0.3.7 devtools_2.4.0
[29] rprojroot_2.0.2 glue_1.4.2 R6_2.5.0 processx_3.5.1
[33] fansi_0.4.2 rcmdcheck_1.3.3 sessioninfo_1.1.1 callr_3.6.0
[37] purrr_0.3.4 magrittr_2.0.1 ps_1.6.0 ellipsis_0.3.1
[41] usethis_2.0.1 utf8_1.2.1 stringi_1.5.3 cachem_1.0.4
[45] crayon_1.4.1
Follow-up: The above strange error is the behavior if you do not have rmarkdown
installed. If you install.packages("rmarkdown")
, then it works as expected.
The error messages are a bit confusing if you lack rmarkdown
, I presume because then knitr
falls back to markdown
, and then yields the error suggesting to add markdown
to suggests, instead of rmarkdown
.
I think instead, the error message should reflect what is in the vignette builder and not depend on the package available in the current session.
Thanks but in my case the CI already installed rmarkdown==2.7
and still failed with that error. I also installed it explicitly here and got the same result (link to build).
The error messages are a bit confusing if you lack
rmarkdown
, I presume because thenknitr
falls back tomarkdown
, and then yields the error suggesting to addmarkdown
to suggests, instead ofrmarkdown
.I think instead, the error message should reflect what is in the vignette builder and not depend on the package available in the current session.
@nsheff That's true, and also what I intended to do in the future:
Line 96 in 3f89d2a
I thought it would be unlikely that package authors don't have rmarkdown installed when they have R Markdown vignettes. Anyway, I agree I should clarify it in the error message that rmarkdown must be installed in order to build R Markdown vignettes. Thanks!
I thought it would be unlikely that package authors don't have rmarkdown installed when they have R Markdown vignettes
Indeed; in this case I wanted to start fresh, with the latest dev version of R, and so didn't have any packages pre-populated. But most of the time you're probably right. Still, it could bite people who took the route I did.
@terrytangyuan You can ignore everything in this thread and just do exactly what the error message told you to do. That is, add markdown
to Suggests
(you still got the error because you added rmarkdown
instead of markdown
).
After you did that (terrytangyuan/autoplotly@614ba38), your CI passed: https://travis-ci.org/github/terrytangyuan/autoplotly/builds/767311497.
@terrytangyuan Just to reiterate, your vignette does not appear to be using rmarkdown, so you should put markdown
in the Suggests
. I'm in a different situation since I'm using rmarkdown
in my vignette, see: https://github.com/databio/loadr/blob/0124ef5548801bc5d82469313791045ca308d9f3/vignettes/loadrIntro.Rmd#L6
Yep got it working now. Thanks!
Gosh, this change in knitr 1.32 breaks 333 253 Bioconductor packages in release and 340 255 in devel!!
- https://bioconductor.org/checkResults/3.12/bioc-LATEST/
- https://bioconductor.org/checkResults/3.13/bioc-LATEST/
Both markdown and rmarkdown are installed on the BioC builders so there's no reason for knitr to not just go ahead and process the vignettes. As a general rule, if a package doesn't declare a dependency it's their problem and I don't think it should be knitr's business. Why not just move markdown and/or rmarkdown to Suggests in knitr and let things happen naturally? People will start declaring markdown or rmarkdown as a dependency of their package when it starts failing on a build machine where these packages are not installed.
@hpages First of all, my apologies for the hassle!
The error is not completely new---it was first introduced on June 21 last year, but it was signaled only during the CRAN incoming check, i.e. R CMD check
with _R_CHECK_CRAN_INCOMING_REMOTE_=true
. It seems BioC doesn't use this variable, so no one was aware of this upcoming change in knitr.
What I did in the last CRAN release of knitr was that I raised the error earlier, i.e., in R CMD build
instead of R CMD check
. For CRAN packages, this means package authors will only see the error locally when they build the source package, but not on CRAN. This was to minimize the impact of this change, but I didn't know that BioC runs R CMD build
before R CMD check
. I'm not familiar with the BioC package submission or build process.
Trust me that I hate disturbing package authors like this. I didn't set a deadline for this issue and was okay with waiting for a couple of years. However, after a few email exchanges with CRAN, it became clear that my previous gentle warning conditional on _R_CHECK_CRAN_INCOMING_REMOTE_=true
was troublesome to CRAN, so I changed the strategy to avoid disturbing CRAN maintainers, but unfortunately it affected BioC.
Both markdown and rmarkdown are installed on the BioC builders so there's no reason for knitr to not just go ahead and process the vignettes.
There is a reason... If you don't declare the (soft) dependency, these packages won't be available during R CMD check --as-cran
, even if they are installed (because _R_CHECK_SUGGESTS_ONLY_=true
). Perhaps BioC uses a different setting for checking packages.
Why not just move markdown and/or rmarkdown to Suggests in knitr and let things happen naturally?
I've wished I could do that for several years, but that means about 450 CRAN packages would fail on CRAN, and CRAN usually gives only about two weeks for authors to fix check issues (although in this case, they allowed for about one month). Again, I didn't want to push package authors to fix this problem in a hurry, so I've been trying to hide the problem (with a fallback mechanism) from R CMD check
for years.
If there is a way to detect if R CMD build
or check
is running on BioC (e.g., an env var), I'll be more than happy to use it to "let things happen naturally."
Apologies again, and thanks for your understanding!
testthat::skip_on_bioc()
uses Sys.getenv("BBS_HOME")
to detect if R is running on a BioC build machine. But...
There is a reason... If you don't declare the (soft) dependency, these packages won't be available during R CMD check --as-cran, even if they are installed (because R_CHECK_SUGGESTS_ONLY=true)
I understand but this is a particular setup, not the default. The default is to use R CMD check
(without --as-cran
) and to not have any _R_CHECK_*_
environment variable defined. This default setup also happens to be the Bioconductor setup. Ideally you'd want to let things happen naturally by default and enable the new check specifically for the CRAN setup, rather than enabling the new check by default and disabling it specifically on the Bioconductor build machines.
Thanks!
Okay. Thanks for the info! I'll probably make a patch release in the next few days to bypass the check for dependency during R CMD build
on BioC.
Ideally you'd want to let things happen naturally by default and enable the new check specifically for the CRAN setup, rather than enabling the new check by default and disabling it specifically on the Bioconductor build machines.
Ideally, yes, I hate using hacks as much as everyone else does. All I've been trying to achieve is to send a reminder to package authors as gently as possible, so they can declare the new dependency without having CRAN chasing after them. I thought R CMD build
would be a good place to do this because only they would see the error by themselves, but I didn't realize BioC has a different process.
Ok, thanks a lot for helping us with this! Hopefully your patch can make it to CRAN soon. Sorry for being pushy but our next release is next month and we're going to freeze the current release (BioC 3.12) and stop the BioC 3.12 daily builds very soon (in a couple of weeks). Really bad timing.
@yihui We've just installed knitr 1.32.4 on all our build machines. Let's see what happens on the build report tomorrow. Fingers crossed. Thanks again for helping us with this.
Let me check and make sure that the knitr issue is gone. Today's report was posted just before I joined a meeting and I didn't have time to look at it yet. I'll get back to you in a few minutes.
No more "r/markdown package should be declared as a dependency" error on the report but now I see this one:
It seems you should call rmarkdown::render() instead of knitr::knit2html() because overview.Rmd appears to be an R Markdown v2 document.
It's breaking R CMD build
on 60+ packages.
Correction: it's breaking 35 software packages in BioC 3.13. e.g. https://bioconductor.org/checkResults/3.13/bioc-LATEST/AWFisher/nebbiolo1-buildsrc.html
Okay. I just addressed this problem in the dev version of knitr. You may install it and let's see if the dust settles tomorrow.
Thanks. Today's builds have started already so it's too late for going on the build machines to update knitr. We'll do this tomorrow (Thursday) and wait until Friday morning to see what happens on the build report.
Alright, latest knitr is now on all the Bioconductor build machines. We're ready to go!
Hi,
I have a package vignette that uses the engine knitr::rmarkdown
, and in my DESCRIPTION file I have declared:
Suggests: rmarkdown
VignetteBuilder: knitr
I believe that this structure follows the recommendation in the initial post for vignettes that use knitr::rmarkdown
. My issue is that certain users who do not have rmarkdown installed are getting the following error when they try to install the package (Similar to the error in the initial post):
--- re-building 'doc.Rmd' using rmarkdown
Error: processing vignette 'doc.Rmd' failed with diagnostics:
The 'markdown' package should be declared as a dependency of the 'PKG' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package. Please see https://github.com/yihui/knitr/issues/1864 for more information.
--- failed re-building 'doc.Rmd'
SUMMARY: processing the following file failed:
'doc.Rmd'
Error: Vignette re-building failed.
Execution halted
If the user installs rmarkdown, the error goes away and the package installs fine.
I am confused why the message says that I should add markdown to the 'Suggests' field when rmarkdown is already there and my vignette engine is knitr::rmarkdown
. It also isn't exactly a helpful error message for the user to see when the solution is simply installing rmarkdown. Is there a better solution such as moving rmarkdown to the 'Depends' field?
I am confused why the message says that I should add markdown to the 'Suggests' field when rmarkdown is already there and my vignette engine is knitr::rmarkdown. It also isn't exactly a helpful error message for the user to see when the solution is simply installing rmarkdown. Is there a better solution such as moving rmarkdown to the 'Depends' field?
@yangjasp this is exactly the issue I raised earlier in this thread, see above, and @yihui's response:
The error messages are a bit confusing if you lack
rmarkdown
, I presume because thenknitr
falls back tomarkdown
, and then yields the error suggesting to addmarkdown
to suggests, instead ofrmarkdown
.
I think instead, the error message should reflect what is in the vignette builder and not depend on the package available in the current session.@nsheff That's true, and also what I intended to do in the future:
Line 96 in 3f89d2a
I thought it would be unlikely that package authors don't have rmarkdown installed when they have R Markdown vignettes. Anyway, I agree I should clarify it in the error message that rmarkdown must be installed in order to build R Markdown vignettes. Thanks!
Hi @yihui,
Today's Bioconductor build report is out: https://bioconductor.org/checkResults/3.13/bioc-LATEST/
I don't see any knitr-related error, only this warning: https://bioconductor.org/checkResults/3.13/bioc-LATEST/HCAExplorer/nebbiolo1-buildsrc.html
(and this package is deprecated anyways)
So we're good! :-)
How long do you think it's going to take to get the latest knitr on CRAN?
Thanks!
@nsheff Thanks for the response! I must have missed that detail, but in my case the 'Suggests' field DOES contain Suggests: rmarkdown
. My understanding was that the problem for you and @mschubert was that you needed to add rmarkdown to 'Suggests', and doing so solved your issue.
My R CMD check runs fine, but the issue is coming up when users who do not have rmarkdown installed try to build the vignettes. Their errors are telling me that I need to add markdown (or rmarkdown, based on this comment) to my suggests when rmarkdown is already there, and the real issue is that they just need to install rmarkdown themselves. Is there anything I can do about this on my end?
How long do you think it's going to take to get the latest knitr on CRAN?
@hpages Hopefully in the next two days. I still have a Solaris issue reported by CRAN that needs to be addressed (but as usual, not straightforward to reproduce, unfortunately).
the real issue is that they just need to install rmarkdown themselves. Is there anything I can do about this on my end?
@yangjasp Nothing else needs to be done on your end. The error message will be clearer in the next version of knitr that they need to install rmarkdown.
I still have a Solaris issue reported by CRAN ... but as usual, not straightforward to reproduce
C'mon, everybody should have a spare Solaris box at home to test their package! /s
Thanks for the response! I must have missed that detail, but in my case the 'Suggests' field DOES contain Suggests: rmarkdown
@yangjasp That was an earlier comment, not the one I was quoting. I'm referring to the comment where I say, exactly what you said, I had rmarkdown in suggests and... Just look a bit further down in the thread and you'll find this issue you raise, and the solution yihui had proposed... Anyway, it doesn't matter, just wait for the fixed message like yihui said, that's all I was trying to say. You can scroll up and read those other posts if you want more detail about why it works that way.
Wow, that was fast! Excellent. Thanks again for all the help.
Note that you can also get the misleading error about markdown
if you are having trouble with your Pandoc version, even if you've correctly specified knitr
as your builder and put rmarkdown
in SUGGESTS. If rmarkdown
can't find a recent enough version of Pandoc it will automatically fall back to markdown
which will trigger the error.
* creating vignettes ... ERROR
Error: --- re-building ‘curve-creation.Rmd’ using rmarkdown
Warning in engine$weave(file, quiet = quiet, encoding = enc) :
Pandoc (>= 1.12.3) not available. Falling back to R Markdown v1.
Error: Error: processing vignette 'curve-creation.Rmd' failed with diagnostics:
The 'markdown' package should be installed and declared as a dependency of the 'culvrt' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package. Please see https://github.com/yihui/knitr/issues/1864 for more information.
In my case I was using github actions to run R CMD CHECK, and it threw this error because I had not added uses: r-lib/actions/setup-pandoc@v1
to my YAML.
@mkoohafkan I just made the error more accurate in case Pandoc is not installed. Thanks!
I have found a possibly related issue and would like to confirm this before opening a new one.
The Description
file of the febr package Imports
knitr and Suggests
rmarkdown because they are used to build the vignette. When running CRAN checks, the following NOTE is issued: Including not using Suggests conditionally
.
For example https://win-builder.r-project.org/X079z5f4WZ1T/00check.log
I could not find a solution for using rmarkdown conditionally. If you think that this is not a related issue, I can open a new one.
The package source code is available on https://github.com/samuel-rosa/febr-package/
@samuel-rosa I don't think the NOTE was from the current version of your package, but from the archived version instead.
* checking CRAN incoming feasibility ... NOTE
Maintainer: 'Alessandro Samuel-Rosa <alessandrosamuelrosa@gmail.com>'
New submission
Package was archived on CRAN
CRAN repository db overrides:
X-CRAN-Comment: Archived on 2020-10-24 as problems were not corrected
in time.
Including not using Suggests conditionally.
That is, "not using Suggests conditionally" was the reason (or one of the reasons) why your previous version of package was archived on CRAN. As long as you have addressed this problem, and R CMD check --as-cran
doesn't reveal new problems, I think you are good to go.
@yihui it seems that this is another CRAN message that needs improvement. :-| Many thanks!!!
Hi @yihui ,
Now that the dust of the last Bioconductor release has settled down, we would like to be able to control this behavior on the Bioconductor build machines. Would you be amenable to introduce a dedicated environment variable for this? E.g. something like:
diff --git a/R/utils.R b/R/utils.R
index 515f9638..c1e7b907 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -1062,7 +1062,7 @@ test_desc_dep = function(pkg, dir = '.') {
# TODO: remove this hack in the future when no CRAN/BioC packages have the issue
test_vig_dep = function(pkg) {
- if (skip_cran_bioc() || test_desc_dep(pkg, '..')) return()
+ if (no_test_vig_dep() || test_desc_dep(pkg, '..')) return()
p = read.dcf(file.path('..', 'DESCRIPTION'), fields = 'Package')[1, 1]
stop2(
"The '", pkg, "' package should be installed and declared as a dependency of the '", p,
@@ -1072,4 +1072,8 @@ test_vig_dep = function(pkg) {
)
}
-skip_cran_bioc = function() is_R_CMD_check() || Sys.getenv('BBS_HOME') != ''
+no_test_vig_dep = function() is_R_CMD_check() || tolower(Sys.getenv("KNITR_NO_TEST_VIGNETTE_DEP", "true")) == "true"
I can turn this into a PR if you want. Let me know.
Thanks!
H.
@hpages Yes, I'll be more than happy to merge the PR, or just push the commit myself since it is simple enough. Should Sys.getenv("KNITR_NO_TEST_VIGNETTE_DEP", "true")
be just Sys.getenv("KNITR_NO_TEST_VIGNETTE_DEP")
? It seems you want to set KNITR_NO_TEST_VIGNETTE_DEP
to true on Bioconductor to skip the test.
Should Sys.getenv("KNITR_NO_TEST_VIGNETTE_DEP", "true") be just Sys.getenv("KNITR_NO_TEST_VIGNETTE_DEP")?
Oops! Yes, absolutely. This should not be true by default. Thanks for the catch.
Okay. Just to make sure, this env var has already been set on Bioconductor, right? (meaning I no longer need to test Sys.getenv('BBS_HOME') != ''
in future versions of knitr)
Correct. You no longer need to test Sys.getenv('BBS_HOME') != ''
in future versions of knitr. Using BBS_HOME
to control whether to skip the vignette dep test or not is not flexible and makes reproducibility difficult. Using a dedicated env var like KNITR_NO_TEST_VIGNETTE_DEP
addresses that. Thanks!
That makes perfect sense. I just pushed the commit. Thanks!
@yihui This change, surprisingly, caused an error in my shiny app. The fix is straightforward enough but now CRAN will not accept the new package version due to an issue in the vignette. Or rather, it seems there is an issue with how CRAN windows builds parse URLs for CSS in Rmarkdown. The URL for the CSS file in the header shown below clearly has two /
s. However, when processed by CRAN on windows, one of those /
s is magicked away (see message and cran-check link below). I have sent a few emails to CRAN but I'm not making any progress. Students using my app at different schools are getting errors so there is some urgency to get the new version on CRAN.
Do you have any suggestions on how I could change the url specification in the vignette header below to pass the CRAN windows builds?
pandoc.exe: Could not fetch http:/netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css
See also: https://www.r-project.org/nosvn/R.check/r-release-windows-ix86+x86_64/radiant-00check.html
---
title: Programming with Radiant
author: "Vincent R. Nijs, Rady School of Management (UCSD)"
date: "`r Sys.Date()`"
output:
rmarkdown::html_vignette:
css: https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.15.3/css/all.min.css
vignette: >
%\VignetteIndexEntry{Programming with Radiant}
%\VignetteEngine{knitr::rmarkdown}
\usepackage[utf8]{inputenc}
---
@vnijs I don't know where the URL http:/netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css
came from. This is the css
option you used in the current version of radiant (v1.3.2):
output:
rmarkdown::html_vignette:
css: http://netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css
I don't know if the change to https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.15.3/css/all.min.css
will help, since I don't know the Pandoc version CRAN uses. I guess the best you could do for now is to see if the problem is reproducible on win-builder (try both v1.3.2 and your current development version, and see if the former throws the same error and the latter passes).
@vnijs this particular "could not fetch" issue is a bug in rmarkdown that we unfortunately missed in last release in our tests, and in reverse dependencies checks. I'll fix that today (rstudio/rmarkdown#2163)
I am no expert in R and I got the following error. How do I add the markdown to suggest?
devtools::install_github("kenhanscombe/ukbtools", build_vignettes = TRUE, dependencies = TRUE)
Downloading GitHub repo kenhanscombe/ukbtools@HEAD
✔ checking for file ‘/tmp/RtmprNPa9Z/remotes73b52d1262aa/kenhanscombe-ukbtools-4015440/DESCRIPTION’ ...
─ preparing ‘ukbtools’:
✔ checking DESCRIPTION meta-information ...
─ installing the package to process help pages
E creating vignettes (4.5s)
--- re-building ‘explore-ukb-data.Rmd’ using rmarkdown
Warning in engine$weave(file, quiet = quiet, encoding = enc) :
Pandoc (>= 1.12.3) not available. Falling back to R Markdown v1.
Error: processing vignette 'explore-ukb-data.Rmd' failed with diagnostics:
The 'markdown' package should be installed and declared as a dependency of the 'ukbtools' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package. Please see #1864 for more information.
--- failed re-building ‘explore-ukb-data.Rmd’
SUMMARY: processing the following file failed:
‘explore-ukb-data.Rmd’
Error: Vignette re-building failed.
Execution halted
Error: Failed to install 'ukbtools' from GitHub:
System command 'R' failed, exit status: 1, stdout + stderr (last 10 lines):
E> Pandoc (>= 1.12.3) not available. Falling back to R Markdown v1.
E> Error: processing vignette 'explore-ukb-data.Rmd' failed with diagnostics:
E> The 'markdown' package should be installed and declared as a dependency of the 'ukbtools' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package. Please see #1864 for more information.
E> --- failed re-building ‘explore-ukb-data.Rmd’
E>
E> SUMMARY: processing the following file failed:
E> ‘explore-ukb-data.Rmd’
E>
E> Error: Vignette re-building failed.
E> Execution halted
@jameskozubek If you do not install the package inside RStudio, you have to install Pandoc: https://pandoc.org/installing.html Your ran into the error because Pandoc was not available:
Warning in engine$weave(file, quiet = quiet, encoding = enc) :
Pandoc (>= 1.12.3) not available. Falling back to R Markdown v1.
The error could be circumvented by adding markdown
to Suggests
in DESCRIPTION
, but I don't recommend that. BTW, it seems you are not the package author of ukbtools, so it will not be straightforward for you to do that.
Thank you. Installing pandoc worked.
The error could be circumvented by adding
markdown
toSuggests
inDESCRIPTION
, but I don't recommend that.
Hi Yihui, can you please elaborate on why adding markdown to suggests is not a good idea if using knitr::rmarkdown. A colleague who tested this for me gets the install error telling them about adding rmarkdown to Suggests, but I already have it there. He is not using RStudio but native R, and so maybe this is a pandoc issue? Can you recommend the best way to handle this?
My package repository is aharmer/morphoBlocks.
Thanks in advanced.
Aaron.
why adding markdown to suggests is not a good idea if using knitr::rmarkdown.
@aharmer If you use the vignette engine knitr::rmarkdown
, you mean to use Pandoc to build your vignettes. Adding markdown to Suggests means to use the markdown package to build vignettes, which is a workaround. Since markdown is not Pandoc and doesn't cover Pandoc's Markdown features, you could end up with an unexpected vignette.
He is not using RStudio but native R, and so maybe this is a pandoc issue? Can you recommend the best way to handle this?
He needs to install Pandoc if he wants to build the package from source with vignettes. Your instructions are perfectly accurate: https://github.com/aharmer/morphoBlocks#installation
Thanks so much @yihui, this really clarifies things for me about the difference between the two packages. In the end it turned out the issue was I didn't include dependencies = TRUE
in the install_github
instructions on my README. I wrongly assumed specifying build_vignettes = TRUE
would install the Suggests.
Thanks again,
Aaron.
@yihui We just installed the latest version of R on the Bioconductor builders end of last week and installed knitr version 1.33.8 and the ERROR no longer occurs for packages that are using knitr but do not have rmarkdown/markdown in Suggest. It seems like you have decided not to throw this ERROR anymore or there is a bug in your code?
@lshep Bioconductor is a different story. This issue only affects R CMD check --as-cran
. As @hpages replied above, Bioc doesn't set the env var _R_CHECK_SUGGESTS_ONLY_
, so it's not affected by this R CMD check
issue (it was affected once but the problem has been fixed in knitr long ago).
@yihui this may have been the wrong issue to post on but what I'm concerned with it how to reproduce the ERROR we were seeing on the Bioconductor build report since it has disappeared with knitr1.33.8? As this seemed to be a real ERROR that you wanted corrected and we were able to see the ERROR with knitr 1.33.3. My understanding was the work around was a temporary fix since the original updated was right before the Bioconductor release and breaking numerous packages; now we wanted the ERROR to appear so maintainers would correct?
@lshep Looking at the changes from 1.33.3 to 1.33.8 (ae6ab6e...fad3ead), I can't think of a possible reason why the error could have disappeared in 1.33.8. The only possibly relevant commit is 93720d3, which was @hpages's suggestion from #1864 (comment), but that shouldn't make the error disappear. I don't know why 1.33.3 could cause the error, either...
@yihui Correction: we were using knitr 1.33.5 on our build machines, which was raising the R CMD build
error if either markdown or rmarkdown was not declared as a dep. With the latest knitr (1.33.8), we only see the error if markdown is not declared as a dep but we don't see it anymore for rmarkdown. The change is in commit c56fd61 from Aug 25.
So for example, a package like RMassBank (which uses rmarkdown to build its vignette) was getting an R CMD build
error with knitr 1.33.5 but not anymore with knitr 1.33.8. Is this intended?
We've contacted many developers to let them know about these errors and ask them to fix their package. Now they're getting back to us telling us they don't know what we're talking about because they don't see the error on our daily build report. What should we tell them? Thanks
@hpages I'm confused now. Neither v1.33.5 nor v1.33.8 should cause an error on Bioc, because both markdown and rmarkdown are available on Bioc. If I understood you correctly last time (93720d3), you have set KNITR_NO_TEST_VIGNETTE_DEP=true
on Bioc, right? With that, no Bioc packages should need to do anything, and R CMD build
should not throw an error.
That said, I just committed 0dcd462, which completely removed the possibility of build error as long as markdown is available (which is true on Bioc).
@yihui We only use KNITR_NO_TEST_VIGNETTE_DEP=true
for our release builds (BioC 3.13). We don't set this variable for our devel builds (BioC 3.14) because we want BioC developers to fix whatever error knitr detects in the devel version of their package.
I'm confused now. Neither v1.33.5 nor v1.33.8 should cause an error on Bioc, because both markdown and rmarkdown are available on Bioc.
Both markdown and rmarkdown have always been available on our build machines and yet we were getting these R CMD build
errors. The error message was:
The 'markdown' package should be installed and declared as a dependency of the 'foo' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package.
The message complains that markdown should be declared as a dependency. AFAIK the fact that a package needs to declare another package as a dependency has nothing to do with what's installed on the system where the package is being tested. So if knitr considers it is a mistake that markdown or rmarkdow are not listed as a dependency, this is a fact that remains true whether these packages are installed or not.
With knitr v1.33.9, R CMD build RMassBank
still doesn't give the error we used to get with knitr 1.33.5, despite the fact that RMassBank still uses rmarkdown to build its vignette but does not list it as a dep.
With knitr v1.33.9,
R CMD build RMassBank
still doesn't give the error we used to get with knitr 1.33.5, despite the fact that RMassBank still uses rmarkdown to build its vignette but does not list it as a dep.
Okay, that's the (new) behavior that I expect on Bioc now. Once CRAN submission is re-opened, I'll try to submit a new version of knitr to CRAN. Thanks!
Okay, that's the (new) behavior that I expect on Bioc now.
Focusing on the behavior on Bioc is not that helpful. When Bioc developers run R CMD build
on their package, they don't do it on the Bioconductor build machines. They do it on their own machine and with a stock knitr installed from CRAN (knitr 1.33). This version of knitr gives them an error about markdown or rmarkdown not being in Suggests
. This has been the case for months now (since knitr 1.33 was released on CRAN), and during all these months we've been using a setup on our devel build machines that was consistent with what people get on their own machine.
Even though this new check by knitr came as a surprise in April and broke hundreds of Bioconductor packages a few days before the BioC 3.13 release (see #1864 (comment)), we embraced it in BioC 3.14 and have been working all these months with our developers to help them fix their package.
But now it seems that you've changed your mind. Are you saying that knitr will no longer consider that it's an error when a package uses markdown or rmarkdown to build its vignette and doesn't depend on it? Just want to make sure so we can clarify the situation with our developers. Thanks!
Are you saying that knitr will no longer consider that it's an error when a package uses markdown or rmarkdown to build its vignette and doesn't depend on it?
Yes. I'm following your original suggestion in #1864 (comment), i.e., "let things happen naturally." I will no longer throw an error on my side. If the build/check platform (e.g., CRAN) thinks it is an error not to declare the dependency, I'll just let the platform throw the error. The previous error on my side was meant to serve as a precaution and avoid future breakages on CRAN.
Now that I have gotten the permission from CRAN (and was even strongly encouraged by them) that it's time to break packages that still haven't made the correction, I'll just let them break, although personally I really hope to give them more time.
Thanks for clarifying. I know, it was my suggestion to let things happen naturally. More generally I don't think a package should meddle too much in what its reverse deps do with their Suggests
field. We just need to know where you are going so we can follow, anticipate, and avoid miscommunication/confusion with our developers. Thanks again.
For the record, I continue getting this message, despite both rmarkdown and markdown being declared in Suggests or Imports of my package, as well as having both of them installed in the environment.
@stefanoborini If you get the message even if you declare them in Imports, then there must be something else wrong. Chances are your .libPaths()
is not properly configured on your computer and you installed rmarkdown/markdown to a custom library path that R is not able to find during R CMD build
or R CMD check
. You may try to configure R_LIBS_USER
in ~/.Renviron
.
@yihui I use .Rprofile
to extend the libPaths indeed, but then I don't understand why devtools::build
works and R CMD build
does not. Why would they behave differently? Don't they work exactly in the same way?
@stefanoborini I don't know why (I'm not the author of either of them), but my experience is that R commands (such as R CMD build
or check
) works better with .Renviron
in general. If you want to set a custom library path, I recommend that you set it via R_LIBS_USER
in .Renviron
instead.
This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.