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

Support Debian #44

Closed
ejona86 opened this issue Jun 17, 2015 · 31 comments
Closed

Support Debian #44

ejona86 opened this issue Jun 17, 2015 · 31 comments
Assignees
Milestone

Comments

@ejona86
Copy link
Member

ejona86 commented Jun 17, 2015

For gRPC, we're interested in encouraging users to use tcnative instead of jetty-apln. This would work pretty well, except that lots of our users would be Debian/Ubuntu users. Having a pre-built binary for Debian is a prerequisite for us encouraging our users to use tcnative.

We would have little problem making our own builds of tcnative, but it seems that solving the "Debian vs Red Hat" problem would help tcnative in general.

The main problem I see is what to specify as the classifier for any Debian binary. The OS detector doesn't vary based on RHEL/Debian. We could just add "-debian" to the end and force users to hard-code when they are running on Debian. Not great, but better than nothing.

If combined with #24, we could solve it more cleanly as both variants of the Linux .so could be included in the JAR.

@nmittler
Copy link
Member

nmittler commented Jul 1, 2015

@normanmaurer @trustin @Scottmitch any thoughts on this?

@Scottmitch
Copy link
Member

@nmittler - I would like to see Debian supported. I wonder if we could make an "intermediate" dependency which pulls in the correct one depending on linux distro (and maybe for mac, windows, etc..).

  1. Users include tc-native.
  2. The tc-natvie pom file detects which architecture (maybe runs shell cods if necessary) to figure out tc-native-<architecture> and includes it.
  3. We build/publish artifacts for tc-native-<architecture>.

Does this sound feasible?

@normanmaurer
Copy link
Member

@nmittler I would love to see Debian support as well but I was just too busy to think about it.. gimme some more time.

@nmittler
Copy link
Member

nmittler commented Jul 1, 2015

@Scottmitch yeah that might work. So maybe have an intermediate dependency called tc-native-linux that switches on the distro.

It would also be nice to have the os-maven-plugin updated to provide more information.

@Scottmitch
Copy link
Member

@nmittler - Maybe but what I was originally thinking was having 3 layers:

  1. tc-native (users directly include this in their POMs)
  2. tc-native-arch-detector (tc-native from layer 1 includes this. this does the work of picking the correct dependency from the layer 3 below).
  3. Each platform would publish a jar at this level (tc-native-linux-rhel, tc-native-linux-debian, tc-native-windows-..., tc-native-osx-..., etc... does it make sense to also include 64 bit 32 bit in jar versions?).

If you wanted you could include a 4th layer which knows more details about its respective platform (layer 3 would be layer 4...and the new layer 3 would include 1 pom for windows, 1 for linux, 1 for mac, etc...)

@nmittler
Copy link
Member

nmittler commented Jul 1, 2015

@Scottmitch So what we have now is layers 1 & 3. I think we can avoid layer 2 if we modify the os-maven-plugin to add an additional field (e.g. os.detected.family), which would only be used on linux and would contain the value of ID_LIKE from /etc/os-release. Then we could have the plugin optionally include ${os.detected.family} in the value it generates for ${os.detected.classifier}, giving us something like linux_debian_x86_64.

@nmittler
Copy link
Member

nmittler commented Jul 1, 2015

@normanmaurer any thoughts?

@trustin
Copy link
Member

trustin commented Jul 2, 2015

We could add os.detected.family and let it be generic or something similar for undetected families and the OSes without diversity. Then we could use ${os.detected.classifier}_${os.detected.family}? Alternatively, we can just break the backward compatibility and let os.detected.classifier includes os.detected.family. It's just a classpath changes, so I guess it shouldn't be a big deal.

However, I think we could solve this problem more nicely. Please see the issues filed by @cconroy - #33 and netty/netty#3502 We could revise our release (and deployment) process so that our CI server takes care of producing the JAR with the native libraries of all platforms, then the problem gets much simpler.

@nmittler
Copy link
Member

nmittler commented Jul 6, 2015

@trustin regarding #33 and netty/netty#3502, wouldn't we still need a separate build for debian/rhel due to the difference of soname for libssl.so and libcrypto.so?

@normanmaurer
Copy link
Member

@nmittler yes we would. That said I would like to also release a version build against libressl ans maybe also boringssl. So let me pick this up next week and get all of it done.

@nmittler
Copy link
Member

nmittler commented Jul 6, 2015

@normanmaurer that would be awesome! Let me know if there's anything I can help out with.

@zhangkun83
Copy link

Alternatively, we can just break the backward compatibility and let os.detected.classifier includes os.detected.family. It's just a classpath changes, so I guess it shouldn't be a big deal.

@trustin Protobuf team and gRPC team are using os.detected.classifier in the pre-compiled protoc and codegen artifacts published on Maven Central. This would break us as we are not going to publish artifacts per family.

@nmittler
Copy link
Member

nmittler commented Jul 8, 2015

If we make the inclusion of ${os.detected.family} in the classifier opt-in (see #44 (comment)) then that wouldn't break us.

@normanmaurer thoughts?

@zhangkun83
Copy link

I would not change what os.detected.classifier returns. It would break everything around protoc and grpc codegen artifacts, including the publishing process, and Gradle and Maven plugins that consume them.

IMO the biggest value of os-maven-plugin is producing unified identifiers for OS, arch, etc. Providing a unified classifier is not universally useful because different projects may want to include different fields in the classifier. It would be more reasonable for different projects to compose their own classifiers from the individual fields provided by os-maven-plugin. Maybe we should eventually remove os.detected.classifier.

@trustin
Copy link
Member

trustin commented Jul 9, 2015

On Thu, Jul 9, 2015, at 04:15 AM, Kun Zhang wrote:

I would not change what os.detected.classifier returns. It would break
everything around protoc and grpc codegen artifacts, including the
publishing process, and Gradle and Maven plugins that consume them.

IMO the biggest value of os-maven-plugin is producing unified
identifiers for OS, arch, etc. Providing a unified classifier is not
universally useful because different projects may want to include
different fields in the classifier. It would be more reasonable for
different projects to compose their own classifiers from the
individual fields provided by os-maven-plugin. Maybe we should
eventually remove os.detected.classifier.

Agreed. +1 for not changing os.detected.classifier. Not sure about
deprecating it since it should still be useful for statically linked
libraries. The problem we are seeing here is basically we don't want to
link statically with glibc(xx) and openssl.

Let me try to release os-maven-plugin with my spare cycle.

@nmittler
Copy link
Member

@trustin +1, I see no need in deprecating os.detected.classifier. It's there as a utility if folks want to use it.

@normanmaurer is this still on your radar?

@normanmaurer
Copy link
Member

@nmittler yeah kind of... just too busy and not really very high on my to do list atm :(

@nmittler
Copy link
Member

@normanmaurer anything I could do to help out? Maybe I could take a crack at the changes to the os-maven-plugin to get the ball rolling?

@nmittler
Copy link
Member

FYI, I just created trustin/os-maven-plugin#10 to add detection of OS variants to the os-maven-plugin. It should be fairly easy to retrofit netty-tcnative to use a custom deployment for debian.

@nmittler
Copy link
Member

@normanmaurer gentle ping. Now that the os-maven-plugin has been released, we should be able to release netty_tcnative for debian.

@normanmaurer
Copy link
Member

@nmittler will do tomorrow. Sorry for the slowness 👎

@nmittler
Copy link
Member

@normanmaurer sgtm! No worries, I know you're busy! :)

@nmittler
Copy link
Member

@normanmaurer Just to clarify, you'll also release a "_debian" artifact built on wheezy?

@normanmaurer
Copy link
Member

@nmittler that's the plan

@nmittler
Copy link
Member

👍

@nmittler
Copy link
Member

@normanmaurer I think we can close this now?

@Scottmitch
Copy link
Member

@nmittler
Copy link
Member

It brings a tear to my eye!!! Closing!!! :)

Thanks @normanmaurer @Scottmitch @trustin!!!

@normanmaurer
Copy link
Member

Boom!

Am 28.08.2015 um 16:08 schrieb Nathan Mittler notifications@github.com:

Closed #44.


Reply to this email directly or view it on GitHub.

@nmittler
Copy link
Member

LOL ... what he said!

@trustin
Copy link
Member

trustin commented Aug 29, 2015

@nmittler High-five! ✋

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

No branches or pull requests

6 participants