Don't error when running quarto run from Lua on windows

[cderv]

closes #10134

Issue happens because quarto.cmd is called from Lua, inside Quarto context where all the environment variable will be set. This leads to QUARTO_DENO_EXTRA_OPTIONS already set with --v8-flags and we are adding it again

Some flags can't be duplicated like --v8-flags and create an error when running deno.

This PR correctly check when QUARTO_DENO_ env var are set and only add flags to it if not present already.

To be clear, this is one quick fix. Other solution could be

• Call deno run instead of quarto run is we really need only deno, and no quarto context.
• Find a way to run safely in controlled environment quarto from Lua (currently an issue on windows with pandoc.system.with_environment()
• Other ideas ?

I'll check linux script to understand why we haven't found this sooner, and also why our test suites did not error on this case.

[cderv]

I'll check linux script to understand why we haven't found this sooner,

I commented on main issue about this #10134 (comment)

We get the issue only on Windows with quarto.cmd because our current script forces us to us SET to define variable. This will effectively define environment variable for the child process, and this is why it gets conflict when calling it nested.

So current fix seems the most appropriate right now. At some point we would need to rewrite the dev script to use powershell maybe, or different structure with temporary variable but I don't think that really exist in BAT script

[cderv]

and also why our test suites did not error on this case.

I don't think we test for this in our smoke-test. Only in our feature-matrix part, and I need to fix the CI to handle the puppeteer problem which create failing runs most of the time, and a notification mechanism to know when we have failling tests in feature-matrix.

in the meantime, I believe we need to add smoke-test documents for this Juice processing IMO.

[cderv]

It seems like all tests are passing. This fixes the problem for me.

@cscheid following our discussion I understand this is an ok fix for now to solve the problem of quarto.cmd script setting environment variable.

If so, I'll merge it tomorrow my morning. I'll add a test for the missing juice feature too.

@gordonwoodhull I tweak the call to juice also so that we would have discovered this earlier with less silent error. If you can have a look.

Thanks :!

[gordonwoodhull]

Excellent error handling support, thanks! Wish this had already been there to save you some perplexity.

Sorry you lost a day on this, and thanks for pointing out that we need tests that exercise juice - see #10148

I guess those wouldn't have helped since this was a dev environment issue.

[cderv]

I guess those wouldn't have helped since this was a dev environment issue.

Yes you're right. it is not helpful in fact as we see CI passes in #10184. We are running dev environmment on CI but in fact we are not calling quarto render directly using the CLI. We are calling run-tests script, which will call deno run on the *.test.ts file after environment setup. And these tests files are calling directly quarto() function from Typescript.

Conclusion: Our CI does not test the CLI currently. nor the binary or the dev scripts.

Only quarto-web is currently testing that our pre-release build is still able to render all the website. But some edge case like this are not found.

I'll think about something for next CI improvement in 1.6 or 1.7.