diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..6313b56c5 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text=auto eol=lf diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 000000000..049dcdb7c --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,105 @@ +name: Test + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: + contents: read + +jobs: + # Lint, unit and integration tests run on Cloud Build (cloudbuild-test.yaml); this job only + # builds the packages and the smoke test bundle that the Windows job needs. + build-linux: + name: Build packages and smoke test bundle (Linux) + runs-on: ubuntu-latest + timeout-minutes: 60 + permissions: + contents: read + steps: + - name: Checkout code + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Setup Bazel + uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3.0.0 + + # MODULE.bazel sets `use_home_npmrc = True`, so aspect_rules_js requires ~/.npmrc to exist. + # An empty file keeps the default public npm registry (Cloud Build writes its own). + - name: Create an empty ~/.npmrc + run: touch ~/.npmrc + + - name: Build package tarballs and the Windows smoke test bundle + run: | + bazel build \ + //packages/@dataform/cli:package_tar \ + //packages/@dataform/core:package_tar \ + //cli/tests/windows:smoke_test_bundle + + - name: Upload CLI tarball + uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1 + with: + name: dataform-cli-tarball + path: bazel-bin/packages/@dataform/cli/package.tar.gz + + - name: Upload Core tarball + uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1 + with: + name: dataform-core-tarball + path: bazel-bin/packages/@dataform/core/package.tar.gz + + - name: Upload Windows smoke test bundle + uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1 + with: + name: windows-smoke-test + path: bazel-bin/cli/tests/windows/smoke_test_bundle.js + + test-windows: + name: CLI smoke tests (Windows, Node ${{ matrix.node-version }}) + needs: build-linux + runs-on: windows-latest + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + # Oldest supported Node.js LTS and the current one (also used by the Bazel toolchain). + node-version: [22, 24] + permissions: + contents: read + steps: + - name: Setup Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: ${{ matrix.node-version }} + + - name: Download CLI tarball + uses: actions/download-artifact@cc203385981b70ca67e1cc392babf9cc229d5806 # v4.1.9 + with: + name: dataform-cli-tarball + path: dist/cli + + - name: Download Core tarball + uses: actions/download-artifact@cc203385981b70ca67e1cc392babf9cc229d5806 # v4.1.9 + with: + name: dataform-core-tarball + path: dist/core + + - name: Download Windows smoke test bundle + uses: actions/download-artifact@cc203385981b70ca67e1cc392babf9cc229d5806 # v4.1.9 + with: + name: windows-smoke-test + path: dist/smoke + + - name: Install @dataform/cli globally from tarball + run: npm install -g dist/cli/package.tar.gz + + - name: Run Windows smoke tests in PowerShell + shell: powershell + run: | + node dist/smoke/smoke_test_bundle.js --shell powershell --core-tarball dist/core/package.tar.gz + + - name: Run Windows smoke tests in cmd.exe + shell: cmd + run: | + node dist\smoke\smoke_test_bundle.js --shell cmd --core-tarball dist\core\package.tar.gz diff --git a/cli/api/commands/base_worker.ts b/cli/api/commands/base_worker.ts index 941fdae9f..11dc2b8d1 100644 --- a/cli/api/commands/base_worker.ts +++ b/cli/api/commands/base_worker.ts @@ -19,7 +19,7 @@ export abstract class BaseWorker { ): Promise { const forkScript = this.resolveScript(); const child = fork(forkScript, [], { - stdio: [0, 1, 2, "ipc", "pipe"], + stdio: [0, 1, 2, "ipc"], }); return new Promise((resolve, reject) => { diff --git a/cli/api/commands/compile.ts b/cli/api/commands/compile.ts index 5cace57af..993e9ac88 100644 --- a/cli/api/commands/compile.ts +++ b/cli/api/commands/compile.ts @@ -96,7 +96,7 @@ export async function compile( compiledGraph = dataform.CompiledGraph.create(decodedResult.compile.compiledGraph); if (workflowSettingsDataformCoreVersion) { - fs.rmSync(temporaryProjectPath, { recursive: true }); + fs.rmSync(temporaryProjectPath, { recursive: true, force: true, maxRetries: 3 }); } return compiledGraph; diff --git a/cli/commands/format_command.ts b/cli/commands/format_command.ts index 2f6fda427..3c613aef5 100644 --- a/cli/commands/format_command.ts +++ b/cli/commands/format_command.ts @@ -14,6 +14,8 @@ import { printError, printFormatFilesResult, printSuccess } from "df/cli/console import { ICommand, INamedOption } from "df/cli/yargswrapper"; import { formatFile } from "df/sqlx/format"; +const normalizeLineEndings = (text: string) => text.replace(/\r\n/g, "\n"); + export interface IFormatArgs extends IProjectDirArgs, IActionsArgs { ignoreJsFiles: boolean; check: boolean; @@ -50,7 +52,13 @@ export const formatCommand: ICommand = { actions = argv.actions; } const filenames = actions - .map((action: string) => glob.sync(action, { cwd: argv.projectDir })) + .map((action: string) => + glob.sync(action, { + cwd: argv.projectDir, + // Treat "\" as a path separator only on Windows; on POSIX it remains a glob escape. + windowsPathsNoEscape: process.platform === "win32", + }), + ) .flat(); const isCheckMode = argv.check; @@ -70,7 +78,8 @@ export const formatCommand: ICommand = { }); return { filename, - needsFormatting: fileContent !== formattedContent, + needsFormatting: + normalizeLineEndings(fileContent) !== normalizeLineEndings(formattedContent), }; } else { // Normal formatting mode diff --git a/cli/index_project_test.ts b/cli/index_project_test.ts index 14bf61aca..ec0d47ec5 100644 --- a/cli/index_project_test.ts +++ b/cli/index_project_test.ts @@ -110,5 +110,31 @@ modules.exports = { const bufFromContents = Buffer.from(jsContents, "utf-8"); expect(bufFromContents.equals(bufFromFile)).equals(true); }); + + test("format --check accepts correctly formatted files with CRLF line endings", async () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + await setupProject(tmpDirFixture, projectDir); + writeDefinitionFile( + projectDir, + "crlf.sqlx", + ` +config { type: "table" } +SELECT 1 as test +`, + ); + expect((await runCli("format", [projectDir])).exitCode).equals(0); + + // Same formatted content, but with Windows line endings (e.g. a Git checkout with + // core.autocrlf=true). + const crlfFilePath = path.join(projectDir, "definitions", "crlf.sqlx"); + const formattedContent = fs.readFileSync(crlfFilePath, "utf8"); + expect(formattedContent).not.contains("\r\n"); + fs.writeFileSync(crlfFilePath, formattedContent.replace(/\n/g, "\r\n")); + + const checkResult = await runCli("format", [projectDir, "--check"]); + + expect(checkResult.exitCode).equals(0); + expect(checkResult.stdout).contains("All files are formatted correctly"); + }); }); }); diff --git a/cli/tests/windows/BUILD b/cli/tests/windows/BUILD new file mode 100644 index 000000000..00443b257 --- /dev/null +++ b/cli/tests/windows/BUILD @@ -0,0 +1,22 @@ +load("//packages:index.bzl", "pkg_bundle") +load("//tools:ts_library.bzl", "ts_library") + +package(default_visibility = ["//visibility:public"]) + +ts_library( + name = "smoke_test_lib", + srcs = ["smoke_test.ts"], + deps = [ + "//:node_modules/@types/node", + ], +) + +# Self-contained CommonJS bundle (Node built-ins only) that the Windows CI runner executes with +# plain `node`; it is built on the Linux job and shipped as a workflow artifact. +pkg_bundle( + name = "smoke_test_bundle", + allow_node_builtins = True, + entry_point = "smoke_test.js", + externals = [], + deps = [":smoke_test_lib"], +) diff --git a/cli/tests/windows/smoke_test.ts b/cli/tests/windows/smoke_test.ts new file mode 100644 index 000000000..36c467156 --- /dev/null +++ b/cli/tests/windows/smoke_test.ts @@ -0,0 +1,533 @@ +/** + * End-to-end smoke tests for the Dataform CLI on Windows. + * + * The CLI is expected to be installed globally (`npm install -g `). Each flow is + * executed through the shell selected with `--shell` so that the `dataform.cmd` shim is exercised + * the same way a user would run it: + * + * node smoke_test_bundle.js --shell powershell [--core-tarball ] [--dataform-bin ] + * node smoke_test_bundle.js --shell cmd [--core-tarball ] [--dataform-bin ] + * + * Without `--shell`, `bundle.js` is run directly with Node (no shell and no launcher), which also + * works on Linux and macOS. + * + * `--core-tarball` installs `@dataform/core` from a local tarball through `package.json` + * (`"file:"`), the same flow used by the CLI unit tests; CI always passes it. Note that + * `dataform init` installs nothing, it only pins the CLI's own version. Without `--core-tarball` + * (local runs), Flow 3B installs that pinned version from the npm registry. + * + * This file is bundled with Bazel (`//cli/tests/windows:smoke_test_bundle`) into a single + * CommonJS file that only depends on Node built-ins, so the Windows CI runner needs nothing but + * Node.js to execute it. + */ + +import { spawn, spawnSync, SpawnSyncReturns } from "child_process"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +type ShellMode = "cmd" | "powershell" | "direct"; + +interface IDataformBinary { + /** `dataform.cmd` shim on Windows or the `dataform` launcher elsewhere; null if unknown. */ + shimPath: string | null; + /** `node_modules/@dataform/cli/bundle.js`; used to spawn the CLI without any shell. */ + bundlePath: string | null; +} + +interface IExecOptions { + cwd?: string; + timeoutMillis?: number; +} + +const WATCH_TIMEOUT_MILLIS = 45_000; +const DEFAULT_TIMEOUT_MILLIS = 120_000; +const SAMPLE_DESCRIPTION = "Sample table description"; +const SAMPLE_TABLE_SQLX = + 'config {\n type: "table",\n description: getContents("./sample_description.md")\n}\n\n' + + "SELECT\n 1 AS sample_col\n"; + +// --------------------------------------------------------------------------- +// Command line flags +// --------------------------------------------------------------------------- + +const argv = process.argv.slice(2); + +function getFlag(flag: string): string | null { + const index = argv.indexOf(flag); + return index !== -1 && index + 1 < argv.length ? argv[index + 1] : null; +} + +function parseShellMode(value: string | null): ShellMode { + switch (value) { + case null: + case "direct": + return "direct"; + case "cmd": + case "powershell": + if (os.platform() !== "win32") { + throw new Error(`--shell ${value} is only supported on Windows`); + } + return value; + default: + throw new Error(`Unknown --shell value "${value}"; expected cmd, powershell or direct`); + } +} + +const shellMode = parseShellMode(getFlag("--shell")); +const customBinary = getFlag("--dataform-bin") || process.env.DATAFORM_BIN || null; +const coreTarball = getFlag("--core-tarball"); + +// --------------------------------------------------------------------------- +// Minimal assertion helpers (keeps the bundle free of non built-in dependencies) +// --------------------------------------------------------------------------- + +function check(condition: boolean, message: string): void { + if (!condition) { + throw new Error(message); + } +} + +function checkEqual(actual: T, expected: T, message: string): void { + check(actual === expected, `${message}\n expected: ${expected}\n actual: ${actual}`); +} + +function checkIncludes(haystack: string, needle: string, message: string): void { + check(haystack.includes(needle), `${message}\n expected to find: ${needle}\n in:\n${haystack}`); +} + +function describeResult(result: SpawnSyncReturns): string { + return `exit=${result.status} signal=${result.signal}\nstdout:\n${result.stdout}\nstderr:\n${result.stderr}`; +} + +// --------------------------------------------------------------------------- +// Locating and invoking the CLI +// --------------------------------------------------------------------------- + +function findOnPath(executable: string): string | null { + for (const dir of (process.env.PATH || "").split(path.delimiter)) { + const candidate = path.join(dir, executable); + if (dir && fs.existsSync(candidate)) { + return candidate; + } + } + return null; +} + +function findBundleNextToShim(shimPath: string): string | null { + const shimDir = path.dirname(shimPath); + const candidates = [ + // npm global prefix on Windows: %APPDATA%\npm\dataform.cmd + %APPDATA%\npm\node_modules\... + path.join(shimDir, "node_modules", "@dataform", "cli", "bundle.js"), + // npm global prefix on POSIX: /bin/dataform + /lib/node_modules/... + path.join(shimDir, "..", "lib", "node_modules", "@dataform", "cli", "bundle.js"), + ]; + try { + // On POSIX the launcher is a symlink straight to bundle.js. + candidates.unshift(fs.realpathSync(shimPath)); + } catch (e) { + // Ignore: the shim does not exist or is not a symlink. + } + const found = candidates.find( + (candidate) => candidate.endsWith(".js") && fs.existsSync(candidate), + ); + return found ? path.resolve(found) : null; +} + +function resolveDataformBinary(): IDataformBinary { + if (customBinary) { + const resolved = path.resolve(customBinary); + if (resolved.endsWith(".js")) { + return { shimPath: null, bundlePath: resolved }; + } + return { shimPath: resolved, bundlePath: findBundleNextToShim(resolved) }; + } + const shimPath = findOnPath(os.platform() === "win32" ? "dataform.cmd" : "dataform"); + if (!shimPath) { + throw new Error( + "dataform is not on PATH; install it with `npm install -g` or pass --dataform-bin", + ); + } + return { shimPath, bundlePath: findBundleNextToShim(shimPath) }; +} + +const binary = resolveDataformBinary(); +if (shellMode === "direct" && !binary.bundlePath) { + throw new Error( + "Direct mode runs bundle.js with node, but it could not be located next to the dataform " + + "launcher; pass --dataform-bin ", + ); +} + +/** + * The program and leading arguments that start the CLI. The cmd.exe and PowerShell modes go + * through the launcher (e.g. `dataform.cmd`), as a user would. Direct mode runs + * `[node, bundle.js]`, because Node refuses to spawn a `.cmd` file without a shell (EINVAL since + * the CVE-2024-27980 fix). + */ +function cliCommand(): string[] { + if (shellMode !== "direct" && binary.shimPath) { + return [binary.shimPath]; + } + return [process.execPath, binary.bundlePath]; +} + +function quoteForCmd(arg: string): string { + // cmd.exe has no escape for an embedded double quote; the arguments used here never need one. + check(!arg.includes('"'), `cmd.exe argument must not contain double quotes: ${arg}`); + return /[\s&|<>^()]/.test(arg) ? `"${arg}"` : arg; +} + +function quoteForPowerShell(arg: string): string { + return `'${arg.replace(/'/g, "''")}'`; +} + +function execCli(args: string[], options: IExecOptions = {}): SpawnSyncReturns { + const spawnOptions = { + cwd: options.cwd, + env: process.env, + encoding: "utf8" as const, + timeout: options.timeoutMillis || DEFAULT_TIMEOUT_MILLIS, + }; + const command = [...cliCommand(), ...args]; + switch (shellMode) { + case "powershell": + return spawnSync( + "powershell", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + ["&", ...command.map(quoteForPowerShell)].join(" "), + ], + spawnOptions, + ); + case "cmd": + // The whole command line is built here and passed verbatim, so Node does not re-quote it + // (and does not emit DEP0190, which is triggered by `shell: true` with an argument array). + // `/s` makes cmd.exe strip only the outer pair of quotes. + return spawnSync("cmd.exe", ["/d", "/s", "/c", `"${command.map(quoteForCmd).join(" ")}"`], { + ...spawnOptions, + windowsVerbatimArguments: true, + }); + default: + return spawnSync(command[0], command.slice(1), spawnOptions); + } +} + +// --------------------------------------------------------------------------- +// Test runner +// --------------------------------------------------------------------------- + +let passed = 0; +let failed = 0; + +async function runTest(name: string, fn: () => void | Promise): Promise { + process.stdout.write(`TEST: ${name} ... `); + try { + await fn(); + console.log("PASSED"); + passed++; + } catch (e) { + console.log("FAILED"); + console.error(` ${e instanceof Error ? e.stack || e.message : String(e)}`); + failed++; + } +} + +function createTempDir(prefix: string): string { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); +} + +function removeDir(dir: string): void { + // Windows may still hold handles on recently closed files; retry a few times. + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 5 }); +} + +function toPosixPath(filePath: string): string { + return filePath.replace(/\\/g, "/"); +} + +// --------------------------------------------------------------------------- +// CLI flows +// --------------------------------------------------------------------------- + +async function runCliFlows(projectDir: string): Promise { + const definitionsDir = path.join(projectDir, "definitions"); + const workflowSettingsPath = path.join(projectDir, "workflow_settings.yaml"); + const sampleTablePath = path.join(definitionsDir, "sample_table.sqlx"); + + // `run` and `test` read credentials before doing anything else. Neither the dry run (Flow 5) + // nor test discovery (Flow 6) contacts BigQuery, so placeholder credentials are enough. `init` + // only refuses directories that already contain a project, so the file can be written first. + fs.writeFileSync( + path.join(projectDir, ".df-credentials.json"), + JSON.stringify({ projectId: "test-project" }, null, 2), + ); + + await runTest("CLI Flow 1: dataform --help", () => { + const result = execCli(["--help"]); + checkEqual(result.status, 0, `--help must exit with 0: ${describeResult(result)}`); + checkIncludes(result.stdout, "dataform [command]", "--help must print the usage line"); + }); + + await runTest("CLI Flow 2: dataform init creates the project structure", () => { + const result = execCli(["init", projectDir, "test-db", "us-central1"]); + checkEqual(result.status, 0, `init must exit with 0: ${describeResult(result)}`); + check(fs.existsSync(workflowSettingsPath), "workflow_settings.yaml must exist"); + const workflowSettings = fs.readFileSync(workflowSettingsPath, "utf8"); + checkIncludes(workflowSettings, "defaultProject: test-db", "defaultProject must be set"); + checkIncludes(workflowSettings, "defaultLocation: us-central1", "defaultLocation must be set"); + check(fs.existsSync(definitionsDir), "definitions/ must exist"); + check(fs.existsSync(path.join(projectDir, "includes")), "includes/ must exist"); + }); + + await runTest("CLI Flow 3A: dataform install refuses projects with dataformCoreVersion", () => { + const result = execCli(["install", projectDir]); + checkEqual(result.status, 1, `install must exit with 1: ${describeResult(result)}`); + checkIncludes( + result.stderr, + "No installation is needed when using workflow_settings.yaml", + "install must explain that packages are installed at runtime", + ); + }); + + await runTest("CLI Flow 3B: dataform install installs @dataform/core from package.json", () => { + // Switch the project to the package.json flow (the same setup as the CLI unit tests): the + // version pinned by `dataform init` moves from workflow_settings.yaml to package.json, or is + // replaced by the local tarball when one was provided. + const workflowSettings = fs.readFileSync(workflowSettingsPath, "utf8"); + const versionMatch = /^dataformCoreVersion:\s*(\S+)\s*$/m.exec(workflowSettings); + check(!!versionMatch, "workflow_settings.yaml written by init must pin dataformCoreVersion"); + fs.writeFileSync(workflowSettingsPath, workflowSettings.replace(versionMatch[0], "")); + const coreDependency = coreTarball + ? `file:${toPosixPath(path.resolve(coreTarball))}` + : versionMatch[1]; + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ dependencies: { "@dataform/core": coreDependency } }, null, 2), + ); + + const result = execCli(["install", projectDir]); + checkEqual(result.status, 0, `install must exit with 0: ${describeResult(result)}`); + check( + fs.existsSync(path.join(projectDir, "node_modules", "@dataform", "core", "package.json")), + "node_modules/@dataform/core must be installed", + ); + }); + + await runTest("CLI Flow 4A: dataform compile (issue #1565)", () => { + fs.writeFileSync(path.join(definitionsDir, "sample_description.md"), SAMPLE_DESCRIPTION); + fs.writeFileSync(sampleTablePath, SAMPLE_TABLE_SQLX); + const result = execCli(["compile", projectDir]); + checkEqual(result.status, 0, `compile must exit with 0: ${describeResult(result)}`); + checkIncludes(result.stdout, "Compiled 1 action(s).", "compile must report one action"); + }); + + await runTest("CLI Flow 4B: dataform compile --json uses forward slashes (issue #510)", () => { + const result = execCli(["compile", projectDir, "--json"]); + checkEqual(result.status, 0, `compile --json must exit with 0: ${describeResult(result)}`); + const graph = JSON.parse(result.stdout); + check(Array.isArray(graph.tables), "compiled graph must contain tables"); + const table = graph.tables.find((t: any) => t.target && t.target.name === "sample_table"); + check(!!table, "sample_table must be in the compiled graph"); + checkEqual(table.fileName, "definitions/sample_table.sqlx", "fileName must use '/'"); + checkEqual( + table.actionDescriptor && table.actionDescriptor.description, + SAMPLE_DESCRIPTION, + "getContents must resolve relative paths inside definitions/", + ); + }); + + await runTest("CLI Flow 4C: dataform compile with a notebook declared in actions.yaml", () => { + const actionsYamlPath = path.join(definitionsDir, "actions.yaml"); + const notebookPath = path.join(definitionsDir, "sample_notebook.ipynb"); + fs.writeFileSync( + actionsYamlPath, + "actions:\n- notebook:\n filename: sample_notebook.ipynb\n", + ); + fs.writeFileSync(notebookPath, JSON.stringify({ cells: [] })); + try { + const result = execCli(["compile", projectDir, "--json"]); + checkEqual(result.status, 0, `compile must exit with 0: ${describeResult(result)}`); + const graph = JSON.parse(result.stdout); + check(Array.isArray(graph.notebooks), "compiled graph must contain notebooks"); + const notebook = graph.notebooks.find( + (n: any) => n.target && n.target.name === "sample_notebook", + ); + check(!!notebook, "sample_notebook must be in the compiled graph"); + checkEqual(notebook.fileName, "definitions/sample_notebook.ipynb", "fileName must use '/'"); + } finally { + fs.rmSync(actionsYamlPath, { force: true }); + fs.rmSync(notebookPath, { force: true }); + } + }); + + await runTest("CLI Flow 4D: dataform compile --watch recompiles after a change", () => { + check( + !!binary.bundlePath, + "bundle.js could not be located next to the dataform launcher; pass --dataform-bin ", + ); + return new Promise((resolve, reject) => { + // Spawn node directly, without cmd.exe or PowerShell in between, so that child.kill() + // terminates the dataform process itself rather than the shell. + const child = spawn(process.execPath, [binary.bundlePath, "compile", projectDir, "--watch"], { + env: process.env, + }); + let output = ""; + let changeWritten = false; + let finished = false; + const compiledCount = () => output.split("Compiled 1 action(s).").length - 1; + const finish = (error?: Error) => { + if (finished) { + return; + } + finished = true; + clearTimeout(timer); + child.kill(); + if (changeWritten) { + fs.writeFileSync(sampleTablePath, SAMPLE_TABLE_SQLX); + } + if (error) { + reject(error); + } else { + resolve(); + } + }; + const timer = setTimeout( + () => finish(new Error(`Timed out waiting for a recompilation. Output:\n${output}`)), + WATCH_TIMEOUT_MILLIS, + ); + const onData = (chunk: Buffer) => { + output += chunk.toString(); + if (!changeWritten && output.includes("Watching for changes...")) { + // The watcher is ready; touch a definition and wait for the second compilation. + changeWritten = true; + fs.appendFileSync(sampleTablePath, "\n-- watched change\n"); + } else if (changeWritten && compiledCount() >= 2) { + finish(); + } + }; + child.stdout.on("data", onData); + child.stderr.on("data", onData); + child.on("error", (error) => finish(error)); + child.on("exit", (code) => { + if (compiledCount() < 2) { + finish(new Error(`watch exited early with code ${code}. Output:\n${output}`)); + } + }); + }); + }); + + await runTest("CLI Flow 5: dataform run --dry-run --json", () => { + const operationPath = path.join(definitionsDir, "sample_operation.sqlx"); + fs.writeFileSync(operationPath, 'config {\n type: "operations"\n}\n\nSELECT 1 AS op_col\n'); + try { + const result = execCli([ + "run", + projectDir, + "--dry-run", + "--json", + "--actions", + "sample_operation", + ]); + checkEqual(result.status, 0, `run --dry-run must exit with 0: ${describeResult(result)}`); + const executionGraph = JSON.parse(result.stdout); + check(Array.isArray(executionGraph.actions), "execution graph must contain actions"); + check( + executionGraph.actions.some((a: any) => a.target && a.target.name === "sample_operation"), + "execution graph must include sample_operation", + ); + } finally { + fs.rmSync(operationPath, { force: true }); + } + }); + + await runTest("CLI Flow 6: dataform test", () => { + const noTestsResult = execCli(["test", projectDir]); + checkEqual(noTestsResult.status, 1, `test must exit with 1: ${describeResult(noTestsResult)}`); + checkIncludes(noTestsResult.stderr, "No unit tests found.", "test must report missing tests"); + + const testPath = path.join(definitionsDir, "table_test.sqlx"); + fs.writeFileSync( + testPath, + 'config {\n type: "test",\n dataset: "sample_table"\n}\n\n' + + 'input "sample_table" {\n SELECT\n 1 AS sample_col\n}\n\nSELECT\n 1 AS sample_col\n', + ); + try { + // Only compilation and test discovery are verified; executing the test needs BigQuery. + const result = execCli(["test", projectDir], { timeoutMillis: 60_000 }); + checkIncludes(result.stdout, "Running 1 unit tests...", "test must discover the unit test"); + } finally { + fs.rmSync(testPath, { force: true }); + } + }); + + await runTest("CLI Flow 7: dataform format and --check with CRLF line endings", () => { + const filePath = path.join(definitionsDir, "to_format.sqlx"); + // Pass the relative path with the OS separator ("definitions\to_format.sqlx" on Windows) to + // verify that `--actions` treats "\" as a path separator rather than a glob escape on Windows. + const relativeActionPath = path.join("definitions", "to_format.sqlx"); + try { + fs.writeFileSync(filePath, 'config { type: "table" } SELECT 1 AS num'); + const unformattedCheck = execCli([ + "format", + projectDir, + "--check", + "--actions", + relativeActionPath, + ]); + checkEqual( + unformattedCheck.status, + 1, + `--check must fail: ${describeResult(unformattedCheck)}`, + ); + checkIncludes(unformattedCheck.stderr, "to_format.sqlx", "--check must list the file"); + + const format = execCli(["format", projectDir, "--actions", relativeActionPath]); + checkEqual(format.status, 0, `format must exit with 0: ${describeResult(format)}`); + const formatted = fs.readFileSync(filePath, "utf8"); + check(!formatted.includes("\r\n"), "format must write LF line endings"); + + // The same formatted content checked out with CRLF (git core.autocrlf=true) must pass. + fs.writeFileSync(filePath, formatted.replace(/\n/g, "\r\n")); + const crlfCheck = execCli(["format", projectDir, "--check", "--actions", relativeActionPath]); + checkEqual(crlfCheck.status, 0, `--check must accept CRLF: ${describeResult(crlfCheck)}`); + checkIncludes(crlfCheck.stdout, "All files are formatted correctly", "--check must pass"); + } finally { + fs.rmSync(filePath, { force: true }); + } + }); + + await runTest("CLI Flow 8: dataform init-creds --help", () => { + const result = execCli(["init-creds", "--help"]); + checkEqual(result.status, 0, `init-creds --help must exit with 0: ${describeResult(result)}`); + checkIncludes(result.stdout, ".df-credentials.json", "help must mention the credentials file"); + checkIncludes(result.stdout, "--test-connection", "help must mention --test-connection"); + }); +} + +async function main(): Promise { + console.log("=== Dataform CLI smoke tests ==="); + console.log(`OS: ${os.platform()} ${os.release()}, Node: ${process.version}`); + console.log(`Shell: ${shellMode}`); + console.log(`Launcher: ${binary.shimPath || "(none)"}`); + console.log(`Bundle: ${binary.bundlePath || "(not found)"}`); + console.log(`Core tarball: ${coreTarball || "(none; installing from the npm registry)"}`); + console.log("================================\n"); + + const projectDir = createTempDir("df-smoke-"); + try { + await runCliFlows(projectDir); + } finally { + removeDir(projectDir); + } + + console.log(`\nResults: ${passed} passed, ${failed} failed.`); + process.exit(failed > 0 ? 1 : 0); +} + +main().catch((error) => { + console.error("Unhandled error in the smoke test runner:", error); + process.exit(1); +}); diff --git a/contributing.md b/contributing.md index 96047b8a6..0fae0b8ef 100644 --- a/contributing.md +++ b/contributing.md @@ -115,6 +115,62 @@ pnpm will use the registry specified in ~/.npmrc by default. When building in th Additionally, installed NPM dependencies need to be added to the `deps` of `ts_library` rules by prefixing them with `//:node_modules/...`. +## Windows Support + +The Dataform CLI is supported on Windows 10 and 11 in both **Command Prompt (`cmd.exe`)** and **PowerShell**. + +### Prerequisites & Setup + +- **Node.js**: Node 22+ LTS is required. +- **Git line endings**: This repository includes a `.gitattributes` file (`* text=auto eol=lf`) that enforces LF line endings automatically without changing your global Git configuration. For user Dataform projects checked out with CRLF (`core.autocrlf=true`), `dataform format` and `dataform format --check` are CRLF-aware and normalize line endings before comparing. +- **Symlinks & Long Paths**: If using Bazel or deep `node_modules` hierarchies on Windows, enable [Windows Developer Mode](https://learn.microsoft.com/en-us/windows/apps/get-started/enable-your-device-for-development) or enable `core.longpaths` for this repository: + ```bash + git config core.longpaths true + ``` + +### Development & Path Conventions + +- **Canonical Forward Slashes**: Dataform's internal graph contracts, table names, and proto file paths require canonical POSIX forward slashes (`/`). Even on Windows, definitions must resolve as `definitions/my_table.sqlx` rather than with backslashes (`\`). +- **Cross-Platform Path Helpers**: Use the `Path` utility in `core/path.ts` (`Path.toPosixPath`, `Path.startsWithPath`, `Path.join`, `Path.normalize`, `Path.relativePath`, `Path.dirName`, `Path.basename`). It handles mixed separators and Windows drive letters (e.g. `C:\` vs `c:/`) automatically. + +### Running Windows Smoke Tests + +The smoke test suite lives in `cli/tests/windows/smoke_test.ts` and is bundled by Bazel into a +single file that only needs Node.js to run. Build it on Linux, macOS or WSL (or download the +`windows-smoke-test` artifact from a CI run of `.github/workflows/test.yaml`): + +```bash +bazel build //cli/tests/windows:smoke_test_bundle //packages/@dataform/cli:package_tar //packages/@dataform/core:package_tar +# -> bazel-bin/cli/tests/windows/smoke_test_bundle.js +``` + +Then, on Windows, install the CLI globally and run the bundle through the shell you want to test: + +- **In PowerShell**: + + ```powershell + npm install -g path\to\cli\package.tar.gz + node smoke_test_bundle.js --shell powershell --core-tarball path\to\core\package.tar.gz + ``` + +- **In Command Prompt (`cmd.exe`)**: + + ```cmd + npm install -g path\to\cli\package.tar.gz + node smoke_test_bundle.js --shell cmd --core-tarball path\to\core\package.tar.gz + ``` + +Without `--core-tarball`, the `@dataform/core` version pinned by `dataform init` is installed from +the npm registry instead. Without `--shell`, the CLI bundle is run directly with Node (bypassing +the `dataform.cmd` launcher), which also works on Linux and macOS. + +### Windows CI on GitHub Actions + +All pull requests run the `CLI smoke tests (Windows, Node 22)` and +`CLI smoke tests (Windows, Node 24)` jobs on `windows-latest` via `.github/workflows/test.yaml`. +Each installs the CLI tarball built by the Linux job and runs the smoke tests under both PowerShell +and `cmd.exe`, with `@dataform/core` installed from the locally built tarball. + ## The Contribution Process 1. Decide on what you'd like to contribute. The majority of open-source contributions come from: diff --git a/core/BUILD b/core/BUILD index 10c88a9a1..b8e1d94cf 100644 --- a/core/BUILD +++ b/core/BUILD @@ -94,6 +94,7 @@ ts_test_suite( "main_session_test.ts", "main_sqlx_chars_test.ts", "main_workflow_settings_test.ts", + "path_test.ts", "utils_test.ts", "workflow_settings_test.ts", ], diff --git a/core/compilers.ts b/core/compilers.ts index 38cf3823c..bc87242d7 100644 --- a/core/compilers.ts +++ b/core/compilers.ts @@ -68,7 +68,7 @@ function compileSqlx(rootNode: SyntaxTreeNode, path: string): string { return `dataform.sqlxAction({ sqlxConfig: { - name: "${Path.escapedBasename(path)}", + name: ${JSON.stringify(Path.basename(path))}, type: "operations", ...${config || "{}"} }, diff --git a/core/compilers_test.ts b/core/compilers_test.ts index d5e526552..7481582b6 100644 --- a/core/compilers_test.ts +++ b/core/compilers_test.ts @@ -16,6 +16,11 @@ suite("core/compilers", () => { expect(result).to.include("select 1"); }); + test("escapes the action name derived from the file name", () => { + const result = compile("select 1", 'definitions/a"b.sqlx'); + expect(result).to.include('name: "a\\"b"'); + }); + test("compiles yml to js", () => { const code = "foo: bar"; const path = "definitions/foo.yml"; diff --git a/core/main.ts b/core/main.ts index d218d9c7c..7cf49bd26 100644 --- a/core/main.ts +++ b/core/main.ts @@ -38,6 +38,11 @@ export function main(coreExecutionRequest: Uint8Array | string): Uint8Array | st request = dataform.CoreExecutionRequest.decode(coreExecutionRequest); } const compileRequest = request.compile; + if (compileRequest?.compileConfig?.filePaths) { + compileRequest.compileConfig.filePaths = compileRequest.compileConfig.filePaths.map( + Path.toPosixPath, + ); + } // Allow extensions to populate settings by themselves. const failIfMissing = !compileRequest?.compileConfig?.extension?.compilationMode; diff --git a/core/main_js_api_test.ts b/core/main_js_api_test.ts index 396bfe956..625c05356 100644 --- a/core/main_js_api_test.ts +++ b/core/main_js_api_test.ts @@ -1,4 +1,6 @@ import { expect } from "chai"; +import * as fs from "fs"; +import * as path from "path"; import { dataform, google } from "df/protos/ts"; import { @@ -623,6 +625,29 @@ dataform.jitData("key", {test: () => {}}); ); }); + test("getContents resolves parent-relative paths that remain inside rootDir", () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + writeWorkflowSettingsFile(projectDir, VALID_WORKFLOW_SETTINGS_YAML); + fs.mkdirSync(path.join(projectDir, "docs")); + fs.writeFileSync(path.join(projectDir, "docs", "descriptions.md"), "# Shared description"); + writeDefinitionFile( + projectDir, + "table.sqlx", + `config { + type: "table", + description: getContents('../docs/descriptions.md'), + } + SELECT 1 AS test`, + ); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(result.compile.compiledGraph.tables[0].actionDescriptor.description).to.equal( + `# Shared description`, + ); + }); + test("throws error for invalid missing markedown", () => { const projectDir = tmpDirFixture.createNewTmpDir(); writeWorkflowSettingsFile(projectDir, VALID_WORKFLOW_SETTINGS_YAML); @@ -662,6 +687,32 @@ dataform.jitData("key", {test: () => {}}); "outside the project directory", ); }); + + test("throws error for a sibling directory differing only by case", () => { + // With root ".../Proj", "../../proj/secret.md" must not be readable on case-sensitive + // file systems, even though it would be the same directory on Windows. + const parentDir = tmpDirFixture.createNewTmpDir(); + const projectDir = path.join(parentDir, "Proj"); + fs.mkdirSync(projectDir); + fs.mkdirSync(path.join(parentDir, "proj")); + fs.writeFileSync(path.join(parentDir, "proj", "secret.md"), "secret"); + writeWorkflowSettingsFile(projectDir, VALID_WORKFLOW_SETTINGS_YAML); + writeDefinitionFile( + projectDir, + "table.sqlx", + `config { + type: "table", + description: getContents('../../proj/secret.md'), + } + SELECT 1 AS test`, + ); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors[0].message).to.include( + "outside the project directory", + ); + }); }); suite("invalid options", () => { [ diff --git a/core/main_session_test.ts b/core/main_session_test.ts index 0dce5d3a1..7007471ad 100644 --- a/core/main_session_test.ts +++ b/core/main_session_test.ts @@ -279,6 +279,27 @@ actions: ]); }); + test("backslash file paths in the compile config are normalized to forward slashes", () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + writeWorkflowSettingsFile(projectDir, VALID_WORKFLOW_SETTINGS_YAML); + writeDefinitionFile(projectDir, "a.sqlx", `config { type: "table" } SELECT 1 AS test`); + const coreExecutionRequest = dataform.CoreExecutionRequest.create({ + compile: { + compileConfig: { + projectDir, + filePaths: ["definitions\\a.sqlx"], + }, + }, + }); + + const result = runMainInVm(coreExecutionRequest); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(result.compile.compiledGraph.tables.map((table) => table.fileName)).deep.equals([ + "definitions/a.sqlx", + ]); + }); + test("fails when non-unique target", () => { const projectDir = tmpDirFixture.createNewTmpDir(); writeWorkflowSettingsFile(projectDir, VALID_WORKFLOW_SETTINGS_YAML); diff --git a/core/path.ts b/core/path.ts index 51f0e74f2..1aded435b 100644 --- a/core/path.ts +++ b/core/path.ts @@ -1,37 +1,88 @@ -export const separator = (() => { - if (typeof process !== "undefined") { - return process.platform === "win32" ? "\\" : "/"; +export const separator = "/"; + +/** + * Replaces every backslash with a forward slash, on every platform, so the compiled graph is the + * same on Windows and POSIX. As a result, a literal backslash in a POSIX file name (e.g. + * `definitions/a\b.sqlx`) is treated as a separator; such files can't be checked out on Windows + * anyway. Repeated separators are deliberately kept, so a UNC root such as `\\server\share\proj` + * becomes `//server/share/proj` rather than a POSIX-looking `/server/share/proj`. + */ +export function toPosixPath(path: string): string { + return path ? path.replace(/\\/g, "/") : path; +} + +const WINDOWS_DRIVE_LETTER = /^[a-zA-Z]:(?=\/|$)/; + +/** + * Normalizes a path for comparison purposes: forward slashes and a lower-cased Windows drive + * letter, so that `C:/proj` and `c:/proj` compare equal. Everything else keeps its case, because + * POSIX file systems are case-sensitive. + */ +export function comparablePath(path: string): string { + const posixPath = toPosixPath(path); + return WINDOWS_DRIVE_LETTER.test(posixPath) + ? posixPath[0].toLowerCase() + posixPath.slice(1) + : posixPath; +} + +/** + * Returns true if `path` is `base` itself or is located inside `base`. Only Windows drive letters + * are compared case-insensitively. + */ +export function startsWithPath(path: string, base: string): boolean { + if (!path || !base) { + return false; } - return "/"; -})(); + const withTrailingSlash = (p: string) => (p.endsWith("/") ? p : p + "/"); + return withTrailingSlash(comparablePath(path)).startsWith( + withTrailingSlash(comparablePath(base)), + ); +} +/** + * Returns `fullPath` relative to `base`, using forward slashes: + * - `definitions/a.sqlx` for a path inside `base`, + * - `""` when `fullPath` is `base` itself, + * - the normalized `fullPath` unchanged when `base` is empty or `fullPath` lies outside of it. + */ export function relativePath(fullPath: string, base: string) { + const normalizedFull = toPosixPath(fullPath); if (base.length === 0) { - return fullPath; + return normalizedFull; } - const stripped = fullPath.substr(base.length); - if (stripped.startsWith(separator)) { - return stripped.substr(1); - } else { - return stripped; + const normalizedBase = toPosixPath(base); + if (!startsWithPath(normalizedFull, normalizedBase)) { + return normalizedFull; } + const stripped = normalizedFull.slice(normalizedBase.length); + return stripped.startsWith("/") ? stripped.slice(1) : stripped; } export function filename(path: string) { - return path.split(separator).slice(-1)[0]; + return toPosixPath(path).split("/").slice(-1)[0]; } export function basename(path: string) { const f = filename(path); - return f.substring(0, f.lastIndexOf(".")); + const dotIndex = f.lastIndexOf("."); + return dotIndex <= 0 ? f : f.substring(0, dotIndex); } export function dirName(fullPath: string) { - return fullPath.slice(0, fullPath.lastIndexOf(separator)); + const normalized = toPosixPath(fullPath); + const lastSlash = normalized.lastIndexOf("/"); + if (lastSlash === -1) { + return ""; + } + if (lastSlash === 0 || /^[a-zA-Z]:$/.test(normalized.slice(0, lastSlash))) { + return normalized.slice(0, lastSlash + 1); + } + return normalized.slice(0, lastSlash); } export function join(...paths: string[]) { return paths + .map(toPosixPath) .map((path) => { if (path.startsWith(separator)) { path = path.slice(1); @@ -41,21 +92,21 @@ export function join(...paths: string[]) { } return path; }) + .filter((path) => path.length > 0) .join(separator); } -export function escapedBasename(path: string) { - return basename(path).replace(/\\/g, "\\\\"); -} - export function fileExtension(fullPath: string) { - return fullPath.split(".").slice(-1)[0]; + const f = filename(fullPath); + const dotIndex = f.lastIndexOf("."); + return dotIndex <= 0 ? "" : f.slice(dotIndex + 1); } export function normalize(path: string) { + const normalized = toPosixPath(path); const parts = []; let dotDotCount = 0; - for (const part of path.split("/").filter((p) => !!p && p !== ".")) { + for (const part of normalized.split("/").filter((p) => !!p && p !== ".")) { if (part === "..") { if (parts.length === 0) { dotDotCount++; @@ -66,7 +117,7 @@ export function normalize(path: string) { parts.push(part); } } - if (path.startsWith("/")) { + if (normalized.startsWith("/")) { if (parts.length === 0) { return "/"; } diff --git a/core/path_test.ts b/core/path_test.ts new file mode 100644 index 000000000..9c9a77c81 --- /dev/null +++ b/core/path_test.ts @@ -0,0 +1,152 @@ +import { expect } from "chai"; + +import * as Path from "df/core/path"; +import { suite, test } from "df/testing"; + +suite("path utils", () => { + test("separator is always forward slash", () => { + expect(Path.separator).equals("/"); + }); + + test("toPosixPath converts Windows backslashes to forward slashes", () => { + expect(Path.toPosixPath("definitions\\table.sqlx")).equals("definitions/table.sqlx"); + expect(Path.toPosixPath("C:\\Users\\runner\\project\\file.sqlx")).equals( + "C:/Users/runner/project/file.sqlx", + ); + expect(Path.toPosixPath("definitions/table.sqlx")).equals("definitions/table.sqlx"); + expect(Path.toPosixPath("")).equals(""); + }); + + test("toPosixPath keeps the UNC prefix of Windows network paths", () => { + expect(Path.toPosixPath("\\\\server\\share\\proj")).equals("//server/share/proj"); + expect(Path.toPosixPath("\\\\server\\share\\proj\\definitions\\a.sqlx")).equals( + "//server/share/proj/definitions/a.sqlx", + ); + }); + + test("comparablePath lower-cases only the Windows drive letter", () => { + expect(Path.comparablePath("C:\\Proj\\Defs")).equals("c:/Proj/Defs"); + expect(Path.comparablePath("c:/Proj")).equals("c:/Proj"); + expect(Path.comparablePath("C:")).equals("c:"); + expect(Path.comparablePath("/home/Alice")).equals("/home/Alice"); + expect(Path.comparablePath("Definitions/A.sqlx")).equals("Definitions/A.sqlx"); + // Not a drive letter: no slash (or end of string) after the colon. + expect(Path.comparablePath("C:Proj")).equals("C:Proj"); + }); + + test("startsWithPath is case-sensitive except for Windows drive letters", () => { + expect(Path.startsWithPath("/tmp/Proj/definitions/a.sqlx", "/tmp/Proj")).equals(true); + expect(Path.startsWithPath("/tmp/Proj", "/tmp/Proj")).equals(true); + expect(Path.startsWithPath("/tmp/Proj/", "/tmp/Proj")).equals(true); + expect(Path.startsWithPath("/tmp/Proj", "/tmp/Proj/")).equals(true); + expect(Path.startsWithPath("/tmp/Proj/a.sqlx", "/tmp/Proj/")).equals(true); + expect(Path.startsWithPath("/tmp/proj/secret.json", "/tmp/Proj")).equals(false); + expect(Path.startsWithPath("/tmp/Projects/a.sqlx", "/tmp/Proj")).equals(false); + expect(Path.startsWithPath("/other/a.sqlx", "/tmp/Proj")).equals(false); + expect(Path.startsWithPath("/tmp/Proj", "")).equals(false); + expect(Path.startsWithPath("", "/tmp/Proj")).equals(false); + expect(Path.startsWithPath("c:\\proj\\a.sqlx", "C:/proj")).equals(true); + expect(Path.startsWithPath("C:/proj/a.sqlx", "c:\\proj")).equals(true); + expect(Path.startsWithPath("C:/Proj/a.sqlx", "C:/proj")).equals(false); + expect(Path.startsWithPath("D:/proj/a.sqlx", "C:/proj")).equals(false); + expect( + Path.startsWithPath("\\\\server\\share\\proj\\a.sqlx", "\\\\server\\share\\proj"), + ).equals(true); + expect(Path.startsWithPath("/server/share/proj/a.sqlx", "\\\\server\\share\\proj")).equals( + false, + ); + }); + + test("filename extracts file name with POSIX and Windows slashes", () => { + expect(Path.filename("definitions/table.sqlx")).equals("table.sqlx"); + expect(Path.filename("definitions\\table.sqlx")).equals("table.sqlx"); + expect(Path.filename("C:\\Users\\runner\\project\\definitions\\table.sqlx")).equals( + "table.sqlx", + ); + expect(Path.filename("table.sqlx")).equals("table.sqlx"); + }); + + test("basename extracts base name without extension", () => { + expect(Path.basename("definitions/table.sqlx")).equals("table"); + expect(Path.basename("definitions\\table.sqlx")).equals("table"); + expect(Path.basename("C:\\Users\\runner\\project\\definitions\\table.sqlx")).equals("table"); + expect(Path.basename("no_ext")).equals("no_ext"); + expect(Path.basename("multi.dot.name.sqlx")).equals("multi.dot.name"); + expect(Path.basename(".gitignore")).equals(".gitignore"); + expect(Path.basename("definitions/.gitignore")).equals(".gitignore"); + expect(Path.basename(".df-credentials.json")).equals(".df-credentials"); + }); + + test("dirName extracts directory path with POSIX and Windows slashes", () => { + expect(Path.dirName("definitions/table.sqlx")).equals("definitions"); + expect(Path.dirName("definitions\\table.sqlx")).equals("definitions"); + expect(Path.dirName("C:\\project\\definitions\\table.sqlx")).equals("C:/project/definitions"); + expect(Path.dirName("/table.sqlx")).equals("/"); + expect(Path.dirName("C:\\table.sqlx")).equals("C:/"); + expect(Path.dirName("C:/table.sqlx")).equals("C:/"); + expect(Path.dirName("table.sqlx")).equals(""); + }); + + test("relativePath normalizes to forward slashes across platforms", () => { + expect(Path.relativePath("/path/to/project/definitions/file.sqlx", "/path/to/project")).equals( + "definitions/file.sqlx", + ); + expect(Path.relativePath("/path/to/project/definitions/file.sqlx", "/path/to/project/")).equals( + "definitions/file.sqlx", + ); + expect(Path.relativePath("C:\\project\\definitions\\file.sqlx", "C:\\project")).equals( + "definitions/file.sqlx", + ); + expect(Path.relativePath("c:\\project\\definitions\\file.sqlx", "C:\\project")).equals( + "definitions/file.sqlx", + ); + expect(Path.relativePath("C:/project/definitions/file.sqlx", "C:\\project")).equals( + "definitions/file.sqlx", + ); + expect(Path.relativePath("file.sqlx", "")).equals("file.sqlx"); + }); + + test("relativePath ignores case only in the Windows drive letter", () => { + expect(Path.relativePath("/home/Alice/definitions/a.sqlx", "/home/alice")).equals( + "/home/Alice/definitions/a.sqlx", + ); + expect(Path.relativePath("C:/Project/definitions/a.sqlx", "C:/project")).equals( + "C:/Project/definitions/a.sqlx", + ); + }); + + test("relativePath returns empty string for the base itself and the full path when outside", () => { + expect(Path.relativePath("/home/alice", "/home/alice")).equals(""); + expect(Path.relativePath("/home/alice/", "/home/alice")).equals(""); + expect(Path.relativePath("/home/alice", "/home/alice/")).equals(""); + expect(Path.relativePath("C:\\project", "c:/project")).equals(""); + expect(Path.relativePath("/other/z.sqlx", "/home/alice")).equals("/other/z.sqlx"); + expect(Path.relativePath("/home/alice-other/z.sqlx", "/home/alice")).equals( + "/home/alice-other/z.sqlx", + ); + }); + + test("join combines paths with forward slashes", () => { + expect(Path.join("a", "b", "c")).equals("a/b/c"); + expect(Path.join("a\\b", "c\\d")).equals("a/b/c/d"); + expect(Path.join("/a/", "/b/")).equals("a/b"); + }); + + test("fileExtension extracts extension", () => { + expect(Path.fileExtension("definitions/file.sqlx")).equals("sqlx"); + expect(Path.fileExtension("definitions\\file.sqlx")).equals("sqlx"); + expect(Path.fileExtension("no_extension")).equals(""); + expect(Path.fileExtension(".gitignore")).equals(""); + expect(Path.fileExtension("definitions/.gitignore")).equals(""); + expect(Path.fileExtension(".df-credentials.json")).equals("json"); + // Only the file name is inspected, not dots in directory names. + expect(Path.fileExtension("definitions.v2/no_extension")).equals(""); + expect(Path.fileExtension("definitions.v2/file.sqlx")).equals("sqlx"); + }); + + test("normalize handles both slash types and dot navigation", () => { + expect(Path.normalize("a/b/../c")).equals("a/c"); + expect(Path.normalize("a\\b\\..\\c")).equals("a/c"); + expect(Path.normalize("/a/b/../../c")).equals("/c"); + }); +}); diff --git a/core/session.ts b/core/session.ts index d0ac460f5..e596eb9ac 100644 --- a/core/session.ts +++ b/core/session.ts @@ -104,13 +104,14 @@ export class Session { public getContents(filePath: string): string { const callerFile = utils.getCallerFile(this.rootDir); const callerDir = Path.dirName(callerFile); - const resolvedPath = Path.join(callerDir, filePath); - const absolutePath = Path.separator + Path.normalize(Path.join(this.rootDir, resolvedPath)); - const rootDir = this.rootDir.endsWith(Path.separator) - ? this.rootDir - : this.rootDir + Path.separator; - - if (!absolutePath.startsWith(rootDir)) { + const rootDir = Path.toPosixPath(this.rootDir); + // Keep the root's leading slashes verbatim: "/" for POSIX, "//" for UNC (\\server\share) and + // none for drive-letter paths (C:/...). Path.join and Path.normalize strip them. + const rootPrefix = /^\/*/.exec(rootDir)[0]; + const absolutePath = + rootPrefix + Path.normalize(Path.join(rootDir.slice(rootPrefix.length), callerDir, filePath)); + + if (!Path.startsWithPath(absolutePath, rootDir)) { throw new Error(`Cannot read "${filePath}": path resolves outside the project directory.`); } diff --git a/core/utils.ts b/core/utils.ts index 59bf628a8..c4c09f391 100644 --- a/core/utils.ts +++ b/core/utils.ts @@ -47,11 +47,13 @@ export function getCallerFile(rootDir: string) { let lastfile: string; const stack = getCurrentStack(); while (stack.length) { - const nextLastfile = stack.shift().getFileName(); - if (!nextLastfile) { + const rawNextLastfile = stack.shift().getFileName(); + if (!rawNextLastfile) { continue; } - if (!nextLastfile.includes(rootDir)) { + const nextLastfile = Path.toPosixPath(rawNextLastfile); + // An empty rootDir (a host that doesn't set projectDir) matches every file, as before. + if (rootDir && !Path.startsWithPath(nextLastfile, rootDir)) { continue; } if (nextLastfile.includes("node_modules")) { @@ -60,10 +62,7 @@ export function getCallerFile(rootDir: string) { // If it's in the root directory we'll take it, but keep searching // for a better match. lastfile = nextLastfile; - if (!( - nextLastfile.includes(`definitions${Path.separator}`) || - nextLastfile.includes(`models${Path.separator}`) - )) { + if (!(nextLastfile.includes("definitions/") || nextLastfile.includes("models/"))) { continue; } break; diff --git a/core/utils_test.ts b/core/utils_test.ts index 1b16b8e72..c03a08b3c 100644 --- a/core/utils_test.ts +++ b/core/utils_test.ts @@ -1,5 +1,6 @@ import { expect } from "chai"; import { + getCallerFile, getConnectionForIcebergTable, getEffectiveBucketName, getEffectiveTableFolderRoot, @@ -360,4 +361,41 @@ suite("Dataform Utility Validations", () => { ); }); }); + + suite("getCallerFile", () => { + test("returns a relative POSIX path for Windows paths with differing drive-letter case", () => { + const globalAny = global as any; + const previousCurrentFile = globalAny.__dataform_current_file; + try { + globalAny.__dataform_current_file = "c:\\project\\definitions\\sub\\table.sqlx"; + expect(getCallerFile("C:\\project")).to.equal("definitions/sub/table.sqlx"); + } finally { + globalAny.__dataform_current_file = previousCurrentFile; + } + }); + + test("returns a relative POSIX path for Windows UNC paths", () => { + const globalAny = global as any; + const previousCurrentFile = globalAny.__dataform_current_file; + try { + globalAny.__dataform_current_file = "\\\\server\\share\\proj\\definitions\\table.sqlx"; + expect(getCallerFile("\\\\server\\share\\proj")).to.equal("definitions/table.sqlx"); + } finally { + globalAny.__dataform_current_file = previousCurrentFile; + } + }); + + test("accepts any caller file when rootDir is empty", () => { + const globalAny = global as any; + const previousCurrentFile = globalAny.__dataform_current_file; + try { + // Hosts that don't set projectDir pass an empty rootDir; the stack must still be searched + // instead of failing with "Unable to find valid caller file". + globalAny.__dataform_current_file = undefined; + expect(() => getCallerFile("")).to.not.throw(); + } finally { + globalAny.__dataform_current_file = previousCurrentFile; + } + }); + }); }); diff --git a/vscode/workflow_settings_yaml.schema.json b/vscode/workflow_settings_yaml.schema.json index 89e76da0f..37a2a95db 100644 --- a/vscode/workflow_settings_yaml.schema.json +++ b/vscode/workflow_settings_yaml.schema.json @@ -1,58 +1,58 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "$comment": "Created from 'protos/configs.proto'. Even options are specified as 'required' in proto, they might be specified from the CLI options, so leave them optional except for the dataformCoreVersion which is required in compile.", - "additionalProperties": false, - "properties": { - "dataformCoreVersion": { - "type": "string", - "description": "The desired dataform core version to compile against." - }, - "defaultProject": { - "type": "string", - "description": "The default Google Cloud project (database)." - }, - "defaultDataset": { - "type": "string", - "description": "The default dataset (schema)." - }, - "defaultLocation": { - "type": "string", - "description": "The default BigQuery location to use." - }, - "defaultAssertionDataset": { - "type": "string", - "description": "The default dataset (schema) for assertions." - }, - "vars": { - "type": "object", - "description": "User-defined variables that are made available to project code during compilation. An object containing a list of key-value pairs.", - "additionalProperties": { - "type": "string" - } - }, - "projectSuffix": { - "type": "string", - "description": "The suffix to append to all Google Cloud project references." - }, - "datasetSuffix": { - "type": "string", - "description": "The suffix to append to all dataset references." - }, - "namePrefix": { - "type": "string", - "description": "The prefix to append to all action names." - }, - "defaultNotebookRuntimeOptions": { - "type": "object", - "description": "Default runtime options for Notebook actions.", - "outputBucket": { - "type": "string", - "description": "Storage bucket to output notebooks to after their execution." - } - } - }, - "required": [ - "dataformCoreVersion" - ] +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "$comment": "Created from 'protos/configs.proto'. Even options are specified as 'required' in proto, they might be specified from the CLI options, so leave them optional except for the dataformCoreVersion which is required in compile.", + "additionalProperties": false, + "properties": { + "dataformCoreVersion": { + "type": "string", + "description": "The desired dataform core version to compile against." + }, + "defaultProject": { + "type": "string", + "description": "The default Google Cloud project (database)." + }, + "defaultDataset": { + "type": "string", + "description": "The default dataset (schema)." + }, + "defaultLocation": { + "type": "string", + "description": "The default BigQuery location to use." + }, + "defaultAssertionDataset": { + "type": "string", + "description": "The default dataset (schema) for assertions." + }, + "vars": { + "type": "object", + "description": "User-defined variables that are made available to project code during compilation. An object containing a list of key-value pairs.", + "additionalProperties": { + "type": "string" + } + }, + "projectSuffix": { + "type": "string", + "description": "The suffix to append to all Google Cloud project references." + }, + "datasetSuffix": { + "type": "string", + "description": "The suffix to append to all dataset references." + }, + "namePrefix": { + "type": "string", + "description": "The prefix to append to all action names." + }, + "defaultNotebookRuntimeOptions": { + "type": "object", + "description": "Default runtime options for Notebook actions.", + "outputBucket": { + "type": "string", + "description": "Storage bucket to output notebooks to after their execution." + } + } + }, + "required": [ + "dataformCoreVersion" + ] } \ No newline at end of file