search

apache / pekko-http

The Streaming-first HTTP server/module of Apache Pekko
https://pekko.apache.org/
Apache License 2.0
149 stars 36 forks source link

Consider putting `akka.parboiled2.util.Base64` into its own sbt module/artifact #40

Closed mdedetrich closed 1 year ago

mdedetrich commented 1 year ago

See comment at https://github.com/apache/incubator-pekko-http/issues/39#issuecomment-1427878059

jrudolph commented 1 year ago

IMO splitting something off will only create extra work but not solve the underlying situation.

mdedetrich commented 1 year ago

Maybe I am misunderstanding, as I see the problem behind the underlying situation is because this specific source file has its own licenses we need to manually take this into account (this is the same problem in peko with the protobuff modules).

In pekko the solution was fairly straight forward because the protobuff was its own sbt modules. This made it easy to granularly apply an overridden META-INF just for artifacts behind those modules and for the source package, we just need to annotate at the bottom of the LICENSE the other licenses (see https://github.com/apache/incubator-pekko/blob/main/LICENSE#L205-L207) i.e.

pekko-protobuf contains the sources of Google protobuf 2.5.0 runtime support, moved into the source package org.apache.pekko.protobuf so as to avoid version conflicts. For license information see COPYING.protobuf

I would imagine if base64 was put into its own sbt module/artifact then hypothetically speaking you would then have a similar text, i.e.

pekko-http-base64 contains additional licenses etc etc etc

Of course I am only talking about technical parts here, legal would have to review irregardless but my assumption is that since there is precedent of this in another pekko project it shouldn't be that exceptional.

pjfanning commented 1 year ago

When I highlighted this file, it was just to enforce my main argument that I would prefer to bring #1 into scope for the pekko-http 1.0.0 release.

If we choose to leave #1 off the agenda, then we will just need to take the hit and update our source and pekko-parsing jar license/notice files accordingly. That work on getting those license/notice files right has been complicated. We get guidance but I would have very low confidence in any of the changes I make to those files. I am not a lawyer and looking around the apache projects, there seem to be different solutions - it really is very hard to know what the right solution is when it comes to adding text to license and/or notice. We may even need to change the license headers in the actual source files (moving the copyrights to the notice files).

mdedetrich commented 1 year ago

When I highlighted this file, it was just to enforce my main argument that I would prefer to bring https://github.com/apache/incubator-pekko-http/issues/1 into scope for the pekko-http 1.0.0 release.

This I don't have arguments against this, my main concern was replacing akka.parboiled2.util.Base64 with JVM's base 64

If we choose to leave https://github.com/apache/incubator-pekko-http/issues/1 off the agenda, then we will just need to take the hit and update our source and pekko-parsing jar license/notice files accordingly.

I don't think this is mutually exclusive though? I was just thinking of moving that single akka.parboiled2.util.Base64 source file into its own sbt project/artifact while the parboiled2 upgrade is done separately as a different task (and both can be done for 1.0.0 if need be)

That work on getting those license/notice files right has been complicated. We get guidance but I would have very low confidence in any of the changes I make to those files.

I guess I have a different tolerance for "complicated", I see it more as a hassle in the sense that it may take some time until legal gets around viewing it but the eventual action points/results are not technically that complicated. I am also saying this in context of the fact that as we spoke elsewhere, we aren't in a rush to release pekko-http (our main focus is pekko-core)

I am not a lawyer and looking around the apache projects, there seem to be different solutions - it really is very hard to know what the right solution is when it comes to adding text to license and/or notice. We may even need to change the license headers in the actual source files (moving the copyrights to the notice files).

My strategy to tackle this would be to get the appropriate person/s and/or teams to look at it and see what they say. I wasn't paying too much attention (or I have forgotten) how it worked with the protobuf/google case but I think it was similar?

pjfanning commented 1 year ago

I'm happy to use parboiled2 jar's class. Binary linking is simpler license wise than copy/pasting the code. Basically, using https://github.com/apache/incubator-pekko-http/pull/14

This Base64 is a bit uglier because it has multiple authors but maybe it isn't much more complicated than the google protobuf code.

mdedetrich commented 1 year ago

This Base64 is a bit uglier because it has multiple authors but maybe it isn't much more complicated than the google protobuf code.

Yup this is my thinking as well, and making it its own artifact so that META-INF only contains the relevant licenses/notices for that file might be a hassle but its not hard to do

pjfanning commented 1 year ago

My preference is to import org.parboiled2.util.Base64 class and use that. Binary linking as opposed to copy/paste of code.

mdedetrich commented 1 year ago

My preference is to import org.parboiled2.util.Base64 class and use that. Binary linking as opposed to copy/paste of code.

Oh I think now I know why I am confused, I was under the impression that this was custom written for akka-http specifically and not part of parboiled. If that isn't the case then obviously linked to the binary is far easier

mdedetrich commented 1 year ago

Closing issue because I incorrectly thought that org.parboiled2.util.Base64 was custom written for akka-http due to it being pointed out.