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

[WIP] Fixes #227; add mill clean #315

Merged
merged 8 commits into from
May 16, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update "clean all" to keep all 'out/mill-*' paths
guilgaly committed May 12, 2018
commit 26ec458c7140efbd7635d730895dfabee33745ff
9 changes: 3 additions & 6 deletions main/src/mill/main/MainModule.scala
Original file line number Diff line number Diff line change
@@ -190,8 +190,8 @@ trait MainModule extends mill.Module{
def clean(evaluator: Evaluator[Any], targets: String*) = mill.T.command {
val rootDir = ammonite.ops.pwd / OutDir

val KeepPattern = "(mill-worker-[0-9]+)".r.anchored
val KeepPattern = "(mill-.+)".r.anchored
Copy link
Member

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 as mill clean __? That should correctly delete everything that needs to be deleted while leaving the not-to-be-deleted files untouched

Copy link
Contributor Author

@guilgaly guilgaly May 14, 2018

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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 __.

Copy link
Member

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 ExternalModules into an mill-external folder, and add that folder to the hardcoded list of things to rm?


def keepPath(path: Path) = path.segments.lastOption match {
case Some(KeepPattern(_)) => true
case _ => false
@@ -212,10 +212,7 @@ trait MainModule extends mill.Module{
case Left(err) =>
Result.Failure(err)
case Right(paths) =>
paths.foreach { p =>
println(p)
ammonite.ops.rm(p)
}
paths.foreach(ammonite.ops.rm)
Result.Success(())
}
}