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

consider using parboiled2 as a jar dependency in v1.0.0 #39

Closed
pjfanning opened this issue Feb 13, 2023 · 8 comments
Closed

consider using parboiled2 as a jar dependency in v1.0.0 #39

pjfanning opened this issue Feb 13, 2023 · 8 comments

Comments

@pjfanning
Copy link
Contributor

pjfanning commented Feb 13, 2023

Relates to #35

@pjfanning
Copy link
Contributor Author

pjfanning commented Feb 13, 2023

@jrudolph @mdedetrich this is just for discussion but I'd be interested in your views on this

@mdedetrich
Copy link
Contributor

mdedetrich commented Feb 13, 2023

one particularly problematic file is https://github.com/apache/incubator-pekko-http/blob/2e8b2fade52cf0a85d0150a407226851cf00c683/akka-parsing/src/main/java/akka/parboiled2/util/Base64.java

  • has multiple authors at various stages and BSD and Apache licensing
  • even maybe possible to use Java's Base64 class at this stage

I will comment on the other stuff later, but I wouldn't advocate for removing/changing this for Java's Base64 class. There is a reason why this Base64 implementation is there, Base64 is used frequently in the context of http and as you can read from the comments its a far more efficient implementation then Java's Base64. Generally speaking its also not unusual for the Akka/Pekko ecosystem to reimplement JVM API's if they are not very efficient (which isn't uncommon) and one of Akka/Pekko's main selling points is performance.

As an alternative we can make this base64 into its own sbt module/artifact which would make it fairly easy to add in custom licenses in a similar to what was done with pekko-core's protobuf modules? This can also provide other benefits, i.e. other projects can include specifically just the base64 artifact rather than entire pekko-http if thats all they need.

@jrudolph
Copy link
Contributor

This is a duplicate of #1. Basically, I agree with this move. In #14, @mdedetrich suggested to move that change to the 1.1.x release but if it is easy enough to do I would also concur with doing this earlier if it simplifies the license situation.

@jrudolph
Copy link
Contributor

Btw. IIRC the original reasoning for vendoring was to avoid a transitive dependency on shapeless (which has now been removed from parboiled2) to avoid having to delay releases until shapeless is out.

@mdedetrich
Copy link
Contributor

@jrudolph Do you have any comments on the Base64 situation?

@mdedetrich
Copy link
Contributor

mdedetrich commented Feb 13, 2023

In #14, @mdedetrich suggested to move that change to the 1.1.x release but if it is easy enough to do I would also concur with doing this earlier if it simplifies the license situation.

I am fine with doing the change in 1.0.x release if it simplifies the license situation dramatically. pekko-http due to being a http web server/client doesn't have as strict requirements as something like pekko-core

@pjfanning
Copy link
Contributor Author

pjfanning commented Feb 13, 2023

I'll close this in favour of #1 and link this discussion there

@mdedetrich
Copy link
Contributor

Also created #40 to discuss Base64 situation specifically

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

3 participants