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

Fix #1038: Consider unsupported platforms as exotic #1039

Closed
wants to merge 3 commits into from

Conversation

parttimenerd
Copy link
Contributor

@parttimenerd parttimenerd commented Jan 25, 2022

Only fall back on Linuxx64 on platforms not mentioned in
https://github.com/torvalds/linux/blob/5bfc75d92efd494db37f5c4c173d3639d4772966/tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py#L56
(excluding the i*86 platforms).
This prevents the installation of a x86_64 JDK on PowerPC (and multiple other platforms).

Fixes #1038.

@marc0der
Copy link
Member

marc0der commented Feb 12, 2022

@parttimenerd happy to improve this, but I do expect some test coverage here. The appropriate test to look at is the PlatformSpec.

Also, I would be as bold as to say that we should make Exotic the fallthrough option, and ensure that x86_64 really is the only possibility for standard 64 bit Linux architecture.

Finally, we also need to ensure that the backend knows what to do with Exotic.

@parttimenerd
Copy link
Contributor Author

Thanks for the review. I will look into adding test coverage next week.

@marc0der
Copy link
Member

@parttimenerd I'd still be really keen to take this forward if you were willing to take this on. Please shout if you need any help with tests.

@parttimenerd
Copy link
Contributor Author

I had other JDK work to do, but I'm going to look into it this evening.

@parttimenerd
Copy link
Contributor Author

parttimenerd commented Mar 15, 2022

I've modified the PR accordingly and added a PR (sdkman/sdkman-broker#15) to sdkman-broker. Is there anything more to do?

@marc0der
Copy link
Member

marc0der commented Mar 20, 2022

@parttimenerd Thanks for contributing this! I had to rebase against master and GitHub was not smart enough to pick that up. I also fixed some of the failing tests. Your changes are now merged and available for testing on the beta channel. Please let me know if this fixes your issues.

@marc0der marc0der closed this Mar 20, 2022
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.

Bug: PowerPC linux is inferred as LinuxX64 platform
2 participants