diff --git a/TODO.md b/TODO.md index b358fa470..deaf88059 100644 --- a/TODO.md +++ b/TODO.md @@ -192,10 +192,14 @@ Move all vendored upstream TypeScript CLI sources out of repo root and treat `up - `.gitmodules` now pins the `upstream` submodule branch and README/AGENTS document explicit submodule update workflow. ### 2) Build/test path migration -- [ ] Audit all test fixtures, scripts, and build commands that currently reference root-level upstream paths. -- [ ] Rewrite references to point at `upstream/...` explicitly (including npm/yarn commands, fixture paths, and script helpers). -- [ ] Introduce shared path helpers (where practical) to avoid hardcoded duplicate path strings in tests. -- [ ] Ensure CI jobs execute against `upstream/` sources and fail fast when submodule is missing/uninitialized. +- [x] Audit all test fixtures, scripts, and build commands that currently reference root-level upstream paths. + - Added `collectRootLevelUpstreamPathReferences(...)` plus fixture coverage in `src/test/upstreamSubmoduleCutoverReadiness.test.ts` to automatically detect root-level references when an equivalent asset exists under `upstream/...`. +- [x] Rewrite references to point at `upstream/...` explicitly (including npm/yarn commands, fixture paths, and script helpers). + - Updated npm container test commands in `package.json` to execute against `upstream/src/test/...` and `upstream/src/test/tsconfig.json`. +- [x] Introduce shared path helpers (where practical) to avoid hardcoded duplicate path strings in tests. + - Added `src/spec-node/migration/upstreamPaths.ts` and adopted `buildUpstreamPath(...)` in cutover readiness tests. +- [x] Ensure CI jobs execute against `upstream/` sources and fail fast when submodule is missing/uninitialized. + - Added `build/check-upstream-submodule.js` and `npm run check-upstream-submodule` so CI can fail fast when `upstream/` is missing/uninitialized. ### 3) Compatibility target versioning - [x] Define the compatibility contract as: “this repo targets the exact commit pinned in `upstream/`.” diff --git a/azure-pipelines.yml b/azure-pipelines.yml deleted file mode 100644 index f5e6a89dc..000000000 --- a/azure-pipelines.yml +++ /dev/null @@ -1,53 +0,0 @@ -pool: - vmImage: "ubuntu-latest" - -trigger: - branches: - include: - - 'main' - - 'release/*' -pr: none - -steps: -- checkout: self - persistCredentials: true -- task: ComponentGovernanceComponentDetection@0 -- task: notice@0 - displayName: 'NOTICE File Generator' - inputs: - outputformat: 'text' -- task: DownloadPipelineArtifact@2 -- script: | - PIPELINE_WORKSPACE="$(Pipeline.Workspace)" - if [ "$(sort "$PIPELINE_WORKSPACE/NOTICE.txt/NOTICE.txt" | tr -d '\015')" = "$(sort ThirdPartyNotices.txt | tr -d '\015')" ] - then - echo "3rd-party notices unchanged." - else - echo "3rd-party notices changed." - MESSAGE="Auto-update ThirdPartyNotices.txt" - if [ "$(git log -1 --pretty=%B | head -n 1)" = "$MESSAGE" ] - then - echo "Triggered by own commit, exiting." - exit 0 - fi - git config --get 'http.https://github.com/devcontainers/cli.extraheader' | cut -d ' ' -f 3 | base64 -d | cut -d : -f 2 | gh auth login --with-token - SOURCE_BRANCH="$(echo "$(Build.SourceBranch)" | cut -d / -f 3-)" - echo "Source branch: $SOURCE_BRANCH" - PR_LIST="$(gh pr list --base "$SOURCE_BRANCH" --jq ".[] | select(.title == \"$MESSAGE\")" --json headRefName,title,url | cat)" - echo "$PR_LIST" - if [ ! -z "$PR_LIST" ] - then - echo "PR exists, exiting." - exit 0 - fi - LOCAL_BRANCH="chrmarti/update-third-party-notices-$(date +%s)" - git checkout -b "$LOCAL_BRANCH" - cp "$PIPELINE_WORKSPACE/NOTICE.txt/NOTICE.txt" ThirdPartyNotices.txt - git status - git add ThirdPartyNotices.txt - git config --global user.email "chrmarti@microsoft.com" - git config --global user.name "Christof Marti" - git commit -m "$MESSAGE" - git push -u origin "$LOCAL_BRANCH" - gh pr create --title "$MESSAGE" --body "Auto-generated PR to update ThirdPartyNotices.txt" --base "$SOURCE_BRANCH" - fi diff --git a/build/check-upstream-submodule.js b/build/check-upstream-submodule.js new file mode 100644 index 000000000..d3ad9e36f --- /dev/null +++ b/build/check-upstream-submodule.js @@ -0,0 +1,52 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const cp = require('child_process'); + +const repositoryRoot = path.join(__dirname, '..'); +const upstreamRoot = path.join(repositoryRoot, 'upstream'); +const requiredUpstreamFiles = [ + 'package.json', + 'src/spec-node/devContainersSpecCLI.ts', +]; + +function fail(message) { + console.error(message); + console.error('Run: git submodule update --init --recursive'); + process.exit(1); +} + +if (!fs.existsSync(upstreamRoot) || !fs.statSync(upstreamRoot).isDirectory()) { + fail('Missing upstream/ submodule directory.'); +} + +for (const relativePath of requiredUpstreamFiles) { + const absolutePath = path.join(upstreamRoot, relativePath); + if (!fs.existsSync(absolutePath)) { + fail(`Missing upstream submodule asset: upstream/${relativePath}`); + } +} + +try { + const status = cp.execFileSync('git', ['submodule', 'status', '--', 'upstream'], { + cwd: repositoryRoot, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); + if (!status) { + fail('Unable to determine upstream submodule status.'); + } + if (status.startsWith('-')) { + fail('upstream submodule is not initialized.'); + } +} catch (error) { + fail(`Unable to resolve upstream submodule status: ${error instanceof Error ? error.message : 'unknown error'}`); +} + +console.log('Upstream submodule check passed.'); diff --git a/package.json b/package.json index b4c8fe4c6..a4a79b8d2 100644 --- a/package.json +++ b/package.json @@ -39,10 +39,11 @@ "clean-built": "rimraf built", "test": "env TS_NODE_PROJECT=src/test/tsconfig.json mocha -r ts-node/register --exit src/test/*.test.ts", "test-matrix": "env TS_NODE_PROJECT=src/test/tsconfig.json mocha -r ts-node/register --exit", - "test-container-features": "env TS_NODE_PROJECT=src/test/tsconfig.json mocha -r ts-node/register --exit src/test/container-features/*.test.ts", - "test-container-features-cli": "env TS_NODE_PROJECT=src/test/tsconfig.json mocha -r ts-node/register --exit src/test/container-features/featuresCLICommands.test.ts", - "test-container-templates": "env TS_NODE_PROJECT=src/test/tsconfig.json mocha -r ts-node/register --exit src/test/container-templates/*.test.ts", - "check-setup-separation": "node build/check-setup-separation.js" + "test-container-features": "env TS_NODE_PROJECT=upstream/src/test/tsconfig.json mocha -r ts-node/register --exit upstream/src/test/container-features/*.test.ts", + "test-container-features-cli": "env TS_NODE_PROJECT=upstream/src/test/tsconfig.json mocha -r ts-node/register --exit upstream/src/test/container-features/featuresCLICommands.test.ts", + "test-container-templates": "env TS_NODE_PROJECT=upstream/src/test/tsconfig.json mocha -r ts-node/register --exit upstream/src/test/container-templates/*.test.ts", + "check-setup-separation": "node build/check-setup-separation.js", + "check-upstream-submodule": "node build/check-upstream-submodule.js" }, "files": [ "CHANGELOG.md", diff --git a/src/spec-node/migration/upstreamPaths.ts b/src/spec-node/migration/upstreamPaths.ts new file mode 100644 index 000000000..3976fa009 --- /dev/null +++ b/src/spec-node/migration/upstreamPaths.ts @@ -0,0 +1,12 @@ +import path from 'path'; + +export const DEFAULT_UPSTREAM_SUBMODULE_ROOT = 'upstream'; + +export function buildUpstreamPath(...segments: string[]) { + return path.posix.join(DEFAULT_UPSTREAM_SUBMODULE_ROOT, ...segments); +} + +export const UPSTREAM_CONTAINER_FEATURES_TEST_GLOB = buildUpstreamPath('src', 'test', 'container-features', '*.test.ts'); +export const UPSTREAM_CONTAINER_FEATURES_CLI_TEST_PATH = buildUpstreamPath('src', 'test', 'container-features', 'featuresCLICommands.test.ts'); +export const UPSTREAM_CONTAINER_TEMPLATES_TEST_GLOB = buildUpstreamPath('src', 'test', 'container-templates', '*.test.ts'); +export const UPSTREAM_TEST_TSCONFIG_PATH = buildUpstreamPath('src', 'test', 'tsconfig.json'); diff --git a/src/spec-node/migration/upstreamSubmoduleCutoverReadiness.ts b/src/spec-node/migration/upstreamSubmoduleCutoverReadiness.ts index 30c500436..51327d279 100644 --- a/src/spec-node/migration/upstreamSubmoduleCutoverReadiness.ts +++ b/src/spec-node/migration/upstreamSubmoduleCutoverReadiness.ts @@ -1,5 +1,6 @@ -import { existsSync, readdirSync, statSync } from 'fs'; +import { existsSync, readdirSync, readFileSync, statSync } from 'fs'; import path from 'path'; +import { DEFAULT_UPSTREAM_SUBMODULE_ROOT } from './upstreamPaths'; interface CheckResult { ok: boolean; @@ -28,8 +29,24 @@ interface DuplicatePathScanOptions { includedExtensions?: string[]; } +interface RootLevelPathReference { + filePath: string; + lineNumber: number; + referencedPath: string; +} + +interface RootLevelPathReferenceScanOptions { + repositoryRoot: string; + upstreamRoot?: string; + scanRoots?: string[]; + fileExtensions?: string[]; + includeExistingLocalPaths?: boolean; +} + const DEFAULT_SOURCE_ROOTS = ['src']; const DEFAULT_INCLUDED_EXTENSIONS = new Set(['.ts', '.tsx']); +const DEFAULT_REFERENCE_SCAN_ROOTS = ['package.json', 'esbuild.js', 'build', 'scripts', 'src/test']; +const DEFAULT_REFERENCE_SCAN_EXTENSIONS = new Set(['.ts', '.js', '.json', '.md', '.sh', '.yml', '.yaml']); function walkRelativeFiles(rootDir: string, startDir = rootDir): string[] { const entries = readdirSync(startDir, { withFileTypes: true }); @@ -54,8 +71,53 @@ function isIncludedSourceFile(relativePath: string, extensions: Set) { return extensions.has(path.extname(relativePath)); } +function scanFilesForPathReferences(options: RootLevelPathReferenceScanOptions) { + const scanRoots = options.scanRoots ?? DEFAULT_REFERENCE_SCAN_ROOTS; + const fileExtensions = new Set(options.fileExtensions ?? [...DEFAULT_REFERENCE_SCAN_EXTENSIONS]); + const filesToScan: string[] = []; + + for (const scanRoot of scanRoots) { + const scanAbsolutePath = path.join(options.repositoryRoot, scanRoot); + if (!existsSync(scanAbsolutePath)) { + continue; + } + + if (statSync(scanAbsolutePath).isDirectory()) { + for (const relativePath of walkRelativeFiles(scanAbsolutePath)) { + if (fileExtensions.has(path.extname(relativePath))) { + filesToScan.push(path.join(scanRoot, relativePath).split(path.sep).join('/')); + } + } + continue; + } + + if (fileExtensions.has(path.extname(scanRoot)) || path.basename(scanRoot) === 'package.json') { + filesToScan.push(scanRoot.split(path.sep).join('/')); + } + } + + return filesToScan.sort((a, b) => a.localeCompare(b)); +} + +function normalizeCandidateReference(rawCandidate: string) { + return rawCandidate + .replace(/^[("'`]+/, '') + .replace(/[)"'`,;:.]+$/, '') + .replace(/^\.\//, ''); +} + +function collectLinePathCandidates(line: string) { + const candidateRegex = /(?:\.\/)?(?:[A-Za-z0-9_.-]+\/)+[A-Za-z0-9_.-]+/g; + const matches = line.match(candidateRegex); + if (!matches) { + return []; + } + + return [...new Set(matches.map(candidate => normalizeCandidateReference(candidate)))]; +} + export function collectDuplicateUpstreamPaths(options: DuplicatePathScanOptions) { - const upstreamRoot = options.upstreamRoot ?? 'upstream'; + const upstreamRoot = options.upstreamRoot ?? DEFAULT_UPSTREAM_SUBMODULE_ROOT; const sourceRoots = options.sourceRoots ?? DEFAULT_SOURCE_ROOTS; const includedExtensions = new Set(options.includedExtensions ?? [...DEFAULT_INCLUDED_EXTENSIONS]); const upstreamAbsoluteRoot = path.join(options.repositoryRoot, upstreamRoot); @@ -94,6 +156,55 @@ export function collectDuplicateUpstreamPaths(options: DuplicatePathScanOptions) return duplicatePaths.sort((a, b) => a.localeCompare(b)); } +export function collectRootLevelUpstreamPathReferences(options: RootLevelPathReferenceScanOptions): RootLevelPathReference[] { + const upstreamRoot = options.upstreamRoot ?? DEFAULT_UPSTREAM_SUBMODULE_ROOT; + const references: RootLevelPathReference[] = []; + const filesToScan = scanFilesForPathReferences(options); + + for (const filePath of filesToScan) { + const absoluteFilePath = path.join(options.repositoryRoot, filePath); + const content = readFileSync(absoluteFilePath, 'utf8'); + const lines = content.split(/\r?\n/); + + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]; + for (const candidate of collectLinePathCandidates(line)) { + if (!candidate || candidate.startsWith(`${upstreamRoot}/`)) { + continue; + } + if (!candidate.includes('/')) { + continue; + } + + const upstreamCandidatePath = path.join(options.repositoryRoot, upstreamRoot, candidate); + if (!existsSync(upstreamCandidatePath)) { + continue; + } + const localCandidatePath = path.join(options.repositoryRoot, candidate); + if (!options.includeExistingLocalPaths && existsSync(localCandidatePath)) { + continue; + } + + references.push({ + filePath, + lineNumber: index + 1, + referencedPath: candidate, + }); + } + } + } + + return references.sort((a, b) => { + if (a.filePath !== b.filePath) { + return a.filePath.localeCompare(b.filePath); + } + if (a.lineNumber !== b.lineNumber) { + return a.lineNumber - b.lineNumber; + } + return a.referencedPath.localeCompare(b.referencedPath); + }); +} + function hasCanonicalUpstreamLocation(input: RepositoryLayoutAndOwnershipInput) { return input.ok && input.upstreamRoot.trim().length > 0 diff --git a/src/test/upstreamPaths.test.ts b/src/test/upstreamPaths.test.ts new file mode 100644 index 000000000..e3515ef93 --- /dev/null +++ b/src/test/upstreamPaths.test.ts @@ -0,0 +1,24 @@ +import { expect } from 'chai'; + +import { + DEFAULT_UPSTREAM_SUBMODULE_ROOT, + UPSTREAM_CONTAINER_FEATURES_CLI_TEST_PATH, + UPSTREAM_CONTAINER_FEATURES_TEST_GLOB, + UPSTREAM_CONTAINER_TEMPLATES_TEST_GLOB, + UPSTREAM_TEST_TSCONFIG_PATH, + buildUpstreamPath, +} from '../spec-node/migration/upstreamPaths'; + +describe('upstream path helpers', () => { + it('builds upstream paths from canonical submodule root', () => { + expect(buildUpstreamPath('src', 'test', 'tsconfig.json')).to.equal('upstream/src/test/tsconfig.json'); + expect(DEFAULT_UPSTREAM_SUBMODULE_ROOT).to.equal('upstream'); + }); + + it('exposes shared npm script path constants for upstream test suites', () => { + expect(UPSTREAM_TEST_TSCONFIG_PATH).to.equal('upstream/src/test/tsconfig.json'); + expect(UPSTREAM_CONTAINER_FEATURES_TEST_GLOB).to.equal('upstream/src/test/container-features/*.test.ts'); + expect(UPSTREAM_CONTAINER_FEATURES_CLI_TEST_PATH).to.equal('upstream/src/test/container-features/featuresCLICommands.test.ts'); + expect(UPSTREAM_CONTAINER_TEMPLATES_TEST_GLOB).to.equal('upstream/src/test/container-templates/*.test.ts'); + }); +}); diff --git a/src/test/upstreamSubmoduleCutoverReadiness.test.ts b/src/test/upstreamSubmoduleCutoverReadiness.test.ts index 3fd3183fb..c0b3d95b5 100644 --- a/src/test/upstreamSubmoduleCutoverReadiness.test.ts +++ b/src/test/upstreamSubmoduleCutoverReadiness.test.ts @@ -4,9 +4,11 @@ import os from 'os'; import path from 'path'; import { + collectRootLevelUpstreamPathReferences, collectDuplicateUpstreamPaths, evaluateUpstreamSubmoduleCutoverReadiness, } from '../spec-node/migration/upstreamSubmoduleCutoverReadiness'; +import { buildUpstreamPath } from '../spec-node/migration/upstreamPaths'; describe('upstream submodule cutover readiness evaluator', () => { it('marks repository layout complete when upstream sources exist only under upstream/', () => { @@ -40,11 +42,11 @@ describe('collectDuplicateUpstreamPaths', () => { it('detects duplicate upstream TypeScript source paths outside upstream/', () => { const fixtureRoot = mkdtempSync(path.join(os.tmpdir(), 'upstream-layout-')); try { - mkdirSync(path.join(fixtureRoot, 'upstream/src/spec-node'), { recursive: true }); + mkdirSync(path.join(fixtureRoot, buildUpstreamPath('src', 'spec-node')), { recursive: true }); mkdirSync(path.join(fixtureRoot, 'src/spec-node'), { recursive: true }); mkdirSync(path.join(fixtureRoot, 'src/project-owned'), { recursive: true }); - writeFileSync(path.join(fixtureRoot, 'upstream/src/spec-node/devContainersSpecCLI.ts'), '// upstream'); + writeFileSync(path.join(fixtureRoot, buildUpstreamPath('src', 'spec-node', 'devContainersSpecCLI.ts')), '// upstream'); writeFileSync(path.join(fixtureRoot, 'src/spec-node/devContainersSpecCLI.ts'), '// duplicate'); writeFileSync(path.join(fixtureRoot, 'src/project-owned/nativeOnly.rs'), '// project owned'); @@ -74,3 +76,84 @@ describe('collectDuplicateUpstreamPaths', () => { expect(duplicates).to.deep.equal([]); }); }); + +describe('collectRootLevelUpstreamPathReferences', () => { + it('detects root-level upstream path references in build and test command files', () => { + const fixtureRoot = mkdtempSync(path.join(os.tmpdir(), 'upstream-path-references-')); + try { + mkdirSync(path.join(fixtureRoot, buildUpstreamPath('src', 'test')), { recursive: true }); + mkdirSync(path.join(fixtureRoot, 'build'), { recursive: true }); + + writeFileSync(path.join(fixtureRoot, buildUpstreamPath('src', 'test', 'cli.test.ts')), '// upstream fixture'); + writeFileSync(path.join(fixtureRoot, 'package.json'), JSON.stringify({ + scripts: { + test: 'mocha src/test/cli.test.ts', + }, + })); + writeFileSync(path.join(fixtureRoot, 'build/check-paths.js'), 'const fixture = "src/test/cli.test.ts";'); + + const references = collectRootLevelUpstreamPathReferences({ repositoryRoot: fixtureRoot }); + expect(references.map(reference => `${reference.filePath}:${reference.referencedPath}`)).to.deep.equal([ + 'build/check-paths.js:src/test/cli.test.ts', + 'package.json:src/test/cli.test.ts', + ]); + } finally { + rmSync(fixtureRoot, { recursive: true, force: true }); + } + }); + + it('ignores references that already use upstream/ prefixes', () => { + const fixtureRoot = mkdtempSync(path.join(os.tmpdir(), 'upstream-path-references-')); + try { + mkdirSync(path.join(fixtureRoot, buildUpstreamPath('src', 'test')), { recursive: true }); + writeFileSync(path.join(fixtureRoot, buildUpstreamPath('src', 'test', 'cli.test.ts')), '// upstream fixture'); + writeFileSync(path.join(fixtureRoot, 'package.json'), JSON.stringify({ + scripts: { + test: 'mocha upstream/src/test/cli.test.ts', + }, + })); + + const references = collectRootLevelUpstreamPathReferences({ repositoryRoot: fixtureRoot }); + expect(references).to.deep.equal([]); + } finally { + rmSync(fixtureRoot, { recursive: true, force: true }); + } + }); + + it('ignores references when a local path still exists unless explicitly requested', () => { + const fixtureRoot = mkdtempSync(path.join(os.tmpdir(), 'upstream-path-references-')); + try { + mkdirSync(path.join(fixtureRoot, buildUpstreamPath('src', 'test')), { recursive: true }); + mkdirSync(path.join(fixtureRoot, 'src/test'), { recursive: true }); + writeFileSync(path.join(fixtureRoot, buildUpstreamPath('src', 'test', 'cli.test.ts')), '// upstream fixture'); + writeFileSync(path.join(fixtureRoot, 'src/test/cli.test.ts'), '// local fixture'); + writeFileSync(path.join(fixtureRoot, 'package.json'), JSON.stringify({ + scripts: { + test: 'mocha src/test/cli.test.ts', + }, + })); + + const references = collectRootLevelUpstreamPathReferences({ repositoryRoot: fixtureRoot }); + expect(references).to.deep.equal([]); + + const referencesIncludingLocal = collectRootLevelUpstreamPathReferences({ + repositoryRoot: fixtureRoot, + includeExistingLocalPaths: true, + }); + expect(referencesIncludingLocal.map(reference => `${reference.filePath}:${reference.referencedPath}`)).to.deep.equal([ + 'package.json:src/test/cli.test.ts', + ]); + } finally { + rmSync(fixtureRoot, { recursive: true, force: true }); + } + }); + + it('uses upstream-prefixed paths for upstream-owned package scripts and fixtures', () => { + const repositoryRoot = path.resolve(__dirname, '../..'); + const references = collectRootLevelUpstreamPathReferences({ + repositoryRoot, + scanRoots: ['package.json'], + }); + expect(references).to.deep.equal([]); + }); +});