Miscellaneous Scala clean up #4055
Conversation
Scott-Guest
force-pushed
the
scala-cleanup
branch
from
6d6d72b
to
094f303
Compare
|
NixOS/nixpkgs#292323 for posterity, I implemented a feature in our backported maven builder that we can hopefully get upstreamed back into Nixpkgs |
|
Going to hold off on merging this until @goodlyrottenapple can weigh in; the fixed-output derivation hash failure introduced in 8fe2f88 is something we've seen before with this Nix / Maven setup. As I recall, the issue is that any change to the We discussed pulling this module out in #4063; doing so might be worth doing in order to make the Scala KORE parser a proper first-class dependency (and therefore avoid this problem). Footnotes
|
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
Confirmed with @goodlyrottenapple that extracting the code we currently push as a JAR to maven repositories to its own repo, and releasing versioned JARs rather than snapshots. In the interests of letting @Scott-Guest's Scala upgrade work go through, I'm going to merge this PR even though doing so will mean dealing with hash mismatches for a few days. |
Reverts #4055 We will want to reinstate this PR once the required `scala-kore` infrastructure is actually ready to use.
#1006 is failing because the Scala Maven plugin is trying to download the sources for the compiler bridge, but can't do so outside of a Nix FOD. This PR adds the bridge to the manually-downloaded Maven artifacts (as we did in runtimeverification/k#4055)
This PR makes a couple quick changes analogous to runtimeverification/k#4055: - Update `scala-maven-plugin` to its latest version - Set the Java version by relying on `<release>` in `maven-compiler-plugin` inherited from the parent POM, rather than manually setting `-source` and `-target` for `javac` - Disable various Scala language extensions and enable `-Werror` - One warning about pattern matching on an erased type - Remaining warnings were exhaustiveness issues fixed with a `case _ => ???`. This is poor style and an abuse of `???` IMO, but it's already pervasive throughout our codebase. --------- Co-authored-by: Bruce Collie <[email protected]>
This reverts commit ec70555, which was itself a revert of #4055 because of breakages in our Nix builds because of issues around the `scala-kore` dependency. This PR reinstates the original change now that `scala-kore` has been properly released and versioned, and also makes a fix (see comment below) for an issue with Nix / Maven integration that was producing spurious hash invalidations. --------- Co-authored-by: Scott Guest <[email protected]> Co-authored-by: rv-jenkins <[email protected]>
This PR makes a couple quick miscellaneous changes:
maven-compiler-pluginandscala-maven-pluginto their latest versions<release>rather than<source>and<target><source>and<target>only set the source code and generated class file formats, but do not take into account API changes between Java releases<release>tag should automatically be respected byscala-maven-pluginas well (once Java 17 (still) not supported inkoresubmodule #4041 is fixed)-WerrorKORE.Att()sugar forAtt.empty()ScalaSugarThe commit history is clean, and I recommend reviewing this one commit at a time.