Issue 102: mtn diff output no longer in alphabetical order - A distributed version control system

Reported by joe 23, Nov 7, 2010

Steps to reproduce the problem:
-------------------------------
mtn diff, or mtn diff filea fileb filec

Expected result:
----------------
Both the header of the diff and the diff sections should be in 
alphabetical order by filename.

Actual results:
---------------
Header (listing the files) is in alphabetical order but the diff 
sections seem to be in random order. This makes it difficult to 
review diffs across a whole project.

Pretty sure this changed somewhere between 0.44 and 0.48 (i.e. 0.44 
was alphabetical, but 0.48 wasn't)

Output of `mtn version --full`:
-------------------------------
monotone 0.99.1 (base revision: 
8973482283db7c36780dce2b54721ccc0f5b7388)
Running on          : Linux 2.6.32-25-generic #45-Ubuntu SMP Sat Oct 
16 19:48:22 UTC 2010 i686
C++ compiler        : GNU C++ version 4.3.2
C++ standard library: GNU libstdc++ version 20080905
Boost version       : 1_40
SQLite version      : 3.5.9 (compiled against 3.5.9)
Lua version         : Lua 5.1
PCRE version        : 7.6 2008-01-28 (compiled against 7.6)
Botan version       : 1.8.9 (compiled against 1.8.9)
Changes since base revision:
format_version "1"

new_manifest [c1270158b7fa91abf8235ad129b0476943bde1ed]

old_revision [8973482283db7c36780dce2b54721ccc0f5b7388]

  Generated from data cached in the distribution;
  further changes may have been made.

Comment 1 by joe 23, Nov 8, 2010

More info:

The old behavior was the new files alphabetically, followed by the 
changed files alphabetically.

The new behavior is everything in somewhat random order.

My preference would be that the new and changed files are merged 
into a single sort (if I get to vote), but the old behavior would be 
a lot better than random.

Comment 2 by Thomas Keller, Dec 28, 2010

Labels: Milestone:1.0

Comment 3 by Richard Levitte, Jan 1, 2011

Looking at the code, I'm concluding that the order is by node_id...  
and the way the code currently looks, turning it to some kind of 
alphabetical order seems a bit difficult.

To be honest, I'm not sure I see this as very high priority, and 
definitely not a show stopper for 1.0.

Labels: Priority:Low ~~Priority:Medium~~

Comment 4 by Thomas Keller, Jan 1, 2011

I looked at the code myself a couple of days ago and yes, its not 
easy. The best I came up with was recording the individual diffs 
from the make_diff calls in dump_diffs in some 
std::multimap<path,diff>. multimap because we'd need to use 
the old path in this map for deleted nodes and the new path for 
renamed / added nodes, which of course makes problems for example in 
a case of a circular rename for a regular map.

If I read the stl docs correctly, we'd get the ordering for free 
when we use the standard iterators afterwards to output the diffs in 
order. So I'd still like to see this in 1.0. Accepting and assigning 
to myself.

Status: Accepted
Owner: tommyd

Comment 5 by Richard Levitte, Jan 9, 2011

I've just had a second look at the code at hand, and am noticing 
that dump_headers uses cset order (basically, what write_cset gives) 
while dump_diffs uses roster order (which really is node number 
order, as I understand it).

I'm thinking that the cset order used by dump_headers is a good 
thing, so the conclusion would be that we should have dump_diffs 
produce diffs in the same order, and therefore iterate over a cset.
The trouble with that thought, I guess, is that there's no real 
mapping from a cset back to the nodes in a roster, a cset seems 
pretty disconnected.  That would need to be solved, in other words...

Comment 6 by Thomas Keller, Jan 14, 2011

Fixed in rev e7de71b0f9fd6113f13b00eeddfdcd8aa57bcd78

Status: Fixed