From 620cb0b212a99dde59a675b7635c52734d899b3b Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 2 May 2019 08:25:51 -0700 Subject: [PATCH 1/2] Remove .glob converter from File I didn't mean to leave this in and there is already the toGlob conversion provided by sbt.nio.file.syntax that does the same thing. --- io/src/main/scala/sbt/nio/file/Glob.scala | 1 - io/src/test/scala/sbt/nio/GlobPathFinderSpec.scala | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/io/src/main/scala/sbt/nio/file/Glob.scala b/io/src/main/scala/sbt/nio/file/Glob.scala index 69686963..99820ac7 100644 --- a/io/src/main/scala/sbt/nio/file/Glob.scala +++ b/io/src/main/scala/sbt/nio/file/Glob.scala @@ -476,7 +476,6 @@ private[sbt] object FileGlobApi { } def /(component: String): Glob = Glob(file.toPath.resolve(component)) def \(component: String): Glob = this / component - def glob: Glob = Glob(file) def glob(filter: FileFilter): Glob = filter match { case AllPassFilter => Glob(absolutePath(file), AnyPath) case f => Glob(absolutePath(file)) / convert(f) diff --git a/io/src/test/scala/sbt/nio/GlobPathFinderSpec.scala b/io/src/test/scala/sbt/nio/GlobPathFinderSpec.scala index 7e388cda..2247cb9d 100644 --- a/io/src/test/scala/sbt/nio/GlobPathFinderSpec.scala +++ b/io/src/test/scala/sbt/nio/GlobPathFinderSpec.scala @@ -6,6 +6,7 @@ import org.scalatest.FlatSpec import sbt.io.syntax._ import sbt.io.{ AllPassFilter, IO, NothingFilter, PathFinder } import sbt.nio.file.{ AnyPath, Glob, RecursiveGlob } +import sbt.nio.file.syntax._ object GlobPathFinderSpec { implicit class PathFinderOps[P](val p: P)(implicit f: P => PathFinder) { @@ -56,7 +57,7 @@ class GlobPathFinderSpec extends FlatSpec { } it should "implicitly build a glob" in IO.withTemporaryDirectory { dir => // These use the FileBuilder extension class for file. - assert(dir.glob == Glob(dir)) + assert(dir.toGlob == Glob(dir)) assert(dir * AllPassFilter == Glob(dir, AnyPath)) assert((dir glob AllPassFilter) == Glob(dir, AnyPath)) assert(dir ** AllPassFilter == Glob(dir, RecursiveGlob)) From fa0d0eed5dbaf9c188e6bf4762fd1ca61291b95e Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 2 May 2019 13:36:34 -0700 Subject: [PATCH 2/2] Drop exceptions in deleteIfExists The legacy behavior of IO.delete is that it leaves behind any files in the tree that it is unable to delete. This is because File.delete only throws if an AccessDeniedException occurs. On windows, it is not unusual for deletions to fail because a jar or class file is in use somewhere. As a result, we leak temporary directories all over the place. These silent failures also impacts the DefaultBackgroundJobService which does not actually correctly clean up jar files when sbt exits. At any rate, much of sbt relies on the legacy behavior so I had to add logic to swallow exceptions. I didn't switch back to File.delete to make it clear that this method is broken semantically. --- io/src/main/scala/sbt/io/IO.scala | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/io/src/main/scala/sbt/io/IO.scala b/io/src/main/scala/sbt/io/IO.scala index 241eefd5..3ba6b2fc 100644 --- a/io/src/main/scala/sbt/io/IO.scala +++ b/io/src/main/scala/sbt/io/IO.scala @@ -551,17 +551,23 @@ object IO { deleteEmpty(parents(files.toSet)) } - /** Deletes `file`, recursively if it is a directory. */ + /** + * Deletes `file`, recursively if it is a directory. Note that this method may silently fail to + * delete the file (or directory) if any errors occur. + */ def delete(file: File): Unit = Retry { try { FileTreeView.default.list(file.toPath).foreach { case (dir, attrs) if attrs.isDirectory => delete(dir.toFile) - case (f, _) => Files.deleteIfExists(f) + case (f, _) => + try Files.deleteIfExists(f) + catch { case _: IOException => } } } catch { case _: NotDirectoryException => } - Files.deleteIfExists(file.toPath) + try Files.deleteIfExists(file.toPath) + catch { case _: IOException => } () }