Patchwork D2576: xdiff: add a python wrapper

login
register
mail settings
Submitter phabricator
Date March 3, 2018, 12:25 a.m.
Message ID <differential-rev-PHID-DREV-32kx5nsyzgmjqqukt7cb-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/28758/
State Superseded
Headers show

Comments

phabricator - March 3, 2018, 12:25 a.m.
ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Implement a `mercurial.cext.xdiff` module that exposes the xdiff algorithm.
  
  `xdiff.blocks` should be a drop-in replacement for `bdiff.blocks`.
  
  In theory we can change the pure C version of `bdiff.c` directly. However
  that means we lose bdiff entirely. It seems more flexible to have both at
  the same time so they can be easily switched via Python code. Hence the
  Python module approach.

TEST PLAN
  `make local`. And test from `hg debugshell`:
  
    In [1]: from mercurial.cext import bdiff, xdiff
    
    In [2]: bdiff.blocks('b\nc\nd\n', 'a\nb\nc\ne\nf\n')
    Out[2]: [(0, 2, 1, 3), (3, 3, 5, 5)]
    
    In [3]: xdiff.blocks('b\nc\nd\n', 'a\nb\nc\ne\nf\n')
    Out[3]: [(0, 2, 1, 3), (3, 3, 5, 5)]
    
    In [4]: bdiff.blocks('a\nb', '')
    Out[4]: [(2, 2, 0, 0)]
    
    In [5]: xdiff.blocks('a\nb', '')
    Out[5]: [(2, 2, 0, 0)]
    
    In [6]: bdiff.blocks('a\nb', 'a\nb\n')
    Out[6]: [(0, 1, 0, 1), (2, 2, 2, 2)]
    
    In [7]: xdiff.blocks('a\nb', 'a\nb\n')
    Out[7]: [(0, 1, 0, 1), (2, 2, 2, 2)]
    
    In [8]: xdiff.blocks('', '')
    Out[8]: [(0, 0, 0, 0)]
    
    In [9]: bdiff.blocks('', '')
    Out[9]: [(0, 0, 0, 0)]
    
    In [10]: xdiff.blocks('', '\n')
    Out[10]: [(0, 0, 1, 1)]

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2576

AFFECTED FILES
  contrib/check-code.py
  mercurial/thirdparty/xdiff.c
  setup.py

CHANGE DETAILS




To: ryanmce, #hg-reviewers
Cc: mercurial-devel
phabricator - March 3, 2018, 1:53 a.m.
quark added a comment.


  Traditionally things under `mercurial.cext` has a pure equivalent. So it's cleaner to either make the changes directly in `mercurial.cext.bdiff`, or `mercurial/bdiff.c`. Or add a xdiff pure shim.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2576

To: ryanmce, #hg-reviewers
Cc: quark, mercurial-devel
phabricator - March 3, 2018, 2:49 p.m.
durin42 added inline comments.

INLINE COMMENTS

> setup.py:223
>      # import py2exe's patched Distribution class
> -    from distutils.core import Distribution
> +    from distutils.core import Distribution # noqa: F811
>  except ImportError:

wat?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2576

To: ryanmce, #hg-reviewers
Cc: durin42, quark, mercurial-devel
phabricator - March 3, 2018, 8:08 p.m.
quark commandeered this revision.
quark added a reviewer: ryanmce.
quark added a comment.


  Not going to add another module.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2576

To: quark, #hg-reviewers, ryanmce
Cc: durin42, quark, mercurial-devel

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -220,7 +220,7 @@ 
     py2exe.Distribution # silence unused import warning
     py2exeloaded = True
     # import py2exe's patched Distribution class
-    from distutils.core import Distribution
+    from distutils.core import Distribution # noqa: F811
 except ImportError:
     py2exeloaded = False
 
@@ -874,6 +874,28 @@ 
               extra_compile_args=osutil_cflags,
               extra_link_args=osutil_ldflags,
               depends=common_depends),
+    Extension('mercurial.cext.xdiff',
+              sources=[
+                  'mercurial/thirdparty/xdiff/xdiffi.c',
+                  'mercurial/thirdparty/xdiff/xemit.c',
+                  'mercurial/thirdparty/xdiff/xmerge.c',
+                  'mercurial/thirdparty/xdiff/xprepare.c',
+                  'mercurial/thirdparty/xdiff/xutils.c',
+                  'mercurial/thirdparty/xdiff.c',
+              ],
+              include_dirs=common_include_dirs + [
+                  'mercurial/thirdparty/xdiff',
+              ],
+              depends=common_depends + [
+                  'mercurial/thirdparty/xdiff/xdiff.h',
+                  'mercurial/thirdparty/xdiff/xdiffi.h',
+                  'mercurial/thirdparty/xdiff/xemit.h',
+                  'mercurial/thirdparty/xdiff/xinclude.h',
+                  'mercurial/thirdparty/xdiff/xmacros.h',
+                  'mercurial/thirdparty/xdiff/xprepare.h',
+                  'mercurial/thirdparty/xdiff/xtypes.h',
+                  'mercurial/thirdparty/xdiff/xutils.h',
+              ]),
     Extension('hgext.fsmonitor.pywatchman.bser',
               ['hgext/fsmonitor/pywatchman/bser.c']),
     ]
diff --git a/mercurial/thirdparty/xdiff.c b/mercurial/thirdparty/xdiff.c
new file mode 100644
--- /dev/null
+++ b/mercurial/thirdparty/xdiff.c
@@ -0,0 +1,96 @@ 
+/*
+ xdiff.c: simple Python wrapper for xdiff library
+
+ Copyright (c) 2018 Facebook, Inc.
+
+ This software may be used and distributed according to the terms of the
+ GNU General Public License version 2 or any later version.
+*/
+
+#include "xdiff.h"
+#include "Python.h"
+
+static int hunk_consumer(long a1, long a2, long b1, long b2, void *priv)
+{
+	PyObject *rl = (PyObject *)priv;
+	PyObject *m = Py_BuildValue("llll", a1, a2, b1, b2);
+	if (!m)
+		return -1;
+	if (PyList_Append(rl, m) != 0) {
+		Py_DECREF(m);
+		return -1;
+	}
+	return 0;
+}
+
+static PyObject *blocks(PyObject *self, PyObject *args)
+{
+	char *sa, *sb;
+	int na, nb;
+
+	if (!PyArg_ParseTuple(args, "s#s#", &sa, &na, &sb, &nb))
+		return NULL;
+
+	mmfile_t a = {sa, na}, b = {sb, nb};
+
+	PyObject *rl = PyList_New(0);
+	if (!rl)
+		return PyErr_NoMemory();
+
+	xpparam_t xpp = {
+	    0,    /* flags */
+	    NULL, /* anchors */
+	    0,    /* anchors_nr */
+	};
+	xdemitconf_t xecfg = {
+	    0,                  /* ctxlen */
+	    0,                  /* interhunkctxlen */
+	    XDL_EMIT_BDIFFHUNK, /* flags */
+	    NULL,               /* find_func */
+	    NULL,               /* find_func_priv */
+	    hunk_consumer,      /* hunk_consume_func */
+	};
+	xdemitcb_t ecb = {
+	    rl,   /* priv */
+	    NULL, /* outf */
+	};
+
+	if (xdl_diff(&a, &b, &xpp, &xecfg, &ecb) != 0) {
+		Py_DECREF(rl);
+		return PyErr_NoMemory();
+	}
+
+	return rl;
+}
+
+static char xdiff_doc[] = "xdiff wrapper";
+
+static PyMethodDef methods[] = {
+    {"blocks", blocks, METH_VARARGS,
+     "(a: str, b: str) -> List[(a1, a2, b1, b2)].\n"
+     "Yield matched blocks. (a1, a2, b1, b2) are line numbers.\n"},
+    {NULL, NULL},
+};
+
+static const int version = 1;
+
+#ifdef IS_PY3K
+static struct PyModuleDef bdiff_module = {
+    PyModuleDef_HEAD_INIT, "bdiff", xdiff_doc, -1, methods,
+};
+
+PyMODINIT_FUNC PyInit_xdiff(void)
+{
+	PyObject *m;
+	m = PyModule_Create(&xdiff_module);
+	PyModule_AddIntConstant(m, "version", version);
+	return m;
+}
+#else
+PyMODINIT_FUNC initxdiff(void)
+{
+	PyObject *m;
+	m = Py_InitModule3("xdiff", methods, xdiff_doc);
+	PyModule_AddIntConstant(m, "version", version);
+}
+#endif
diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -249,7 +249,8 @@ 
     (r"[^_]_\([ \t\n]*(?:'[^']+'[ \t\n+]*)+%", "don't use % inside _()"),
     (r'(\w|\)),\w', "missing whitespace after ,"),
     (r'(\w|\))[+/*\-<>]\w', "missing whitespace in expression"),
-    (r'^\s+(\w|\.)+=\w[^,()\n]*$', "missing whitespace in assignment"),
+#   false positive on multi-line keyword arguments
+#    (r'^\s+(\w|\.)+=\w[^,()\n]*$', "missing whitespace in assignment"),
     (r'\w\s=\s\s+\w', "gratuitous whitespace after ="),
     ((
         # a line ending with a colon, potentially with trailing comments