[MERGE REVIEW] Revert destroys file contents produced by merge

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
|>Files whose contents were modified by merge are deleted without being
|>backed up.
|
|
| Deleted, and then replaced by the reverted-to version?

Right.  I should have said 'replaced' rather than 'deleted'.  Living too
close to the code, I guess.

|>Files that were added by merge are deleted instead of being
|>unversioned.
|
|
|>However, if the files are modified afterward by the user, then revert
|>will handle them normally.
|
|
| So if they were added and then modified, they remain as unversioned.
| But if they were modified by merge, then modified by the user, they get
| reverted?

If a file is added by a merge, then modified by a user, revert will
retain it as an unversioned file.  If a file is modified by a merge,
then modified by a user, revert will make a backup before replacing the
contents.

|>This also introduces the splatfile format, a very simple format for
|>representing lists of lists of unicode strings.
|>
|>Advantages over rio:
|> - all unicode strings are legal values
|> - very simple API
|> - aside from header, 1-to-1 correspondance between lines and stanzas
|> - escaping built-in and minimal.
|
|
| This looks pretty good; to me it seems to provide a "list of strings"
| companion to the "list of multimaps" in rio.

Isn't rio a multimap, not a list of them?

| I think both have a place
| - splatfile more so when we don't expect new fields to be added within a
| particular format or to need repeated fields.  Do you agree?

I guess I do see them more as competitors.  I think they represent
different design decisions about where flexibility is needed.  I'm not
tied to splatfile; we could make changes to rio instead, if you'd prefer
that.

I should explain that I'm using splatfile to represent a dict in this
case, which is clearly a subset of what rio can represent.  But the keys
of my dict are illegal values in rio.  I could encode them as hex, but
I'd prefer to be able to read the output files with the naked eye.

And I find there's a Unicode representation of almost everything I want
to store, which is why splatfile is an always-Unicode format.

Before I did the ancestry cache, I just assumed rio would be a good
format for all our needs, but when I tried to use it for the ancestry
cache, I had problems because of the restrictions on keys.  I actually
did hex-encode everything at one point, but that made it hard to debug
the problems I was having; rio seemed to be losing records, but I
couldn't eyeball the file to see what was going on.

So I guess I'm disappointed with rio, and that's why I came up with
splatfile.

| The rio api should probably be simpler; I don't think there's much value
| exposing the reader or writer classes.
|
| Every unicode string is valid on the RHS as far as I know.

Right, but when you consider that we will often want to use file-ids and
revision-ids on the LHS, I think we need to accept more (preferably all)
values on the LHS.

|>+SPLATFILE_1_HEADER = "BZR Splatfile Format 1"
|
|
| Perhaps it would be better to set the header according to what it
| contains, rather than how the data is encode?  In other words perhaps
| 'bzr merge-modified list format 1' - that implies the data format
| anyhow.

I would rather have the physical format be explicit.  Much like XML has
both an XML format version and a schema.  Not because I want to be able
to vary both independently, but to ensure the splatfile parser can't
return garbage.  Perhaps two headers, or a combined header, then?

Two headers might be:

BZR merge-modified list format 1
BZR Splatfile format 1

Combined might be:
BZR merge-modified list format 1 / BZR Splatfile format 1

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEAerP0F+nu1YWqI0RAsm/AJ4tIbJn5uq12byBgQit88+7oU3PaACfbZlB
Ublg5byCmACC34EGA7fer4w=
=Ztwp
-----END PGP SIGNATURE-----