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

feature: Better Windows Support #3366

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

KartikSoneji
Copy link

Windows Fixes

Release Improvements

Provides prebuilt binaries for anchor and avm

  • for all platforms: Windows, MacOS Intel, MacOS ARM, Linux
  • commits to main publish the binaries as artifacts
  • tagging a commit creates a GitHub release

TODO: The avm install script still needs to be changed to use the prebuilt binaries instead of building from source.

Copy link

vercel bot commented Nov 14, 2024

@KartikSoneji is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thanks! As we've discussed in Discord, please only make the changes necessary to support Windows (no CI changes).

There are also so many more commands that the CLI makes use of — are you sure these are all the changes we need?

@KartikSoneji
Copy link
Author

I'll move the CI commits to a separate branch.
As for the commands, I wanted to start with the most used ones.
Is there a test suite I can run to check the rest of the commands?
Or a list?

@acheroncrypto
Copy link
Collaborator

As for the commands, I wanted to start with the most used ones.
Is there a test suite I can run to check the rest of the commands?
Or a list?

You can find all of them by searching for Command::new usage in the CLI codebase.

However, it seems strange to me that Rust doesn't already take care of making commands work by their name on Windows by default. Do all CLIs that want to support Windows have to run <name>.cmd?

In any case, we could just implement a std::process::Command wrapper that does this automatically rather than having to change every command individually.

@KartikSoneji
Copy link
Author

However, it seems strange to me that Rust doesn't already take care of making commands work by their name on Windows by default. Do all CLIs that want to support Windows have to run .cmd?

Ah so this is actually a quirk of node and node based executables.
Node commands like npm, yarn etc are written in JS and need to be run in a node runtime.
To abstract away this implementation detail, node commands user wrapper scripts:

  • <command> for unix (eg npm)
  • <command>.cmd for Windows cmd (eg npm.cmd)

Running npm in the shell on Windows calls the correct npm.cmd script because the PATHEXT variable contains .BAT

But std::process::Command doesn't take PATHEXT into consideration so fails to find the correct file to call.

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