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

Changing the 'os.detected.arch' property for s390 and s390x #43

Closed

Conversation

namrata-ibm
Copy link

os.detected.arch on s390 is returned as s390_32 and for s390x it is returned as s390_64 which needs to be corrected. This PR is for fixing the same.

@namrata-ibm
Copy link
Author

@trustin Could you please check?

@namrata-ibm
Copy link
Author

@trustin Could you please have a look?

@trustin
Copy link
Owner

trustin commented May 23, 2020

Hi! Sorry I'm late. Is s390 and s390x completely different from each other? If s390x is a 64-bit variant of s390, then I think it's better keeping the current naming for consistency, like we have x86_32 and x86_64. If this is not the case, please let me know with some relevant documentation.

@namrata-ibm
Copy link
Author

@trustin s390x is 64 bit variant. Came across an issue while building SonarQube which looks up for protoc-linux-s390_64.exe on Maven Central. This name is formed using osdetector-gradle-plugin (which internally uses this plugin).
However wanted to get to your notice that binaries for s390x would always be created with "s390x" suffix and not as per the naming convention used here. (unlike x86, for which both x86_32 and x86_64 are valid identifiers)

eg: https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.12.1/
https://search.maven.org/search?q=g:org.wildfly.openssl
https://repo1.maven.org/maven2/org/eclipse/swt/org.eclipse.swt.gtk.linux.s390x/4.3/

Please let me know your comments.

@trustin
Copy link
Owner

trustin commented May 25, 2020

I see a similar issue with i686 vs x86_32. Instead of breaking backward compatibility, how about adding some mapping configuration that allows a user to map certain values (s390_32 and s390_64 in this case) to user-specified ones (s390 and s390x in this case)?

@namrata-ibm
Copy link
Author

I am not sure whether I can follow above suggestion correctly. s390_32/s390_64 are invalid.
Could you please elaborate more about mapping at user end?

@trustin
Copy link
Owner

trustin commented Feb 6, 2021

@namrata-ibm Sorry for a late reply. I meant letting a user create a file (.os-mappings.properties) that contains something like this:

arch.s390_32=s390
arch.s390_64=s390x

.. so that this plugin picks this file up.

@namrata-ibm
Copy link
Author

@trustin so this would mean creating above file in every project which uses this plugin?

@trustin
Copy link
Owner

trustin commented Feb 8, 2021

Yes, because I assume there are projects that use s390_32 and s390_64 already in production. However, if we are sure there are no projects that use them, we could make a breaking change with calculated risk. What do you think?

@namrata-ibm
Copy link
Author

@trustin tried searching but seems difficult to determine who all use this in production already. Hence will close this PR as handling it at project level seems a safer approach.
Thank you for your comments.

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.

None yet

2 participants