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

Make ScalaBuff compiler work with windows paths as well. #37

Conversation

bantonsson
Copy link
Contributor

Hi,
We've started using ScalaBuff in one part of the Akka project, and we need it to work on windows as well.

It would be great if you could release a 1.1.3 version with these changes.

@SandroGrzicic
Copy link
Owner

Hi,

This is obviously a reasonable request.

However, I have a question: have you actually tested this on Windows and found it not to work?

The reason that I'm asking is that I actually develop on Windows (7) and in my past years of JVM usage haven't ever seen paths with / fail on Windows for any reason. That's why I treat them as "cross platform" paths.

Again, please tell me if I'm wrong, and if so what specific Windows and Java versions fail with the / paths.

Thanks!

@bantonsson
Copy link
Contributor Author

Hi,

Yes, we have tested this on windows. Both Windows 7 and Windows XP, and it doesn't work if you give windows paths to the compiler from the command line.

It's not about backwards slash paths not working in Java (they get canonicalized), but that you generate a mixed backwards and forwards slash path if you get a backwards slash path as input. The windows style paths are generated by the build tools, so it's not an option to just change them.

Here is an example:

java net.sandrogrzicic.scalabuff.compiler.ScalaBuff --scala_out=akka-cluster\target\src_managed\scala akka-cluster\src\main\protobuf\ClusterMessages.proto

* Error: Unable to write output file: akka/cluster/protobuf/msg/Akka-cluster\src
\main\protobuf\ClusterMessages.scala

@SandroGrzicic
Copy link
Owner

Right, you're absolutely correct. Thanks for the PR! Feel free to discuss ScalaBuff on the Google Group if you have any other proposed changes, enhancements, etc.! ;)

@SandroGrzicic SandroGrzicic reopened this Apr 22, 2013
SandroGrzicic added a commit that referenced this pull request Apr 22, 2013
…indows-paths-ban

Make ScalaBuff compiler work with Windows paths as well.
@SandroGrzicic SandroGrzicic merged commit 91db612 into SandroGrzicic:master Apr 22, 2013
@SandroGrzicic
Copy link
Owner

Note: The next release will be 1.2.0 due to some relatively minor but generated-message-changing changes (the type of repeated fields will be changed from Vector to Seq - the actual default instance will remain Vector but now you'll be able to use any collection as a repeated field) and I also have some other code changes planned. However I'm very busy right now so the release will probably happen tomorrow or in a few days maximum. I hope that works for you.

@bantonsson
Copy link
Contributor Author

Thanks for the update. That's fine. We're able to work around things.

@viktorklang
Copy link
Collaborator

Remember to make it immutable.Seq as Seq is collection.Seq and can as such be mutable, which is not desirable.
Cheers,

@SandroGrzicic
Copy link
Owner

@bantonsson I've published a 1.2.0-SNAPSHOT (for 2.10 only) on Sonatype that you can use before I publish the release.
@viktorklang Awesome, thanks for the catch, I've replied in the commit line note.

@bantonsson bantonsson deleted the wip-make-scalabuff-work-with-windows-paths-ban branch April 23, 2013 07:38
@thomasschoeftner
Copy link

ScalaBuff seems to work fine for me on Win7 using the Oracle JDK and sbt 0.13.

The sbt-scalabuff plugin, however, may be problematic. - I had to rebuild it for sbt 0.13 and upgrade it to scalabuff 1.3.6 to make it work... sbt-scalabuff 0.3-SNAPSHOT is still using ScalaBuff 1.1.2 internally.

This did it for me: https://github.com/thomasschoeftner/sbt-scalabuff

Edit:
My flavor of sbt-scalabuff is using 1.3.7-SNAPSHOT of scalabuff now..
I also modified the mechanism how to feed the .proto files to the scalabuff compiler, because of problems with Protobuf imports when processing single files. Now it is using --proto_path instead.

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

Successfully merging this pull request may close these issues.

4 participants