watchman: windows fixup a few issues

* On 64-bit systems we need to handle the long long type in the python
  bser implementation
* Address a race condition if a query comes in almost immediately after
  a watch is established
* Fix construction of relative root string on windows
* Add named pipe client transport support to pywatchman
* Tweak windows build to coexist on a shared drive with a unix build
* Update make.bat to do the right thing for me

The python based test suite now passes 100% most of the time (there's
a couple of intermittent issues)

Refs: facebook#19

.gitignore

@@ -9,6 +9,7 @@ install-sh
 missing
 config.log
 .deps/
+.windeps
 *.in
 Makefile
 *.o
@@ -31,6 +32,7 @@ config.guess
 configure.lineno
 python/build
 *.pyc
+*.pyd
 *.so
 *.obj
 *.exe

cfg.c

@@ -34,7 +34,7 @@ void cfg_load_global_config_file(void)
     return;
   }
 
-  if (!w_path_exists(cfg_file) && errno == ENOENT) {
+  if (!w_path_exists(cfg_file)) {
     return;
   }
 

log.c

@@ -161,7 +161,7 @@ void w_log(int level, WATCHMAN_FMT_STRING(const char *fmt), ...)
     buf[sizeof(buf)-1] = 0;
   }
 
-  len = strlen(buf);
+  len = (int)strlen(buf);
 
   if (buf[len - 1] != '\n') {
     if (len < (int)sizeof(buf) - 1) {

make.bat

@@ -1 +1,8 @@
+if "%VS120COMNTOOLS%" == "" (
+  @echo "Setting up build environment"
+  "c:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\vcvarsall.bat" amd64
+)
+@rem Allow python build to succeed:
+@rem http://stackoverflow.com/questions/2817869/error-unable-to-find-vcvarsall-bat
+SET VS90COMNTOOLS=%VS120COMNTOOLS%
 nmake /nologo /s /f winbuild\Makefile %1

python/pywatchman/__init__.py

@@ -55,6 +55,10 @@ class Transport(object):
     """ communication transport to the watchman server """
     buf = None
 
+    def close(self):
+        """ tear it down """
+        raise NotImplementedError()
+
     def readBytes(self, size):
         """ read size bytes """
         raise NotImplementedError()
@@ -112,6 +116,10 @@ def __init__(self, sockpath, timeout):
             raise WatchmanError('unable to connect to %s: %s' %
                                 (self.sockpath, e))
 
+    def close(self):
+        self.sock.close()
+        self.sock = None
+
     def readBytes(self, size):
         try:
             buf = [self.sock.recv(size)]
@@ -128,6 +136,25 @@ def write(self, data):
             raise WatchmanError('timed out sending query command')
 
 
+class WindowsNamedPipeTransport(Transport):
+    """ connect to a named pipe """
+
+    def __init__(self, sockpath, timeout):
+        self.sockpath = sockpath
+        self.timeout = timeout
+        self.pipe = os.open(sockpath, os.O_RDWR | os.O_BINARY)
+
+    def close(self):
+        os.close(self.pipe)
+        self.pipe = None
+
+    def readBytes(self, size):
+        return os.read(self.pipe, size)
+
+    def write(self, data):
+        return os.write(self.pipe, data)
+
+
 class CLIProcessTransport(Transport):
     """ open a pipe to the cli to talk to the service
     This intended to be used only in the test harness!
@@ -154,6 +181,11 @@ def __init__(self, sockpath, timeout):
         self.sockpath = sockpath
         self.timeout = timeout
 
+    def close(self):
+        if self.proc:
+            self.proc.kill()
+            self.proc = None
+
     def _connect(self):
         if self.proc:
             return self.proc
@@ -192,21 +224,21 @@ class BserCodec(Codec):
     """ use the BSER encoding.  This is the default, preferred codec """
 
     def receive(self):
-
         buf = [self.transport.readBytes(sniff_len)]
         if not buf[0]:
             raise WatchmanError('empty watchman response')
 
         elen = bser.pdu_len(buf[0])
-        rlen = len(buf[0])
 
+        rlen = len(buf[0])
         while elen > rlen:
             buf.append(self.transport.readBytes(elen - rlen))
             rlen += len(buf[-1])
 
         response = ''.join(buf)
         try:
-            return bser.loads(response)
+            res = bser.loads(response)
+            return res
         except ValueError as e:
             raise WatchmanError('watchman response decode error: %s' % e)
 
@@ -242,14 +274,17 @@ class client(object):
     recvCodec = None
     sendConn = None
     recvConn = None
+    tport = None
 
     def __init__(self, sockpath=None, timeout=1.0, transport=None,
                  sendEncoding=None, recvEncoding=None):
         self.sockpath = sockpath
         self.timeout = timeout
 
         transport = transport or os.getenv('WATCHMAN_TRANSPORT') or 'local'
-        if transport == 'local':
+        if transport == 'local' and os.name == 'nt':
+            self.transport = WindowsNamedPipeTransport
+        elif transport == 'local':
             self.transport = UnixSocketTransport
         elif transport == 'cli':
             self.transport = CLIProcessTransport
@@ -309,9 +344,19 @@ def _connect(self):
         if self.sockpath is None:
             self.sockpath = self._resolvesockname()
 
-        transport = self.transport(self.sockpath, self.timeout)
-        self.sendConn = self.sendCodec(transport)
-        self.recvConn = self.recvCodec(transport)
+        self.tport = self.transport(self.sockpath, self.timeout)
+        self.sendConn = self.sendCodec(self.tport)
+        self.recvConn = self.recvCodec(self.tport)
+
+    def __del__(self):
+        self.close()
+
+    def close(self):
+        if self.tport:
+            self.tport.close()
+            self.tport = None
+            self.recvConn = None
+            self.sendConn = None
 
     def receive(self):
         self._connect()

python/pywatchman/bser.c

@@ -29,6 +29,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 
 #include <Python.h>
+#ifdef _MSC_VER
+#define inline __inline
+#include <stdint.h>
+#endif
 
 /* Return the smallest size int that can store the value */
 #define INT_SIZE(x) (((x) == ((int8_t)x))  ? 1 :    \
@@ -120,7 +124,7 @@ static void bser_dtor(bser_t *bser)
   bser->buf = NULL;
 }
 
-static int bser_long(bser_t *bser, long val)
+static int bser_long(bser_t *bser, int64_t val)
 {
   int8_t i8;
   int16_t i16;
@@ -225,6 +229,10 @@ static int bser_recursive(bser_t *bser, PyObject *val)
     return bser_long(bser, PyInt_AS_LONG(val));
   }
 
+  if (PyLong_Check(val)) {
+    return bser_long(bser, PyLong_AsLongLong(val));
+  }
+
   if (PyString_Check(val) || PyUnicode_Check(val)) {
     return bser_string(bser, val);
   }

query/parse.c

@@ -206,8 +206,9 @@ static bool parse_relative_root(w_root_t *root, w_query *res, json_t *query)
   path = w_string_new(json_string_value(relative_root));
   canon_path = w_string_canon_path(path);
   res->relative_root = w_string_path_cat(root->root_path, canon_path);
-  res->relative_root_slash = w_string_make_printf("%.*s/",
-        res->relative_root->len, res->relative_root->buf);
+  res->relative_root_slash = w_string_make_printf("%.*s%c",
+        res->relative_root->len, res->relative_root->buf,
+        WATCHMAN_DIR_SEP);
   w_string_delref(path);
   w_string_delref(canon_path);
 

runtests.py

@@ -29,16 +29,14 @@
 # We test for this in a test case
 os.environ['WATCHMAN_EMPTY_ENV_VAR'] = ''
 
+# Ensure that we find the watchman we built in the tests
+os.environ['PATH'] = '.' + os.pathsep + os.environ['PATH']
+
 unittest.installHandler()
 
 # We'll put all our temporary stuff under one dir so that we
 # can clean it all up at the end
 temp_dir = tempfile.mkdtemp(prefix='watchmantest')
-if args.keep:
-    atexit.register(sys.stdout.write,
-                    'Preserving output in %s\n' % temp_dir)
-else:
-    atexit.register(shutil.rmtree, temp_dir)
 # Redirect all temporary files to that location
 tempfile.tempdir = temp_dir
 
@@ -48,6 +46,28 @@
 inst = WatchmanInstance.Instance()
 inst.start()
 
+def retry_rmtree(top):
+    # Keep trying to remove it; on Windows it may take a few moments
+    # for any outstanding locks/handles to be released
+    for i in xrange(1, 10):
+        shutil.rmtree(top, ignore_errors=True)
+        if not os.path.isdir(top):
+            return
+        time.sleep(0.2)
+    sys.stdout.write('Failed to completely remove ' + top)
+
+def cleanup():
+    global inst
+    inst.stop()
+
+    if args.keep:
+        sys.stdout.write('Preserving output in %s\n' % temp_dir)
+        return
+    retry_rmtree(temp_dir)
+
+atexit.register(cleanup)
+
+
 # Allow tests to locate our instance by default
 os.environ['WATCHMAN_SOCK'] = inst.getSockPath()
 

tests/integration/WatchmanInstance.py

@@ -18,7 +18,10 @@ def __init__(self, config={}):
         self.base_dir = tempfile.mkdtemp(prefix='inst')
         self.cfg_file = os.path.join(self.base_dir, "config.json")
         self.log_file_name = os.path.join(self.base_dir, "log")
-        self.sock_file = os.path.join(self.base_dir, "sock")
+        if os.name == 'nt':
+            self.sock_file = '\\\\.\\pipe\\watchman-test-%s' % int(time.time())
+        else:
+            self.sock_file = os.path.join(self.base_dir, "sock")
         self.state_file = os.path.join(self.base_dir, "state")
         with open(self.cfg_file, "w") as f:
             f.write(json.dumps(config))
@@ -34,11 +37,12 @@ def stop(self):
         if self.proc:
             self.proc.kill()
             self.proc.wait()
+            self.proc = None
         self.log_file.close()
 
     def start(self):
         args = [
-            './watchman',
+            'watchman',
             '--foreground',
             '--sockname={}'.format(self.sock_file),
             '--logfile={}'.format(self.log_file_name),

tests/integration/WatchmanTestCase.py

@@ -123,8 +123,13 @@ def normFileList(self, files):
     def assertFileList(self, root, files=[], cursor=None,
                        relativeRoot=None, message=None):
         expected_files = self.normFileList(files)
-        st, res = self.waitFor(
-            lambda: self.getFileList(root, cursor=cursor,
-                                     relativeRoot=relativeRoot
-                                     ) == expected_files)
+        if (cursor is not None) and cursor[0:2] == 'n:':
+            # it doesn't make sense to repeat named cursor queries, as
+            # the cursor moves each time
+            self.getFileList(root, cursor=cursor, relativeRoot=relativeRoot)
+        else:
+            st, res = self.waitFor(
+                lambda: self.getFileList(root, cursor=cursor,
+                                         relativeRoot=relativeRoot
+                                         ) == expected_files)
         self.assertEqual(self.last_file_list, expected_files, message)

tests/integration/test_since.py

@@ -50,9 +50,11 @@ def test_sinceIssue2(self):
             'foo/bar/222'])
 
         # now check the delta for the since
-        self.assertFileList(root, cursor='n:foo', files=[
-            'foo/bar',
-            'foo/bar/222'])
+        expected = ['foo/bar', 'foo/bar/222']
+        if os.name == 'nt' or os.uname()[0] == 'SunOS':
+            # These systems also show the containing dir as modified
+            expected.append('foo')
+        self.assertFileList(root, cursor='n:foo', files=expected)
 
     def test_sinceRelativeRoot(self):
         root = tempfile.mkdtemp()
@@ -97,7 +99,8 @@ def test_sinceRelativeRoot(self):
             'since': res['clock'],
             'relative_root': 'subdir',
             'fields': ['name']})
-        self.assertEqual(self.normFileList(res['files']), ['dir2', 'dir2/bar'])
+        self.assertEqual(self.normFileList(res['files']),
+                         self.normFileList(['dir2', 'dir2/bar']))
 
     def assertFreshInstanceForSince(self, root, cursor, empty=False):
         res = self.watchmanCommand('query', root, {

watcher/win32.c

@@ -113,8 +113,11 @@ static void *readchanges_thread(void *arg) {
   DWORD err, filter;
   OVERLAPPED olap;
   BOOL initiate_read = true;
+  bool did_signal_init = false;
   HANDLE handles[2] = { state->olap, state->ping };
 
+  w_set_thread_name("readchange %.*s", root->root_path->len, root->root_path->buf);
+
   // Block until winmatch_root_st is waiting for our initialization
   pthread_mutex_lock(&state->mtx);
 
@@ -131,9 +134,7 @@ static void *readchanges_thread(void *arg) {
     goto out;
   }
 
-  // Signal that we are done with init
-  pthread_cond_signal(&state->cond);
-  pthread_mutex_unlock(&state->mtx);
+  w_log(W_LOG_DBG, "ReadDirectoryChangesW signalling as init done");
 
   while (!root->cancelled) {
     DWORD bytes;
@@ -154,6 +155,17 @@ static void *readchanges_thread(void *arg) {
       }
     }
 
+    if (!did_signal_init) {
+      // Signal that we are done with init.  We MUST do this after our first
+      // successful ReadDirectoryChangesW, otherwise there is a race condition
+      // where we'll miss observing the cookie for a query that comes in
+      // after we've crawled but before the watch is established.
+      pthread_cond_signal(&state->cond);
+      pthread_mutex_unlock(&state->mtx);
+      did_signal_init = true;
+    }
+
+    w_log(W_LOG_DBG, "waiting for change notifications");
     DWORD status = WaitForMultipleObjects(2, handles, FALSE, INFINITE);
 
     if (status == WAIT_OBJECT_0) {