Skip to content

Conversation

@gmishkin
Copy link
Contributor

@gmishkin gmishkin commented May 5, 2019

Change to use the platform-specific path separator. The stub template script already sets IFS=';' on Windows so having colons in CLASSPATH means it doesn't get split properly.

@gmishkin gmishkin requested a review from johnynek as a code owner May 5, 2019 04:19
@gmishkin gmishkin changed the title Portable classpath Portable classpath in stub template May 5, 2019
@johnynek
Copy link
Contributor

johnynek commented May 8, 2019

Thanks for the catch.

It would be great to follow this up with a PR that would exercise this in a test so that we don't regress on this since it may be important to you.

@johnynek johnynek merged commit b5daa06 into bazel-contrib:master May 8, 2019
@gmishkin
Copy link
Contributor Author

gmishkin commented May 8, 2019

There's another thing I need to do to get the whole thing working (bazel-deps), which I thought I would get back to sooner (but didn't) and then think about how to test it.

For some reason templates/jar_artifact_resource.bzl is ending up at the root of the C: drive instead of in the jar or wherever, so the parse target doesn't run. I'm guessing it has something to do with how the resources attribute is handled in scala_binary targets with Windows paths. I need to look into it more.

The other thought I had which I'm not sure is a direction you'd consider going in but would apply to this class path issue too: would it be possible to share more with the native Java rules? Those create a native .exe on Windows instead of using the stub template.

@gmishkin gmishkin deleted the portable-classpath branch May 8, 2019 23:01
@johnynek
Copy link
Contributor

johnynek commented May 9, 2019

you did see this PR, right: #718 will this be helpful?

@gmishkin
Copy link
Contributor Author

gmishkin commented May 9, 2019

Ah, I did see it a while ago before I even ran into this issue, but it was just a curiosity at the time so it slipped my mind.

I think the launcher improvement was not sufficient to get bazel-deps specifically working. Some issue either there, or here in the way these rules handle resources paths on Windows, is still causing jar_artifact_backend.bzl to get dropped in C:\templates instead of where (I suppose) it should be.

Exception in thread "main" java.lang.NullPointerException
        at com.github.johnynek.bazel_deps.Writer$.jarArtifactBackend$lzycompute(Writer.scala:13)
        at com.github.johnynek.bazel_deps.Writer$.jarArtifactBackend(Writer.scala:12)
        at com.github.johnynek.bazel_deps.Writer$.workspace(Writer.scala:149)
        at com.github.johnynek.bazel_deps.MakeDeps$.apply(MakeDeps.scala:79)
        at com.github.johnynek.bazel_deps.ParseProject$.main(ParseProject.scala:17)
        at com.github.johnynek.bazel_deps.ParseProject.main(ParseProject.scala)

gergelyfabian pushed a commit to gergelyfabian/rules_scala that referenced this pull request May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants