Unavoidable "[Importer$] Expected to find just 1 match" warnings in sbt sub-project builds

[rtyley]

Describe the bug
For an sbt multi-project build which uses the ScroogeSBT autoplugin on a subproject, an unavoidable warning message (warn [Importer$] Expected to find just 1 match for ..., introduced March 2020 with 9191deb) will be shown for every single .thrift file found, with no possible way to configure the project to avoid that warning.

To Reproduce
Steps to reproduce the behavior:

1. Create a sbt multi-project build, with ScroogeSBT disabled on all projects, except the subproject containing .thrift files in src/main/thrift
2. Execute sbt compile
3. See a warning for every .thrift file:

2020-11-26 12:43:26.379Z warn [Importer$] Expected to find just 1 match for /Users/roberto_tyley/code/ophan/event-model/src/main/thrift/device.thrift, but found 2, in DirImporter(/Users/roberto_tyley/code/ophan), DirImporter(/Users/roberto_tyley/code/ophan/event-model/src/main/thrift)
2020-11-26 12:43:26.394Z warn [Importer$] Expected to find just 1 match for /Users/roberto_tyley/code/ophan/event-model/src/main/thrift/device.thrift, but found 2, in DirImporter(/Users/roberto_tyley/code/ophan), DirImporter(/Users/roberto_tyley/code/ophan/event-model/src/main/thrift)

Expected behavior

Warnings should not be shown, or it should be at least possible to configure the sbt project in such a way that there are not two DirImporters for the same file. The number of warnings shown at the moment is heavily noisy, at least if you have many .thrift files.

Environment

sbt v1.4.4, scrooge v20.10.0

Additional context

The warning was introduced with 9191deb. It's probably valid, but the fact that there are two paths found (the project root, and the path of the subproject src/main/thrift dir) is because the root is hard-coded into the scrooge Compiler class, and there's no way to turn it off:

scrooge/scrooge-generator/src/main/scala/com/twitter/scrooge/Compiler.scala

Line 48 in 37edfe5

val importer = Importer(new File(".")) +: Importer(config.includePaths.toSeq)

...I've played the scroogeThriftIncludes setting, and could not find any way to remove the warnings.

cc @mosesn

[felixbr]

Until the fix (#345) is merged and released, you can use the same workaround I used in our internal sbt-plugin to at least silence the warning:

  private val importerLogger = java.util.logging.Logger.getLogger("com.twitter.scrooge.frontend.Importer$")
  importerLogger.setLevel(java.util.logging.Level.OFF)

It's hacky but the best solution I could find for the time being 🙂

[muuki88]

Next scrooge sbt version will have a setting to disable this

scroogeThriftIncludeRoot := false

[jstultz]

I am not sure that the issue here is really that the root folder is included by default, but rather with the way absolute paths are handled. Any DirImporter will successfully resolve an absolute path, and when compiling a thrift file, it is the first file that the importer(s) resolve, and is going to be an absolute path. So while not including the root directory as an importer will solve this for some projects, it will still result in spurious warnings any time that a project has scroogeThriftIncludes with more than one entry (which would, I believe, be any project that has any dependencies in the thrift configuration?)

I believe conditioning the warning on a check like this would suffice to solve the problem more generally:

scrooge/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/Importer.scala

Lines 87 to 88 in 76504ba

	val file = new File(filename)
	if (file.isAbsolute)

[jstultz]

(if others agree, I'd be happy to submit a PR)

[niodice]

Also seeing this quite a bit. I tend to ignore it and I always see the correct behavior in the generated code. This makes me think that @jstultz is correct in that these are spurious warnings.

Anyone maintainers have more context here?

[csaltos]

I've added Compile / scroogeThriftIncludeRoot := false to the Thrift configuration subprojects and it's working for me ... thank you very much for the fix and workarounds.