-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add support for Windows #20
Comments
Windows is currently not officially supported, however it shouldn't be too complicated to add. Here is the rough outline: |
What about |
That's a good idea and was also what was used in a very early version of this project. Unfortunately, the |
I see. How about we fall back to these two properties when |
I think we could check the |
My platform is neither Windows nor ARM, but I'm still inconvenienced by the need to spawn an extra process to run
I may be jumping to conclusions too fast, but would it be correct to assume that
I don't know, but IF dragging in What do you think? |
I doubt that anything can be done about changing the property of |
Great! Will you fix that or would you prefer a pull request? |
PR created. |
It's a bit unfortunate that the PR submitted by andraxin was rejected, because assuming the presence of a uname command is not necessarily justified even under Linux, such as when running in a minimal Docker container. The only thing you can really rely on is that Java is installed. For edge-cases where os.arch is not enough, why not have the user set -Dos.arch=whatever or have some other way to override it. |
Hey @shado23 I think we can add an option to overrride the target arch via the java option passed, would you like to help with that? My proposition would be to leave the current behavior as the default one, and to add an option to override it via the |
@pomadchin and @jodersky proposing a alternate solution if it has not been considered already. Regarding the platform identification,
Some other smaller things I identified that block windows support (unrelated to where the
|
hey @chitralverma took some time to respond; I think it is a great idea and worth trying 👍 |
Ref hacky project on what this will look like, |
@chitralverma hmmm, I think it's worth trying making a PR against this repo just to start iterating on this idea. |
The command
uname -sm
only works for Unix-based OSes. Currently I have to manually setThe text was updated successfully, but these errors were encountered: