[WIP] Fixes #227; add mill clean
#315
Conversation
|
Resolving modules/tasks the same way |
|
I'll look into doing just that, probably this weekend. 👍 |
main/src/mill/main/MainModule.scala
Outdated
|
|
||
| val pathsToRemove = | ||
| if (targets.isEmpty) | ||
| Right(List(rootDir)) |
There was a problem hiding this comment.
In this case mill exits with a non-zero exit code, presumably because it fails to delete out/clean, so that's a bit ugly... We should probably list the content of out/ and filter out out/clean.
There was a problem hiding this comment.
You can get output of clean command with T.ctx().dest, and filtering out should be simple in this case
|
|
||
| val pathsToRemove = | ||
| if (targets.isEmpty) | ||
| Right(ammonite.ops.ls(rootDir).filterNot(keepPath)) |
There was a problem hiding this comment.
Turns out the error on "filter out" was caused by deleting out/mill-worker-1. For now I have hard-coded a filter for anything like out/mill-worker-N, but:
- I'm not sure this is what we'd want?
- If we do, I don't know if there is a better way to filter those folders ?
|
perhaps skip everything starting with `mill-`? so far all those
folders/files are mill’s internal machinery rather than user modules or
targets
…On Sat, 12 May 2018 at 8:09 AM, Guillaume Galy ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In main/src/mill/main/MainModule.scala
<#315 (comment)>:
> + * 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-worker-[0-9]+)".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))
Turns out the error on "filter out" was caused by deleting
out/mill-worker-1. For now I have hard-coded a filter for anything like
out/mill-worker-N, but:
- I'm not sure this is what we'd want?
- If we do, I don't know if there is a better way to filter those
folders ?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#315 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ATpwjBfCQFgHoIZrIi5kV-17YnLV2XABks5txvs5gaJpZM4T3J_E>
.
|
|
Looks ok to me. @rockjam want to take a look? If you're happy we can merge this |
| def clean(evaluator: Evaluator[Any], targets: String*) = mill.T.command { | ||
| val rootDir = ammonite.ops.pwd / OutDir | ||
|
|
||
| val KeepPattern = "(mill-.+)".r.anchored |
There was a problem hiding this comment.
Here's an idea: what if instead of this regex, we simply treated mill clean as the same as mill clean __? That should correctly delete everything that needs to be deleted while leaving the not-to-be-deleted files untouched
There was a problem hiding this comment.
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.
@guilgaly do you want to try out this one in this PR?
There was a problem hiding this comment.
@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.
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 ExternalModules into an mill-external folder, and add that folder to the hardcoded list of things to rm?
See issue #227. I'm picking up after pull-request #253 (closed due to inactivity).
This seems to work for simple use cases (
mill clean,mill myModule someOtherModule,mill myModule.myTask...). Some limitations:cleanon a module/task which does not exist...mill _.myTask...Question: To improve that, I guess we'd need to resolve the target similarly to
mill.main.RunScript#resolveTasks, except able to resolve either a module (not the module's default task) or a task? Would that make sense to do?TODO:
mill resolvemill cleanand handle it asmill clean __