Hi!

Today I noticed that sometimes darcs generates strange patches:
http://re.jabber.ru/~alexey/ejabberd/_darcs/patches/20070829161259-9f22b-1bdefba7d07cdcbf79431c0e7de9e8a2ac4f7635.gz
http://re.jabber.ru/~alexey/ejabberd/_darcs/patches/20070829175459-e83eb-caf1155b92f3adec1f8f1c9ffbded79efa7a891a.gz

Look at first hunks, they remove, and then add back the same lines:

hunk ./ChangeLog 1
-2007-08-29  Mickael Remond  <mremond@process-one.net>
-
-	* doc/guide.tex: Documentation for XML based optimisation build
-	time option (EJAB-298)
+2007-08-29  Alexey Shchepin  <alexey@process-one.net>
+
+	* src/mod_muc/mod_muc_room.erl: The mod_muc option max_users now
+	limits max number of users in rooms and max_users_admin_threshold
+	sets a number of admin or owner accounts allowd to join after
+	max_users occupants
+
+2007-08-29  Mickael Remond  <mremond@process-one.net>
+
+	* doc/guide.tex: Documentation for XML based optimisation build
+	time option (EJAB-298)

While I expected to see this:

hunk ./ChangeLog 1
+2007-08-29  Alexey Shchepin  <alexey@process-one.net>
+
+	* src/mod_muc/mod_muc_room.erl: The mod_muc option max_users now
+	limits max number of users in rooms and max_users_admin_threshold
+	sets a number of admin or owner accounts allowd to join after
+	max_users occupants
+

Am I missing something and this is an intended behaviour, or is it a bug?
Darcs version is 1.0.9~rc1-0.1 (from debian etch).

Attachments

• smime.p7s

Hi Alexey,

[pre-pended lines]
> +2007-08-29  Mickael Remond  <mremond@process-one.net>
> +
> +	* doc/guide.tex: Documentation for XML based optimisation build
> +	time option (EJAB-298)

That does not look expected to me, but maybe David should comment.
Does 1.0.9 or perhaps the current stable branch of darcs behave the
same way?

In any case, thanks for the bug report!

Was the patch recorded with Amend-record? If it was, it's
probably just a matter the Amend-record not being totally
optimized, but the patch should be fine although odd looking.

Yes, that is puzzling.  It seems that there's a pretty severe infelicity in
our diff algorithm.  :( On the plus side, it doesn't cause corruption, but
it'd also be nice to figure out what triggers this.
-- 
David Roundy
http://www.darcs.net

Hi!

 TP> Was the patch recorded with Amend-record? If it was, it's probably just a
 TP> matter the Amend-record not being totally optimized, but the patch should
 TP> be fine although odd looking.

Yes, the first one was altered with amend-record to resolve a conflict, however
this one
http://re.jabber.ru/~alexey/ejabberd/_darcs/patches/20070829161259-9f22b-1bdefba7d07cdcbf79431c0e7de9e8a2ac4f7635.gz
containing

hunk ./ChangeLog 1
-2007-08-29  Alexey Shchepin  <alexey@process-one.net>
+2007-08-29  Mickael Remond  <mremond@process-one.net>
+
+	* doc/guide.tex: Documentation for XML based optimisation build
+	time option (EJAB-298)
hunk ./ChangeLog 6
+2007-08-29  Alexey Shchepin  <alexey@process-one.net>
+	

is a result of tailor svn->darcs gate, and I believe it doesn't use
amend-record.

Attachments

• smime.p7s

...
> is a result of tailor svn->darcs gate, and I believe it doesn't use
> amend-record.

I don't think amend-record should be responsible.  It ought to be possible
to reproduce this.  If you grab a repository with one of these two as
the last patch, unrecord and then record again, does it result in the same
diff? (Yes, I could test this myself, but I'm trying to focus on
conflicts...)
-- 
David Roundy
http://www.darcs.net

DR> I don't think amend-record should be responsible.  It ought to be possible
 DR> to reproduce this.  If you grab a repository with one of these two as the
 DR> last patch, unrecord and then record again, does it result in the same
 DR> diff?

The patch I showed in the first message becomes correct after unrecord/record.
The other patch doesn't change, but I found that it's because TAB was
accidentally inserted in the line 7.  Probably a more human-friendly patch
would be this:

hunk ./ChangeLog 1
+2007-08-29  Mickael Remond  <mremond@process-one.net>
+
+	* doc/guide.tex: Documentation for XML based optimisation build
+	time option (EJAB-298)
+
hunk ./ChangeLog 7
-
+^I

but it has the same number of lines, so I think there is no issue with the
second patch.

Attachments

• smime.p7s

I'll do some testing with the provided examples.

Alexey,

I'm not familiar with this "p7s" format. Is there any chance you could test
again with a build of a Darcs 2 pre-release binary? There are some pre-built
from the DarcsTwo page on the wiki. 

Short of that, re-sending a test case in a more common format would be helpful.

smime.p7s files are S/MIME signatures of my emails, so they are not related to
the bug.
The problematic patch is available here:
http://re.jabber.ru/~alexey/ejabberd/_darcs/patches/20070829175459-e83eb-caf1155b92f3adec1f8f1c9ffbded79efa7a891a.gz
I'll try to find out how to reproduce it with darcs 1 and 2.

Attachments

• smime.p7s

I was able to reproduce this (although not in a controlled way, yet).  I can
confirm that amend-record is involved.  I'm guess that it's because of the
coalesced nature of hunks?

We now have bugs/issue525.sh which reproduces an amend-record version of this bug.

Set as resolved based on the following.

Best regards
Thorkil
On Sunday 16 November 2008 21:42, David Roundy wrote:
> On Sat, Nov 15, 2008 at 11:17:52PM +0000, Eric Kow wrote:
> > Hi David,
> > 
> > On Sat, Nov 15, 2008 at 16:25:41 -0500, David Roundy wrote:
> > > Here's a fix for issue525, which turned out to be trivial.
> > 
> > Applied, thanks!  But I confess that I don't fully understand this.
> > 
> > resolve issue525: canonize output of sort_coalesceFL in AmendRecord.
> > --------------------------------------------------------------------
> > > David Roundy <droundy@darcs.net>**20081115211925
> > >  Ignore-this: cb7485c971d7d8d6f7ffce9f9ec40e98
> > > ] hunk ./src/Darcs/Commands/AmendRecord.lhs 193
> > > -    in n2pia $ infodepspatch new_pinf pdeps $ fromPrims $ 
sort_coalesceFL $
> > > -       concatFL $
> > > -       mapFL_FL canonize $ oldchs +>+ chs
> > > +    in n2pia $ infodepspatch new_pinf pdeps $ fromPrims $ concatFL $ 
mapFL_FL canonize
> > > +           $ sort_coalesceFL $ concatFL $ mapFL_FL canonize $ oldchs 
+>+ chs
> > 
> > Do you think you could provide some examples of what a realistic
> > non-canonical representation of a patch would be (compared to the
> > canonical one) and also an explanation of why running sort_coalesceFL
> > on them can result in their decanonicalisation?
> 
> canonize is a very poor name for this function, which really just
> simplifies the patches.
> 
> > Also, is there any reason to still canonize the patches before we
> > sort_coalesce them?
> 
> I'm not sure, but I don't think it'll hurt.
> 
> > Is this snippet from issue525 an example of a non-canonical patch?
> > 
> > hunk ./ChangeLog 1
> > -2007-08-29  Alexey Shchepin  <alexey@process-one.net>
> > +2007-08-29  Mickael Remond  <mremond@process-one.net>
> > +
> > +       * doc/guide.tex: Documentation for XML based optimisation build
> > +       time option (EJAB-298)
> > hunk ./ChangeLog 6
> > +2007-08-29  Alexey Shchepin  <alexey@process-one.net>
> > +       
> 
> No, canonize only operates on single primitive patches, and neither of
> these two primitive patches can be simplified.  This pair of patches can't
> be simplified into the below unless you add some additional information
> about the initial line 2.
> 
> > With its canonical representation being something like the below?
> > 
> > hunk ./ChangeLog 1
> > +2007-08-29  Mickael Remond  <mremond@process-one.net>
> > +
> > +       * doc/guide.tex: Documentation for XML based optimisation build
> > +       time option (EJAB-298)
> > +       
> >
> > If so, that makes me a bit confused about the relationship between
> > coalescing and canonizing...
> _______________________________________________
> darcs-users mailing list
> darcs-users@darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-users
>

In msg7021 Eric Kow asked:
> Do you think you could provide some examples of what a realistic
> non-canonical representation of a patch would be (compared to the
> canonical one) and also an explanation of why running sort_coalesceFL
> on them can result in their decanonicalisation?

Ignoring the issue of different diff algorithms, a non-canonical patch
is a hunk which has common lines in its 'old' and 'new' arguments. The
canonize function splits such a hunk into smaller ones with no common
lines, using the supplied diff algorithm. OTOH, coalesce joins adjacent
or overlapping hunks; this may produce non-canonical hunks when then the
second hunk puts back (a part of) what the first hunk removes. The
simplest example is the sequence (of file states)

  a -> b -> b
            a

corresponding to hunks

  hunk 1 [a] [b]
  hunk 2 [] [a]

which (currently) coalesce to

  hunk 1 [a] [ab]

While this behavior could be fixed for the simple example above, we
cannot easily fix it in general: the second hunk might add many lines
interspersed with arbitrarily many lines from what the first hunk
removes. Finding the list of minimal hunks is exactly what the diff
algorithm is supposed to do.

This means we /first/ have to coalesce and /then/ canonize whenever we
concatenate lists of primitive patches.

Note that the hunks produced by canonizing a single hunk should already
be in coalesced form, since the diff algorithm takes care of that. Thus,
David's solution does unnecessary work: it first canonizes, then
coalesces, then canonizes again. The first part is not needed and can be
removed.

Revert the status to resolved. I did not touch the Status button when I
posted the message. Is this a feature of Roundup?

See patch1719.