search

trustin / os-maven-plugin

A Maven plugin that sets various useful properties detected from ${os.name} and ${os.arch} properties.
Apache License 2.0
296 stars 66 forks source link

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

Closed namrata-ibm closed 3 years ago

namrata-ibm commented 4 years ago

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 commented 4 years ago

@trustin Could you please check?

namrata-ibm commented 4 years ago

@trustin Could you please have a look?

trustin commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

trustin commented 3 years ago

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 commented 3 years ago

@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.