Skip to content

Commit

Permalink
[watchman] jsonify protocol
Browse files Browse the repository at this point in the history
Summary:
fix the protocol handling by adopting json for both the requests and
the response.

This also allows us to remove the dependency on libevent (which is honestly a
bit of a cluster... to handle portably), so now all our dependencies are either
in the base system or are included in our source code.

This depends on D645447 for the integration test pieces.

We always return a JSON object response that includes our version number.
Errors will always have an "error" object field that holds the text of the
error.  The rest of the commands are still transitional when it comes to their
response format, but I'll be updating the README as I review/revise each of
those.

I've pulled in version 2.4 of the Jansson JSON library, which is compatibly MIT
licensed.  Doc on it can be found here: http://www.digip.org/jansson/doc/2.4/

Test Plan: `make integration`

Reviewers: jakubv, keegancsmith, zoel

CC: pieter, fugalh, security-diffs@lists

Differential Revision: https://phabricator.fb.com/D645534
  • Loading branch information
wez committed Dec 2, 2012
1 parent 3bc515a commit 2be9fca
Show file tree
Hide file tree
Showing 25 changed files with 4,939 additions and 178 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ arcanist/.phutil_module_cache
*.swp
compile
stamp-h1
watchman-config.h
config.h
thirdparty/jansson/jansson_config.h
compile
tests/*.t
.dirstamp
*.a
25 changes: 25 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
bin_PROGRAMS = watchman

THIRDPARTY_CPPFLAGS = -Ithirdparty/jansson

watchman_CPPFLAGS = $(THIRDPARTY_CPPFLAGS)
watchman_LDADD = -L. -lwmanjson
watchman_SOURCES = \
argv.c \
hash.c \
Expand All @@ -13,6 +17,26 @@ noinst_HEADERS = \
watchman.h \
watchman_hash.h

noinst_LIBRARIES = libwmanjson.a

# bundled json library
libwmanjson_a_CPPFLAGS = $(THIRDPARTY_CPPFLAGS)
libwmanjson_a_SOURCES = \
thirdparty/jansson/dump.c \
thirdparty/jansson/error.c \
thirdparty/jansson/hashtable.c \
thirdparty/jansson/hashtable.h \
thirdparty/jansson/jansson_private.h \
thirdparty/jansson/load.c \
thirdparty/jansson/memory.c \
thirdparty/jansson/pack_unpack.c \
thirdparty/jansson/strbuffer.c \
thirdparty/jansson/strbuffer.h \
thirdparty/jansson/strconv.c \
thirdparty/jansson/utf.c \
thirdparty/jansson/utf.h \
thirdparty/jansson/value.c

# unit tests
TESTS = tests/argv.t
noinst_PROGRAMS = tests/argv.t
Expand All @@ -25,6 +49,7 @@ build-tests: $(TESTS)
integration:
arc unit --engine WatchmanIntegrationEngine

tests_argv_t_CPPFLAGS = $(THIRDPARTY_CPPFLAGS)
tests_argv_t_SOURCES = \
thirdparty/tap.c \
tests/argv.c \
Expand Down
26 changes: 23 additions & 3 deletions arcanist/lib/WatchmanInstance.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class WatchmanInstance {
private $dir;
private $invocations = 0;
private $debug = false;
private $sock;

function __construct() {
$this->dir = PhutilDirectoryFixture::newEmptyFixture();
Expand All @@ -43,6 +44,27 @@ function command() {
return newv('ExecFuture', $args);
}

function request() {
$args = func_get_args();

if (!$this->invocations) {
return call_user_func_array(
array($this, 'resolveCommand'),
$args);
}

// Use a socket instead
if (!$this->sock) {
$this->sock = fsockopen('unix://' .
$this->dir->getPath() . '/.watchman.' .
getenv('USER'));
}

fwrite($this->sock, json_encode($args));
$data = fgets($this->sock);
return json_decode($data, true);
}

function resolveCommand() {
$args = func_get_args();

Expand All @@ -53,9 +75,7 @@ function resolveCommand() {
if ($this->debug) {
echo "running: " . $future->getCommand() . "\n";
}
list($out) = $future->resolvex();

return $out;
return $future->resolveJSON();
}

function __destruct() {
Expand Down
2 changes: 1 addition & 1 deletion arcanist/lib/WatchmanTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function watchmanCommand() {
}

return call_user_func_array(
array($this->watchman, 'resolveCommand'),
array($this->watchman, 'request'),
$args);
}

Expand Down
29 changes: 15 additions & 14 deletions configure.ac
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
AC_INIT([watchman], [1.0], [], [watchman])
AM_INIT_AUTOMAKE([dist-bzip2 subdir-objects -Wall])
AM_INIT_AUTOMAKE([dist-bzip2 subdir-objects])
AC_PROG_CC
AC_C_BIGENDIAN
AC_C_INLINE
AC_PROG_RANLIB

AM_PROG_CC_C_O
AM_PROG_AS

AC_SEARCH_LIBS(
[event_init],
[event event-1.4 event2],
[found_libevent=yes],
[found_libevent=no]
)
dnl could do some feature testing here to turn on a safe set
dnl of warnings, and also add an option to turn on -Werror
dnl for maintainers.
if test -n "$GCC" ; then
CFLAGS="$CFLAGS -Wall -Wextra -Wdeclaration-after-statement"
dnl -Werror
fi

AC_CHECK_HEADERS(event.h sys/inotify.h sys/event.h)
AC_SEARCH_LIBS([pthread_create], [pthread])

AC_CHECK_FUNCS(kqueue inotify_init)
AC_CHECK_HEADERS(locale.h sys/inotify.h sys/event.h)
AC_CHECK_FUNCS(kqueue inotify_init strtoll localeconv)

if test "x$found_libevent" = "xno" ; then
AC_MSG_ERROR("we need libevent")
fi
AC_CONFIG_HEADER([watchman-config.h])
AC_CONFIG_FILES([Makefile])
AC_CONFIG_HEADER([config.h])
AC_CONFIG_FILES([Makefile thirdparty/jansson/jansson_config.h])
AC_OUTPUT

dnl vim:ts=2:sw=2:
Expand Down
Loading

0 comments on commit 2be9fca

Please sign in to comment.