Scalafmt not respecting SBT resolvers setting

[ceedubs]

This template is a guideline, not a strict requirement.

• Version: 2.0.4
• Integration: SBT

Problem

This is my understanding of the problem that I'm seeing. I apologize if I've misdiagnosed what's happening.

The Scalafmt SBT plugin seems to use a hard-coded list of resolvers for dynamic resolution, ignoring the resolvers that have been configured the standard SBT ways (ex: resolvers or externalResolvers). It looks to me like the relevant code is here. This shows up as a problem in an environment where network configuration prevents hitting public repositories (Maven Central, Sonatype Releases) directly. It also causes issues if a noisy neighbor causes Maven Central to temporarily ban your IP address...

Expectation

When resolving artifacts, I would expect Scalafmt to only try to hit the repositories that are returned by the sbt resolvers command.

Workaround

So far I don't know of any.

[ceedubs]

Oops I just realized that https://github.com/scalameta/sbt-scalafmt also exists. The code that I linked to in the description is within this repository, so I'm not sure where to open this issue. Let me know if I should move it over there.

[poslegm]

It looks like related to #1415. scalafmt uses coursier for dynamic resolution. So you can try to update sbt plugin to 2.0.6 and set environment variable COURSIER_REPOSITORIES.

If COURSIER_REPOSITORIES doesn't work with sbt-scalafmt v2.0.6 — it's a bug.

[ceedubs]

@poslegm thanks I'll give that a try. I still think that it would be nice if sbt-scalafmt had a way of passing along the settings that are configured in the standard way within SBT. We have a somewhat convoluted way of populating the repositories and credentials within SBT. We can probably set up environment variables for them, but it is going to be a little bit of a hassle.

[henricook]

I don't think COURSIER_REPOSITORIES is honoured. I don't know why that's an sbt-scalafmt problem, please tell me if i'm missing something with my PR

[mihaisoloi]

@poslegm please excuse my ignorance, but why is it required to define a repositories array, when you could get it from sbt or env vars?

this requires extra configuration to be added, i.e. COURSIER_REPOSITORIES, where the information is already passed via MAVENPROXY_URL or resolvers in sbt

[poslegm]

@mihaisoloi I don't know reasons of this decision. Maybe it was done because of the use coursier-small and now hard-coded repositories array is redundant. Probably @olafurpg can tell you more

[henricook]

Interesting question @mihaisoloi - if the sonatype repositories aren't necessary we can just use coursier defaults, by default

[henricook]

Unfortunately even after my recent change on a CI build internally i'm still getting this

In the code this means it couldn't download the dependency, but doesn't give out the usual information about why, and what repositories are tried. I may raise a PR to enhance error reporting.

Generally speaking I think the whole concept of scalafmt-dynamic is flawed here - it's a nightmare having a dependency that tries to download its own dependencies with its own rules, at least as soon as you put it into a controlled environment (no outbound internet allowed) like this

[olafurpg]

The motivation for downloading scalafmt on the fly is

• avoid binary incompatibility with sbt boot classpath, adding scalafmt-core to an sbt plugin results in runtime crashes while parsing XML tokens due to the fact that sbt v1.x uses an outdated version of fastparse
• ensure the command line/sbt/intellij/vscode/... use the same scalafmt version. If the sbt plugin uses a different version of scalafmt than intellij then you end up jumping between minor diffs

Contributions are welcome to make this setup work with corporate VPNs and other controlled environments.

[henricook]

Good to know the motivation thanks @olafurpg

Can i ask for your thoughts on adding cause to the message here - from my screenshot i'm not sure the cause is printing out?

case CannotDownload(configPath, version, cause) =>
        val message = s"failed to resolve Scalafmt version '$version'"
        cause match {
          case Some(e) => reporter.error(configPath, message, e) <---------- this line
          case None => reporter.error(configPath, message)
        }

[tanishiking]

As @ceedubs said in #1521 (comment)

it would be nice if sbt-scalafmt had a way of passing along the settings that are configured in the standard way within SBT

it might be useful for sbt-scalafmt to evaluate the -Dsbt.config.repository so that users don't have to specify their custom repositories both in -Dsbt.config.repository (for sbt) and COUSIER_REPOSITORIES (for scalafmt).
To implement this feature, we have to transform and pass the sbt configuration to scalafmt-dynamic somehow.

---

Maybe we can pass the repositories information by adding .withRepositories(...) or something to scalafmt-interface (and of course, scalafmt-dynamic). I think this is a straightforward way to passing repositories from sbt-scalafmt to scalafmt-dynamic .

I feel adding such a method to scalafmt-interface could be a bit wired, because the interface shouldn't concern about the behavior of the implementation (in this case, scalafmt-dynamic) and adding .withRepositories(...) to the interface implies that the interface knows that the implementation might download something from the repositories.
However, maybe we should compromise and add the method to the interface for the sake of convenience.

[t-botz]

@tanishiking Not familiar with scalafmt architecture, so sorry if my comments are not relevant.

Agreed with your sentiment that it would be weird to add method related to repositories in scalafmt. I already feel weird about scalafmt caring about dependency version right now. IMO scalafmt shouldn't care at all about it, it should just care about "Do I understand the version number specified in the conf? yes / no"

Thinking about alternative solutions, would that make sense to leverage sbt and not use scalafmt-dynamic at all.

The sbt plugin could be adding the right scalafmt-core version to the project setting in it's own scope like :

   libraryDependencies += "org.scalameta" % "sbt-scalafmt" % "2.2.1" % ScalaFmtScope,

I feel like that's a very common approach for sbt plugins and I don't think scalafmt has anything special about it. I don't think classpath conflict is a problem either, you can always run in a forked jvm.

Happy to try an implementation, if there is no concern about that approach.

[t-botz]

@olafurpg I am far from being a sbt expert but I still fail to understand the motivation

* avoid binary incompatibility with sbt boot classpath, adding scalafmt-core to an sbt plugin results in runtime crashes while parsing XML tokens due to the fact that sbt v1.x uses an outdated version of fastparse

That problem can be solved by running on a separate process, scalafmt-core definitely doesn't need to share classpath with sbt.

* ensure the command line/sbt/intellij/vscode/... use the same scalafmt version. If the sbt plugin uses a different version of scalafmt than intellij then you end up jumping between minor diffs

If scalafmt rely on the underlying dependency manager (sbt, gradle, ...), then you'll definitely have a better experience than if you bring an other dependency manager on top of it.

[olafurpg]

I estimate that making sbt-scalafmt respect the resolvers setting in sbt will require less work than using the built-in sbt resolver, which as of sbt v1.3.x is Coursier, the same resolver Scalafmt uses.

A good place to start looking would be extending the Scalafmt.java interface to accept new resolvers: https://github.com/scalameta/scalafmt/blob/c5cbcaa8e5c5e9808bbc988d3152ba7569c7224b/scalafmt-interfaces/src/main/java/org/scalafmt/interfaces/Scalafmt.java

Next, update sbt-scalafmt to pass in the custom repositories here https://github.com/scalameta/sbt-scalafmt/blob/022e89216571a0c1a03939643961dcd0c2c135f2/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala#L91

A workaround in the meantime is to export the COURSIER_REPOSITORIES environment variable. Alternatively, you might be able to configure a global mirror to replace Maven Central https://get-coursier.io/blog/2019/07/05/1.1.0-M14.html#mirrors

Someone who is blocked by this issue is encouraged to make a contribution to fix the issue. A lot of people are already using the current solution without problems so I wouldn't wait for them to pull of their sleeves to work on this.

I totally agree that dependency management should ideally be outside the scope of a formatter. I resisted this change for a long time. However, we experienced a lot of issues where the build tool was using a different version of Scalafmt compared to editors like IntelliJ, VS Code, Vim, Emacs. One problem we experienced in particular with editor integrations is that they need to support is different Scalafmt versions between different repositories. Previously, IntelliJ was using v1.5.1 for all projects, which forced people to stay on older Scalafmt versions as we released new features. This discrepancy between the build tool and editors was causing a lot of pain, which is mostly gone now since we introduced scalafmt-dynamic.

[howardrotterdam]

I think it's easier for the users to ensure the versions they choose for Scalafmt is the same in the build tool and IDE, intead of letting the code formatter do this and fail to resolve dependency in certain environment.

[slivkamiro]

Hi all, I've tried to implement this - see #1586. I'm struggling a bit with sbt-scalafmt part.

[slivkamiro]

Nevermind, I found my way around :) Any volunteers for the review?

[olafurpg]

#1586 added the possibility to include custom resolvers to scalafmt-dynamic. Thank you @slivkamiro for taking the lead on this change 🙏 This feature has been released in v2.3.2

The next step is to update the build integrations like sbt-scalafmt to populate this new setting based on the build

[poslegm]

sbt-scalafmt v2.3.0 with @slivkamiro's changes has been published.

Please try to update plugin and check how resolvers settings work

addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.3.0")

[pascals-ager]

Is it just me with:

module not found: org.scalameta#sbt-scalafmt;2.3.0

v2.2.1 works perfectly well (except the build problem in a controlled CI environment ofc)

[poslegm]

@pascals-ager What version of sbt do you use?

[pascals-ager]

@pascals-ager What version of sbt do you use?

1.2.8

[poslegm]

@pascals-ager I tried to build project with sbt 1.2.8 and sbt-scalafmt 2.3.0 and it works fine. Is there more details about module not found error?

[pascals-ager]

@poslegm I tried using v2.3.0 just now, and it works for me now! I haven't tested the "building in a controlled CI environment" though..

[ceedubs]

Works for me too. Thank you!

edit: I was mistaken. This does not seem to work for me in a controlled CI environment.

[sideeffffect]

tested the "building in a controlled CI environment"

I did, and it still doesn't work

[milanvdm]

We have the same issue. It fails when you run it against your own mirrored artifactory since it does not use the same repositories as sbt.
We run sbt-scalafmt v2.3.1 and scalafmt v2.3.2.

[slivkamiro]

I think I found the issue with my previous work on this. see scalameta/sbt-scalafmt#87

[tomasherman]

scalameta/sbt-scalafmt#87 fixed the issue for us

[ceedubs]

I posted this on scalameta/sbt-scalafmt#87 a while ago, but it probably makes sense to post it here.

I gave it a try. It looks like it's now failing because it isn't picking up the credentials associated with the custom resolvers.

[poslegm]

@ceedubs can you provide error messages or other context?

[ceedubs]

@poslegm I'm sorry for the slow response and for what I think was a false error report. The issue that I was seeing doesn't actually appear to be a credentials issue. I was seeing some errors that I don't quite understand, but updating from SBT 1.3.3 to 1.3.8 seems to have resolved the issue. I'm going to close out this issue. Thanks a lot to @slivkamiro, @poslegm, and to anyone else who spent tim/effort looking into and resolving this!

[clemensutschig]

tried with 1.3.8 and fmt 2.3.0 -> same issue :(
we run this in a corp environment,.. and while 2.3.0 is downloaded - it#s not resolved

[debug] Updating ProjectRef(uri("file:/tmp/workspace/odsst-cd/odsst-cd-odsp3-4play1-master/project/"), "odsst-cd-odsp3-4play1-master-build")...
[debug] downloaded https://.../repository/ivy-releases/org.scalameta/sbt-scalafmt/scala_2.12/sbt_1.0/2.3.0/ivys/ivy.xml
[debug] downloaded https://..../repository/ivy-releases/org.scalameta/sbt-scalafmt/scala_2.12/sbt_1.0/2.3.0/ivys/ivy.xml.sha1
[debug] downloaded https://..../repository/maven-public/org/scalameta/sbt-scalafmt_2.12_1.0/2.3.0/sbt-scalafmt-2.3.0.pom.sha1

[info] Set current project to odsp3-4play1 (in build file:/tmp/workspace/odsst-cd/odsst-cd-odsp3-4play1-master/)
[success] Total time: 0 s, completed Apr 29, 2020 7:33:56 AM
[info] Checking 2 Scala sources...
[error] failed to resolve Scalafmt version '2.3.0': /tmp/workspace/odsst-cd/odsst-cd-odsp3-4play1-master/.scalafmt.conf
[error] (Compile / scalafmtSbtCheck) failed to resolve Scalafmt version '2.3.0': /tmp/workspace/odsst-cd/odsst-cd-odsp3-4play1-master/.scalafmt.conf
[error] Total time: 2 s, completed Apr 29, 2020 7:33:58 AM

[clemensutschig]

so 1.3.8 and 2.3.2 fmt - solve this :)