Skip to content

Detect M1 mac in test and substitute container image (#57) #75

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

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

alpreu
Copy link
Contributor

@alpreu alpreu commented Aug 29, 2022

This is a best-effort solution for issue #57.

It uses the JVMs system properties to detect the M1 architecture.
This solution will fail if the JVM is running through Rosetta emulation. Other approaches that are independent of the JVM suffer similar problems (see this question about detecting the M1 from the cli for example) so I believe there is no really good solution for this edge case.

@@ -38,4 +39,18 @@ protected static String getHttpServiceUrl() {
return PULSAR_CONTAINER.getHttpServiceUrl();
}

private static boolean isRunningOnMacM1() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It uses the JVMs system properties to detect the M1 architecture.
This solution will fail if the JVM is running through Rosetta emulation. Other approaches that are independent of the JVM suffer similar problems (see this question about detecting the M1 from the cli for example) so I believe there is no really good solution for this edge case.

I think this is a pragmatic choice. We could later provide an env var that users in the "rosetta" group could set as an override/last resort. Again, I think this is fine for now.

If we start needing to do this same logic in other places we could add something like MacM1Detector that encapsulates how we determine this info.

Copy link
Collaborator

@onobc onobc left a comment

Choose a reason for hiding this comment

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

Looks good @alpreu !

@onobc onobc merged commit dfa1e36 into spring-projects:main Aug 29, 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.

2 participants