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

Move the markdown package from Imports to Suggests #1864

Closed
yihui opened this issue Jun 21, 2020 · 86 comments · Fixed by #2020
Closed

Move the markdown package from Imports to Suggests #1864

yihui opened this issue Jun 21, 2020 · 86 comments · Fixed by #2020
Labels
feature Feature requests next Issues/PRs considered for the next release

Comments

@yihui
Copy link
Owner

yihui commented Jun 21, 2020

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!

yihui added a commit that referenced this issue Jun 21, 2020
… package containing vignettes based on the knitr::knitr engine
@tdhock
Copy link

tdhock commented Jun 24, 2020

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?

@yihui
Copy link
Owner Author

yihui commented Jun 24, 2020

Yes, you have to.

@tdhock
Copy link

tdhock commented Jun 24, 2020

ok

@mschubert
Copy link

mschubert commented Jun 27, 2020

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.

@yihui
Copy link
Owner Author

yihui commented Jun 27, 2020

@mschubert I need a reproducible example. Could you provide a link to your package (presumably on Github?)? Thanks!

@mschubert
Copy link

mschubert commented Jun 27, 2020

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.

@yihui
Copy link
Owner Author

yihui commented Jun 27, 2020

@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.

@mschubert
Copy link

mschubert commented Jun 27, 2020

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).

@yihui
Copy link
Owner Author

yihui commented Jun 27, 2020

@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!

kaz-yos added a commit to kaz-yos/tableone that referenced this issue Jul 13, 2020
Experienced the following in winbuild

* checking re-building of vignette outputs ... [1s] WARNING
Error(s) in re-building vignettes:
--- re-building 'introduction.Rmd' using rmarkdown
Warning in engine$weave(file, quiet = quiet, encoding = enc) :
  The vignette engine knitr::rmarkdown is not available, because the rmarkdown package is not installed. Please install it.
  Error: processing vignette 'introduction.Rmd' failed with diagnostics:
  The 'markdown' package should be declared as a dependency of the 'tableone' package (e.g., in the  'Suggests' field of DESCRIPTION), because it contains vignette(s) built with the 'markdown' package. Please see yihui/knitr#1864 for more information.
  --- failed re-building 'introduction.Rmd'

--- re-building 'smd.Rmd' using rmarkdown
Warning in engine$weave(file, quiet = quiet, encoding = enc) :
  The vignette engine knitr::rmarkdown is not available, because the rmarkdown package is not installed. Please install it.
  Error: processing vignette 'smd.Rmd' failed with diagnostics:
  The 'markdown' package should be declared as a dependency of the 'tableone' package (e.g., in the  'Suggests' field of DESCRIPTION), because it contains vignette(s) built with the 'markdown' package. Please see yihui/knitr#1864 for more information.
  --- failed re-building 'smd.Rmd'

SUMMARY: processing the following files failed:
  'introduction.Rmd' 'smd.Rmd'

Error: Vignette re-building failed.
@nsheff
Copy link

nsheff commented Jul 14, 2020

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).

codycarroll added a commit to functionaldata/tPACE that referenced this issue Jul 28, 2020
yihui added a commit that referenced this issue Sep 22, 2020
…the vignette engine knitr::rmarkdown is used (#1864)
mikejohnson51 added a commit to mikejohnson51/dataRetrieval that referenced this issue Sep 22, 2020
* creating vignettes ... ERROR

--- re-building 'dataRetrieval.Rmd' using rmarkdown

Warning in engine$weave(file, quiet = quiet, encoding = enc) :

  The vignette engine knitr::rmarkdown is not available because the rmarkdown package is not available. Did you forget to add it to Suggests in DESCRIPTION? Please see yihui/knitr#1864 for more information.

Loading required namespace: sf

--- finished re-building 'dataRetrieval.Rmd'
@yihui
Copy link
Owner Author

yihui commented Sep 30, 2020

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.

jarioksa added a commit to vegandevs/vegan that referenced this issue Nov 26, 2020
@jangorecki
Copy link

Any idea when the actual moving to suggests may happen?

@yihui
Copy link
Owner Author

yihui commented Dec 2, 2020

@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.

@dnadave
Copy link

dnadave commented Dec 16, 2020

Is there a way to get around this issue? This error is preventing us from submitting two packages to Bioconductor...

@yihui
Copy link
Owner Author

yihui commented Dec 16, 2020

@dnadave Yes, please do what the error message told you to do.

alhauser added a commit to google/CausalImpact that referenced this issue Dec 21, 2020
bgctw added a commit to bgctw/logitnorm that referenced this issue Jan 10, 2021
@yihui
Copy link
Owner Author

yihui commented Jan 22, 2021

@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.

jonchang added a commit to jonchang/fishtree that referenced this issue Jan 26, 2021
C.f. yihui/knitr#1864

R currently looks for both pandoc and pandoc-citeproc, but the latter is
deprecated upstream.
@lshep
Copy link

lshep commented Aug 30, 2021

@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?

@yihui
Copy link
Owner Author

yihui commented Aug 30, 2021

@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...

@hpages
Copy link

hpages commented Aug 30, 2021

@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

yihui added a commit that referenced this issue Aug 30, 2021
…ON or throw an error (#1864)

this will break a number of CRAN packages, but is what CRAN  tells me to do
@yihui
Copy link
Owner Author

yihui commented Aug 30, 2021

@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).

@hpages
Copy link

hpages commented Aug 31, 2021

@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.

@yihui
Copy link
Owner Author

yihui commented Aug 31, 2021

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!

@hpages
Copy link

hpages commented Aug 31, 2021

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!

@yihui
Copy link
Owner Author

yihui commented Aug 31, 2021

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.

@hpages
Copy link

hpages commented Aug 31, 2021

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.

yihui added a commit that referenced this issue Sep 8, 2021
…but rmarkdown/markdown/Pandoc is not available (#1864)

this is like a trolley problem to me---I still can't accept the consequence that I'm going to kill 200 packages on CRAN if I throw an error when the package author didn't declare dependence on rmarkdown or markdown
clrpackages pushed a commit to clearlinux-pkgs/R-knitr that referenced this issue Sep 14, 2021
Andrew Heiss (1):
      fix #2038: add ability to pass arguments to dvisvgm through engine.opts (#2039)

Christophe Dervieux (3):
      fix #1993: ignore some values of `fig.keep` (e.g., fig.keep = 'last') when there is only one figure in a chunk (#1996)
      Throw an error if inline result cannot be coerced to character (#2007)
      `include_*`

David C Hall (1):
      Use `conditionMessage()` to render conditions (#2016)

Jamie Lentin (1):
      utils-vignettes: Don't disable vtangle if .Rout.save exists (#2018)

Kenneth Blake Vernon (1):
      fix #1935: make collapse and strip.white = TRUE independent (#2011)

Will Landau (1):
      Add a new targets engine (#2031)

Yihui Xie (19):
      start the next version
      stop() early on CI servers if Pandoc is not found before building R Markdown vignettes: yihui/knitr#1864 (comment)
      use the chunk option engine.opts$classoption to customize the class options of `standalone` for the tikz engine: yihui/knitr#1985 (comment)
      close #1992 (originally reported at #492): allow customizing the tilde symbol ~ with the package option latex.tilde
      add the attribute data=external="1" to <iframe> generated by include_url() so that Pandoc won't base64 encode the URL: https://stackoverflow.com/q/67477667/559676
      close #1997: remove the duplicate \format{} on the help page ?opts_chunk
      use a dedicated env var to decide whether to check DESCRIPTION: yihui/knitr#1864 (comment)
      fix rstudio/rmarkdown#2165: emit a message instead of a warning when building rmarkdown vignettes without pandoc
      remove the code that was for backward compatibility with a very old version of knitr
      remove is_abs_path from knitr, which is no longer used in bookdown >= v0.22 (rstudio/bookdown@0098721)
      signal an error when buiding rmarkdown vignettes but Pandoc is not available, unless the vignette is build during R CMD check (in which case a message is emitted instead)
      specify the package name in \link[]
      `write_bib()` only uses the first URL if multiple are found in a package (#2028)
      close #1864: move markdown to Suggests (#2020)
      no longer test the declaration of dependency on markdown in DESCRIPTION or throw an error (#1864)
      update URLs
      do not throw an error in R CMD check when checking a package on CRAN but rmarkdown/markdown/Pandoc is not available (#1864)
      respect the env var R_CRANDALF: yihui/crandalf@d5949e5
      CRAN release v1.34

christophe dervieux (1):
      re-document. Trailing whitespace are removed in code now.

knokknok (1):
      cache.globals = FALSE means detecting all variables in a code chunk (#1898)
richfitz added a commit to vimc/orderly that referenced this issue Sep 20, 2021
flyrain pushed a commit to flyrain/spark that referenced this issue Sep 21, 2021
…SparkR package

### What changes were proposed in this pull request?
Declare the markdown package as a dependency of the SparkR package

### Why are the changes needed?
If we didn't install pandoc locally, running make-distribution.sh will fail with the following message:
```
— re-building ‘sparkr-vignettes.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 'sparkr-vignettes.Rmd' failed with diagnostics:
The 'markdown' package should be declared as a dependency of the 'SparkR' package (e.g., in the 'Suggests' field of DESCRIPTION), because the latter contains vignette(s) built with the 'markdown' package. Please see yihui/knitr#1864 for more information.
— failed re-building ‘sparkr-vignettes.Rmd’
```

### Does this PR introduce _any_ user-facing change?
Yes. Workaround for R packaging.

### How was this patch tested?
Manually test. After the fix, the command `sh dev/make-distribution.sh -Psparkr -Phadoop-2.7 -Phive -Phive-thriftserver -Pyarn` in the environment without pandoc will pass.

Closes apache#32270 from xuanyuanking/SPARK-35171.

Authored-by: Yuanjian Li <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 8e9e700)
Signed-off-by: Dongjoon Hyun <[email protected]>
urswilke added a commit to urswilke/pyramidi that referenced this issue Nov 2, 2021
jakubsob pushed a commit to Appsilon/semantic.dashboard that referenced this issue Nov 18, 2021
GregGuerin added a commit to ternaustralia/ausplotsR that referenced this issue Nov 23, 2021
Add markdown to suggests, along with knitr and rmarkdown following guidance in yihui/knitr#1864 and error message trying to build vignette in install_github
@stefanoborini
Copy link

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.

@yihui
Copy link
Owner Author

yihui commented Dec 7, 2021

@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.

@stefanoborini
Copy link

stefanoborini commented Dec 7, 2021

@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?

@yihui
Copy link
Owner Author

yihui commented Dec 7, 2021

@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.

miracum-bot pushed a commit to miracum/dqa-miracumdqa that referenced this issue May 9, 2022
@github-actions
Copy link

github-actions bot commented Jun 8, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Feature requests next Issues/PRs considered for the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.