Repository navigation
fix: ensure installation commands are using .quiet - #5758
Conversation
There was a problem hiding this comment.
woudlnt u need to do this for all these?
Hmmm and we use "$" elsewhere too would this affect everything?
Are we sure there isn't just a arg we need to send to $ to ensure it doesnt "take over" like it was?
Also did u see open bun discussions?
There was a problem hiding this comment.
I tried all args and methods of Bun.$ and nothing worked.
I tested now also other commands for yarn, pnpm and bun and both yarn and pnpm triggered the same bug, even though they don't get executed on my system without manual modification of the code. I'm going to modify the pull request to include those as well.
For the other Bun.$ usages, I see that it quite used in the codebase but I'm not sure if all of those usages are affected. The other usages $ with npm, pnpm and yarn seem to be the ones affected so it might be necessary to add the wrapper there as well.
Potentially other commands might be affected but I haven't found them.
Which bun discussion are you referring to? I only saw the latest comment on #5489, but I don’t really have enough informations about Bun’s internals to tell whether it’s the same issue or not. This one seems specifically related to Bun.$, but I’m not sure if that’s the root cause or if there’s a better workaround.
There was a problem hiding this comment.
that makes sense ig I was just asking if u saw a discussion on bun like if u know if anyoen else has seen this issue basically
There was a problem hiding this comment.
Oh got it, I’ve seen other issues about Bun not handling Ctrl+C correctly on Windows but in other conditions, nothing specifically related to Bun.$
|
I changed all occurrences of npm, pnpm and yarn to use spawnWrapper, excluding the ones that don't get used during OpenCode execution (for example as a standalone script like the publish script) |
|
IT seems like the bug actually is in Bun |
|
I'm going to update this to use that and merge it, if that doesnt do the fix then we can explore this again |
rekram1-node
changed the title
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Co-authored-by: Aiden Cline <[email protected]> Co-authored-by: Aiden Cline <[email protected]>
Closes #5071
To fix the bug described in the issue, I introduced a small wrapper around
Bun.spawnthat kind of mimics theBun.$interface for the usage patterns (.text()and.throw(false)) related to the two problematic commands (npm listandnpm config get registry).I replaced the
$calls with this wrapper, and as a result the Ctrl+C handling now behaves correctly on Windows: it exits if no input prompt is present, otherwise it deletes the current input.I placed the wrapper file in the same directory as the affected file to keep it close, since the bug currently impacts only that file. Additionally, the issue may be fixed upstream in Bun, in which case this wrapper would no longer be necessary.