fix(release): state what the echo helper does, and drop two dead claims
Three corrections from review, none of which change behaviour. attemptStreaming promised output "as the command produces it", which spawnSync cannot do: it returns only after the child exits, and the two streams are echoed one after the other, so their interleaving is lost. For an npm publish that is visible — notices go to stderr while the `+ name@version` confirmation goes to stdout, so the confirmation prints first. The helper is now attemptEchoed and its contract says buffered, echoed after exit, stdout before stderr; live progress would need an asynchronous spawn with data listeners. The traversal comment claimed a node on the stack is a cycle only peer edges can form, and that skipping it drops just that edge. The cycle does carry a peer edge, because the install edges were proved acyclic a moment earlier, but the back edge that reaches the stacked node need not be the peer one — which is what the post-condition exists to catch, so the comment now points at it instead of asserting an invariant the traversal does not have. The `if (placed.has(member.name)) return` after leaving the stack was unreachable: a re-entrant visit returns at the top guard while the member is on the stack, so it can never be placed by the time the recursion unwinds.
This commit is contained in:
@@ -18,7 +18,7 @@ import { join, resolve } from 'node:path'
|
||||
import { setTimeout as sleep } from 'node:timers/promises'
|
||||
import { parseArgs } from 'node:util'
|
||||
import { releaseFamily } from './families.ts'
|
||||
import { attempt, attemptStreaming, isEntry } from './process.ts'
|
||||
import { attempt, attemptEchoed, isEntry } from './process.ts'
|
||||
import { packedIdentity, readPublishOrder } from './tarball.ts'
|
||||
|
||||
/**
|
||||
@@ -102,7 +102,7 @@ async function publishTarball(tarball: string, name: string, version: string): P
|
||||
// command-line flag could not serve both and would override the manifest
|
||||
// that does. Each packed manifest decides, and
|
||||
// check-workspace-constraints holds every manifest to its sequence's level.
|
||||
const result = attemptStreaming('npm', ['publish', tarball, ...tagArgs])
|
||||
const result = attemptEchoed('npm', ['publish', tarball, ...tagArgs])
|
||||
const output = `${result.stdout}${result.stderr}`
|
||||
if (result.status === 0) return
|
||||
|
||||
|
||||
Reference in New Issue
Block a user