darcs

Issue 525 amend-record => darcs patches show duplicate additions

Title amend-record => darcs patches show duplicate additions
Priority bug Status resolved
Milestone Resolved in
Superseder Nosy List alexey, darcs-devel, dmitry.kurochkin, kowey, markstos, thorkilnaur, tommy
Assigned To
Topics

Created on 2007-08-29.21:18:16 by alexey, last changed 2009-08-27.14:14:35 by admin.

Messages
msg2108 (view) Author: alexey Date: 2007-08-29.21:18:15
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
msg2109 (view) Author: kowey Date: 2007-08-30.03:54:15
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!
msg2110 (view) Author: tommy Date: 2007-08-30.09:46:10
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.
msg2111 (view) Author: daveroundy Date: 2007-08-30.12:41:50
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
msg2112 (view) Author: alexey Date: 2007-08-30.18:07:44
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
msg2113 (view) Author: daveroundy Date: 2007-08-30.18:15:10
...
> 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
msg2114 (view) Author: alexey Date: 2007-08-30.19:22:09
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
msg3153 (view) Author: markstos Date: 2008-02-06.16:18:55
I'll do some testing with the provided examples.
msg3170 (view) Author: markstos Date: 2008-02-07.04:12:16
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.
msg3241 (view) Author: alexey Date: 2008-02-08.17:56:26
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
msg4816 (view) Author: kowey Date: 2008-05-21.12:50:49
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?
msg4886 (view) Author: kowey Date: 2008-05-29.19:52:09
We now have bugs/issue525.sh which reproduces an amend-record version of this bug.
msg7021 (view) Author: thorkilnaur Date: 2009-01-08.20:56:07
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
>
History
Date User Action Args
2007-08-29 21:18:17alexeycreate
2007-08-30 03:54:17koweysetstatus: unread -> unknown
nosy: kowey, beschmi, droundy, alexey, tommy
messages: + msg2109
2007-08-30 09:46:11tommysetnosy: kowey, beschmi, droundy, tommy, alexey
messages: + msg2110
2007-08-30 12:41:52daveroundysetnosy: + daveroundy
messages: + msg2111
2007-08-30 18:07:45alexeysetfiles: + smime.p7s
messages: + msg2112
2007-08-30 18:15:11daveroundysetmessages: + msg2113
2007-08-30 19:23:08alexeysetfiles: + smime.p7s
messages: + msg2114
2007-09-21 17:07:43droundyset
2008-02-06 16:18:56markstossetpriority: wishlist -> bug
nosy: + markstos
title: darcs generates strange patches -> darcs patches show duplicate additions
messages: + msg3153
assignedto: markstos
2008-02-07 04:12:17markstossetstatus: unknown -> waiting-for
nosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
messages: + msg3170
2008-02-08 17:56:27alexeysetfiles: + smime.p7s
nosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
messages: + msg3241
2008-02-08 18:47:04markstossetfiles: - smime.p7s
nosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
2008-02-08 18:47:09markstossetfiles: - smime.p7s
nosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
2008-02-08 18:47:15markstossetfiles: - smime.p7s
nosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
2008-02-08 18:47:22markstossetfiles: - smime.p7s
nosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
2008-02-08 18:47:54markstossetnosy: droundy, tommy, beschmi, kowey, markstos, alexey, daveroundy
assignedto: markstos -> alexey
2008-05-21 12:50:51koweysetnosy: + dagit
title: darcs patches show duplicate additions -> amend-record => darcs patches show duplicate additions
messages: + msg4816
assignedto: alexey ->
2008-05-29 19:52:11koweysetstatus: waiting-for -> unknown
nosy: droundy, tommy, beschmi, kowey, markstos, dagit, alexey, daveroundy
messages: + msg4886
2008-09-28 20:49:38adminsetnosy: + simon, thorkilnaur, - daveroundy
2009-01-08 20:56:11thorkilnaursetstatus: unknown -> resolved
nosy: + dmitry.kurochkin
messages: + msg7021
2009-08-06 17:37:47adminsetnosy: + jast, Serware, darcs-devel, zooko, mornfall, - droundy, alexey
2009-08-06 20:34:31adminsetnosy: - beschmi
2009-08-10 22:07:49adminsetnosy: + alexey, - darcs-devel, zooko, jast, Serware, mornfall
2009-08-11 00:02:13adminsetnosy: - dagit
2009-08-25 17:26:14adminsetnosy: + darcs-devel, - simon
2009-08-27 14:14:35adminsetnosy: tommy, kowey, markstos, darcs-devel, thorkilnaur, alexey, dmitry.kurochkin