[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