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

CRAN check failure: package 'quarto' but has not declared a SystemRequirements #71

Open
jhelvy opened this issue Oct 6, 2023 · 7 comments · Fixed by #73
Open

CRAN check failure: package 'quarto' but has not declared a SystemRequirements #71

jhelvy opened this issue Oct 6, 2023 · 7 comments · Fixed by #73

Comments

@jhelvy
Copy link
Owner

jhelvy commented Oct 6, 2023

We got notified from CRAN that we have several check failures:

Please see the problems shown on https://cran.r-project.org/web/checks/check_results_renderthis.html.
Please correct before 2023-10-19 to safely retain your package on CRAN.

A follow up email suggests that it has to do with the Quarto installation on CRAN:

The new check failures are because CRAN was requested to install Quarto. Your package makes use of it via package 'quarto' but has not declared a SystemRequirements (and quarto passes its checks with or without quarto-cli).

@gadenbuie
Copy link
Collaborator

gadenbuie commented Oct 6, 2023

Your package makes use of it via package 'quarto' but has not declared a SystemRequirements (and quarto passes its checks with or without quarto-cli).

I'm pretty sure this is just a snide passing comment and not at all related to the problem. If we depend on the R package quarto and quarto declares its SystemRequirements, then we should be covered. For example: does every package that uses rmarkdown declare a system requirement of pandoc? No, the fact that rmarkdown does is sufficient.

So I'm reading these emails as broadly: now that quarto-cli is installed on CRAN machines our tests are not passing.

The good news/bad news is that I can reproduce the issue locally and after some digging it seems that our tests are failing legitimately. If I dig into to_html() and turn on debugging I can see that we're trying to render the slides.qmd to output in a temporary file, but quarto is throwing

ERROR: --output option cannot specify a relative or absolute path

Error: --output option cannot specify a relative or absolute path
    at parseRenderFlags (file:///Applications/quarto/bin/quarto.js:42392:31)
    at Command.fn (file:///Applications/quarto/bin/quarto.js:90845:25)
    at Command.execute (file:///Applications/quarto/bin/quarto.js:8437:24)
    at Command.parseCommand (file:///Applications/quarto/bin/quarto.js:8333:25)
    at async quarto (file:///Applications/quarto/bin/quarto.js:127540:5)
    at async file:///Applications/quarto/bin/quarto.js:127558:9

I figured out a fix and proposed a related issue in quarto-dev/quarto-r#126

@chainsawriot
Copy link

The package is now archived on CRAN. I checked with the current r-devel using Rocker and I cannot reproduce the same errors anymore. So I think these issues are now gone.

https://cran-archive.r-project.org/web/checks/2023/2023-10-22_check_results_renderthis.html

@jhelvy
Copy link
Owner Author

jhelvy commented Jul 10, 2024

I still see these errors. I don't know how to address this though because quarto_render() won't allow us to specify an output path. @gadenbuie any ideas? Perhaps we could disable quarto support for to_html() until this is supported by quarto_render()? We could through an error message if the user attempts to render a qmd to html, just saying that it is recommended to first render to html via quarto directly instead of {renderthis}. That will enable the package to get back on CRAN.

@jhelvy jhelvy reopened this Jul 10, 2024
@gadenbuie
Copy link
Collaborator

I still see these errors.

Are you seeing these errors locally? IIRC this was working in CI too after #73. Maybe we should kick off a new set of checks to see if the error has resurfaced.

@jhelvy
Copy link
Owner Author

jhelvy commented Jul 10, 2024

I still see these errors.

Are you seeing these errors locally? IIRC this was working in CI too after #73. Maybe we should kick off a new set of checks to see if the error has resurfaced.

When I run devtools::check() I get 15 test failures.

@gadenbuie
Copy link
Collaborator

Those are new! Looks like there have been changes in withr that have broken how we handle temporary file cleanup. Ironically, if renderthis had been on CRAN, withr would have seen our breakage with their reverse dependency checks

@jhelvy
Copy link
Owner Author

jhelvy commented Jul 10, 2024

I mean, I appreciate that CRAN makes people keep their stuff up to date, it feels like a good overall approach to make sure the whole ecosystem is working well. But things like this then fall through the cracks if the timing is off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants