diff --git a/.github/workflow.lock.yml b/.github/workflow.lock.yml index ee6b042..b7b60aa 100644 --- a/.github/workflow.lock.yml +++ b/.github/workflow.lock.yml @@ -3,23 +3,23 @@ entries: .github/workflows/src/release-commit.yml: output: .github/workflows/release-commit.yml dependencies: - - package: github:sxzz/workflows//.github/workflows/release-commit.yml + - package: github:sxzz/workflows/.github/workflows/release-commit.yml requested: main foundAt: .github/workflows/src/release-commit.yml#jobs.release.uses .github/workflows/src/release.yml: output: .github/workflows/release.yml dependencies: - - package: github:sxzz/workflows//.github/workflows/release.yml + - package: github:sxzz/workflows/.github/workflows/release.yml requested: main foundAt: .github/workflows/src/release.yml#jobs.release.uses .github/workflows/src/unit-test.yml: output: .github/workflows/unit-test.yml dependencies: - - package: github:sxzz/workflows//.github/workflows/unit-test.yml + - package: github:sxzz/workflows/.github/workflows/unit-test.yml requested: main foundAt: .github/workflows/src/unit-test.yml#jobs.unit-test.uses packages: - github:actions/checkout//.: + github:actions/checkout: source: github owner: actions repo: checkout @@ -30,7 +30,7 @@ packages: dependencies: [] requested: de0fac2e4500dabe0009e67214ff5f5447ce83dd resolved: de0fac2e4500dabe0009e67214ff5f5447ce83dd - github:actions/setup-node//.: + github:actions/setup-node: source: github owner: actions repo: setup-node @@ -41,7 +41,7 @@ packages: dependencies: [] requested: 48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e resolved: 48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e - github:pnpm/action-setup//.: + github:pnpm/action-setup: source: github owner: pnpm repo: action-setup @@ -52,7 +52,7 @@ packages: dependencies: [] requested: 0e279bb959325dab635dd2c09392533439d90093 resolved: 0e279bb959325dab635dd2c09392533439d90093 - github:sxzz/workflows//.github/workflows/release-commit.yml: + github:sxzz/workflows/.github/workflows/release-commit.yml: source: github owner: sxzz repo: workflows @@ -60,12 +60,12 @@ packages: type: reusable-workflow contentDigest: sha256:06630f463980c0f109857593e1205d302aaca93766e6337790c7f319976b9da5 dependencies: - - package: github:sxzz/workflows//setup-js + - package: github:sxzz/workflows/setup-js requested: main foundAt: sxzz/workflows/.github/workflows/release-commit.yml#jobs.release.steps[0].uses requested: main resolved: e10bbede8ba835eaee8df633bb472d303aba8509 - github:sxzz/workflows//.github/workflows/release.yml: + github:sxzz/workflows/.github/workflows/release.yml: source: github owner: sxzz repo: workflows @@ -73,12 +73,12 @@ packages: type: reusable-workflow contentDigest: sha256:716fb2eae20dfe7e87c92eabd1b4be35deeb2f6fca8f999f63412d26904abbf8 dependencies: - - package: github:sxzz/workflows//setup-js + - package: github:sxzz/workflows/setup-js requested: main foundAt: sxzz/workflows/.github/workflows/release.yml#jobs.release.steps[0].uses requested: main resolved: e10bbede8ba835eaee8df633bb472d303aba8509 - github:sxzz/workflows//.github/workflows/unit-test.yml: + github:sxzz/workflows/.github/workflows/unit-test.yml: source: github owner: sxzz repo: workflows @@ -86,12 +86,12 @@ packages: type: reusable-workflow contentDigest: sha256:0cff202ed73f1bd09e8e8727ec40aed870f0159d3d9e9e7e910bf89c369c8d3c dependencies: - - package: github:sxzz/workflows//setup-js + - package: github:sxzz/workflows/setup-js requested: main foundAt: sxzz/workflows/.github/workflows/unit-test.yml#jobs.lint.steps[0].uses requested: main resolved: e10bbede8ba835eaee8df633bb472d303aba8509 - github:sxzz/workflows//setup-js: + github:sxzz/workflows/setup-js: source: github owner: sxzz repo: workflows @@ -99,13 +99,13 @@ packages: type: composite contentDigest: sha256:ee6696753c61e7206ab75bc50a88f8854a2ea5ceb668cbc2d813658bef7aa993 dependencies: - - package: github:actions/checkout//. + - package: github:actions/checkout requested: de0fac2e4500dabe0009e67214ff5f5447ce83dd foundAt: sxzz/workflows/setup-js#runs.steps[0].uses - - package: github:pnpm/action-setup//. + - package: github:pnpm/action-setup requested: 0e279bb959325dab635dd2c09392533439d90093 foundAt: sxzz/workflows/setup-js#runs.steps[1].uses - - package: github:actions/setup-node//. + - package: github:actions/setup-node requested: 48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e foundAt: sxzz/workflows/setup-js#runs.steps[2].uses requested: main diff --git a/src/commands.ts b/src/commands.ts index 9def7b3..217f056 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -1,6 +1,6 @@ import { execFile } from 'node:child_process' import { promisify } from 'node:util' -import { pack, verify } from './pack.ts' +import { pack, packScanned, verify } from './pack.ts' import { scan } from './scan.ts' import { discoverConfig, @@ -27,9 +27,9 @@ export async function update(options: UpdateOptions = {}): Promise { const cwd = resolveCwd(options.cwd) const current = await readLockfile(cwd) const refreshPackages = selectRefreshPackages(current, options.packageName) - await scan({ ...options, cwd, refreshPackages }) + const scanResult = await scan({ ...options, cwd, refreshPackages }) if (!options.lockfileOnly) { - await pack(options) + await packScanned(scanResult, options) } } diff --git a/src/pack.ts b/src/pack.ts index cceaeac..053ef4e 100644 --- a/src/pack.ts +++ b/src/pack.ts @@ -38,6 +38,14 @@ export async function pack( } const scanResult = await scan(options) + return packScanned(scanResult, options) +} + +export async function packScanned( + scanResult: CommandResult, + options: FlowpackOptions = {}, +): Promise { + const cwd = resolveCwd(options.cwd) const github = options.github ?? new HttpGitHubClient() for (const entry of scanResult.entries) { diff --git a/src/scan.ts b/src/scan.ts index 35a277c..73bcfb7 100644 --- a/src/scan.ts +++ b/src/scan.ts @@ -143,30 +143,28 @@ export async function resolveQueue( context: ScanContext, ): Promise { const seen = new Set() + const resolvedRefs = new Map() + const warnings = new Set() while (queue.length > 0) { const dependency = queue.shift()! - const existing = lockfile.packages[dependency.package] const shouldRefresh = refreshPackages.has(dependency.package) - if ( - existing && - !shouldRefresh && - seen.has(`${dependency.package}@${existing.resolved}`) - ) { - continue - } - const previousPackage = previous.packages[dependency.package] const resolved = previousPackage?.resolved && !shouldRefresh ? previousPackage.resolved - : await resolveDependencyRef(dependency, github, context) + : await resolveDependencyRef(dependency, github, context, resolvedRefs) + const seenKey = `${dependency.package}@${resolved}` + if (seen.has(seenKey)) { + continue + } const scanned = await scanRemotePackage( dependency.remote, resolved, github, context, + warnings, ) lockfile.packages[dependency.package] = { ...scanned, @@ -174,7 +172,7 @@ export async function resolveQueue( resolved, } - seen.add(`${dependency.package}@${resolved}`) + seen.add(seenKey) queue.push( ...scanned.dependencies.map((item) => ({ ...item, @@ -189,6 +187,7 @@ async function scanRemotePackage( resolved: string, github: GitHubClient, context: ScanContext, + warnings: Set, ): Promise> { if (remote.kind === 'reusable-workflow') { if (isExternal(remote, context.external)) { @@ -237,8 +236,10 @@ async function scanRemotePackage( const using = typeof runs?.using === 'string' ? runs.using.toLowerCase() : undefined if (using !== 'composite') { - context.stderr?.write( - `${styleText('yellow', `Warning: Unsupported action type for ${remote.owner}/${remote.repo}/${remote.path}@${resolved}: ${using ?? 'unknown'}; marking external`)}\n`, + warnOnce( + context, + warnings, + `Unsupported action type for ${remote.owner}/${remote.repo}/${remote.path}@${resolved}: ${using ?? 'unknown'}; marking external`, ) return { ...externalPackage(remote, 'external-action'), @@ -265,15 +266,38 @@ function resolveDependencyRef( dependency: ResolvedDependency, github: GitHubClient, context: ScanContext, + resolvedRefs: Map, ): Promise { + const key = `${dependency.package}@${dependency.requested}` + const resolved = resolvedRefs.get(key) + if (resolved) { + return Promise.resolve(resolved) + } context.stdout?.write( `Resolving ${dependency.remote.owner}/${dependency.remote.repo}${dependency.remote.path === '.' ? '' : `/${dependency.remote.path}`}@${dependency.requested}\n`, ) - return github.resolveRef( - dependency.remote.owner, - dependency.remote.repo, - dependency.requested, - ) + return github + .resolveRef( + dependency.remote.owner, + dependency.remote.repo, + dependency.requested, + ) + .then((value) => { + resolvedRefs.set(key, value) + return value + }) +} + +function warnOnce( + context: ScanContext, + warnings: Set, + message: string, +): void { + if (warnings.has(message)) { + return + } + warnings.add(message) + context.stderr?.write(`${styleText('yellow', `Warning: ${message}`)}\n`) } function isExternal(remote: RemoteRef, selectors: string[]): boolean { @@ -320,18 +344,19 @@ export async function readActionMetadata( } function parseRemoteUsesFromDependency(dependency: LockDependency): RemoteRef { - const match = /^github:([^/]+)\/([^/]+)\/\/(.+)$/u.exec(dependency.package) + const match = /^github:([^/]+)\/([^/]+)(?:\/(.+))?$/u.exec(dependency.package) if (!match) { throw new Error(`Unsupported package key: ${dependency.package}`) } const [, owner, repo, packagePath] = match - const kind = packagePath!.startsWith('.github/workflows/') + const remotePath = packagePath ?? '.' + const kind = remotePath.startsWith('.github/workflows/') ? 'reusable-workflow' : 'action' return { owner: owner!, repo: repo!, - path: packagePath!, + path: remotePath, ref: dependency.requested, package: dependency.package, kind, diff --git a/src/utils/ref.ts b/src/utils/ref.ts index 32c4ad3..2c32d57 100644 --- a/src/utils/ref.ts +++ b/src/utils/ref.ts @@ -3,7 +3,9 @@ import type { RemoteRef } from '../types.ts' const REMOTE_USES_RE = /^[\w.-]+\/[\w.-]+(?:\/[^@\s]+)?@[^@\s]+$/ export function packageKey(owner: string, repo: string, path: string): string { - return `github:${owner}/${repo}//${path || '.'}` + return path && path !== '.' + ? `github:${owner}/${repo}/${path}` + : `github:${owner}/${repo}` } export function isRemoteUses(value: unknown): value is string { @@ -53,8 +55,8 @@ export function matchesPackageSelector(key: string, selector: string): boolean { : `github:${selector}` return ( key === normalized || - key.startsWith(`${normalized}//`) || - key.includes(`:${selector}//`) + key.startsWith(`${normalized}/`) || + key.includes(`:${selector}/`) ) } @@ -71,7 +73,7 @@ export function matchesExternalSelector( remote.package === packageSelector || fullName === value || withPath === value || - remote.package.startsWith(`${packageSelector}//`) + remote.package.startsWith(`${packageSelector}/`) ) } diff --git a/tests/index.test.ts b/tests/index.test.ts index 3f5fbdc..2f53326 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -21,8 +21,11 @@ import { substituteString, substituteValue } from '../src/utils/substitute.ts' class FixtureGitHub implements GitHubClient { refs = new Map() files = new Map() + resolveCalls = new Map() resolveRef(owner: string, repo: string, ref: string): Promise { + const key = `${owner}/${repo}@${ref}` + this.resolveCalls.set(key, (this.resolveCalls.get(key) ?? 0) + 1) const resolved = this.refs.get(`${owner}/${repo}@${ref}`) if (!resolved) { throw new Error(`missing ref ${owner}/${repo}@${ref}`) @@ -171,8 +174,8 @@ runs: const lockfile = await readYaml(path.join(cwd, '.github/workflow.lock.yml')) expect(Object.keys(lockfile.packages)).toEqual([ - 'github:acme/nested//.', - 'github:acme/root//.', + 'github:acme/nested', + 'github:acme/root', ]) expect(lockfile.entries['.github/workflows/src/ci.yml'].output).toBe( '.github/workflows/ci.yml', @@ -234,15 +237,54 @@ runs: await scan({ cwd, github }) expect( - (await readLockfile(cwd)).packages['github:acme/root//.'].resolved, + (await readLockfile(cwd)).packages['github:acme/root'].resolved, ).toBe('1111111111111111111111111111111111111111') await update({ cwd, github, lockfileOnly: true }) expect( - (await readLockfile(cwd)).packages['github:acme/root//.'].resolved, + (await readLockfile(cwd)).packages['github:acme/root'].resolved, ).toBe('2222222222222222222222222222222222222222') }) + it('does not resolve or warn repeatedly during update packing', async () => { + const cwd = await fixtureRepo({ + '.github/workflows/src/ci.yml': ` +on: + push: +jobs: + one: + runs-on: ubuntu-latest + steps: + - uses: acme/js@main + two: + runs-on: ubuntu-latest + steps: + - uses: acme/js@main +`, + }) + const github = new FixtureGitHub() + github.refs.set('acme/js@main', '4444444444444444444444444444444444444444') + github.files.set( + 'acme/js@4444444444444444444444444444444444444444:action.yml', + ` +name: JS +description: JS action +runs: + using: node20 + main: dist/index.js +`, + ) + const stderr = new TestOutput() + const stdout = new TestOutput() + + await update({ cwd, github, stderr, stdout }) + + expect(github.resolveCalls.get('acme/js@main')).toBe(1) + expect(stdout.text.match(/Resolving acme\/js@main/gu)).toHaveLength(1) + expect(stderr.text.match(/Unsupported action type/gu)).toHaveLength(1) + expect(stdout.text).toContain('Packed 1 workflow\n') + }) + it('inlines safe reusable workflows with renamed jobs and a bridge job', async () => { const cwd = await fixtureRepo({ '.github/workflows/src/ci.yml': ` @@ -400,9 +442,7 @@ jobs: { uses: 'acme/root@6666666666666666666666666666666666666666' }, ]) const lockfile = await readLockfile(cwd) - expect(lockfile.packages['github:acme/root//.'].type).toBe( - 'external-action', - ) + expect(lockfile.packages['github:acme/root'].type).toBe('external-action') }) it('accepts workflow entries from command options', async () => { @@ -476,10 +516,10 @@ entries: .github/workflows/src/ci.yml: output: .github/workflows/ci.yml dependencies: - - package: github:acme/root//. + - package: github:acme/root requested: main packages: - github:acme/root//.: + github:acme/root: source: github owner: acme repo: root @@ -502,15 +542,15 @@ packages: ...previous, packages: { ...previous.packages, - 'github:acme/root//.': { - ...previous.packages['github:acme/root//.'], + 'github:acme/root': { + ...previous.packages['github:acme/root'], resolved: 'new', }, }, } expect(diffLockfiles(previous, current).changed).toEqual([ { - package: 'github:acme/root//.', + package: 'github:acme/root', oldResolved: 'old', newResolved: 'new', dependencyChanged: false,