Skip to content

refactor scalac java_binary target creation #1677

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

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

gergelyfabian
Copy link
Contributor

@gergelyfabian gergelyfabian commented Jan 14, 2025

I'd like to remove Scala compiler extensions from the Scala toolchain, that I could be potentially building myself (in the same monorepo, build the compiler extension, but also use it).

Till now the only way to add Scala compiler dependencies was a toolchain.
If I left the compiler extension as a toolchain dependency there, I could not build the compiler extension due to a cycle in deps (building the compiler extension would depend on itself through the toolchain).

I'd like to work this around by defining my own scalac java_binary target, where I can define my own dependencies (and then create rule names with macros that override the scalac target).

To do this without duplicating code from rules_scala, refactor SCALAC_DEPS parameter and scalac target definitions to a Starlark file to make them reusable for outside users.

Added:

  • define_scalac()
  • define_scalac_bootstrap()

A user could do:

EXTENDED_SCALAC_DEPS = DEFAULT_SCALAC_DEPS + [
    "@//my/compiler/dependency:library",
]

define_scalac(
    name = "scalac_with_extensions",
    deps = EXTENDED_SCALAC_DEPS,
)

And then build that compiler library in the same monorepo, this way, providing compiler dependencies without toolchains.

See #1674 for more information.

@gergelyfabian
Copy link
Contributor Author

Rebased with latest master to ensure the build for latest Bazel does not fail.

@gergelyfabian
Copy link
Contributor Author

In fact, I could move the java_binary's definition to the Starlark file too...

@gergelyfabian
Copy link
Contributor Author

In fact, I could move the java_binary's definition to the Starlark file too...

Just did that.

@gergelyfabian gergelyfabian changed the title refactor scalac deps refactor scalac java_binary target creation Jan 14, 2025
@gergelyfabian
Copy link
Contributor Author

Moved the scalac target definitions to Starlark too, so that those are also reusable. Implemented the idea with Label from @mbland (Thanks!) and added a more detailed description. This could be a user-friendly solution to #1674 in fact.

@gergelyfabian gergelyfabian force-pushed the refactor-scalac-deps branch 2 times, most recently from a1ef249 to cc40b02 Compare January 15, 2025 09:31
@gergelyfabian
Copy link
Contributor Author

Not sure why the build failed, I just rebased with latest master.

@mbland
Copy link
Contributor

mbland commented Jan 15, 2025

This looks like the culprit: https://buildkite.com/bazel/rules-scala-scala/builds/5293#0194694d-dea6-481f-85ca-36bcc3da80d3/126-462

ERROR: /workdir/test/gen_src/BUILD:20:13: scala @//test/gen_src:uses_gen_file failed:
  (Exit 1): scalac failed: error executing command
  (from target //test/gen_src:uses_gen_file)
  bazel-out/k8-opt-exec-2B5CBBC6/bin/src/java/io/bazel/rulesscala/scalac/scalac
  @bazel-out/k8-fastbuild/bin/test/gen_src/uses_gen_file.jar-0.params

bazel-out/k8-fastbuild/bin/test/gen_src/foo.scala:1: error: Invalid literal number

BTW, the other ERROR further down about a nonexistent //test/coverage/... package I've fixed in one of the commits in #1678. That "fix" doesn't actually affect the test result, because both of the builds launched by the script fail in the same way, but it no longer hides the underlying breakage.

The error from this PR seems to be a different issue altogether. I'm not immediately sure what it is.

@gergelyfabian
Copy link
Contributor Author

The error from this PR seems to be a different issue altogether. I'm not immediately sure what it is.

It seems it's non-deterministic (and probably not related to this PR). Amended my commit, force-pushed, and the rerun of the same test passed.

I'd like to remove Scala compiler extensions from the Scala toolchain, that
I could be potentially building myself (in the same monorepo, build the
compiler extension, but also use it).

Till now the only way to add Scala compiler dependencies was a toolchain.
If I left the compiler extension as a toolchain dependency there, I could
not build the compiler extension due to a cycle in deps (building the
compiler extension would depend on itself through the toolchain).

I'd like to work this around by defining my own scalac java_binary target,
where I can define my own dependencies (and then create rule names with
macros that override the scalac target).

To do this without duplicating code from rules_scala, refactor
SCALAC_DEPS parameter and scalac target definitions to a Starlark file to
make them reusable for outside users.

Added:
- define_scalac()
- define_scalac_bootstrap()

A user could do:
```
EXTENDED_SCALAC_DEPS = DEFAULT_SCALAC_DEPS + [
    "@//my/compiler/dependency:library",
]

define_scalac(
    name = "scalac_with_extensions",
    deps = EXTENDED_SCALAC_DEPS,
)
```

And then build that compiler library in the same monorepo, this way,
providing compiler dependencies without toolchains (that would cause
cycles for this usecase).

See bazel-contrib#1674 for more information.
Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @gergelyfabian! Sorry for unreasonable delay,

@simuons simuons merged commit 21d6ed0 into bazel-contrib:master Feb 13, 2025
2 checks passed
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.

3 participants