Skip to content
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

fix bunx on windows with postinstall scripts #17076

Merged
merged 5 commits into from
Feb 6, 2025

Conversation

RiskyMH
Copy link
Member

@RiskyMH RiskyMH commented Feb 5, 2025

fixes #9841
fixes #15783, fixes #15562, fixes #14848, fixes #12336, fixes #10068

context behind change is it runs <current-exe> exec on windows which would be bunx exec but as that subcommand wasn't resolvable it just tried to run exec as a package:

var argv = if (shell_bin != null and !Environment.isWindows) [_]?[*:0]const u8{
shell_bin.?,
"-c",
combined_script,
null,
} else [_]?[*:0]const u8{
try bun.selfExePath(),
"exec",
combined_script,
null,
};


will try to add tests which fail without this

@robobun
Copy link

robobun commented Feb 5, 2025

Updated 9:40 PM PT - Feb 5th, 2025

@RiskyMH, your commit 5fe203d has 1 failures in Build #11177:


🧪   try this PR locally:

bunx bun-pr 17076

@RiskyMH RiskyMH changed the title investigate bunx on windows and its postinstall scripting fix bunx on windows with postinstall scripts Feb 5, 2025
@hakimio
Copy link

hakimio commented Feb 5, 2025

Maybe there should be some test for this?

@RiskyMH
Copy link
Member Author

RiskyMH commented Feb 5, 2025

will try to add tests which fail without this

Yeah i will. Just investigating more rn. Also going to sleep soon, but I wanted to find the cause of this as it was buging me.

Seems simple to add test though, im surprised it hasn't failed anything already but the tests probably don't have bunx simlink and instead rely on effectively bun x.

@RiskyMH RiskyMH requested a review from dylan-conway February 6, 2025 03:39
@RiskyMH RiskyMH marked this pull request as ready for review February 6, 2025 03:42
Copy link
Member

@dylan-conway dylan-conway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. @paperclover should take a look too

Copy link
Member

@paperclover paperclover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great find

src/cli.zig Outdated Show resolved Hide resolved
@Jarred-Sumner Jarred-Sumner merged commit 1684c62 into main Feb 6, 2025
67 of 69 checks passed
@Jarred-Sumner Jarred-Sumner deleted the riskymh/bunx-windows-testing branch February 6, 2025 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants