Skip to content

WIP: Allow some task reference from source tasks #5179

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

lefou
Copy link
Member

@lefou lefou commented May 23, 2025

TBD

With the merge of #4524, source tasks could no longer depend on other tasks. Due to this change, the logic to calculate changes, especially in the --watch mode complicated, could be significantly simplified. Unfortunately, this also broke a common use-case, where sub-modules or cross-modules want to derive there source paths from the outer or parent module to provide a convenient and consistent default behavior. for that reason, the def sourceFolders: Seq[os.SubPath] was introduced, which could be modified in an object-orients fashion to cusomites the default behavior of sources. Although looking simple on paper, when facing an existing grown setup, it makes it really hard to understand the effective source path just from looking at the build script and modules code.

Here are some issues I noticed:

  • sources and sourceFolders can be independently overridden without any consistency guarantees. It's unclear at a concrete module, mixing in various traits what their actual value is
  • sourceFolders isn't a task, hence it can't be easily shown and inspected with mill show.
  • Overriding sources may subsequently ignore or re-use the value of sourceFolders, leaving the maintainer in doubt about the actual paths used
  • The existing module structure does not use sourceFolders. In fact, existing source dir customization (e.g. in the Android and Kotlin modules) complete bypasses sourceFolders and just override sources, so the refactoring towards sourceFolders is incomplete
  • Manually understanding of the actual value of sources is a tedious and error-prone process. I tried to understand the value of AndroidKotlinAppModule.sources in a PR, where I neither could just run mill show nor could I simply wald the super.sources tree up. It costs a lot of time to get an understanding of such simple core setting of a module, leaving too little confidence in a setup involving lots of mix-ins.

Consequences:

I think have two parallel structures the configure the sources (sources and sourceFolders) is not going to age well. I already find it difficult to work with it. As a maintainer of various larger polyglot projects this change significantly raises the mental overhead. Therefore I think we should try to handle Task.Sources derivations not outside of it. Instead we should allow task dependencies as we did before and remove sourceFolders. Each module level can be inspected, the effective values can be shown. There is no need to use println-debugging or a JVM debugger to understand the effective values of sourceFolders.

What this POC tries to change:

Re-enable the ability to let Task.Sources depend on other task, but limit the allowed task types to just Sources tasks or maybe tasks returning the type Seq[PathRef] (and compatible). That way, the logic to re-evaluate the up-to-date state should be kept simple. We keep just checking the PathRefs the final sources task is returning for changes.

@lefou lefou changed the title Make kotlinSources task private WIP: Allow some task reference from source tasks May 23, 2025
@lefou lefou force-pushed the tr-remove-sourceFolders branch from 86e041e to 1b8befa Compare May 26, 2025 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant