When the format contains an unknown secondary format property (a 
property that comes after a '|' in the format file), the commands 
rebase suspend, rebase pull, and rebase apply will corrupt the format 
file. This destroys forward compatibility when later versions want to 
add another such property. (The only existing one is "no-working-
dir".)

This bug affects all darcs versions at least from 2.12.5 up to 
2.14.4.

It is caused by the the coincidence of the following two problems in 
our code, each of which looks harmless in isolation:

(1) The Show instance for RepoProperty prepends a string "Unknown 
property: " to the name of an unknown property. Note that we also use 
this instance when we write the format file.

(2) The affected commands write the format file /before/ checking 
that the format allows us to write the repo. This check is done in a 
generic way in withRepoLock and withRepoLockCanFail, but the code in 
Darcs.Repository.Rebase is executed before these calls.

A related problem, though much less severe, is

(3) The rebase commands do an extra check and crash if the format 
file specifies that a rebase is in progress but we cannot find any 
suspended patches. This also limits forward compatibility in that it 
means we cannot attach secondary format properties to an existing 
"rebase-in-progress" or its replacement in 2.16 in order to indicate 
e.g. that we moved the rebase patch to a different location (e.g. we 
might want to move it into the hashed store). But at least we crash 
before we write the format file, so the repo is not damaged.

I have marked this as "critical" because as long as this bug is 
present in darcs versions that are widely deployed we cannot add any 
new secondary repo property. This /severely/ limits our ability to 
make progress in a user-friendly manner.

The first thing to do is to fix (1), which is a simple one-line 
patch.

Then we need to make a new 2.14.5 release with that fix applied. We 
should reach out to distributors and ask them to upgrade as soon as 
possible. The CHANGELOG entry should be loud and clear and strongly 
recommend to upgrade.

This is of course a blocker for 2.16.

In 2.16 we should document the format of the _darcs/format file and 
explain how primary and secondary format properties work. This 
feature is crude and not intuitively easy to understand, so it *must* 
be documented in the code else we run the risk of making the same 
mistake again. I have a patch for this.

We also need to fix (2). It is *not* acceptable that we write the 
format file before having checked that we are allowed to modify the 
repo, even if what we write is byte for byte identical with the 
original content. When we fix that, we may as well go the whole way 
and postpone writing of the format file until after we have taken the 
repo lock (as suggested in the TODO comment attached to call to 
writeRepoFormat in Darcs.Repository.Rebase.)

I am not yet sure what to do about (3).

Correction: What I wrote about primary and secondary properties is 
nonsense. Sorry. I failed to read the code properly.

The vertical bar between properties on the same line actually means this:

* If we know *any* of these properties, then we can read the repo.
* If we know *all* of them, we can also write the repo.

The notation makes a lot more sense now.

I'll have to think about how that affects my analysis of the bug and 
whether the conclusions are still valid.

As it turns out we do have tests for the format file, including 
alternative format properties, see tests/repoformat.sh. We just didn't 
test it with rebase commands.