-
Notifications
You must be signed in to change notification settings - Fork 0
Harden review workflows and CI coverage #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
86ffaec
9158fb9
af5f5ac
924b11c
8325986
aa09d7f
383391a
a747ecf
345550d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,15 @@ import { execFileSync, spawnSync } from 'node:child_process'; | |
| import { createHash } from 'node:crypto'; | ||
| import { | ||
| existsSync, | ||
| lstatSync, | ||
| mkdirSync, | ||
| readFileSync, | ||
| readlinkSync, | ||
| renameSync, | ||
| statSync, | ||
| writeFileSync, | ||
| } from 'node:fs'; | ||
| import { dirname, relative, resolve } from 'node:path'; | ||
| import { basename, dirname, relative, resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { summaryPath } from './summary-path.mjs'; | ||
|
|
||
|
|
@@ -1088,7 +1090,7 @@ function build() { | |
| const change = changeSummary(sourceSummaries.change, target.changeDefaults); | ||
| const content = { | ||
| repo: { | ||
| name: remoteRepository?.name || repo.split('/').pop(), | ||
| name: remoteRepository?.name || basename(repo), | ||
| root: localWorkspace ? repo : target.remote?.url || repo, | ||
| base: target.base, | ||
| head: target.head, | ||
|
|
@@ -1146,6 +1148,29 @@ function build() { | |
| return true; | ||
| } | ||
|
|
||
| function untrackedFileKind(stat) { | ||
| if (stat.isFile()) return 'file'; | ||
| if (stat.isSymbolicLink()) return 'symlink'; | ||
| if (stat.isDirectory()) return 'directory'; | ||
| return 'other'; | ||
| } | ||
|
|
||
| function fingerprintUntrackedPath(content, path) { | ||
| const stat = lstatSync(resolve(repo, path), { bigint: true }); | ||
| content.update(path); | ||
| content.update('\0'); | ||
| content.update(untrackedFileKind(stat)); | ||
| content.update('\0'); | ||
| content.update(String(stat.size)); | ||
| content.update('\0'); | ||
| content.update(String(stat.mtimeNs)); | ||
|
Comment on lines
+1164
to
+1166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an untracked regular file is rewritten with the same byte length while its mtime is preserved—such as by a timestamp-preserving generator, rsync-like tool, or on a coarse-timestamp filesystem—its kind, size, mtime, path, and Git status all remain unchanged. The watcher therefore never calls Useful? React with 👍 / 👎. |
||
| content.update('\0'); | ||
| if (stat.isSymbolicLink()) { | ||
| content.update(readlinkSync(resolve(repo, path))); | ||
| content.update('\0'); | ||
| } | ||
| } | ||
|
|
||
| function fingerprint() { | ||
| let summariesTime = ''; | ||
| if (!ignoreSummaryWatch && !noSummaries) { | ||
|
|
@@ -1163,7 +1188,7 @@ function fingerprint() { | |
| } | ||
| const content = createHash('sha256'); | ||
| content.update( | ||
| tryRepo(['diff', '--no-ext-diff', '--binary', 'HEAD', '--']), | ||
| tryRepo(['diff', '--no-ext-diff', '--no-textconv', '--binary', 'HEAD', '--']), | ||
| ); | ||
| const untracked = tryRepo([ | ||
| 'ls-files', | ||
|
|
@@ -1175,10 +1200,7 @@ function fingerprint() { | |
| .filter((path) => path && !excludedPaths.has(path)) | ||
| .sort(); | ||
| for (const path of untracked) { | ||
| content.update('\0'); | ||
| content.update(path); | ||
| content.update('\0'); | ||
| content.update(readFileSync(resolve(repo, path))); | ||
| fingerprintUntrackedPath(content, path); | ||
| } | ||
| return [ | ||
| tryRepo(['rev-parse', 'HEAD']), | ||
|
|
@@ -1202,18 +1224,38 @@ const refresh = () => { | |
|
|
||
| const started = refresh(); | ||
| if (watching && started) { | ||
| let last = fingerprint(); | ||
| let last; | ||
| let remoteWait = 0; | ||
| const watcher = setInterval(() => { | ||
| const next = fingerprint(); | ||
| remoteWait += watchInterval; | ||
| const remoteDue = remoteMode && remoteWait >= remoteRefreshInterval; | ||
| if (next !== last || remoteDue || watchContent) { | ||
| last = next; | ||
| remoteWait = 0; | ||
| if (!refresh()) clearInterval(watcher); | ||
| let watcher; | ||
| const stopWatching = (error) => { | ||
| console.error(error instanceof Error ? error.message : String(error)); | ||
| process.exitCode = 1; | ||
| if (watcher) clearInterval(watcher); | ||
| }; | ||
| const poll = () => { | ||
| try { | ||
| const next = fingerprint(); | ||
| if (last === undefined) { | ||
| last = next; | ||
| return true; | ||
| } | ||
| remoteWait += watchInterval; | ||
| const remoteDue = remoteMode && remoteWait >= remoteRefreshInterval; | ||
| if (next !== last || remoteDue || watchContent) { | ||
| last = next; | ||
| remoteWait = 0; | ||
| if (!refresh()) { | ||
| clearInterval(watcher); | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } catch (error) { | ||
| stopWatching(error); | ||
| return false; | ||
| } | ||
| }, watchInterval); | ||
| }; | ||
| if (poll()) watcher = setInterval(poll, watchInterval); | ||
| } else if (watching) { | ||
| process.exitCode = 1; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a clean runner where the preinstalled Node differs from the matrix version, this enables Corepack in the old Node installation and then replaces it on
PATH; ascorepack enable --helpstates, the shims are installed next to the currently resolvedcorepackbinary. Thecache: pnpmsetup subsequently needs to invoke pnpm whileactions/setup-nodeis running, so it can fail before the install step, and the same ordering appears in all five jobs intest-lanes.yml. Install pnpm independently before setup-node (for example with pnpm/action-setup), or otherwise ensure the selected Node toolchain exposes the shim before enabling pnpm caching.AGENTS.md reference: AGENTS.md:L10-L17
Useful? React with 👍 / 👎.