Handle deletes in fresh scm aware queries

Summary: Fresh instance queries in watchman return the full set of files and do not mention any deleted files (it is a fresh query).  For SCM aware queries, it is necessary to return deleted files since the merge base.

Reviewed By: wez

Differential Revision: D5565778

fbshipit-source-id: 0cd1417578497d45d3c77f7169529c529d034366

query/eval.cpp

@@ -48,9 +48,9 @@ void w_query_process_file(
     ctx->file.reset();
   };
 
-  // For fresh instances, only return files that currently exist.
-  if (!ctx->since.is_timestamp && ctx->since.clock.is_fresh_instance &&
-      !ctx->file->exists()) {
+  // For fresh instances, only return files that currently exist
+  if (!ctx->disableFreshInstance && !ctx->since.is_timestamp &&
+      ctx->since.clock.is_fresh_instance && !ctx->file->exists()) {
     return;
   }
 
@@ -62,7 +62,7 @@ void w_query_process_file(
   }
 
   if (ctx->query->dedup_results) {
-    w_string_t *name = w_query_ctx_get_wholename(ctx);
+    w_string_t* name = w_query_ctx_get_wholename(ctx);
 
     auto inserted = ctx->dedup.insert(name);
     if (!inserted.second) {
@@ -133,7 +133,7 @@ bool w_query_file_matches_relative_root(
   // "in relative root" here does not mean exactly the relative root, so compare
   // against the relative root's parent.
   result = w_string_equal(parent_path, ctx->query->relative_root) ||
-           w_string_startswith(parent_path, ctx->query->relative_root_slash);
+      w_string_startswith(parent_path, ctx->query->relative_root_slash);
 
   return result;
 }
@@ -189,8 +189,8 @@ static void execute_common(
     ctx->dedup.reserve(64);
   }
 
-  res->is_fresh_instance = !ctx->since.is_timestamp &&
-    ctx->since.clock.is_fresh_instance;
+  res->is_fresh_instance =
+      !ctx->since.is_timestamp && ctx->since.clock.is_fresh_instance;
 
   if (!(res->is_fresh_instance && ctx->query->empty_on_fresh_instance)) {
     if (!generator) {
@@ -228,8 +228,14 @@ static void execute_common(
   res->dedupedFileNames = std::move(ctx->dedup);
 }
 
-w_query_ctx::w_query_ctx(w_query* q, const std::shared_ptr<w_root_t>& root)
-    : query(q), root(root), resultsArray(json_array()) {
+w_query_ctx::w_query_ctx(
+    w_query* q,
+    const std::shared_ptr<w_root_t>& root,
+    bool disableFreshInstance)
+    : query(q),
+      root(root),
+      resultsArray(json_array()),
+      disableFreshInstance{disableFreshInstance} {
   // build a template for the serializer
   if (query->fieldList.size() > 1) {
     json_array_set_template_new(
@@ -247,6 +253,7 @@ w_query_res w_query_execute(
   w_query_res res;
   std::shared_ptr<w_query> altQuery;
   ClockSpec resultClock(ClockPosition{});
+  bool disableFreshInstance{false};
 
   w_perf_t sample("query_execute");
 
@@ -282,14 +289,15 @@ w_query_res w_query_execute(
       // And switch us over to run the rest of the query on this one
       altQuery = w_query_parse(root, altQuerySpec);
       query = altQuery.get();
+      disableFreshInstance = true;
       // We may have been called with a custom generator; we don't need to use
       // that for this case, so make sure that we use the default generator
       // so that it will actually execute using the pathGenerator.
       generator = nullptr;
     }
   }
 
-  w_query_ctx ctx(query, root);
+  w_query_ctx ctx(query, root, disableFreshInstance);
   if (query->sync_timeout.count() && !root->syncToNow(query->sync_timeout)) {
     throw QueryExecError("synchronization failed: ", strerror(errno));
   }
@@ -316,16 +324,15 @@ w_query_res w_query_execute(
   res.clockAtStartOfQuery.clock = ctx.clockAtStartOfQuery.clock;
 
   // Evaluate the cursor for this root
-  ctx.since = query->since_spec
-      ? query->since_spec->evaluate(
-            ctx.clockAtStartOfQuery.position(),
-            ctx.lastAgeOutTickValueAtStartOfQuery,
-            &root->inner.cursors)
-      : w_query_since();
+  ctx.since = query->since_spec ? query->since_spec->evaluate(
+                                      ctx.clockAtStartOfQuery.position(),
+                                      ctx.lastAgeOutTickValueAtStartOfQuery,
+                                      &root->inner.cursors)
+                                : w_query_since();
 
   if (query->query_spec.get_default("bench")) {
     for (auto i = 0; i < 100; ++i) {
-      w_query_ctx c(query, root);
+      w_query_ctx c(query, root, false);
       w_query_res r;
       c.clockAtStartOfQuery = ctx.clockAtStartOfQuery;
       execute_common(&c, nullptr, &r, generator);

tests/integration/test_scm.py

@@ -75,6 +75,10 @@ def test_scmHg(self):
 |  parent:      1:6b3ecb11785e
 |  summary:     add m1
 |
+| o  changeset:   5:7bc34583612
+|/   bookmark:    feature3
+|    summary:     remove car
+|
 | o  changeset:   2:2db357583971
 |/   bookmark:    feature1
 |    summary:     add f1
@@ -93,13 +97,18 @@ def test_scmHg(self):
         # so we use an alternative similar name.
         self.hg(['book', 'TheMaster'], cwd=root)
         self.touchRelative(root, 'bar')
+        self.touchRelative(root, 'car')
         self.hg(['addremove'], cwd=root)
-        self.hg(['commit', '-m', 'add bar'], cwd=root)
+        self.hg(['commit', '-m', 'add bar and car'], cwd=root)
         self.hg(['book', 'feature1'], cwd=root)
         self.touchRelative(root, 'f1')
         self.hg(['addremove'], cwd=root)
         self.hg(['commit', '-m', 'add f1'], cwd=root)
         self.hg(['co', 'TheMaster'], cwd=root)
+        self.hg(['book', 'feature3'], cwd=root)
+        self.hg(['rm', 'car'], cwd=root)
+        self.hg(['commit', '-m', 'remove car'], cwd=root)
+        self.hg(['co', 'TheMaster'], cwd=root)
         self.touchRelative(root, 'm1')
         self.hg(['addremove'], cwd=root)
         self.hg(['commit', '-m', 'add m1'], cwd=root)
@@ -114,7 +123,7 @@ def test_scmHg(self):
             'expression': ['not', ['anyof', ['name', '.hg'], ['dirname', '.hg']]],
             'fields': ['name']})
         self.assertFileListsEqual(
-            res['files'], ['foo', 'bar', 'm1', 'm2'])
+            res['files'], ['foo', 'bar', 'car', 'm1', 'm2'])
 
         # Verify behavior with badly formed queries
         with self.assertRaises(pywatchman.WatchmanError) as ctx:
@@ -228,6 +237,26 @@ def test_scmHg(self):
                     'mergebase-with': 'TheMaster'}}})
         self.assertEqual(clock['scm'], res['clock']['scm'])
 
+        # Fresh instance queries return the complete set of changes (so there is
+        # no need to provide information on deleted files).  In contrast, SCM
+        # aware queries must contain the deleted files in the result list. Check
+        # that the deleted file is part of the result set for feature3.
+        self.hg(['co', '-C', 'feature3'], cwd=root)
+        res = self.watchmanCommand('query', root, {
+            'expression': ['not', ['anyof', ['name', '.hg'], ['dirname', '.hg']]],
+            'fields': ['name'],
+            'since': res['clock']})
+        self.assertFileListsEqual(res['files'], ['f1', 'car'])
+
+        res = self.watchmanCommand('query', root, {
+            'expression': ['not', ['anyof', ['name', '.hg'], ['dirname', '.hg']]],
+            'fields': ['name'],
+            'since': {
+                'scm': {
+                    'mergebase': '',
+                    'mergebase-with': 'TheMaster'}}})
+        self.assertFileListsEqual(res['files'], ['car'])
+
     def getSubFatClocksOnly(self, subname, root):
         dat = self.waitForSub(subname, root=root)
         return [

watchman_query.h

@@ -111,7 +111,13 @@ struct w_query_ctx {
   // How many times we suppressed a result due to dedup checking
   uint32_t num_deduped{0};
 
-  w_query_ctx(w_query* q, const std::shared_ptr<w_root_t>& root);
+  // Disable fresh instance queries
+  bool disableFreshInstance{false};
+
+  w_query_ctx(
+      w_query* q,
+      const std::shared_ptr<w_root_t>& root,
+      bool disableFreshInstance);
   w_query_ctx(const w_query_ctx&) = delete;
   w_query_ctx& operator=(const w_query_ctx&) = delete;