-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
[WIP] Fixes #227; add mill clean
#315
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
17a4696
Adding clean as a default task
42af532
Merge branch 'master' into mill-clean
guilgaly 284ea8e
[WIP] Improve 'clean' paths resolution
guilgaly 6f1c247
Improve clean targets resolution mechanism
guilgaly 166e4da
fix error on clean all
guilgaly 26ec458
update "clean all" to keep all 'out/mill-*' paths
guilgaly 15ef865
fix cross module resolution in clean task
guilgaly 8729054
Add documentation for "clean" task
guilgaly File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
package mill.main | ||
|
||
import ammonite.ops.Path | ||
import mill.define.{NamedTask, Task} | ||
import mill.eval.{Evaluator, Result} | ||
import mill.util.{EitherOps, ParseArgs, PrintLogger, Watched} | ||
import mill.util.{PrintLogger, Watched} | ||
import pprint.{Renderer, Truncated} | ||
import upickle.Js | ||
|
||
object MainModule{ | ||
def resolveTasks[T](evaluator: Evaluator[Any], targets: Seq[String], multiSelect: Boolean) | ||
(f: List[NamedTask[Any]] => T) = { | ||
|
@@ -35,6 +37,9 @@ trait MainModule extends mill.Module{ | |
println(res) | ||
res | ||
} | ||
|
||
private val OutDir: String = "out" | ||
|
||
/** | ||
* Resolves a mill query string and prints out the tasks it resolves to. | ||
*/ | ||
|
@@ -177,4 +182,39 @@ trait MainModule extends mill.Module{ | |
} | ||
} | ||
} | ||
|
||
/** | ||
* Deletes the given targets from the out directory. Providing no targets | ||
* will clean everything. | ||
*/ | ||
def clean(evaluator: Evaluator[Any], targets: String*) = mill.T.command { | ||
val rootDir = ammonite.ops.pwd / OutDir | ||
|
||
val KeepPattern = "(mill-.+)".r.anchored | ||
|
||
def keepPath(path: Path) = path.segments.lastOption match { | ||
case Some(KeepPattern(_)) => true | ||
case _ => false | ||
} | ||
|
||
val pathsToRemove = | ||
if (targets.isEmpty) | ||
Right(ammonite.ops.ls(rootDir).filterNot(keepPath)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out the error on "filter out" was caused by deleting
|
||
else | ||
RunScript.resolveTasks( | ||
mill.main.ResolveSegments, evaluator, targets, multiSelect = true | ||
).map( | ||
_.map { segments => | ||
Evaluator.resolveDestPaths(rootDir, segments).out | ||
}) | ||
|
||
pathsToRemove match { | ||
case Left(err) => | ||
Result.Failure(err) | ||
case Right(paths) => | ||
paths.foreach(ammonite.ops.rm) | ||
Result.Success(()) | ||
} | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an idea: what if instead of this regex, we simply treated
mill clean
as the same asmill clean __
? That should correctly delete everything that needs to be deleted while leaving the not-to-be-deleted files untouchedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just beat me to it! I was starting to think along those line, mainly for better consistency with the other tasks in the main module, but you're right that it should also remove the special-case code with the somewhat arbitrary regex. 👌
I'll try to implement that and see if there is any issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guilgaly do you want to try out this one in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rockjam I'd say this PR is already useful, so I'll rather try to write the improved version for another future PR.
@lihaoyi & @rockjam : one thing I'm wondering about is wether we have a way to list all available external modules (like the GenIdea one)? As we'll want to clean their outputs too when doing
mill clean __
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a way to list all of them right now; they just live arbitrarily on the classpath, and JVM classpath-scanning is not well supported (slow, unreliable, ...). Currently, their target directories are also randomly scattered throughout the
out/
folder.Perhaps we could just put all the
ExternalModule
s into anmill-external
folder, and add that folder to the hardcoded list of things torm
?