Skip to content
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

Add support for .super suffix in Mill task resolution #4637

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

zelosleone
Copy link
Contributor

Add support for .super suffix in Mill task resolution & enhance ParseArgs to handle .super suffix in task selectors:

  • Implement processSuperSuffix method to modify segments with .super
  • Update extractSegments to post-process selector segments
  • Add comprehensive test cases for various .super task resolution scenarios
  • Introduce SuperTask in Resolved to handle super task resolution
  • Implement SuperTaskHandler to manage super task logic in ResolveCore
  • Add methods in Resolve to instantiate and handle super tasks
  • Extend existing resolution and instantiation methods to support super tasks
  • Add comprehensive test cases for single and multi-level task overrides

fixes #4119

* Add support for `.super` suffix in Mill task resolution

Enhance `ParseArgs` to handle `.super` suffix in task selectors:
- Implement `processSuperSuffix` method to modify segments with `.super`
- Update `extractSegments` to post-process selector segments
- Add comprehensive test cases for various `.super` task resolution scenarios

* Implement `.super` task resolution in Mill's task system

Add support for resolving base tasks using the `.super` suffix:
- Introduce `SuperTask` in `Resolved` to handle super task resolution
- Implement `SuperTaskHandler` to manage super task logic in `ResolveCore`
- Add methods in `Resolve` to instantiate and handle super tasks
- Extend existing resolution and instantiation methods to support super tasks
- Add comprehensive test cases for single and multi-level task overrides
@zelosleone zelosleone changed the title Add support for .super suffix in Mill task resolution (#7) Add support for .super suffix in Mill task resolution Mar 2, 2025
@lihaoyi
Copy link
Member

lihaoyi commented Mar 3, 2025

Doesn't seem to work in some cases when I run it manually:

$ ./mill dist.installLocal  

$ ci/patch-mill-bootstrap.sh 

$ ./mill-assembly.jar show contrib.testng.artifactSuffix.super
show java.nio.file.NoSuchFileException: /Users/lihaoyi/Github/mill/out/contrib/testng/artifactSuffix.super.json
    java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
    java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
    java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
    java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:218)
    java.base/java.nio.file.Files.newByteChannel(Files.java:380)
    java.base/java.nio.file.Files.newByteChannel(Files.java:432)
    java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:422)
    java.base/java.nio.file.Files.newInputStream(Files.java:160)
    ujson.Readable$$anon$1.transform(Readable.scala:26)
    ujson.Readable$.transform(Readable.scala:15)
    ujson.Readable$.transform(Readable.scala:14)
    upickle.core.BufferedValue$.maybeSortKeysTransform(BufferedValue.scala:76)
    ujson.package$.transform(package.scala:8)
    ujson.package$.read$$anonfun$1(package.scala:15)
    upickle.core.TraceVisitor$.withTrace(TraceVisitor.scala:18)
    ujson.package$.read(package.scala:15)
    mill.main.MainModule$.$anonfun$10(MainModule.scala:343)
    scala.collection.immutable.List.map(List.scala:247)
    scala.collection.immutable.List.map(List.scala:79)
    mill.main.MainModule$.show0$$anonfun$1(MainModule.scala:339)
    mill.api.Result$Success.flatMap(Result.scala:26)
    mill.main.MainModule$.mill$main$MainModule$$$show0(MainModule.scala:333)
    mill.main.MainModule.show$$anonfun$1(MainModule.scala:139)
    mill.define.NamedTask.evaluate(Task.scala:245)
    mill.define.NamedTask.evaluate$(Task.scala:233)
    mill.define.Command.evaluate(Task.scala:387)

Add support for super task resolution in Mill

Implement enhanced task resolution for super tasks, allowing:
- Direct super task resolution
- Qualified super task resolution across module hierarchies
- Preserving original selector for qualified super tasks

Changes include:
- Modifying `Resolve` to handle super task selectors
- Updating `ResolveCore` to support super task resolution
- Adding comprehensive test cases for super task resolution in different module hierarchies
@zelosleone zelosleone closed this Mar 4, 2025
@zelosleone
Copy link
Contributor Author

Jesus, i didnt realize a lot of commits happened, i am going to make a new PR to fix these conflicts sorry about that. @lihaoyi

@jseteny
Copy link

jseteny commented Mar 5, 2025

$ ./mill-assembly.jar show contrib.testng.artifactSuffix.super

@lihaoyi which def do you expect to get called?
Is it JavaModule: def artifactSuffix: T[String] = platformSuffix()

If yes, how it could be called, since JavaModule is a trait.

@lihaoyi
Copy link
Member

lihaoyi commented Mar 5, 2025

Yes, i expect the overriden version from the trait to be called, not the override defined in the package.mill file

@zelosleone zelosleone reopened this Mar 5, 2025
@zelosleone zelosleone marked this pull request as draft March 5, 2025 23:32
Implement enhanced task resolution for super tasks, allowing:
- Direct super task resolution
- Qualified super task resolution across module hierarchies
- Preserving original selector for qualified super tasks

Changes include:
- Modifying `Resolve` to handle super task selectors
- Updating `ResolveCore` to support super task resolution
- Adding comprehensive test cases for super task resolution in different module hierarchies
@zelosleone
Copy link
Contributor Author

@lihaoyi should be fine now, sorry for the trouble!

@zelosleone zelosleone requested a review from lihaoyi March 5, 2025 23:35
@zelosleone zelosleone marked this pull request as ready for review March 5, 2025 23:35
@lihaoyi
Copy link
Member

lihaoyi commented Mar 7, 2025

@zelosleone the manual testing steps I provided still fail. Please make sure it works before asking for a review

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.

Allow invocation of foo.super tasks from the command line (500USD Bounty)
3 participants