-
Notifications
You must be signed in to change notification settings - Fork 182
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
Separate versioning of sbt-scalafix and scalafix-core #1146
Comments
From my experience with |
I agree that it's quite annoying to specify the sbt-scalafix version, and then add a version inside the scalafix.conf from the user point of view, that looks almost the same. Moreover, I wonder if people will update the version inside .scalafix.conf (except if scala steward is able to manage this). On the other hand, I think that those two versions should be independent since semantically they are different and they could occur independently. It will make easier the release management. I wonder if it's possible to have a default value for scalafix version, provided by the sbt plugin, which will need a commit to evolve, but not an urgent one (avoiding the error when fetching artifacts for really early adopters ^^ ). The scafix-version config, will be used only for the advanced user (metals for example). So my idea is not to show directly this version conf to avoid the confusion that raises from "versions remaining almost the same". What do you think ? |
I think it's a good idea to have clearly distinct version numbers for sbt-scalafix and scalafix-core, for example sbt-scalafix:20.0.0 and scalafix-core:1.0.0. It might be a bit weird at first sight but it helps clarify that the two versions are independent. I regret we didn't do that with sbt-scalafmt (it was proposed). I would require the version field in .scalafix.conf from scalafix-core:1.0.0 going forward. Otherwise I am concerned many users will stay on old scalafix versions using the latest default from scalafix. One problem I see with this scheme is that sbt-scalafix won't know what version of SemanticDB to install since it's provided by scalafix-interfaces. The options I see to fix this problem are
I'm not sure what the best solution is 🤔 |
why couldn't we evolve the default value of scalafix with releases? I don't think users should specify semanticdb version, it's internal to scalafix (they don't know much about which one is the correct one). |
That looks like the best option to me. Implementation-wise, https://github.com/scalacenter/sbt-scalafix/blob/9d7d17743208a4a04fac5138d947f5c1ecfabe93/src/main/scala/scalafix/sbt/ScalafixPlugin.scala#L54-L55 could be wrapped in a key (backed by |
👍 |
The biggest motivation to separate versioning of sbt-scalafix and scalafix-core is that it means other integrations (mill, gradle, maybe metals) also don't need to cut a new release for every new Scalafix release. It would be hugely beneficial if all clients can use scalafix-interfaces to support any version of Scalafix. The current situation is the sbt-scalafix and scalafix-core must be released together, and it usually means that other clients end up staying on old Scalafix versions.
I agree. One option to avoid the eager sbt init lookup is to have users install sbt-scalafix in the meta-meta-build |
Indeed, although technically it's a good workaround! Would it be an option to challenge in sbt the fact that |
Yes I don't think there is a need for it to be a setting key. Regarding sbt 1.3.x: I know it's possible to declare a task key twice, so maybe we could have the "new one" in autoImports and affect libraryDependencies ourselves. Not sure how the duplicated key in scope for sbt 1.4.x clients would play though. |
As a starting point, I think it's fine to fetch the semanticdb version via scalafix-interfaces.jar using coursier resolution during sbt init. There will be minor overhead from the resolution but I estimate it will only be a few milliseconds when cached (if I had to take a wild guess, around 20ms). This overhead is unavoidable regardless of the approach we take. |
I started looking into that for good and quickly faced another challenge: since Since 3694af6, clients are passing a binary Scala version, that I see 3 options:
I am thinking about opening a PR to implement (3) as a preliminary step to this ticket. WDYT @olafurpg @mlachkar ? |
Approach 3 sounds like the best option. I’m wondering, would this change no longer be necessary if we could automatically release sbt-Scalafix together with Scalafix-cli? Maybe it’s possible to merge the repos somehow |
I didn't go as far as challenging this issue itself as I thought the need for it was already discussed before #1108, but it's interesting indeed. I have seen that the sbt plugin used to be part of the main repo - what was the rationale at that time to spin it off? Merging it back would of course makes things easier (and make all discussions in this thread irrelevant), but I like the fact that it's currently separated as it forces us to think like other scalafix-interface clients (IDE or build tool) do. |
I tried to sum up the current constraints about Scala versions before embarking on (3), and I could not find any (new) blocker. Please correct me if I got something wrong!
|
Edited: not a problem, see 2 messages below. |
Usually, you depend on scalafix-rules in your |
@mlachkar not really, this isn't related to |
I will try to find some time over the next few weeks to resume my work on this. |
It has been a few weeks now 😅 so before getting back to that, I wanted to re-evaluate (3) at the light of
In a nutshell, I'd like to see if we could work towards removing the need for passing a Scala version (even binary) to
What do you think @olafurpg & @mlachkar? Maybe it's worth getting an opinion from someone at the scalacenter involved in the compiler? |
I tried to catch up on the discussion but it's not clear to me what is the exact goal here.
Can you elaborate on the motivation for this? The full Scala version is passed along to rules via
I had assumed the Scala version was relatively easy for clients to provide.
Compiler APIs tend to break in minor ways so I would be careful about relying on compatibility guarantees between minor 3.x releases. Some rules like Not sure if this is relevant but I recently implemented a resource generator for the sbt-sourcegraph plugin to identify the latest SemanticDB version for all Scala 2.x versions. A similar solution based on |
The compilers, both Scala 2 and 3, make no guarantee whatsoever about their binary or source API. Same goes for Publishing against only the binary-version (as opposed to the full-version) something that depends on the compiler is a recipe for disaster later. In the worst case (like for the |
Thanks @olafurpg & @sjrd for pointing out the rationale behind full-version publishing!
Indeed it's drifting from the original goal - I am just trying to challenge all versioning schemes to anticipate any other potential breaking change for users and/or rule authors so that we can batch it with that one.
If we stricly follow this guideline, I believe we should publish
If I understand well, that could be a source of binary incompatibility when rules built against different versions of scalameta run in the same classloader (which can become a problem as the number of community rules increase) ? Using a full-version for
|
I think the dependency on |
Indeed, that part looks safe. But what about the dependency from scalameta to scalap? Even if the scalap API remains effectively stable (despite not having a MiMa setup), usage of scala-compiler from scalap might break if their versions are not aligned. |
With Scala 3 out and with no plans for Scala 2.14 then I'd try to avoid complicating the existing cross-publishing schemes. I think it's OK for rules to report a Scalafix error if they require a specific Scala patch version.
This should be a a low-risk dependency. The transitive dependency on scala-compiler via scalameta->scalap should also not be a concern.
FWIW, you can layer the classloaders so that scala-library types are shared in the parent classloader. |
Separate versioning of sbt-scalafix and scalafix-core similar to sbt-scalafmt and scalafmt-core. Users should declare the Scalafix version in .scalafix.conf so that clients like Metals or maybe IntelliJ can load the correct version of Scalafix without having to inspect project/plugins.sbt
It will make easier the release of scalafix, which is now depending on sbt-scalafix.
#fixes #1084
The text was updated successfully, but these errors were encountered: