diff --git a/io/src/main/scala/sbt/internal/io/Milli.scala b/io/src/main/scala/sbt/internal/io/Milli.scala index d953a645..03d3c9be 100644 --- a/io/src/main/scala/sbt/internal/io/Milli.scala +++ b/io/src/main/scala/sbt/internal/io/Milli.scala @@ -237,22 +237,17 @@ private trait Mac extends Library with Posix[Long] { options: Int): Int } private object MacMilli extends PosixMilliLong[Mac] { - private final val ATTR_BIT_MAP_COUNT: Short = 5 - private final val ATTR_CMN_MODTIME: Int = 0x00000400 + private val attr = new Attrlist + attr.bitmapcount = 5 // ATTR_BIT_MAP_COUNT + attr.commonattr = 0x00000400 // ATTR_CMN_MODTIME protected def getModifiedTimeNative(filePath: String) = { - val attr = new Attrlist - attr.bitmapcount = ATTR_BIT_MAP_COUNT - attr.commonattr = ATTR_CMN_MODTIME val buf = new TimeBuf checkedIO(filePath) { libc.getattrlist(filePath, attr, buf, 20 /* buf size */, 0) } (buf.tv_sec, buf.tv_nsec) } protected def setModifiedTimeNative(filePath: String, mtimeNative: (Long, Long)): Unit = { - val attr = new Attrlist - attr.bitmapcount = ATTR_BIT_MAP_COUNT - attr.commonattr = ATTR_CMN_MODTIME val buf = new Timespec val (sec, nsec) = mtimeNative buf.tv_sec = sec diff --git a/io/src/main/scala/sbt/io/IO.scala b/io/src/main/scala/sbt/io/IO.scala index 932148b0..a8c5cc96 100644 --- a/io/src/main/scala/sbt/io/IO.scala +++ b/io/src/main/scala/sbt/io/IO.scala @@ -15,7 +15,7 @@ import java.io.{ OutputStream, PrintWriter } -import java.io.{ ObjectInputStream, ObjectStreamClass } +import java.io.{ ObjectInputStream, ObjectStreamClass, FileNotFoundException } import java.net.{ URI, URISyntaxException, URL } import java.nio.charset.Charset import java.nio.file.FileSystems @@ -306,7 +306,7 @@ object IO { } } if (preserveLastModified) - setModifiedTime(target, entry.getTime) + setModifiedTimeOrFalse(target, entry.getTime) } else { //log.debug("Ignoring zip entry '" + name + "'") } @@ -543,7 +543,7 @@ object IO { def makeFileEntry(file: File, name: String) = { // log.debug("\tAdding " + file + " as " + name + " ...") val e = createEntry(name) - e setTime getModifiedTime(file) + e setTime getModifiedTimeOrZero(file) e } def addFileEntry(file: File, name: String) = { @@ -643,7 +643,7 @@ object IO { preserveLastModified: Boolean, preserveExecutable: Boolean )(from: File, to: File): File = { - if (overwrite || !to.exists || getModifiedTime(from) > getModifiedTime(to)) { + if (overwrite || !to.exists || getModifiedTimeOrZero(from) > getModifiedTimeOrZero(to)) { if (from.isDirectory) createDirectory(to) else { @@ -728,7 +728,7 @@ object IO { } } if (preserveLastModified) { - copyModifiedTime(sourceFile, targetFile) + copyLastModified(sourceFile, targetFile) () } if (preserveExecutable) { @@ -737,33 +737,6 @@ object IO { } } - /** - * Transfers the last modified time of `sourceFile` to `targetFile`. - * - * Note: this method has an underspecified, and generally inconsistent - * behavior. In particular, if the source file is missing, it will - * silently set the target modification time to 1st January 1970, - * returning success. Also, because it uses lastModified(), it may - * trim away the millisecond part of the timestamp without notice. - * - * After sbt/io v1.0.2 (for the 1.0.x series) and v1.1.1 (for - * the 1.1.x series) a new method copyModifiedTime() has been added - * to sbt.io.IO. That method will correctly throw a FileNotFoundException - * if any of the two files are missing, or an IOException in case an - * IO exception occurs. - * - * The current behavior of copyLastModified() is retained for compatibility, - * but client code should be migrated to copyModifiedTime(), instead. - * The copyLastModified() method may be removed in future versions of sbt.io.IO. - */ - @deprecated("Use copyModifiedTime() instead.", "1.0.3") - def copyLastModified(sourceFile: File, targetFile: File): Boolean = { - val last = sourceFile.lastModified - // lastModified can return a negative number, but setLastModified doesn't accept it - // see Java bug #6791812 - targetFile.setLastModified(math.max(last, 0L)) - } - /** Transfers the executable property of `sourceFile` to `targetFile`. */ def copyExecutable(sourceFile: File, targetFile: File) = { val executable = sourceFile.canExecute @@ -1126,7 +1099,7 @@ object IO { /** * Return the last modification timestamp of the specified file, - * in milliseconds from the Unix epoch (January 1, 1970 UTC). + * in milliseconds since the Unix epoch (January 1, 1970 UTC). * This method will use whenever possible native code in order to * return a timestamp with a 1-millisecond precision. * @@ -1157,9 +1130,16 @@ object IO { * and Java's get/setLastModifiedTime() will be used instead. This * setting applies to setModifiedTime() and copyModifiedTime() as well. * + * This method was added in sbt/io v1.1.2, but it may be + * replaced in the future by a similar method that returns + * a Try, rather than throwing an exception. + * It is therefore marked as deprecated, since we cannot + * guarantee that it will remain available in future versions. + * * @see setModifiedTime * @see copyModifiedTime */ + @deprecated("This method might be removed in the future, also see getModifiedOrZero()", "1.1.3") def getModifiedTime(file: File): Long = Milli.getModifiedTime(file) /** @@ -1174,9 +1154,19 @@ object IO { * contrast, Java's traditional setLastModified() will return a * boolean false value if an error occurs. * + * This method may not work correctly if mtime is negative. + * + * This method was added in sbt/io v1.1.2, but it may be + * replaced in the future by a similar method that returns + * a Try, rather than throwing an exception. + * It is therefore marked as deprecated, since we cannot + * guarantee that it will remain available in future versions. + * * @see getModifiedTime * @see copyModifiedTime */ + @deprecated("This method might be removed in the future, also see setModifiedTimeOrFalse()", + "1.1.3") def setModifiedTime(file: File, mtime: Long): Unit = Milli.setModifiedTime(file, mtime) /** @@ -1192,9 +1182,82 @@ object IO { * If the timestamp cannot be copied, this method will throw * a FileNotFoundException or an IOException, as appropriate. * + * This method was added in sbt/io v1.1.2, but it may be + * replaced in the future by a similar method that returns + * a Try, rather than throwing an exception. + * It is therefore marked as deprecated, since we cannot + * guarantee that it will remain available in future versions. + * * @see getModifiedTime * @see setModifiedTime */ + @deprecated("This method might be removed in the future, also see copyLastModified()", "1.1.3") def copyModifiedTime(fromFile: File, toFile: File): Unit = Milli.copyModifiedTime(fromFile, toFile) + + /** + * Return the last modification timestamp of the specified file, + * in milliseconds since the Unix epoch (January 1, 1970 UTC). + * + * If the specified file does not exist, this method will return 0L. + * + * The deprecated method getModifiedTime() has similar semantics, + * but will throw an exception if the file does not exist. + * Please refer to its documentation for additional details. + * + * @see getModifiedTime + */ + def getModifiedTimeOrZero(file: File): Long = + try { + getModifiedTime(file) + } catch { + case _: FileNotFoundException => 0L + } + + /** + * Sets the modification time of the file argument, in milliseconds + * since the Unix epoch (January 1, 1970 UTC). + * + * If the specified file does not exist, this method will return false. + * It will return true if the file modification time was successfully changed. + * + * The deprecated method setModifiedTime() has similar semantics, + * but will throw an exception if the file does not exist. + * Please refer to its documentation for additional details. + * + * This method may not work correctly if mtime is negative. + * + * @see setModifiedTime + */ + def setModifiedTimeOrFalse(file: File, mtime: Long): Boolean = + try { + Milli.setModifiedTime(file, mtime) + true + } catch { + case _: FileNotFoundException => false + } + + /** + * Transfers the last modified time of `sourceFile` to `targetFile`. + * + * Note: this method has a special semantics if files are missing. + * In particular, if the source file is missing, it will silently + * set the target modification time to 1st January 1970, which + * corresponds to the Unix epoch. + * + * The method returns true if the target file modification time was + * successfully changed, false otherwise. + * + * The deprecated related method copyModifiedTime() has a somewhat different + * semantics, please refer to its documentation for additional details. + * + * @see copyModifiedTime + */ + def copyLastModified(sourceFile: File, targetFile: File): Boolean = { + val last = getModifiedTimeOrZero(sourceFile) + // getModifiedTimeOrZero can return a negative number, but setLastModified + // (which may be used by setModifiedTimeOrFalse) doesn't accept it, + // see Java bug #6791812 + setModifiedTimeOrFalse(targetFile, math.max(last, 0L)) + } } diff --git a/io/src/main/scala/sbt/io/Path.scala b/io/src/main/scala/sbt/io/Path.scala index 0b05dc12..fa2862de 100644 --- a/io/src/main/scala/sbt/io/Path.scala +++ b/io/src/main/scala/sbt/io/Path.scala @@ -20,7 +20,7 @@ final class RichFile(val asFile: File) extends AnyVal with RichNioPath { def isDirectory: Boolean = asFile.isDirectory /** The last modified time of the wrapped file.*/ - def lastModified: Long = IO.getModifiedTime(asFile) + def lastModified: Long = IO.getModifiedTimeOrZero(asFile) /** * True if and only if the wrapped file `asFile` exists and the file 'other' @@ -277,7 +277,7 @@ object Path extends Mapper { separated.mkString(sep) } def newerThan(a: File, b: File): Boolean = - a.exists && (!b.exists || IO.getModifiedTime(a) > IO.getModifiedTime(b)) + a.exists && (!b.exists || IO.getModifiedTimeOrZero(a) > IO.getModifiedTimeOrZero(b)) /** The separator character of the platform.*/ val sep: Char = java.io.File.separatorChar diff --git a/io/src/main/scala/sbt/io/PollingWatchService.scala b/io/src/main/scala/sbt/io/PollingWatchService.scala index 0eadad4d..0669d78e 100644 --- a/io/src/main/scala/sbt/io/PollingWatchService.scala +++ b/io/src/main/scala/sbt/io/PollingWatchService.scala @@ -93,7 +93,7 @@ class PollingWatchService(delay: FiniteDuration) extends WatchService { watched.toSeq.sortBy(_._1)(pathLengthOrdering).foreach { case (p, _) => if (!results.contains(p)) - p.toFile.allPaths.get.foreach(f => results += f.toPath -> IO.getModifiedTime(f)) + p.toFile.allPaths.get.foreach(f => results += f.toPath -> IO.getModifiedTimeOrZero(f)) } results.toMap } diff --git a/io/src/test/scala/sbt/internal/io/SourceModificationWatchSpec.scala b/io/src/test/scala/sbt/internal/io/SourceModificationWatchSpec.scala index db26490c..ac5a849c 100644 --- a/io/src/test/scala/sbt/internal/io/SourceModificationWatchSpec.scala +++ b/io/src/test/scala/sbt/internal/io/SourceModificationWatchSpec.scala @@ -21,10 +21,9 @@ abstract class SourceModificationWatchSpec( IO.write(file, "foo") - // watchTest(parentDir) { - // IO.write(file, "bar") - // } - pending // until fixed https://github.com/sbt/io/issues/82 + watchTest(parentDir) { + IO.write(file, "bar") + } } it should "watch a directory for file creation" in IO.withTemporaryDirectory { dir =>