Skip to content

Resolve scala version per namespace in bazel mbt importer - #8612

Draft
jchyb wants to merge 1 commit into
scalameta:main-v2from
jchyb:fix-mixed-scala-bazel
Draft

Resolve scala version per namespace in bazel mbt importer#8612
jchyb wants to merge 1 commit into
scalameta:main-v2from
jchyb:fix-mixed-scala-bazel

Conversation

@jchyb

@jchyb jchyb commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Previously a single scala version was picked for all namespaces:

  • first by looking for scala_library dependencies in the repo and matching version through that (additional query)
  • then if those don't exist, by trying to match the MODULE.baze/WORKSPACE entry via a regex (doesn't work with variables, and load calls in WORKSPACE/ include calls in MODULE.bazel)

Now, we try to get the version for each namespace, without any additional query calls (mostly because I couldn't find one that would work consistently, even with cquery):

  • we check the already queried data for scala_version entries
  • we try the same lookup of scala_library dependencies as before, except we use the already queried data to determine it
  • if some namespaces still don't have a scala_version found, we calculate a single global default version:
    • we look into a generated scala_config file used by rules_scala (the exact path will differ between MODULE.bazel and WORKSPACE, so the code there is a bit janky...)
    • if that doesn't exist, we try to parse the WORKSPACE/MODULE.bvazel file like previously
    • if that doesn't work, we run the additional query like previously (this was moved to last, since it will inevitably pick the max scala version used in the repo, we can't tell with one is the default one here, like we can do when we parse MODULE/WORKSPACE)

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8260f987-9739-4787-b94f-09432ba8fbbe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jchyb
jchyb force-pushed the fix-mixed-scala-bazel branch from 2445c7a to 938a46a Compare July 1, 2026 10:01
@jchyb
jchyb marked this pull request as ready for review July 1, 2026 11:13
@maksymilianrozanski

Copy link
Copy Markdown
Contributor

I have work in progress PR with similar changes. I think we can get accurate Scala versions for each source if we replace Bazel query xml output with streamed_jsonproto or streamed_proto.

#8494

@tgodzik

tgodzik commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@jchyb what do you think about changes in #8494 ?

@tgodzik tgodzik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the change looks good, we need to rebase.

The main question is whether using the alternate PR with streamproto might be better?

@jchyb

jchyb commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Yes, I missed that one previously, unfortunately. I don't really like the way we reach rules_scala_config here, so the streamproto one is probably better. I'll test it on the repo I'm working on and report here.

@jchyb
jchyb marked this pull request as draft July 3, 2026 15:04
@jchyb

jchyb commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Tested #8494 on the repo I'm working on and it does work, so I think this PR can be closed, unless we want an intermediate solution before #8494 gets merged

@tgodzik

tgodzik commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This one is a much smaller PR, so might be worth merging unless you are able to help minimize the changes to the other PR?

@jchyb
jchyb force-pushed the fix-mixed-scala-bazel branch from 938a46a to 9971a0a Compare July 9, 2026 16:12
@jchyb

jchyb commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased it just in case, but, for now, I think #8443 is a bigger win, and those two will have conflicts after merging one of them. I will look into #8494 and try to minimize that regardless of this one getting merged, since conceptually it does sound better

@jchyb
jchyb force-pushed the fix-mixed-scala-bazel branch from 9971a0a to 1ec7c5f Compare July 9, 2026 17:00
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