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

On Windows, support Bun.stdin, Bun.stdout, Bun.stderr in Bun.write when the other argument is a file #12411

Merged
merged 7 commits into from
Jul 9, 2024

Conversation

Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Jul 7, 2024

What does this PR do?

The following now work as expected on Windows:

await Bun.write(Bun.stdout, Bun.file(path.join(import.meta.dir, "hello-world.txt")));
await Bun.write(Bun.stderr, Bun.file(path.join(import.meta.dir, "hello-world.txt")));

This also addresses a bug where we would have an assertion failure when passing a NUL file path on Windows. In most cases, we will now use the Windows GetFinalPathByHandleW function instead of Zig's implementation (ziglang/zig#18756).

Fixes #12404
Fixes #12134
Fixes #11352 (most likely)
Fixes #10914 (most likely)

How did you verify your code works?

The BUN_FEATURE_FLAG_DISABLE_UV_FS_COPYFILE environment variable forces the code path which does a read-write loop in Bun.write. We re-run the tests for Bun.write with this environment variable set on Windows. Also, added a couple more tests.

@Jarred-Sumner Jarred-Sumner requested review from a team, erik-dunteman and paperdave and removed request for a team and erik-dunteman July 7, 2024 11:10
Copy link
Contributor

github-actions bot commented Jul 7, 2024

@Jarred-Sumner, your commit has failing tests :(

🪟💻 3 failing tests Windows x64 baseline

  • test/cli/install/registry/bun-install-registry.test.ts 2 failing
  • test/cli/install/registry/bun-install-windowsshim.test.ts code 1
  • test/js/node/child_process/child_process.test.ts 1 failing

🪟💻 3 failing tests Windows x64

  • test/cli/install/registry/bun-install-registry.test.ts 2 failing
  • test/cli/install/registry/bun-install-windowsshim.test.ts code 1
  • test/js/node/child_process/child_process.test.ts 1 failing

View logs

src/bun.js/webcore/blob.zig Outdated Show resolved Hide resolved
src/bun.js/webcore/blob.zig Show resolved Hide resolved
src/bun.js/webcore/blob.zig Show resolved Hide resolved
Co-authored-by: dave caruso <me@paperdave.net>
@Jarred-Sumner Jarred-Sumner merged commit 9ae8705 into main Jul 9, 2024
50 of 52 checks passed
@Jarred-Sumner Jarred-Sumner deleted the jarred/windows-handle branch July 9, 2024 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants