diff --git a/src/command/render/render-files.ts b/src/command/render/render-files.ts index c2bdd8dc276..d7b36962fa1 100644 --- a/src/command/render/render-files.ts +++ b/src/command/render/render-files.ts @@ -232,6 +232,7 @@ export async function renderExecute( previewServer: context.options.previewServer, handledLanguages: languages(), project: context.project, + env: {}, }; // execute computations setExecuteEnvironment(executeOptions); diff --git a/src/core/jupyter/jupyter-embed.ts b/src/core/jupyter/jupyter-embed.ts index 16586e825de..dd2ac2e7061 100644 --- a/src/core/jupyter/jupyter-embed.ts +++ b/src/core/jupyter/jupyter-embed.ts @@ -68,6 +68,7 @@ import { ProjectContext } from "../../project/types.ts"; import { logProgress } from "../log.ts"; import * as ld from "../../../src/core/lodash.ts"; import { texSafeFilename } from "../tex.ts"; +import { ExecuteOptions } from "../../execute/types.ts"; export interface JupyterNotebookAddress { path: string; @@ -415,7 +416,7 @@ export async function notebookMarkdown( // Wrap any injected cells with a div that includes a back link to // the notebook that originated the cells - const notebookMarkdown = ( + const notebookMarkdownInner = ( nbAbsPath: string, cells: JupyterCellOutput[], title?: string, @@ -454,7 +455,7 @@ export async function notebookMarkdown( return cell; } }); - return notebookMarkdown(nbAbsPath, theCells, notebookInfo.title); + return notebookMarkdownInner(nbAbsPath, theCells, notebookInfo.title); } else if (nbAddress.indexes) { // Filter and sort based upon cell indexes const theCells = nbAddress.indexes.map((idx) => { @@ -472,11 +473,11 @@ export async function notebookMarkdown( return cell; } }); - return notebookMarkdown(nbAbsPath, theCells, notebookInfo.title); + return notebookMarkdownInner(nbAbsPath, theCells, notebookInfo.title); } else { // Return all the cell outputs as there is no addtional // specification of cells - const notebookMd = notebookMarkdown( + const notebookMd = notebookMarkdownInner( nbAbsPath, notebookInfo.outputs, notebookInfo.title, @@ -595,7 +596,8 @@ async function getCachedNotebookInfo( // Get the markdown for the notebook const format = context.format; - const executeOptions = { + const executeOptions: ExecuteOptions = { + env: {}, target: context.target, resourceDir: resourcePath(), tempDir: context.options.services.temp.createDir(), 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 07589a60bdc..1871201b829 100644 --- a/src/execute/rmd.ts +++ b/src/execute/rmd.ts @@ -159,6 +159,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. @@ -259,6 +261,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) @@ -291,6 +294,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 489b378cc80..cae98d2fb22 100644 --- a/src/execute/types.ts +++ b/src/execute/types.ts @@ -81,6 +81,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/src/resources/jupyter/notebook.py b/src/resources/jupyter/notebook.py index 2d4ca301525..76afadd03e4 100644 --- a/src/resources/jupyter/notebook.py +++ b/src/resources/jupyter/notebook.py @@ -1,26 +1,24 @@ # pyright: reportMissingImports=false -import os -import re +import asyncio import atexit +import base64 +import copy import glob -import sys import json +import os import pprint -import copy -import base64 - +import re +import sys from pathlib import Path -from yaml import safe_load as parse_string -from yaml import safe_dump - -from log import trace import nbformat -from nbclient import NotebookClient from jupyter_client import KernelManager from jupyter_core_utils_vendor import run_sync -import asyncio +from log import trace +from nbclient import NotebookClient +from yaml import safe_dump +from yaml import safe_load as parse_string # optional import of papermill for params support try: @@ -129,6 +127,8 @@ def set_env_vars(options): else: os.environ["QUARTO_FIG_DPI"] = str(options["fig_dpi"]) os.environ["QUARTO_FIG_FORMAT"] = options["fig_format"] + for key, value in options.get("env", {}).items(): + os.environ[key] = value def retrieve_nb_from_cache(nb, status, input, **kwargs): @@ -201,6 +201,7 @@ def notebook_execute(options, status): quiet = quarto_kernel_setup_options["quiet"] resource_dir = quarto_kernel_setup_options["resource_dir"] eval = quarto_kernel_setup_options["eval"] + quarto_kernel_setup_options["env"] = options.get("env", {}) # set environment variables set_env_vars(quarto_kernel_setup_options) diff --git a/tests/docs/smoke-all/engine-reordering/julia-engine/_quarto.yml b/tests/docs/smoke-all/engine/engine-reordering/julia-engine/_quarto.yml similarity index 100% rename from tests/docs/smoke-all/engine-reordering/julia-engine/_quarto.yml rename to tests/docs/smoke-all/engine/engine-reordering/julia-engine/_quarto.yml diff --git a/tests/docs/smoke-all/engine-reordering/julia-engine/notebook.qmd b/tests/docs/smoke-all/engine/engine-reordering/julia-engine/notebook.qmd similarity index 100% rename from tests/docs/smoke-all/engine-reordering/julia-engine/notebook.qmd rename to tests/docs/smoke-all/engine/engine-reordering/julia-engine/notebook.qmd diff --git a/tests/docs/smoke-all/engine-reordering/jupyter-engine/_quarto.yml b/tests/docs/smoke-all/engine/engine-reordering/jupyter-engine/_quarto.yml similarity index 100% rename from tests/docs/smoke-all/engine-reordering/jupyter-engine/_quarto.yml rename to tests/docs/smoke-all/engine/engine-reordering/jupyter-engine/_quarto.yml diff --git a/tests/docs/smoke-all/engine-reordering/jupyter-engine/notebook.qmd b/tests/docs/smoke-all/engine/engine-reordering/jupyter-engine/notebook.qmd similarity index 100% rename from tests/docs/smoke-all/engine-reordering/jupyter-engine/notebook.qmd rename to tests/docs/smoke-all/engine/engine-reordering/jupyter-engine/notebook.qmd diff --git a/tests/docs/smoke-all/engine/env/julia.qmd b/tests/docs/smoke-all/engine/env/julia.qmd new file mode 100644 index 00000000000..d8480f39fe5 --- /dev/null +++ b/tests/docs/smoke-all/engine/env/julia.qmd @@ -0,0 +1,14 @@ +--- +format: html +_quarto: + tests: + html: + ensureFileRegexMatches: + - ["julia.qmd"] + - [] +engine: julia +--- + +```{julia} +ENV["QUARTO_DOCUMENT_FILE"] +``` \ No newline at end of file diff --git a/tests/docs/smoke-all/engine/env/jupyter.qmd b/tests/docs/smoke-all/engine/env/jupyter.qmd new file mode 100644 index 00000000000..fdab65b13c8 --- /dev/null +++ b/tests/docs/smoke-all/engine/env/jupyter.qmd @@ -0,0 +1,16 @@ +--- +format: html +_quarto: + tests: + html: + ensureFileRegexMatches: + - ["jupyter.qmd"] + - [] +--- + +```{python} +import os + +print(os.environ.get("QUARTO_DOCUMENT_FILE", "")) +print("Hello, world") +``` \ No newline at end of file diff --git a/tests/docs/smoke-all/engine/env/knitr.qmd b/tests/docs/smoke-all/engine/env/knitr.qmd new file mode 100644 index 00000000000..fe74496b723 --- /dev/null +++ b/tests/docs/smoke-all/engine/env/knitr.qmd @@ -0,0 +1,13 @@ +--- +format: html +_quarto: + tests: + html: + ensureFileRegexMatches: + - ["knitr.qmd"] + - [] +--- + +```{r} +print(Sys.getenv("QUARTO_DOCUMENT_FILE")) +``` \ No newline at end of file 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 6390aea40e3..5e95ef7ee18 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 TestLogConfig { @@ -69,9 +70,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 @@ -109,11 +107,45 @@ 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.cliPath(), + args: [cmd, ...args], + env + }), + timeout, + ]); + }, + verify, + context: context || {}, + type: "smoke", + }); +} + export function testQuartoCmd( cmd: string, args: string[], @@ -132,7 +164,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, ]); },