Include milliseconds in Windows Stat dates (#2106)

On Windows, we use a third-party library (i.e.
`@gyselroth/windows-fsstat`) to compute file and directory stats
because the non-BigInt version of `fs.stat` would not always return a
unique `inode` value (i.e. the returned `inode` value of 2 files or
directories could be the same).
However, this library would truncate all dates to the second and this
can lead us to discard very close modifications since we now expect
the modification date to be different to process a local event as a
modification.

We've modified our fork of `@gyselroth/windows-fsstat` to include
milliseconds in returned dates and use it in the client.
Since changing the modification date of most files would trigger a lot
of checksums re-computing, we include a "migration" to prevent this
during the first initial scan.

core/app.js

@@ -13,6 +13,7 @@ const url = require('url')
 const uuid = require('uuid/v4')
 const https = require('https')
 const { createGzip } = require('zlib')
+const semver = require('semver')
 
 require('./globals')
 const pkg = require('../package.json')
@@ -373,6 +374,23 @@ class App {
         )
         wasUpdated = false
       }
+
+      // TODO: remove with flag WINDOWS_DATE_MIGRATION_FLAG
+      try {
+        if (
+          semver.lt(
+            clientInfo.configVersion,
+            config.WINDOWS_DATE_MIGRATION_APP_VERSION
+          )
+        ) {
+          this.config.setFlag(config.WINDOWS_DATE_MIGRATION_FLAG, true)
+        }
+      } catch (err) {
+        log.error(
+          { err, sentry: true },
+          `could not set ${config.WINDOWS_DATE_MIGRATION_FLAG} flag`
+        )
+      }
     }
 
     this.instanciate()

core/config.js

@@ -21,6 +21,22 @@ export type WatcherType = 'atom' | 'chokidar'
 type FileConfig = Object
 */
 
+/* Stat dates on Windows were previously truncated to the second while we now
+ * get the milliseconds as well.
+ * To avoid re-calculating all the local checksums during the initial scan
+ * because of the date migration, we'll truncate `scan` events dates to the
+ * second during the first initial scan following the publication of v3.28.1
+ * when checking if the checksum of a given file can be reused or not.
+ *
+ * When publishing v3.28.2 or greater, we can remove WINDOWS_DATE_MIGRATION_FLAG
+ * and stop truncating dates during the initial scan.
+ *
+ * Users who will have skipped the version introducing the flag will see all
+ * their checksums re-computed.
+ */
+const WINDOWS_DATE_MIGRATION_APP_VERSION = '3.28.1'
+const WINDOWS_DATE_MIGRATION_FLAG = 'roundWindowsDatesToSecondInInitialDiff'
+
 const INVALID_CONFIG_ERROR = 'InvalidConfigError'
 const INVALID_CONFIG_MESSAGE = 'Invalid client configuration'
 class InvalidConfigError extends Error {
@@ -170,6 +186,24 @@ class Config {
     return _.get(this.fileConfig, 'flags', {})
   }
 
+  isFlagActive(flagName /*: string */) /*: boolean */ {
+    return this.flags[flagName] || false
+  }
+
+  setFlag(flag /*: string */, isActive /*: boolean */) {
+    if (
+      typeof flag !== 'string' ||
+      typeof isActive !== 'boolean' ||
+      flag === ''
+    ) {
+      throw new Error(
+        `Invalid flag or value: [String(${flag})] → "${String(isActive)}"`
+      )
+    }
+    _.set(this.fileConfig, `flags.${flag}`, isActive)
+    this.persist()
+  }
+
   get version() /*: ?string */ {
     return _.get(this.fileConfig, 'creds.client.softwareVersion', '')
   }
@@ -288,6 +322,8 @@ function validateWatcherType(watcherType /*: ?string */) /*: ?WatcherType */ {
 
 module.exports = {
   INVALID_CONFIG_ERROR,
+  WINDOWS_DATE_MIGRATION_APP_VERSION,
+  WINDOWS_DATE_MIGRATION_FLAG,
   InvalidConfigError,
   Config,
   environmentWatcherType,

core/local/atom/add_checksum.js

@@ -28,6 +28,7 @@ const contentActions = new Set(['created', 'modified', 'renamed', 'scan'])
 
 /*::
 import type Channel from './channel'
+import type { Config } from '../../config'
 import type { Checksumer } from '../checksumer'
 import type { AtomEvent } from './event'
 */
@@ -50,8 +51,10 @@ module.exports = {
  */
 function loop(
   channel /*: Channel */,
-  opts /*: { syncPath: string , checksumer: Checksumer } */
+  opts /*: { config: Config , checksumer: Checksumer } */
 ) /*: Channel */ {
+  const syncPath = opts.config.syncPath
+
   return channel.asyncMap(async events => {
     for (const event of events) {
       try {
@@ -63,7 +66,7 @@ function loop(
             { path: event.path, action: event.action },
             'computing checksum'
           )
-          const absPath = path.join(opts.syncPath, event.path)
+          const absPath = path.join(syncPath, event.path)
           event.md5sum = await opts.checksumer.push(absPath)
         }
       } catch (err) {

core/local/atom/add_infos.js

@@ -22,6 +22,7 @@ const log = logger({
 })
 
 /*::
+import type { Config } from '../../config'
 import type { Pouch } from '../../pouch'
 import type { Metadata } from '../../metadata'
 import type Channel from './channel'
@@ -41,8 +42,10 @@ module.exports = {
  */
 function loop(
   channel /*: Channel */,
-  opts /*: { syncPath: string, pouch: Pouch } */
+  opts /*: { config: Config, pouch: Pouch } */
 ) /*: Channel */ {
+  const syncPath = opts.config.syncPath
+
   return channel.asyncMap(async events => {
     const batch = []
     for (const event of events) {
@@ -55,9 +58,7 @@ function loop(
         if (event.action !== 'initial-scan-done') {
           if (needsStats(event)) {
             log.debug({ path: event.path, action: event.action }, 'stat')
-            event.stats = await stater.stat(
-              path.join(opts.syncPath, event.path)
-            )
+            event.stats = await stater.stat(path.join(syncPath, event.path))
           }
 
           if (event.stats) {

core/local/atom/dispatch.js

@@ -11,6 +11,7 @@ const _ = require('lodash')
 
 const winDetectMove = require('./win_detect_move')
 const { buildDir, buildFile } = require('../../metadata')
+const { WINDOWS_DATE_MIGRATION_FLAG } = require('../../config')
 const logger = require('../../utils/logger')
 
 const STEP_NAME = 'dispatch'
@@ -28,6 +29,7 @@ import type {
 } from './event'
 import type { WinDetectMoveState } from './win_detect_move'
 import type EventEmitter from 'events'
+import type { Config } from '../../config'
 import type Prep from '../../prep'
 import type { Pouch } from '../../pouch'
 import type { Metadata } from '../../metadata'
@@ -41,6 +43,7 @@ type DispatchState = {
 }
 
 type DispatchOptions = {
+  config: Config,
   events: EventEmitter,
   prep: Prep,
   pouch: Pouch,
@@ -133,8 +136,12 @@ async function dispatchEvent(
 }
 
 actions = {
-  initialScanDone: ({ events }) => {
+  initialScanDone: ({ config, events }) => {
     log.info('Initial scan done')
+    // TODO: remove with flag WINDOWS_DATE_MIGRATION_FLAG
+    if (config.isFlagActive(WINDOWS_DATE_MIGRATION_FLAG)) {
+      config.setFlag(WINDOWS_DATE_MIGRATION_FLAG, false)
+    }
     events.emit('initial-scan-done')
   },
 

core/local/atom/incomplete_fixer.js

@@ -35,6 +35,7 @@ const DELAY = 3000
 import type Channel from './channel'
 import type { AtomEvent, AtomBatch } from './event'
 import type { Checksumer } from '../checksumer'
+import type { Config } from '../../config'
 import type { Pouch } from '../../pouch'
 import type { Metadata } from '../../metadata'
 
@@ -44,7 +45,7 @@ type IncompleteItem = {
 }
 
 type IncompleteFixerOptions = {
-  syncPath: string,
+  config: Config,
   checksumer: Checksumer,
   pouch: Pouch
 }
@@ -116,7 +117,7 @@ function completeEventPaths(
 async function rebuildIncompleteEvent(
   previousEvent /*: AtomEvent */,
   nextEvent /*: AtomEvent */,
-  opts /*: { syncPath: string , checksumer: Checksumer, pouch: Pouch } */
+  opts /*: { config: Config , checksumer: Checksumer, pouch: Pouch } */
 ) /*: Promise<Completion> */ {
   const { path: rebuiltPath, oldPath: rebuiltOldPath } = completeEventPaths(
     previousEvent,
@@ -127,7 +128,7 @@ async function rebuildIncompleteEvent(
     return { ignored: true }
   }
 
-  const absPath = path.join(opts.syncPath, rebuiltPath)
+  const absPath = path.join(opts.config.syncPath, rebuiltPath)
   const stats = await stater.statMaybe(absPath)
   const incomplete = stats == null
   const kind = stats ? stater.kind(stats) : previousEvent.kind

core/local/atom/initial_diff.js

@@ -12,11 +12,13 @@
 const _ = require('lodash')
 const path = require('path')
 
+const { WINDOWS_DATE_MIGRATION_FLAG } = require('../../config')
 const { kind } = require('../../metadata')
 const logger = require('../../utils/logger')
 const Channel = require('./channel')
 
 /*::
+import type { Config } from '../../config'
 import type { Pouch } from '../../pouch'
 import type { AtomEvent, AtomBatch, EventKind } from './event'
 import type { Metadata } from '../../metadata'
@@ -68,10 +70,10 @@ module.exports = {
 
 function loop(
   channel /*: Channel */,
-  opts /*: { pouch: Pouch, state: InitialDiffState } */
+  opts /*: { config: Config, state: InitialDiffState } */
 ) /*: Channel */ {
   const out = new Channel()
-  initialDiff(channel, out, opts.state).catch(err => {
+  initialDiff(channel, out, opts).catch(err => {
     log.warn({ err })
   })
   return out
@@ -126,7 +128,7 @@ function clearState(state /*: InitialDiffState */) {
 async function initialDiff(
   channel /*: Channel */,
   out /*: Channel */,
-  state /*: InitialDiffState */
+  { config, state } /*: { config: Config, state: InitialDiffState } */
 ) /*: Promise<void> */ {
   // eslint-disable-next-line no-constant-condition
   while (true) {
@@ -140,6 +142,10 @@ async function initialDiff(
         initialScanDone
       }
     } = state
+    // TODO: remove with flag WINDOWS_DATE_MIGRATION_FLAG
+    const truncateWindowsDates = config.isFlagActive(
+      WINDOWS_DATE_MIGRATION_FLAG
+    )
 
     if (initialScanDone) {
       out.push(events)
@@ -189,7 +195,7 @@ async function initialDiff(
               deletedIno: was.local.fileid || was.local.ino
             })
           }
-        } else if (foundUntouchedFile(event, was)) {
+        } else if (foundUntouchedFile(event, was, truncateWindowsDates)) {
           _.set(event, [STEP_NAME, 'md5sumReusedFrom'], was.local.path)
           event.md5sum = was.local.md5sum
         }
@@ -338,8 +344,10 @@ function fixPathsAfterParentMove(renamedEvents, event) {
   }
 }
 
-function contentUpdateTime(event) {
-  return event.stats.mtime.getTime()
+function contentUpdateTime(event, truncateWindowsDates) {
+  return truncateWindowsDates
+    ? event.stats.mtime.getTime() - event.stats.mtime.getMilliseconds()
+    : event.stats.mtime.getTime()
 }
 
 function docUpdateTime(oldLocal) {
@@ -354,12 +362,16 @@ function foundRenamedOrReplacedDoc(event, was) /*: boolean %checks */ {
   return was != null && was.local != null && was.local.path !== event.path
 }
 
-function foundUntouchedFile(event, was) /*: boolean %checks */ {
+function foundUntouchedFile(
+  event,
+  was,
+  truncateWindowsDates
+) /*: boolean %checks */ {
   return (
     event.kind === 'file' &&
     was != null &&
     was.local != null &&
     was.local.md5sum != null &&
-    contentUpdateTime(event) === docUpdateTime(was.local)
+    contentUpdateTime(event, truncateWindowsDates) === docUpdateTime(was.local)
   )
 }

core/local/atom/producer.js

@@ -15,6 +15,7 @@ const defaultStater = require('../stater')
 const logger = require('../../utils/logger')
 
 /*::
+import type { Config } from '../../config'
 import type { Ignore } from '../../ignore'
 import type { AtomEvent } from './event'
 import type EventEmitter from 'events'
@@ -66,10 +67,10 @@ class Producer {
   scan: Scanner
   */
   constructor(
-    opts /*: { syncPath: string, ignore: Ignore, events: EventEmitter } */
+    opts /*: { config: Config, ignore: Ignore, events: EventEmitter } */
   ) {
     this.channel = new Channel()
-    this.syncPath = opts.syncPath
+    this.syncPath = opts.config.syncPath
     this.ignore = opts.ignore
     this.events = opts.events
     this.watcher = null

core/local/atom/watcher.js

@@ -39,6 +39,7 @@ const dispatch = require('./dispatch')
 const logger = require('../../utils/logger')
 
 /*::
+import type { Config } from '../../config'
 import type { Pouch } from '../../pouch'
 import type Prep from '../../prep'
 import type EventEmitter from 'events'
@@ -48,7 +49,7 @@ import type { AtomEventsDispatcher } from './dispatch'
 import type { Scanner } from './producer'
 
 type AtomWatcherOptions = {
-  syncPath: string,
+  config: Config,
   onAtomEvents?: AtomEventsDispatcher,
   prep: Prep,
   pouch: Pouch,
@@ -107,6 +108,7 @@ const stepsInitialState = (
 
 class AtomWatcher {
   /*::
+  config: Config
   pouch: Pouch
   events: EventEmitter
   checksumer: Checksumer
@@ -118,6 +120,7 @@ class AtomWatcher {
   */
 
   constructor(opts /*: AtomWatcherOptions */) {
+    this.config = opts.config
     this.pouch = opts.pouch
     this.events = opts.events
     this.checksumer = checksumer.init()

core/local/watcher.js

@@ -14,7 +14,7 @@ const { AtomWatcher } = require('./atom/watcher')
 const ChokidarWatcher = require('./chokidar/watcher')
 
 /*::
-import type { WatcherType } from '../config'
+import type { Config } from '../config'
 import type { AtomEventsDispatcher } from './atom/dispatch'
 import type { Pouch } from '../pouch'
 import type Prep from '../prep'
@@ -31,10 +31,7 @@ export interface Watcher {
 }
 
 export type LocalWatcherOptions = {
-  +config: {
-    +syncPath: string,
-    +watcherType: WatcherType
-  },
+  config: Config,
   events: EventEmitter,
   ignore: Ignore,
   onAtomEvents?: AtomEventsDispatcher,
@@ -57,7 +54,7 @@ function build(
 
   if (watcherType === 'atom') {
     return new AtomWatcher({
-      syncPath,
+      config,
       prep,
       pouch,
       events,