diff --git a/src/command/render/render-files.ts b/src/command/render/render-files.ts index 0e8ffec77fd..a6cd5d01165 100644 --- a/src/command/render/render-files.ts +++ b/src/command/render/render-files.ts @@ -228,6 +228,7 @@ export async function renderExecute( previewServer: context.options.previewServer, handledLanguages: languages(), project: context.project, + env: {}, }; // execute computations setExecuteEnvironment(executeOptions); diff --git a/src/execute/environment.ts b/src/execute/environment.ts index 60a29d5de0e..45e55699f18 100644 --- a/src/execute/environment.ts +++ b/src/execute/environment.ts @@ -12,9 +12,9 @@ export const setExecuteEnvironment: (options: ExecuteOptions) => void = ( options, ) => { if (options.projectDir) { - Deno.env.set("QUARTO_PROJECT_ROOT", options.projectDir); - Deno.env.set("QUARTO_DOCUMENT_PATH", dirname(options.target.source)); - Deno.env.set("QUARTO_DOCUMENT_FILE", basename(options.target.source)); + options.env["QUARTO_PROJECT_ROOT"] = options.projectDir; + options.env["QUARTO_DOCUMENT_PATH"] = dirname(options.target.source); + options.env["QUARTO_DOCUMENT_FILE"] = basename(options.target.source); } else { // FIXME: This should not be passthrough anymore as singleFileProjectContext always set `options.projectDir` // https://github.com/quarto-dev/quarto-cli/pull/8771 @@ -23,8 +23,8 @@ export const setExecuteEnvironment: (options: ExecuteOptions) => void = ( "No project directory or current working directory", ); } - Deno.env.set("QUARTO_PROJECT_ROOT", options.cwd); - Deno.env.set("QUARTO_DOCUMENT_PATH", options.cwd); - Deno.env.set("QUARTO_DOCUMENT_FILE", basename(options.target.source)); + options.env["QUARTO_PROJECT_ROOT"] = options.cwd; + options.env["QUARTO_DOCUMENT_PATH"] = options.cwd; + options.env["QUARTO_DOCUMENT_FILE"] = basename(options.target.source); } }; diff --git a/src/execute/jupyter/jupyter-kernel.ts b/src/execute/jupyter/jupyter-kernel.ts index dd864de4e20..59d5cf8807e 100644 --- a/src/execute/jupyter/jupyter-kernel.ts +++ b/src/execute/jupyter/jupyter-kernel.ts @@ -61,6 +61,7 @@ export async function executeKernelOneshot( "execute", { ...options, debug }, options.kernelspec, + options.env, ); if (!result.success) { @@ -186,6 +187,7 @@ async function execJupyter( command: string, options: Record, kernelspec: JupyterKernelspec, + env: Record = {}, ): Promise { try { const cmd = await pythonExec(kernelspec); @@ -196,6 +198,7 @@ async function execJupyter( ...cmd.slice(1), resourcePath("jupyter/jupyter.py"), ], + // FIXME IS THIS NOT SET WITH THE DAEMON? env: { // Force default matplotlib backend. something simillar is done here: // https://github.com/ipython/ipykernel/blob/d7339c2c70115bbe6042880d29eeb273b5a2e350/ipykernel/kernelapp.py#L549-L554 @@ -206,6 +209,7 @@ async function execJupyter( // function within the notebook "MPLBACKEND": "module://matplotlib_inline.backend_inline", "PYDEVD_DISABLE_FILE_VALIDATION": "1", + ...env, }, stdout: "piped", }, diff --git a/src/execute/rmd.ts b/src/execute/rmd.ts index 0ee1b46c3fa..d2a4ef81934 100644 --- a/src/execute/rmd.ts +++ b/src/execute/rmd.ts @@ -156,6 +156,8 @@ export const knitrEngine: ExecutionEngine = { return output; }, + true, + options.env, ); const includes = result.includes as unknown; // knitr appears to return [] instead of {} as the value for includes. @@ -256,6 +258,7 @@ async function callR( quiet?: boolean, outputFilter?: (output: string) => string, reportError = true, + env?: Record, ): Promise { // establish cwd for our R scripts (the current dir if there is an renv // otherwise the project dir if specified) @@ -288,6 +291,7 @@ async function callR( ...rscriptArgsArray, resourcePath("rmd/rmd.R"), ], + env, cwd, stderr: quiet ? "piped" : "inherit", }, diff --git a/src/execute/types.ts b/src/execute/types.ts index 1f57a02c15e..9c8616dcb36 100644 --- a/src/execute/types.ts +++ b/src/execute/types.ts @@ -77,6 +77,7 @@ export interface ExecutionTarget { // execute options export interface ExecuteOptions { + env: Record; target: ExecutionTarget; format: Format; resourceDir: string; diff --git a/src/quarto.ts b/src/quarto.ts index 8a7bbd27568..d87b3778324 100644 --- a/src/quarto.ts +++ b/src/quarto.ts @@ -77,15 +77,11 @@ const checkReconfiguration = async () => { } }; -const passThroughPandoc = async ( - args: string[], - env?: Record, -) => { +const passThroughPandoc = async (args: string[]) => { const result = await execProcess( { cmd: pandocBinaryPath(), args: args.slice(1), - env, }, undefined, undefined, @@ -95,10 +91,7 @@ const passThroughPandoc = async ( Deno.exit(result.code); }; -const passThroughTypst = async ( - args: string[], - env?: Record, -) => { +const passThroughTypst = async (args: string[]) => { if (args[1] === "update") { error( "The 'typst update' command is not supported.\n" + @@ -109,7 +102,6 @@ const passThroughTypst = async ( const result = await execProcess({ cmd: typstBinaryPath(), args: args.slice(1), - env, }); Deno.exit(result.code); }; @@ -117,20 +109,19 @@ const passThroughTypst = async ( export async function quarto( args: string[], cmdHandler?: (command: Command) => Command, - env?: Record, ) { await checkReconfiguration(); checkVersionRequirement(); if (args[0] === "pandoc" && args[1] !== "help") { - await passThroughPandoc(args, env); + await passThroughPandoc(args); } if (args[0] === "typst") { - await passThroughTypst(args, env); + await passThroughTypst(args); } // passthrough to run handlers if (args[0] === "run" && args[1] !== "help" && args[1] !== "--help") { - const result = await runScript(args.slice(1), env); + const result = await runScript(args.slice(1)); Deno.exit(result.code); } @@ -155,13 +146,6 @@ export async function quarto( debug("Quarto version: " + quartoConfig.version()); - const oldEnv: Record = {}; - for (const [key, value] of Object.entries(env || {})) { - const oldV = Deno.env.get(key); - oldEnv[key] = oldV; - Deno.env.set(key, value); - } - const quartoCommand = new Command() .name("quarto") .help({ colors: false }) @@ -191,13 +175,6 @@ export async function quarto( try { await promise; - for (const [key, value] of Object.entries(oldEnv)) { - if (value === undefined) { - Deno.env.delete(key); - } else { - Deno.env.set(key, value); - } - } if (commandFailed()) { exitWithCleanup(1); } diff --git a/tests/smoke/env/install.test.ts b/tests/smoke/env/install.test.ts index 1a1c044c6f8..23ff2450b61 100644 --- a/tests/smoke/env/install.test.ts +++ b/tests/smoke/env/install.test.ts @@ -13,6 +13,7 @@ testQuartoCmd( [ noErrorsOrWarnings, printsMessage({level: "INFO", regex: /tinytex\s+/}), + // FIXME WE SHOULD BE ABLE TO TURN THIS BACK ON // printsMessage({level: "INFO", regex: /chromium\s+/}), // temporarily disabled until we get puppeteer back ], diff --git a/tests/smoke/extensions/extension-render-journals.test.ts b/tests/smoke/extensions/extension-render-journals.test.ts index 0327048fb64..e7c119d1d8d 100644 --- a/tests/smoke/extensions/extension-render-journals.test.ts +++ b/tests/smoke/extensions/extension-render-journals.test.ts @@ -42,6 +42,10 @@ for (const journalRepo of journalRepos) { // Sets up the test setup: async () => { console.log(`using quarto-journals/${journalRepo.repo}`); + + // FIXME THIS DOESN'T WORK + // WE CANNOT GUARANTEE THAT CHDIR WILL BE CONSTANT + // THROUGHOUT THE ASYNC CALL const wd = Deno.cwd(); Deno.chdir(workingDir); await quarto([ diff --git a/tests/smoke/project/project-prepost.test.ts b/tests/smoke/project/project-prepost.test.ts index 3caf6d72419..60e94ead765 100644 --- a/tests/smoke/project/project-prepost.test.ts +++ b/tests/smoke/project/project-prepost.test.ts @@ -8,7 +8,7 @@ import { docs } from "../../utils.ts"; import { join } from "../../../src/deno_ral/path.ts"; import { existsSync } from "../../../src/deno_ral/fs.ts"; -import { testQuartoCmd } from "../../test.ts"; +import { testQuartoCmd, testQuartoCmdWithEnv } from "../../test.ts"; import { fileExists, noErrors, printsMessage, verifyNoPath, verifyPath } from "../../verify.ts"; import { normalizePath, safeRemoveIfExists } from "../../../src/core/path.ts"; @@ -78,22 +78,22 @@ testQuartoCmd( } }); - testQuartoCmd( - "render", - [docs("project/prepost/issue-10828")], - [], - { - env: { - "QUARTO_USE_FILE_FOR_PROJECT_INPUT_FILES": normalizePath(docs("project/prepost/issue-10828/input-files.txt")), - "QUARTO_USE_FILE_FOR_PROJECT_OUTPUT_FILES": normalizePath(docs("project/prepost/issue-10828/output-files.txt")) - }, - teardown: async () => { - const inputPath = normalizePath(docs("project/prepost/issue-10828/input-files.txt")); - const outputPath = normalizePath(docs("project/prepost/issue-10828/output-files.txt")); - verifyPath(inputPath); - safeRemoveIfExists(inputPath); - verifyPath(outputPath); - safeRemoveIfExists(outputPath); - } +testQuartoCmdWithEnv( + "render", + [docs("project/prepost/issue-10828")], + [], + { + "QUARTO_USE_FILE_FOR_PROJECT_INPUT_FILES": normalizePath(docs("project/prepost/issue-10828/input-files.txt")), + "QUARTO_USE_FILE_FOR_PROJECT_OUTPUT_FILES": normalizePath(docs("project/prepost/issue-10828/output-files.txt")) + }, + { + teardown: async () => { + const inputPath = normalizePath(docs("project/prepost/issue-10828/input-files.txt")); + const outputPath = normalizePath(docs("project/prepost/issue-10828/output-files.txt")); + verifyPath(inputPath); + safeRemoveIfExists(inputPath); + verifyPath(outputPath); + safeRemoveIfExists(outputPath); } - ) \ No newline at end of file + } +) \ No newline at end of file diff --git a/tests/test.ts b/tests/test.ts index e4d67ae894c..7ac48a57e16 100644 --- a/tests/test.ts +++ b/tests/test.ts @@ -17,6 +17,7 @@ import { runningInCI } from "../src/core/ci-info.ts"; import { relative, fromFileUrl } from "../src/deno_ral/path.ts"; import { quartoConfig } from "../src/core/quarto.ts"; import { isWindows } from "../src/deno_ral/platform.ts"; +import { execProcess } from "../src/core/process.ts"; export interface TestDescriptor { // The name of the test @@ -55,9 +56,6 @@ export interface TestContext { // control if test is ran or skipped ignore?: boolean; - - // environment to pass to downstream processes - env?: Record; } // Allow to merge test contexts in Tests helpers @@ -95,11 +93,44 @@ export function mergeTestContexts(baseContext: TestContext, additionalContext?: }, // override ignore if provided ignore: additionalContext.ignore ?? baseContext.ignore, - // merge env with additional context taking precedence - env: { ...baseContext.env, ...additionalContext.env }, }; } +// for Quarto tests that new to change the environment, +// we use a subprocess call to run the command, or else we risk +// race conditions with environment variables overwriting +// each other +export function testQuartoCmdWithEnv( + cmd: string, + args: string[], + verify: Verify[], + env: Record, + context?: TestContext, + name?: string +) { + if (name === undefined) { + name = `quarto ${cmd} ${args.join(" ")}`; + } + test({ + name, + execute: async () => { + const timeout = new Promise((_resolve, reject) => { + setTimeout(reject, 600000, "timed out after 10 minutes"); + }); + await Promise.race([ + execProcess({ + cmd: [quartoConfig.binPath(), cmd, ...args], + env + }), + timeout, + ]); + }, + verify, + context: context || {}, + type: "smoke", + }); +} + export function testQuartoCmd( cmd: string, args: string[], @@ -117,7 +148,7 @@ export function testQuartoCmd( setTimeout(reject, 600000, "timed out after 10 minutes"); }); await Promise.race([ - quarto([cmd, ...args], undefined, context?.env), + quarto([cmd, ...args]), timeout, ]); },