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

Upgrade to fastparse v2 #510

Closed
olafurpg opened this issue Oct 25, 2018 · 9 comments
Closed

Upgrade to fastparse v2 #510

olafurpg opened this issue Oct 25, 2018 · 9 comments

Comments

@olafurpg
Copy link
Contributor

Fastparse v2 came out recently (http://www.lihaoyi.com/post/Fastparse2EvenFasterScalaParserCombinators.html) and it has binary breaking changes with v1.0.0. It doesn't look like fastparse v2 will use a custom package name like fastparse.v2 (https://gitter.im/lihaoyi/fastparse?at=5bd18969ae7be94016b25649) so all libraries using fastparse must either upgrade to v2 or shade the fastparse dependency.

Would it be possible for ScalaPB to shade its fastparse dependency? This would allow downstream users of ScalaPB to choose whether they want fastparse v1 or v2 on their classpath.

@thesamet
Copy link
Contributor

If I shade the default artifact, then I'll be forcing users to have our private shaded copy of fastparse in the classpath (+potentially duplicate code for Scala.js users). If I publish this under another artifact name (say scalapb-runtime-shaded), then dependencies such as scalapb-json4s and scalapb-runtime-grpc will transitively bring in the original scalapb-runtime which depends on fastparse 1 which would cause a mess.

Maybe we should isolate the parsing out to an external library, and provide two implementations one for fastparse 1 and one for fastparse 2. Regardless, I think we'll have fastparse 2 as the default in ScalaPB 0.9.0

@olafurpg olafurpg changed the title Shade fastparse dependency Upgrade to fastparse v2 Oct 28, 2018
@olafurpg
Copy link
Contributor Author

Fair enough, I renamed the ticket to "upgrade to fastparse v2". No rush on my side, we don't use the text format parser in Scalameta and we have opted to shade fastparse v1.

@raboof
Copy link
Contributor

raboof commented Jan 31, 2019

Looks like this depends on com-lihaoyi/fastparse#215 (which depends on the Scala Native 0.4.0 release)

raboof added a commit to raboof/ScalaPB that referenced this issue Jan 31, 2019
Disabled 'native' targets for now since fastparse 2 has not been released for
Scala Native yet.
@raboof
Copy link
Contributor

raboof commented Jan 31, 2019

Fastparse 2 is not available for scala 2.10, so this also depends on #542 (no it doesn't, the plugin doesn't depend on fastparse, only the runtime)

raboof added a commit to raboof/ScalaPB that referenced this issue Jan 31, 2019
Disabled 'native' targets for now since fastparse 2 has not been released for
Scala Native yet.
@dgrnbrg
Copy link

dgrnbrg commented Feb 7, 2019

I would like to have my application use ScalaPB & Ammonite. Unfortunately, Ammonite seems to depend on Fastparse 2, which is blocking this inclusion.

So, +1 for this issue.

thesamet pushed a commit that referenced this issue Mar 16, 2019
Disabled 'native' targets for now since fastparse 2 has not been released for
Scala Native yet.
thesamet pushed a commit that referenced this issue Mar 24, 2019
Disabled 'native' targets for now since fastparse 2 has not been released for
Scala Native yet.
@SethTisue
Copy link
Contributor

SethTisue commented Mar 25, 2019

looks like this should be closed now? (is there a release with this change yet?)

@raboof
Copy link
Contributor

raboof commented Mar 25, 2019

Yes - 0.9.0-M1 should contain this change

@thesamet
Copy link
Contributor

Yes, this went in via b467afa

@thesamet
Copy link
Contributor

Thanks @raboof for this PR!

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

5 participants