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

Add support for Windows #20

Open
khanetor opened this issue Jul 18, 2017 · 15 comments
Open

Add support for Windows #20

khanetor opened this issue Jul 18, 2017 · 15 comments

Comments

@khanetor
Copy link

The command uname -sm only works for Unix-based OSes. Currently I have to manually set

nativePlatform := "win32-x86_64"
@jodersky
Copy link
Member

Windows is currently not officially supported, however it shouldn't be too complicated to add. Here is the rough outline:

  • find a way to distinguish the windows architectures (I'm not sure if there are mutliple), similar to what uname can provide on unix
  • use that mechanism in JniNative and JniLoad to bundle and extract the corresponding native libraries

@eaplatanios
Copy link

What about System.getProperty("os.arch") and System.getProperty("os.name"). Could we change the code to use these instead of uname?

@jodersky
Copy link
Member

That's a good idea and was also what was used in a very early version of this project. Unfortunately, the os.arch property is too vague. It is able to distinguish between certain architectures only, such as x86 and ARM for instance. The latter however has several, incompatible, architectures which are indistinguishable with that mechanism. For example armv6 (used in the original raspberry pi) and armv7l (used in subsequent models) are incompatible, yet they both show up as simply "arm" from os.arch.

@eaplatanios
Copy link

I see. How about we fall back to these two properties when uname is not available. Do you think that's acceptable behavior until we find a better solution?

@jodersky
Copy link
Member

I think we could check the os.name property and in case it is windows handle it specifically. Otherwise we should use the standard uname method.

@andraxin
Copy link

andraxin commented Jan 11, 2019

My platform is neither Windows nor ARM, but I'm still inconvenienced by the need to spawn an extra process to run uname during JNI loading, since that's not allowed, causing my application to crash.
My fix was also to use sys.props("os.name") and sys.props("os.arch") to avoid forking a process.

That's a good idea and was also what was used in a very early version of this project. Unfortunately, the os.arch property is too vague. It is able to distinguish between certain architectures only, such as x86 and ARM for instance. The latter however has several, incompatible, architectures which are indistinguishable with that mechanism. For example armv6 (used in the original raspberry pi) and armv7l (used in subsequent models) are incompatible, yet they both show up as simply "arm" from os.arch.

I may be jumping to conclusions too fast, but would it be correct to assume that sys.props("os.arch") == "arm' is the only problem case? Shouldn't this be handled via a trouble report back to Oracle to fix this in Java? That coarse-grain reporting of the underlying architecture is obviously(?) not very helpful and likely to cause pain for others as well...

I think we could check the os.name property and in case it is windows handle it specifically. Otherwise we should use the standard uname method.

I don't know, but IF dragging in uname is only needed on ARM, THEN -perhaps- special-casing Windows is not the optimal approach. In my use case, I'd much rather opt for only falling back to uname when sys.props("os.arch") == "arm'.

What do you think?

@jodersky
Copy link
Member

I doubt that anything can be done about changing the property of os.arch upstream, as that could break user code. However, I think your proposal of using system properties by default and calling uname only on an arm platform is worth a shot!

@andraxin
Copy link

Great! Will you fix that or would you prefer a pull request?

@andraxin
Copy link

PR created.

@fgoepel
Copy link

fgoepel commented Sep 15, 2021

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.
As far as I can tell neither JNA or JNR use this uname cludge.

@pomadchin
Copy link
Member

pomadchin commented Sep 19, 2021

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 os.arch option passed, I see no reason why not to have it.

@chitralverma
Copy link

chitralverma commented Feb 2, 2023

@pomadchin and @jodersky proposing a alternate solution if it has not been considered already.

Regarding the platform identification,

  • currently we are relying on uname which doesn't work with windows and has to be manually overridden in the build.sbt as mentioned in the OP.
  • There were other solutions like relying on os.arch but you are correct, it is natively deciphered in java and can lead to non deterministic results.
  • My suggestion is to not do either. Actually we only need a value for nativePlatform to create directories so why not instead rely on the target triple directly from the 2 supported build tools instead of getting the value from uname and then parsing it.
  • For rust you can simply set the nativePlatform to rustc -vV | sed -n 's|host: ||p' and for cmake we can get it from various places like make -v, gcc -dumpmachine commands.
  • Basically the inference of the nativePlatform can be as per the build tool selected and not something that we should construct.

Some other smaller things I identified that block windows support (unrelated to where the nativePlatform comes from),

  • the path construction is a problem. String interpolation or appending has been done which works on *nix OS but not on windows.
  • This can be easily fixed with java.nio.....Paths but needs to be done in a few places across the repo.
  • Things like this need to be fixed, because for windows, .dll are generated.

@pomadchin
Copy link
Member

hey @chitralverma took some time to respond; I think it is a great idea and worth trying 👍

@chitralverma
Copy link

chitralverma commented Feb 23, 2023

Ref hacky project on what this will look like,
https://github.com/chitralverma/test-native-proj

@pomadchin
Copy link
Member

pomadchin commented Feb 24, 2023

@chitralverma hmmm, I think it's worth trying making a PR against this repo just to start iterating on this idea.

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

No branches or pull requests

7 participants