Pain-point regarding colocating code-generation modules with modules using the generated code

[Baccata]

I'm experiencing a pain-point with respect to code-generation and sandboxing. I'm working on a project that colocates modules (let's call them codegen) containing code-generation logic with modules that use the generated code (let's call them main)

The build is wired in such a way that main's generatedSources task shells out to the codegen mainclass (by means of Jvm.runSubprocess). It's all good and well, but the problem is that in order to produce IDE configuration (be that via GenIdea or Bloop), the generatedSources task has to be invoked, which implies compiling/running the codegen module.

object codegen extends ScalaModule {
    def ivyDeps =//...
}

object main extends ScalaModule {
    def generatedSources = T {
        val outDir = T.ctx().dest / "scala"
        Jvm.runSubprocess(
            classPath = (codegen.runClasspath()).map(_.path),
            mainArgs = Seq(outDir.toString),
            // ...
        )
        Seq(PathRef(outDir))
    }
}

This makes the task of upgrading the libraries the codegen module depends on rather complicated, as source-breaking changes in these libraries are better tackled within the IDE, but I'm necessarily able to produce configuration for said IDE without compiling the codegen module if I've upgraded the libraries. Chicken-and-egg problem, so to say.

In previous versions of mill, I was trying to be smart about it by having the generatedSources not actually run the code-generation, but rather just return a PathRef pointing within its dest, and have another task be responsible for writing within that dest.

object codegen extends ScalaModule {
    def ivyDeps =//...
}

object main extends ScalaModule {
    def outputDir = T {
      T.ctx().dest / "scala" 
    }
    
    def compile() = T {
        // run the codegen here. Breaks sandboxing because the subprocess
        // writes outside of compile's `dest` directory 
        val outDir = outputDir()
        Jvm.runSubprocess(
            classPath = (codegen.runClasspath()).map(_.path),
            mainArgs = Seq(outDir.toString),
            // ...
        )
        super.compile()
    }

    // avoid compiling/running the codegen here to make `GenIdea` happy 
    // even if `codegen` doesn't compile (or crashes at runtime)
    def generatedSources = T {
        Seq(PathRef(outputDir()))
    }
}

Obviously this is not ideal, as compile will need to run twice for the pathrefs to stabilise, with newer versions of mill, this anti-pattern is no-longer possible, but the chicken-and-egg painpoint remains.

Is there a more valid pattern that would would let us address this chicken-and-egg issue ?

[lihaoyi]

It seems the problem is that Mill's evaluation model assumes that when you generate the PathRef pointing at generated code, it means the code has already been generated. This forces the execution of the codegen module.

This seems pretty fundamental to how Mill works today, but it's plausible that there's room for improvement. Essentially we would need to separate the definition of the generated source root folder from the actual generation of those sources, basically what you have done in your snippet above.

[lihaoyi]

It sounds like an SBT-esque solution would be to either have JavaModule define

def generatedSourceDirectories: Seq[os.Path] = ???

Or have the tasks themselves specify it, so we're not limited to generatedSources but any task can define:

def smithyCodegen = Task(generatedSourceDirectories = "scala": os.SubPath){
   ???
}

With the same assumption that IDEs would look at generatedSourceDirectories and assume that any generated sources would be in those folders, and not elsewhere.

It's definitely a but inconvenient to have to duplicate the generated source path in the task header and in its body, but we need some way to specify the metadata in way that does not involve evaluating the task, so perhaps such duplication is unavoidable.

The Task(generatedSourceDirectories = "scala": os.SubPath) seems more appropriate for me, since it would avoid being hardcoded to JavaModule#generatedSourceDirectories and would allow any task to generate sources in a way that IDEs can understand

[Baccata]

but we need some way to specify the metadata in way that does not involve evaluating the task

Yeah that's why I was suggesting case class SourceGenerator(codegenTask : T[Unit], subfolder: Option[RelPath]). My subfolder field is your generatedSourceDirectories

so perhaps such duplication is unavoidable.

Yeah I think so too.

The Task(generatedSourceDirectories = "scala": os.SubPath) seems more appropriate for me

Could you elaborate a little, in particular on how you envision tasks like smithyCodegen would be registered in JavaModule/ScalaModule ?

I think we're somewhat converging towards the same idea, modulo UX details.

[lihaoyi]

What I imagined would be that you would define

def smithyCodegen = Task(generatedSourceDirectories = "scala": os.SubPath){
   ???
}

And generatedSourceDirectories would set a flag on the task smithyCodegen. This would be ignored by the normal Mill evaluation, but BSP/GenIdea would scan all tasks to look for those with generatedSourceDirectories set, and for each one would register a generated sources root in the IDE.

With this approach, there wouldn't be central list of def generatedSourceDirectories: Seq[os.Path] on each module, but instead any task belonging to a Module could mark itself with Task(generatedSourceDirectories = ???) and be discovered by the IDE integration.

[Baccata]

Okay, but the tasks in question would still be wired into the def generatedSources, as they presently are, for the compile task to have knowledge of them, correct ?

And generatedSourceDirectories would set a flag on the task smithyCodegen

Are there existing examples of this pattern, or would it need a change in the Task datatype ? It's been a while since my last contributions to mill.

BSP/GenIdea would scan all tasks to look for those with generatedSourceDirectories

Likewise, any existing example of such a scan that one could draw inspiration from ?

[lihaoyi]

They would need to be wired into sources, whether through generatedSources or otherwise

We'd need to change Task, see how it was done for persistent = true

The BSP and GenIdea code is its own beast, you'll need to dig through it and see how it works and adapt it as necessary

[lihaoyi]

@Baccata I'd like to pull the conversation back here, so it doesn't get to scattered over multiple threads.

I've thought about this a bit more, and I think a def sourcesFolders approach like we do for sources/resources doesn't really make sense for generated sources: we want generated sources to live in the .dest/ folder and not in the source folders next to sources or resources, and the .dest/ folder is unique per task. So you are right that the subfolder metadata for generated sources will have to be a per-task configuration.

As of now it seems the Task(generatedSourceDirectories = os.SubPath){ ... } approach to specifying the sources that I brought up earlier does seem like the way I would approach it. We already have a convention for associated metadata with tasks: exclusive = true, persistent = true, and so on. Having generatedSourceDirectories (or whatever you want to call it) would fit right in.

One concern here would be that requiring people overriding def generatedSources to say def generatedSources = Task(generatedSourceDirectories = "."){ ... } all the time is a pain in the neck. Perhaps we can just hard-code def generatedSources to behave like generatedSourceDirectories = "." in the absence of configuration, just to provide a convenient default for the 99% use case while the other 1% with unusual codegen tasks can customize generatedSourceDirectories as necessary. So by default, IDEs would treat foo/generatedSources.dest/ as a generated source folder (if it exists) without needing to run the task to actually generate or re-generate the sources

Your PR #4621 is indeed how I imagined it would work. I haven't reviewed it thoroughly, but at a glance it all looks straightforward

[lihaoyi]

Using def sourceGenerators: Seq[NamedTask[_]] sounds reasonable. We cannot follow the same approach as def generatedSources: T[Seq[PathRef]] because we need to inspect the individual tasks to get their .dest/ folders without evaluating them, which generatedSources requires.

Leaving generatedSources unchanged in its eager behavior also seems reasonable to me. I don't think there's any right answer here. Eager source generation is more convenient when things work, since you don't need to run compile to get your IDE working, while deferred source generation is more convenient when things don't work.

It feels like we should be able to make def sourceGenerators a complete replacement for def generatedSources: if the IDE import would attempt to generate the sources, but fall back to using the .dest/ folder metadata if generation failed, that would be the best of both worlds right? In that if the source generators were broken the user would get IDE support with the generated folders just missing the sources, while if the source generators are working the user would get IDE support with the generated sources like how they do today

[Baccata]

In that if the source generators were broken the user would get IDE support with the generated folders just missing the sources, while if the source generators are working the user would get IDE support with the generated sources like how they do today

That sounds like a good approach 👍. I'm not sure how much effort it'd take, though, considering it needs to be replicated in 3 different places (BSP/GenIdea/Bloop). Truth be told, I've been wanting a def attempt : Task[mill.api.Result[A]] combinator on tasks for a long time, to avoid having to thread-in evaluators. It would make this rather easy.

I think for now I'm gonna pursue the development that I started on the PR, leaving aside the "attempt to run the task" bit.

[lefou]

Since @lihaoyi mentioned some fall-back behavior in case source generation works or not, I'd like to just bring another idea on the table:

• Support recovery from failed task dependencies #1779

An Task API to depend on the raw result of a task. IDE processes could trigger source generators via that API and could also handle the case where they failed for some reasons.

[lefou]

To elaborate on that: Given the OP case (source generation can sometime fail due to some dependency updates, which require further work in an IDE, but the IDE obviously needs to seen the new dependencies, while it can't generate the sources due to the failure):

Assuming, we have some Task.raw(): Result[T] API, we could handle the failed source generation gracefully.

def generateSourcesWhichCouldFail: T[Seq[PathRef]] = ???
def generatedSources = Task {
  val flakySources = generateSourcesWhichCouldFail.raw() match {
    case Success(srcs) => srcs
    case Failure(msg) => 
      T.log.error("Could not generate sources, continuing with a stub"
      // handle whatever is best, some introspection like we do in bspCompileClassesPath would be possible too.
      Seq(PathRef(T.dest))
  }
  super.generatedSources() ++ flakySources
}

This is just made up to show the options. Obviously you don't want fail without an error, when essential sources are missing. Maybe, you also want to capture some environment setting, to explicitly allow failures like that.

---

In case it helps, here is some other anecdote what we made possible with Mill in another project:

In the IDE, we needed generated MANIFEST.MF files (since we wanted to start an OSGi container fro the IDE for speed). Those files are typically only generated as part of the jar() task and don't end up the classpathes as seen by the IDE, therefore, we make a local toggle in our build.sc.

/**
 * When this is true, we are currently generating Idea project files,
 * and may want to use extra resources.
 */
val ideaScoped = new scala.util.DynamicVariable(false)

Somewhere in the module we generated extra data.

  def ideaResources: Task[Seq[PathRef]] = {
    // this is highly non-reproducible, but required if we want support Idea
    // We add a ready-to-use (generated) Manifest to the resources, so that Idea recognizes
    // OSGi Bundles in the classes files based classpath
    if (ideaScoped.value) T.task { Seq(extractedManifestResources()) }
    else T.task { Seq() }
  }

  override def resources = T.sources {
    super.resources() ++ ideaResources()
  }

To be used consistently, we vendored the entry point to GenIdea to ensure the context is properly set.

def idea(evs: mill.eval.Evaluator.AllBootstrapEvaluators) = T.command {
  ideaScoped.withValue(true) {
    mill.idea.GenIdeaImpl(evaluators = evs.value).run())
  }
}

[Baccata]

I've tagged my present PR as ready : #4621 . It doesn't attempt to run the source generator tasks, but I think we could retrofit that behaviour in the API proposed in the PR, when an .raw() (which I've been wanting) is available.