[MERGE] groupcompress (brisbane)
John Arbash Meinel
john at arbash-meinel.com
Mon Apr 6 15:08:39 BST 2009
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Ian Clatworthy wrote:
> The attached patch covers the changes in brisbane-core
> that introduce the awesome groupcompress stuff. I know
> this horse has probably bolted but I wonder whether a
> different name might be good to minimise any confusion
> with gc=garbage-collection? Maybe multicompress,
> bzrcompress or lifelessjamcompress even? :-)
>
> Anyhow, here are the files involved in this patch:
>
> added:
> bzrlib/_groupcompress_py.py
> bzrlib/_groupcompress_pyx.pyx
> bzrlib/delta.h
> bzrlib/diff-delta.c
> bzrlib/groupcompress.py
> bzrlib/tests/test__groupcompress.py
> bzrlib/tests/test_groupcompress.py
> modified:
> .bzrignore
> bzrlib/tests/__init__.py
> bzrlib/tests/test_versionedfile.py
> bzrlib/versionedfile.py
> setup.py
>
> I'm comfortable that this can be landed in bzr.dev without
> destabilisation given most of it is new code and the
> modified files are all small, obvious changes except
> for test_versionedfile.py that adds a bunch of new tests.
>
> So the areas needing attention IMO by a reviewer are:
>
> 1. the pyrex implementation of groupcompress it's
> pretty new IIUIC so it hasn't necessarily had the
> workout the core groupcompress code has had
As Robert mentioned, the original Python implementation was very heavily
tested (as it was the *original* implementation). And then we evolved
it, and then brought it back again :). The only specific change from
this python implementation and the one that was in the plugin is the
byte stream format. The *matching* code is the same. And I have a
reasonable number of tests that it will continue matching the way we
want. (I make sure that if a delta is added, a future text will compress
against those lines, etc.)
>
> 2. is test coverage of the new code sufficient?
>
> We also need a final word on the GPL 2 vs GPL 2+ issue
> of the C code before landing/merging this into bzr.dev.
>
> Ian C.
>
In the other discussion, I think the answer was "go ahead and do it, as
long as it doesn't lock us into GPL 2 forever". And it doesn't. Whether
we have to rewrite parts of it, or we get approval from the original
authors, etc.
While we are here, I'm thinking to rename "diff-delta.c" to
"_diff_delta.c" or maybe "_groupcompress_delta.c" or something else, so
that it actually fits in with our general naming schemes. And the same
goes for "delta.h".
BB:tweak
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAknaDOcACgkQJdeBCYSNAAPvsgCfZAkBaGHqGGaLXWREd84t8coZ
9aoAoIEHKgQVQWy0wXnAcNTMqLXMhqo8
=e2Si
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list