Generalize handling of source folders in cross-platform/version scenarios #2531
Conversation
| @@ -41,7 +41,13 @@ trait JavaModule | |||
| override def zincWorker: ZincWorkerModule = outer.zincWorker | |||
| override def skipIdea: Boolean = outer.skipIdea | |||
| override def runUseArgsFile: Target[Boolean] = T { outer.runUseArgsFile() } | |||
| override def sources = T.sources { | |||
| for (src <- outer.sources()) yield { | |||
| PathRef(this.millSourcePath / src.path.relativeTo(outer.millSourcePath)) | |||
There was a problem hiding this comment.
What if the user added additional source folders, maybe pointing outside of outer.millSourcePath?
There was a problem hiding this comment.
Seems reasonable to mirror them inside test I think? That's what I'm finding as I port builds over, e.g. maybe someone added src-2.13+/ or src-js-jvm/, and it would make perfect sense to mirror them inside the test submodule. Especially v.s. the current status quo, where the source folder layout of test folders is pretty manual and thus ad-hoc, "same as parent module" seems like a big simplification both in UX as well as internal machinery
We could filter these folders somehow if we really want, but IMO it's not necessary. In most common cases the only downside would be the test module gets a few unused-but-reasonable source folders, and worst comes to worst if someone wants to change this mirroring they can always override it, which is what the CrossSbtModule#Tests already do
There was a problem hiding this comment.
One more consideration is that the current approach of shadowing the Tests trait does not compose. While it works for one use case in CrossScalaModule, you cannot stack a second use case in PlatformScalaModule and have them work together to define the final sources list
We also already delegate a lot of targets from test module to host module, so delegating another one would fit right in
Originally bumped into these issues trying to upgrade fansi/os-lib/upickle to 0.11.0-M9
Generalize
CrossScalaModuleTests#sourcestoJavaModuleTests#sources.CrossScalaModuleinCrossScalaModuleTests, we instead define that allTestsmodule within aJavaModuleof any kind most follow the parent module's source layout, relativized to the test module'smillSourcePath.CrossScalaModule,PlatformScalaModule, etc. without needing it to be aware of every combination of such cross-modulesGeneralize
PlatformScalaModule#sourcesto work for cross modules where the cross segment is the last one, i.e.foo.jvm[2.13.10]vsfoo[2.13.10].jvmTested both changes with an additional example test included in the documentation. This demonstrates an alternative approach to that in
example/web/6-cross-version-platform-publishing/, one commonly used in the com-lihaoyi libraries, so I think it's worth making a full example v.s. just testing this via unit tests