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

Avoid using uname except on ARM #32

Closed
wants to merge 1 commit into from
Closed

Avoid using uname except on ARM #32

wants to merge 1 commit into from

Conversation

andraxin
Copy link

@andraxin andraxin commented Apr 27, 2019

In reference to the open issue Add support for Windows, I'm guessing that the long silence and no action following our last exchange implies that a pull request was the preferred way forward; so, here it comes.

@jodersky
Copy link
Member

jodersky commented May 2, 2019

Thanks for the contribution! The code looks great, but I'm having second thoughts on the issue at hand. I can't find any specification of "os.name" and "os.arch" values. It appears they mostly follow some convention, but I can't find any source of truth that specifies these fields across platforms and JDKs. Specifically on windows, where the most benefits from using system properties would be gained, they seem to vary depending on the version of the OS.

As it stands, I'd rather rely on the stability of uname even if it requires calling out to an external program.

@andraxin
Copy link
Author

andraxin commented May 3, 2019

It's your call, but I'm not sure what kind of "source of truth" is really necessary.

Wouldn't it suffice that the values are unique and consistent on a particular OS version?

BUT, then again, I've only ever used this when building on the same platform (i.e. OS/ARCH combo) as my target, in which case most of this baggage feels fairly unnecessary. Chances are loading the binary on any other platform than what it was built for would fail anyway (although, perhaps, less cleanly, and potentially causing a crash or other harm). If I were to cross-compile for several other targets, the current uname-based approach would get things wrong anyway (since uname would always return the build host's platform, not the build target's).

I may just be missing the point of this feature altogether. (-;

@jodersky jodersky closed this Nov 24, 2019
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