Closed jimjam-slam closed 3 months ago
This is indeed probably due to #9890 . I believe the whole point was to support pre-render and post-render in extensions... so I am a bit surprised it was already working for you.
@cscheid Looking in debugging mode to your example, I noticed several things
We look for pre-render in project context, and it happens we find the extension script at this step already, with its full resolved path '_extensions/jimjam-slam/sverto/sverto-prerender.lua'
Then we look into extension https://github.com/quarto-dev/quarto-cli/blob/98b7fec8d3daf0c549d91785d4afa104eb899748/src/command/render/project.ts#L240-L250
and we find the script a second time but without resolving as an extension so we get sverto-prerender.lua
only.
So we get in the result of getProjectRenderScripts()
two pre-render script.
preRenderScripts
(2) ['_extensions/jimjam-slam/sverto/sverto-prerender.lua', 'sverto-prerender.lua']
0: '_extensions/jimjam-slam/sverto/sverto-prerender.lua'
1: 'sverto-prerender.lua'
length: 2
[[Prototype]]: Object
[[Prototype]]: Object
So it seems we are resolving twice the extensions. I see you added full path resolution at
https://github.com/quarto-dev/quarto-cli/blob/98b7fec8d3daf0c549d91785d4afa104eb899748/src/extension/extension.ts#L787-L803
So oddly, it does not get "cached" correctly in renderOptions?.services.extension
🤔
Though, about #9890 it seems we resolve project extension, with project context already https://github.com/quarto-dev/quarto-cli/blob/98b7fec8d3daf0c549d91785d4afa104eb899748/src/project/project-context.ts#L146-L151
and we merge extension contribution and project configuration https://github.com/quarto-dev/quarto-cli/blob/98b7fec8d3daf0c549d91785d4afa104eb899748/src/project/project-context.ts#L608-L612
I was curious so I tested your additional test at https://github.com/quarto-dev/quarto-cli/commit/853728e555046ef9d9e0501df1baad4cc906ba9b#diff-09e6a0dcf7cdc6dc00aa75a7608456aba19402c7849178958ec7cc840a1fd874 with quarto in a state before #9890 by reverting the merge commit (except test change) and it seems to pass.
So I don't know full context of #9889 but there is something to deal with.
Note that unlike https://github.com/quarto-dev/quarto-cli/issues/9889, Sverto provides a single string for the contributed prerender script, not a list:
Using this form somehow solve the rendering issue (no error)
project:
project:
type: website
pre-render:
- sverto-prerender.lua
But the pre-render script is run twice because the observation above still applies
Hope it helps, and happy to contribute if needed.
I believe the whole point was to support pre-render and post-render in extensions... so I am a bit surprised it was already working for you.
Interesting! I released the current major version of Sverto about three months ago, and it seemed fine up until the issue was reported this week 😅
We look for pre-render in project context, and it happens we find the extension script at this step already, with its full resolved path
'_extensions/jimjam-slam/sverto/sverto-prerender.lua'
Then we look into extension
and we find the script a second time but without resolving as an extension so we get
sverto-prerender.lua
only.
Ah, that explains why the script was running twice! I wasn't sure why that was happening when I finalised the major version but kind of wrote it off at the time 😂
Regarding using
pre-render: sverto-prerender.lua
vs
pre-render:
- sverto-prerender.lua
The glob resolving does expect an array it seems https://github.com/quarto-dev/quarto-cli/blob/98b7fec8d3daf0c549d91785d4afa104eb899748/src/extension/extension.ts#L792-L798
So this finds no file
resolvePathGlobs(
extensionDir,
preRender,
[],
)
{include: Array(0), exclude: Array(0)}
But using an array works
resolvePathGlobs(
extensionDir,
[preRender],
[],
)
{include: Array(1), exclude: Array(0)}
It seems like the preRender as string[]
does not convert and hide the issue.
Because passing a non array as a glob seems bad in our codebase https://github.com/quarto-dev/quarto-cli/blob/98b7fec8d3daf0c549d91785d4afa104eb899748/src/core/path.ts#L279-L288
for (const glob of globs)
will iterate over the letters of the string 😓
Maybe we should make sure that resolveGlobs
gets an array as globs
- it seems string[]
type does not handle this enough... 🤔
I was curious to understand 😅
Yes, this is a bug. Our schemas allow a single string, note maybeArrayOf
from https://github.com/quarto-dev/quarto-cli/blob/14f4e8bf4534a55cdff13fb56d39eacfabc6160e/src/resources/schema/project.yml#L39-L46
I'm trying to understand how to reconcile the behavior described on the original report with what I see on our test tests/docs/project/prepost/extension
. With 1.5.30, that test project doesn't invoke the pre-render script at all, independently of whether pre-render.ts
is in an array or not. On main
, the test project successfully invokes the pre-render script, but Quarto crashes when the pre-render field is a single string (that's a bug)
What I don't understand is how this also causes the project scripts to be executed twice.
I was curious so I tested your additional test at 853728e#diff-09e6a0dcf7cdc6dc00aa75a7608456aba19402c7849178958ec7cc840a1fd874 with quarto in a state before #9890 by reverting the merge commit (except test change) and it seems to pass.
I can't reproduce this - the file doesn't execute for me before the fix from 9890.
I went back to triple-check that I was indeed running 1.5.30 when I successfully rendered our test case. (Not to accuse you of not believing us, @cscheid! I just wanted to make absolutely sure I hadn't misled us all by running a different version than I thought I was running.)
I believe I am, but there is one anomaly in the quarto check
output. First I reverted to 1.5.30:
# in ~/code/quarto-cli
$ git checkout v1.5.30
$ ./configure.sh
Then I did quarto check
from the Quarto CLI directory above. It successfully reports the commit that's checked out, tied to v1.5.30:
$ quarto check
Check file:///Users/jimjamslam/code/quarto-cli/src/quarto.ts
Quarto 99.9.9
[✓] Checking versions of quarto binary dependencies...
Pandoc version 3.1.13: OK
Dart Sass version 1.70.0: OK
Deno version 1.41.0: OK
Typst version 0.11.0: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
Version: 99.9.9
commit: e9d5a9cb82098cae44588c182006b581bb6bf9b4
Path: /Users/jimjamslam/code/quarto-cli/package/dist/bin
Check file:///Users/jimjamslam/code/quarto-cli/src/resources/vendor/deno-land/x/puppeteer@9-0-2/mod.ts
[✓] Checking tools....................OK
TinyTeX: (not installed)
Chromium: (not installed)
[✓] Checking LaTeX....................OK
Tex: (not detected)
[✓] Checking basic markdown render....OK
[✓] Checking Python 3 installation....OK
Version: 3.9.7 (Conda)
Path: /Users/jimjamslam/miniforge3/bin/python
Jupyter: 4.9.2
Kernels: python3
Traceback (most recent call last):
File "/Users/jimjamslam/code/quarto-cli/src/resources/jupyter/jupyter.py", line 21, in <module>
from notebook import notebook_execute, RestartKernel
File "/Users/jimjamslam/Code/quarto-cli/src/resources/jupyter/notebook.py", line 15, in <module>
from yaml import safe_load as parse_string
ModuleNotFoundError: No module named 'yaml'
[✓] Checking Jupyter engine render....OK
ERROR: Error
at renderFiles (file:///Users/jimjamslam/code/quarto-cli/src/command/render/render-files.ts:350:23)
at eventLoopTick (ext:core/01_core.js:153:7)
at async render (file:///Users/jimjamslam/Code/quarto-cli/src/command/render/render-shared.ts:102:18)
at async checkJupyterRender (file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:320:18)
at async file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:280:11
at async withSpinner (file:///Users/jimjamslam/Code/quarto-cli/src/core/console.ts:81:12)
at async checkJupyterInstallation (file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:276:9)
at async check (file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:64:7)
at async Command.actionHandler (file:///Users/jimjamslam/Code/quarto-cli/src/command/check/cmd.ts:33:5)
at async Command.execute (file:///Users/jimjamslam/Code/quarto-cli/src/vendor/deno.land/x/cliffy@v1.0.0-rc.3/command/command.ts:1948:7)
Then I rendered the doc successfully and ran quarto check
afterward. It had the same output, except for the first line (it didn't specify the TS script at the start of the check), and instead of the commit number, it had an error because the folder isn't a git repo (although the subfolder with the bin in it wouldn't be):
$ quarto render
_extensions/jimjam-slam/sverto/sverto-prerender.lua
> svelte-app@1.0.0 build
> rollup -c ./_extensions/jimjam-slam/sverto/rollup.config.js --configQuartoOutPath=/Users/jimjamslam/Code/tests/test-sverto-2/_site --configSvelteInPaths=Circles.svelte:
Circles.svelte → _site...
created _site in 243ms
Sverto pre-render finished!
[1/3] index.qmd
[2/3] about.qmd
[3/3] example.qmd
Output created: _site/index.html
$ quarto check
Quarto 99.9.9
[✓] Checking versions of quarto binary dependencies...
Pandoc version 3.1.13: OK
Dart Sass version 1.70.0: OK
Deno version 1.41.0: OK
Typst version 0.11.0: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
Version: 99.9.9
fatal: not a git repository (or any of the parent directories): .git
Path: /Users/jimjamslam/code/quarto-cli/package/dist/bin
[✓] Checking tools....................OK
TinyTeX: (not installed)
Chromium: (not installed)
[✓] Checking LaTeX....................OK
Tex: (not detected)
[✓] Checking basic markdown render....OK
[✓] Checking Python 3 installation....OK
Version: 3.9.7 (Conda)
Path: /Users/jimjamslam/miniforge3/bin/python
Jupyter: 4.9.2
Kernels: python3
Traceback (most recent call last):
File "/Users/jimjamslam/code/quarto-cli/src/resources/jupyter/jupyter.py", line 21, in <module>
from notebook import notebook_execute, RestartKernel
File "/Users/jimjamslam/Code/quarto-cli/src/resources/jupyter/notebook.py", line 15, in <module>
from yaml import safe_load as parse_string
ModuleNotFoundError: No module named 'yaml'
[✓] Checking Jupyter engine render....OK
ERROR: Error
at renderFiles (file:///Users/jimjamslam/code/quarto-cli/src/command/render/render-files.ts:350:23)
at eventLoopTick (ext:core/01_core.js:153:7)
at async render (file:///Users/jimjamslam/Code/quarto-cli/src/command/render/render-shared.ts:102:18)
at async checkJupyterRender (file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:320:18)
at async file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:280:11
at async withSpinner (file:///Users/jimjamslam/Code/quarto-cli/src/core/console.ts:81:12)
at async checkJupyterInstallation (file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:276:9)
at async check (file:///Users/jimjamslam/code/quarto-cli/src/command/check/check.ts:64:7)
at async Command.actionHandler (file:///Users/jimjamslam/Code/quarto-cli/src/command/check/cmd.ts:33:5)
at async Command.execute (file:///Users/jimjamslam/Code/quarto-cli/src/vendor/deno.land/x/cliffy@v1.0.0-rc.3/command/command.ts:1948:7)
That said, the other parts of the quarto check
do confirm that the development version of Quarto is being used (I was worried I might have a release version lurking around somewhere), and I also checked the path afterward too:
$ which quarto
/Users/jimjamslam/bin/quarto
$ ls -hl ~/bin/quarto
lrwxr-xr-x 1 jimjamslam jimjamslam\admin 55B 12 Jun 09:29 /Users/jimjamslam/bin/quarto -> /Users/jimjamslam/code/quarto-cli/package/dist/bin/quarto
@jimjam-slam https://github.com/quarto-dev/quarto-cli/pull/9953 should be merged soon and will get sverto back into working as it should.
Thanks for giving it a look!
I just tested with the latest prerelease and things look to be working. I'm going to go ahead and close this, but please reopen if something comes up. Thanks!
Bug description
Not sure if this is related to #9889, but a user of the Sverto extension has reported that it no longer works:
https://github.com/jimjam-slam/sverto/issues/27
It appears that on the latest pre-release, the pre-render script provided by the Sverto extension is not found, causing render or preview to fail. On the older pre-release v.1.5.30, it's found without a problem.
Note that unlike #9889, Sverto provides a single string for the contributed
prerender
script, not a list:Steps to reproduce
Set in
_quarto.yml
:Then:
Expected behavior
On v1.5.30:
And then when navigating to
example.html
:Actual behavior
On v1.5.43:
Your environment
Quarto check output