From f26afe6681da174e80e2ca6db8e638e1abf732cc Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Wed, 20 Mar 2019 21:06:10 -0700 Subject: [PATCH] Return (Path, FileAttributes) instead of Stamped.File I realized that Stamped.File was a bad interface that was really just an implementation detail of external hooks. I updated the GlobLister.{ all, unique } methods to return Seq[(Path, FileAttributes)] rather than Stamped.File which is a much more natural api and one I could see surviving the switch to nio based apis planned for 1.4.0/2.0.0. I also added a simple scripted test for glob listing. The GlobLister.all method is implicitly tested all over the place since the compile task uses it, but it's good to have an explicit test. --- main-command/src/main/scala/sbt/Stamped.scala | 17 ++++------ .../scala/sbt/internal/FileAttributes.scala | 22 +++++++++---- .../src/test/scala/sbt/WatchedSpec.scala | 8 ++--- main/src/main/scala/sbt/Defaults.scala | 6 ++-- .../scala/sbt/internal/ExternalHooks.scala | 22 ++++--------- .../main/scala/sbt/internal/FileTree.scala | 16 +++++---- .../main/scala/sbt/internal/GlobLister.scala | 25 +++++++------- sbt/src/sbt-test/io/glob/build.sbt | 1 + sbt/src/sbt-test/io/glob/files/bar.json | 0 sbt/src/sbt-test/io/glob/files/foo.txt | 0 sbt/src/sbt-test/io/glob/files/subdir/baz.yml | 0 sbt/src/sbt-test/io/glob/project/Build.scala | 33 +++++++++++++++++++ sbt/src/sbt-test/io/glob/test | 1 + 13 files changed, 92 insertions(+), 59 deletions(-) create mode 100644 sbt/src/sbt-test/io/glob/build.sbt create mode 100644 sbt/src/sbt-test/io/glob/files/bar.json create mode 100644 sbt/src/sbt-test/io/glob/files/foo.txt create mode 100644 sbt/src/sbt-test/io/glob/files/subdir/baz.yml create mode 100644 sbt/src/sbt-test/io/glob/project/Build.scala create mode 100644 sbt/src/sbt-test/io/glob/test diff --git a/main-command/src/main/scala/sbt/Stamped.scala b/main-command/src/main/scala/sbt/Stamped.scala index 61592f488..668c27510 100644 --- a/main-command/src/main/scala/sbt/Stamped.scala +++ b/main-command/src/main/scala/sbt/Stamped.scala @@ -30,8 +30,10 @@ private[sbt] trait Stamped { */ private[sbt] object Stamped { type File = JFile with Stamped - private[sbt] def file(typedPath: TypedPath, entry: FileAttributes): JFile with Stamped = - new StampedFileImpl(typedPath, entry.stamp) + private[sbt] val file: ((Path, FileAttributes)) => JFile with Stamped = { + case (path: Path, attributes: FileAttributes) => + new StampedFileImpl(path, attributes.stamp) + } /** * Converts a TypedPath instance to a [[Stamped]] by calculating the file hash. @@ -67,14 +69,7 @@ private[sbt] object Stamped { } private final class StampedImpl(override val stamp: Stamp) extends Stamped - private final class StampedFileImpl(typedPath: TypedPath, override val stamp: Stamp) - extends java.io.File(typedPath.toPath.toString) + private final class StampedFileImpl(path: Path, override val stamp: Stamp) + extends java.io.File(path.toString) with Stamped - with TypedPath { - override def exists: Boolean = typedPath.exists - override def isDirectory: Boolean = typedPath.isDirectory - override def isFile: Boolean = typedPath.isFile - override def isSymbolicLink: Boolean = typedPath.isSymbolicLink - override def toPath: Path = typedPath.toPath - } } diff --git a/main-command/src/main/scala/sbt/internal/FileAttributes.scala b/main-command/src/main/scala/sbt/internal/FileAttributes.scala index 2987313c1..1e7abefbe 100644 --- a/main-command/src/main/scala/sbt/internal/FileAttributes.scala +++ b/main-command/src/main/scala/sbt/internal/FileAttributes.scala @@ -23,6 +23,9 @@ import xsbti.compile.analysis.{ Stamp => XStamp } trait FileAttributes { def hash: Option[String] def lastModified: Option[Long] + def isRegularFile: Boolean + def isDirectory: Boolean + def isSymbolicLink: Boolean } object FileAttributes { trait Event { @@ -52,10 +55,10 @@ object FileAttributes { override def toString: String = s"Event($path, $previous, $current)" } private[sbt] def default(typedPath: TypedPath): FileAttributes = - DelegateFileAttributes(Stamped.converter(typedPath)) + DelegateFileAttributes(Stamped.converter(typedPath), typedPath) private[sbt] implicit class FileAttributesOps(val e: FileAttributes) extends AnyVal { private[sbt] def stamp: XStamp = e match { - case DelegateFileAttributes(s) => s + case DelegateFileAttributes(s, _) => s case _ => e.hash .map(Stamp.fromString) @@ -67,8 +70,10 @@ object FileAttributes { private implicit class Equiv(val xstamp: XStamp) extends AnyVal { def equiv(that: XStamp): Boolean = Stamp.equivStamp.equiv(xstamp, that) } - private case class DelegateFileAttributes(private val stamp: XStamp) - extends FileAttributes + private case class DelegateFileAttributes( + private val stamp: XStamp, + private val typedPath: TypedPath + ) extends FileAttributes with XStamp { override def getValueId: Int = stamp.getValueId override def writeStamp(): String = stamp.writeStamp() @@ -83,11 +88,14 @@ object FileAttributes { case _ => None } override def equals(o: Any): Boolean = o match { - case DelegateFileAttributes(thatStamp) => this.stamp equiv thatStamp - case xStamp: XStamp => this.stamp equiv xStamp - case _ => false + case DelegateFileAttributes(thatStamp, thatTypedPath) => + (this.stamp equiv thatStamp) && (this.typedPath == thatTypedPath) + case _ => false } override def hashCode: Int = stamp.hashCode override def toString: String = s"FileAttributes(hash = $hash, lastModified = $lastModified)" + override def isRegularFile: Boolean = typedPath.isFile + override def isDirectory: Boolean = typedPath.isDirectory + override def isSymbolicLink: Boolean = typedPath.isSymbolicLink } } diff --git a/main-command/src/test/scala/sbt/WatchedSpec.scala b/main-command/src/test/scala/sbt/WatchedSpec.scala index 5c66553d3..ed1170fa5 100644 --- a/main-command/src/test/scala/sbt/WatchedSpec.scala +++ b/main-command/src/test/scala/sbt/WatchedSpec.scala @@ -94,14 +94,14 @@ class WatchedSpec extends FlatSpec with Matchers { queue.toIndexedSeq shouldBe Seq(foo) } it should "enforce anti-entropy" in IO.withTemporaryDirectory { dir => - val realDir = dir.toRealPath + val realDir = dir.toRealPath.toPath val queue = new mutable.Queue[Path] - val foo = realDir.toPath.resolve("foo") - val bar = realDir.toPath.resolve("bar") + val foo = realDir.resolve("foo") + val bar = realDir.resolve("bar") val config = Defaults.config( globs = Seq(realDir ** AllPassFilter), preWatch = (count, _) => if (count == 3) CancelWatch else Ignore, - onWatchEvent = _ => Trigger, + onWatchEvent = e => if (e.path != realDir) Trigger else Ignore, triggeredMessage = (tp, _) => { queue += tp; None }, watchingMessage = count => { count match { diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 9ddd1a1d3..a355559bc 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -380,7 +380,7 @@ object Defaults extends BuildCommon { val filter = (includeFilter in unmanagedSources).value -- (excludeFilter in unmanagedSources).value val baseSources = if (sourcesInBase.value) baseDirectory.value * filter :: Nil else Nil - (unmanagedSourceDirectories.value.map(_ ** filter) ++ baseSources).all + (unmanagedSourceDirectories.value.map(_ ** filter) ++ baseSources).all.map(Stamped.file) }, watchSources in ConfigGlobal := (watchSources in ConfigGlobal).value ++ { val baseDir = baseDirectory.value @@ -419,7 +419,7 @@ object Defaults extends BuildCommon { unmanagedResources := { val filter = (includeFilter in unmanagedResources).value -- (excludeFilter in unmanagedResources).value - unmanagedResourceDirectories.value.map(_ ** filter).all + unmanagedResourceDirectories.value.map(_ ** filter).all.map(Stamped.file) }, watchSources in ConfigGlobal := (watchSources in ConfigGlobal).value ++ { val bases = unmanagedResourceDirectories.value @@ -1216,7 +1216,7 @@ object Defaults extends BuildCommon { exclude: ScopedTaskable[FileFilter] ): Initialize[Task[Seq[File]]] = Def.task { val filter = include.toTask.value -- exclude.toTask.value - dirs.toTask.value.map(_ ** filter).all + dirs.toTask.value.map(_ ** filter).all.map(Stamped.file) } def artifactPathSetting(art: SettingKey[Artifact]): Initialize[File] = Def.setting { diff --git a/main/src/main/scala/sbt/internal/ExternalHooks.scala b/main/src/main/scala/sbt/internal/ExternalHooks.scala index e3c9c59cb..df30bcbd3 100644 --- a/main/src/main/scala/sbt/internal/ExternalHooks.scala +++ b/main/src/main/scala/sbt/internal/ExternalHooks.scala @@ -6,14 +6,14 @@ */ package sbt.internal -import java.nio.file.Paths + +import java.nio.file.{ Path, Paths } import java.util.Optional -import sbt.Stamped import sbt.internal.inc.ExternalLookup -import sbt.io.FileTreeDataView.Entry import sbt.io.syntax._ import sbt.io.{ AllPassFilter, Glob, TypedPath } +import sbt.Stamped import xsbti.compile._ import xsbti.compile.analysis.Stamp @@ -22,7 +22,7 @@ import scala.collection.mutable private[sbt] object ExternalHooks { private val javaHome = Option(System.getProperty("java.home")).map(Paths.get(_)) def apply(options: CompileOptions, repo: FileTree.Repository): DefaultExternalHooks = { - def listEntries(glob: Glob): Seq[Entry[FileAttributes]] = repo.get(glob) + def listEntries(glob: Glob): Seq[(Path, FileAttributes)] = repo.get(glob) import scala.collection.JavaConverters._ val sources = options.sources() val cachedSources = new java.util.HashMap[File, Stamp] @@ -36,18 +36,10 @@ private[sbt] object ExternalHooks { case f if f.getName.endsWith(".jar") => // This gives us the entry for the path itself, which is necessary if the path is a jar file // rather than a directory. - listEntries(f.toGlob) foreach { e => - e.value match { - case Right(value) => allBinaries.put(e.typedPath.toPath.toFile, value.stamp) - case _ => - } - } + listEntries(f.toGlob) foreach { case (p, a) => allBinaries.put(p.toFile, a.stamp) } case f => - listEntries(f ** AllPassFilter) foreach { e => - e.value match { - case Right(value) => allBinaries.put(e.typedPath.toPath.toFile, value.stamp) - case _ => - } + listEntries(f ** AllPassFilter) foreach { + case (p, a) => allBinaries.put(p.toFile, a.stamp) } } diff --git a/main/src/main/scala/sbt/internal/FileTree.scala b/main/src/main/scala/sbt/internal/FileTree.scala index b998bcea6..a26bc0bec 100644 --- a/main/src/main/scala/sbt/internal/FileTree.scala +++ b/main/src/main/scala/sbt/internal/FileTree.scala @@ -8,7 +8,7 @@ package sbt package internal -import java.nio.file.{ WatchService => _ } +import java.nio.file.{ Path, WatchService => _ } import sbt.internal.util.appmacro.MacroDefaults import sbt.io.FileTreeDataView.Entry @@ -17,7 +17,9 @@ import sbt.io._ import scala.language.experimental.macros private[sbt] object FileTree { - trait Repository extends sbt.internal.Repository[Seq, Glob, Entry[FileAttributes]] + private def toPair(e: Entry[FileAttributes]): Option[(Path, FileAttributes)] = + e.value.toOption.map(a => e.typedPath.toPath -> a) + trait Repository extends sbt.internal.Repository[Seq, Glob, (Path, FileAttributes)] private[sbt] object Repository { /** @@ -29,19 +31,21 @@ private[sbt] object FileTree { implicit def default: FileTree.Repository = macro MacroDefaults.fileTreeRepository private[sbt] object polling extends Repository { val view = FileTreeView.DEFAULT.asDataView(FileAttributes.default) - override def get(key: Glob): Seq[Entry[FileAttributes]] = view.listEntries(key) + override def get(key: Glob): Seq[(Path, FileAttributes)] = + view.listEntries(key).flatMap(toPair) override def close(): Unit = {} } } private class ViewRepository(underlying: FileTreeDataView[FileAttributes]) extends Repository { - override def get(key: Glob): Seq[Entry[FileAttributes]] = underlying.listEntries(key) + override def get(key: Glob): Seq[(Path, FileAttributes)] = + underlying.listEntries(key).flatMap(toPair) override def close(): Unit = {} } private class CachingRepository(underlying: FileTreeRepository[FileAttributes]) extends Repository { - override def get(key: Glob): Seq[Entry[FileAttributes]] = { + override def get(key: Glob): Seq[(Path, FileAttributes)] = { underlying.register(key) - underlying.listEntries(key) + underlying.listEntries(key).flatMap(toPair) } override def close(): Unit = underlying.close() } diff --git a/main/src/main/scala/sbt/internal/GlobLister.scala b/main/src/main/scala/sbt/internal/GlobLister.scala index cb9016d3c..90d0aaa3e 100644 --- a/main/src/main/scala/sbt/internal/GlobLister.scala +++ b/main/src/main/scala/sbt/internal/GlobLister.scala @@ -8,14 +8,16 @@ package sbt package internal -import sbt.io.{ Glob, TypedPath } +import java.nio.file.Path + +import sbt.io.Glob /** * Retrieve files from a repository. This should usually be an extension class for * sbt.io.internal.Glob (or a Traversable collection of source instances) that allows us to * actually retrieve the files corresponding to those sources. */ -sealed trait GlobLister extends Any { +private[sbt] sealed trait GlobLister extends Any { /** * Get the sources described this [[GlobLister]]. @@ -23,7 +25,7 @@ sealed trait GlobLister extends Any { * @param repository the [[FileTree.Repository]] to delegate file i/o. * @return the files described by this [[GlobLister]]. */ - def all(implicit repository: FileTree.Repository): Seq[Stamped.File] + def all(implicit repository: FileTree.Repository): Seq[(Path, FileAttributes)] /** * Get the unique sources described this [[GlobLister]]. @@ -31,7 +33,7 @@ sealed trait GlobLister extends Any { * @param repository the [[FileTree.Repository]] to delegate file i/o. * @return the files described by this [[GlobLister]] with any duplicates removed. */ - def unique(implicit repository: FileTree.Repository): Seq[Stamped.File] + def unique(implicit repository: FileTree.Repository): Seq[(Path, FileAttributes)] } /** @@ -80,18 +82,15 @@ private[internal] object GlobListers { private def get[T0 <: Traversable[Glob]]( traversable: T0, repository: FileTree.Repository - ): Seq[Stamped.File] = + ): Seq[(Path, FileAttributes)] = traversable.flatMap { glob => - val sourceFilter: TypedPath => Boolean = glob.toTypedPathFilter - repository.get(glob).flatMap { - case e if sourceFilter(e.typedPath) => e.value.toOption.map(Stamped.file(e.typedPath, _)) - case _ => None - } - }.toIndexedSeq: Seq[Stamped.File] + val sourceFilter = glob.toFileFilter + repository.get(glob).filter { case (p, _) => sourceFilter.accept(p.toFile) } + }.toIndexedSeq - override def all(implicit repository: FileTree.Repository): Seq[Stamped.File] = + override def all(implicit repository: FileTree.Repository): Seq[(Path, FileAttributes)] = get(globs, repository) - override def unique(implicit repository: FileTree.Repository): Seq[Stamped.File] = + override def unique(implicit repository: FileTree.Repository): Seq[(Path, FileAttributes)] = get(globs.toSet[Glob], repository) } } diff --git a/sbt/src/sbt-test/io/glob/build.sbt b/sbt/src/sbt-test/io/glob/build.sbt new file mode 100644 index 000000000..a79335a34 --- /dev/null +++ b/sbt/src/sbt-test/io/glob/build.sbt @@ -0,0 +1 @@ +val root = Build.root \ No newline at end of file diff --git a/sbt/src/sbt-test/io/glob/files/bar.json b/sbt/src/sbt-test/io/glob/files/bar.json new file mode 100644 index 000000000..e69de29bb diff --git a/sbt/src/sbt-test/io/glob/files/foo.txt b/sbt/src/sbt-test/io/glob/files/foo.txt new file mode 100644 index 000000000..e69de29bb diff --git a/sbt/src/sbt-test/io/glob/files/subdir/baz.yml b/sbt/src/sbt-test/io/glob/files/subdir/baz.yml new file mode 100644 index 000000000..e69de29bb diff --git a/sbt/src/sbt-test/io/glob/project/Build.scala b/sbt/src/sbt-test/io/glob/project/Build.scala new file mode 100644 index 000000000..d0f9bbb71 --- /dev/null +++ b/sbt/src/sbt-test/io/glob/project/Build.scala @@ -0,0 +1,33 @@ +import java.nio.file.{ Path, Paths } +import sbt._ +import sbt.io.Glob +import sbt.Keys._ + +object Build { + val simpleTest = taskKey[Unit]("Check that glob file selectors work") + val relativeSubdir = Paths.get("subdir") + val relativeFiles = + Seq(Paths.get("foo.txt"), Paths.get("bar.json"), relativeSubdir.resolve("baz.yml")) + val files = taskKey[Path]("The files subdirectory") + val subdir = taskKey[Path]("The subdir path in the files subdirectory") + val allFiles = taskKey[Seq[Path]]("Returns all of the regular files in the files subdirectory") + private def check(actual: Any, expected: Any): Unit = + if (actual != expected) throw new IllegalStateException(s"$actual did not equal $expected") + val root = (project in file(".")) + .settings( + files := (baseDirectory.value / "files").toPath, + subdir := files.value.resolve("subdir"), + allFiles := { + val f = files.value + relativeFiles.map(f.resolve(_)) + }, + simpleTest := { + val allPaths: Glob = files.value.allPaths + val af = allFiles.value.toSet + val sub = subdir.value + check(allPaths.all.map(_._1).toSet, af + sub) + check(allPaths.all.filter(_._2.isRegularFile).map(_._1).toSet, af) + check(allPaths.all.filter(_._2.isDirectory).map(_._1).toSet, Set(sub)) + } + ) +} \ No newline at end of file diff --git a/sbt/src/sbt-test/io/glob/test b/sbt/src/sbt-test/io/glob/test new file mode 100644 index 000000000..3e26c171e --- /dev/null +++ b/sbt/src/sbt-test/io/glob/test @@ -0,0 +1 @@ +> simpleTest \ No newline at end of file