-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Translate the sbt-bridge to Java. #5596
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:
- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
cf0f492
to
0ece6ae
Compare
Well, this doesn't work, because when Zinc compiles the compiler bridge, it assumes it's entirely written in Scala. See So no source ends up being compiled, the jar/class directly is empty, and then of course Zinc cannot find the classes it's looking for. This would require changes in Zinc to move forward. |
* Fix the compiler bridge ClassLoader | ||
* <p> | ||
* Soundtrack: https://www.youtube.com/watch?v=imamcajBEJs | ||
* <p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No </p>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the Javadoc guide does not use the closing </p>
s.
I feared that. I'll look into it. |
1a8ac98
to
f295a26
Compare
All green now :) |
5d5da23
to
720097e
Compare
Only the files in the Compile configuration are translated. Tests are kept in Scala.
Normally, the source of the sbt bridge is fetched from scalaCompilerBridgeSource, compiled, then cached by sbt. Unfortunately the logic in sbt to do this does not take .java source files into account, so the previous commit broke our bridge. Thankfully, it turns out that I have amazing foresight ;). In sbt/sbt#4332 I added scalaCompilerBridgeBinaryJar to sbt, which bypasses the whole bridge compilation and caching logic which is not needed when the bridge is Java-only and thus binary-compatible across Scala releases. So just using this setting is enough to make everything work!
Not needed now that the project is Java-only.
720097e
to
0685e50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed Allan's comments.
@smarter's commits look good to me.
* Fix the compiler bridge ClassLoader | ||
* <p> | ||
* Soundtrack: https://www.youtube.com/watch?v=imamcajBEJs | ||
* <p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the Javadoc guide does not use the closing </p>
s.
These caches are only used with scalaCompilerBridgeSource, so we don't need them now that we switched to scalaCompilerBridgeBinaryJar
57ab07d
to
6da66c2
Compare
You shouldn't have to, we publish binary artefacts for the compiler bridge which are directly usable. In sbt this is handled by https://github.com/sbt/sbt/blob/develop/zinc-lm-integration/src/main/scala/sbt/internal/inc/ZincLmUtil.scala (which is called from https://github.com/sbt/sbt/blob/2a26e8186894dc0af46cd142306727b1acd722f8/main/src/main/scala/sbt/Defaults.scala#L808-L814) |
|
By the way, if you're looking into this to get scala 3 support in gradle, check out gradle/gradle#18001 |
I am! My attempt is at [gradle/gradle#18003] ;) |
Only the files in the Compile configuration are translated. Tests are kept in Scala.