fix(core): apply chained configurators to the cmd log entry - #1496
Open
DevCop95 wants to merge 1 commit into
Open
fix(core): apply chained configurators to the cmd log entry#1496DevCop95 wants to merge 1 commit into
cmd log entry#1496DevCop95 wants to merge 1 commit into
Conversation
A process starts eagerly: `$` calls `run()` synchronously inside the template tag and `exec()` fires `on.start` in the same tick, so the `cmd` entry was emitted before `.quiet()` / `.verbose()` could be applied. The later `stdout`/`stderr`/`end` entries did see them, which is why a single process could log `cmd: verbose=true` next to `stdout: verbose=false`. Defer the entry by a microtask so the synchronous configurator chain lands first, while keeping it ahead of the I/O-driven entries. Sync mode is excluded: it has nothing to chain and emits its output in the same tick. Fixes google#931
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #931
Problem
.quiet()/.verbose()chained onto a template tag are not reflected in thecmdlog entry:The reporter noticed this in 8.2.0; it still reproduces on
main(8.9.0).Root cause
A process starts eagerly:
$callspp.run()synchronously inside the template tag (core.ts,$factory), andexec()fireson.startin the same tick. So thecmdentry is emitted before the chained configurators have run, andisVerbose()is evaluated against a snapshot that is still the default one.stdout/stderr/endentries are unaffected because they arrive on later ticks — hence the inconsistency the issue describes, where a single process logscmd: verbose=truenext tostdout: verbose=false.Instrumented on
main:Fix
Defer the
cmdentry by one microtask so the synchronous configurator chain is applied first.queueMicrotask(notsetImmediate) keeps the entry ahead of any I/O-drivenstdout/stderr/endentry.Sync mode is excluded:
$.syncreturnsProcessOutputrather than aProcessPromise, so there is nothing to chain, andspawnSyncemits its output in the same tick — deferring there would print the command after its output.Output for the script in the issue now matches the pre-8.2.0 behaviour it documents as expected.
Trade-off
Output the caller writes synchronously between the tag and the next
awaitnow lands before the$ cmdbanner:That window is one microtask wide, and it is the cost of letting
.quiet()/.verbose()affect the entry they are supposed to affect.Tests
applies chained configurators to thecmdentry— covers all three symptoms from the issue (.quiet(),.verbose(false), and.quiet(false)re-enabling output). Fails onmain.keeps thecmdentry ahead of the output in sync mode— guards the sync carve-out.