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

Update platform.ts to fix isWindowsAVX2 implementation. #10313

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

liudonghua123
Copy link
Contributor

@liudonghua123 liudonghua123 commented Apr 17, 2024

The isWindowsAVX2 function is not working as expected due to the stdout endwith \r\n. So the simple stdout == "True" will never true.

What does this PR do?

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

I tested the following code on my windows, and it works for me.

// windows-avx2-test.mts
import child_process from "child_process";
import { debug } from "console";

export function spawn(
  cmd: string,
  args: string[],
  options: child_process.SpawnOptions = {},
): {
  exitCode: number;
  stdout: string;
  stderr: string;
} {
  debug("spawn", [cmd, ...args].join(" "));
  const { status, stdout, stderr } = child_process.spawnSync(cmd, args, {
    stdio: "pipe",
    encoding: "utf-8",
    ...options,
  });
  return {
    exitCode: status ?? 1,
    stdout,
    stderr,
  };
}

function isWindowsAVX2(): boolean {
  try {
    const stdout = spawn("powershell", [
        "-c",
        `(Add-Type -MemberDefinition '[DllImport("kernel32.dll")] public static extern bool IsProcessorFeaturePresent(int ProcessorFeature);' -Name 'Kernel32' -Namespace 'Win32' -PassThru)::IsProcessorFeaturePresent(40);`,
    ]).stdout;
    const encoder = new TextEncoder(); 
    console.info(`stdout bytes: ${encoder.encode(stdout)}, True bytes: ${encoder.encode("True")}`)
    return stdout.trim() === "True"
  } catch (error) {
    debug("isWindowsAVX2 failed", error);
    return false;
  }
}

console.info(isWindowsAVX2())
Liu.D.H  playground   5.389s  09:59 > tsx windows-avx2-test.mts
spawn powershell -c (Add-Type -MemberDefinition '[DllImport("kernel32.dll")] public static extern bool IsProcessorFeaturePresent(int ProcessorFeature);' -Name 'Kernel32' -Namespace 'Win32' -PassThru)::IsProcessorFeaturePresent(40);
stdout bytes: 84,114,117,101,13,10, True bytes: 84,114,117,101
true

Liu.D.H  playground   5.729s  10:03 >

The isWindowsAVX2 function is not working as expected due to the stdout endwith `\r\n`. So the simple `stdout == "True"` will never true.
@paperdave paperdave merged commit 51bb5f3 into oven-sh:main Apr 17, 2024
2 of 7 checks passed
@liudonghua123 liudonghua123 deleted the patch-1 branch April 17, 2024 10:48
@liudonghua123
Copy link
Contributor Author

Thanks for the quick merge, the correct bun binary should be downloaded in the next version via npm install. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants