darcs

Issue 1096 patch format should have a version id

Title patch format should have a version id
Priority wishlist Status given-up
Milestone 3.0.0 Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, kowey, mndrix, thorkilnaur
Assigned To
Topics

Created on 2008-09-23.18:19:48 by dagit, last changed 2017-07-31.00:39:19 by gh.

Files
File name Uploaded Type Edit Remove
unnamed dagit, 2008-09-23.22:15:52 text/html
unnamed dagit, 2008-09-23.22:22:04 text/html
Messages
msg6100 (view) Author: dagit Date: 2008-09-23.18:19:45
Patches now generated by will have ignorable ids embedded in the patch descriptions.

They look like this:
The following patch updated the status of issue27 to be resolved:

* resolve issue27: add junk to patch identifiers. 
Ignore-this: b91ab6f6e05e0fda25488fa51653b741


It seems that we're going to be sorry about this change.  From now on darcs
patches will start placing this string "Ignore-this: ..." in every patch
description.  I thought the plan was to improve patch infos but this seems like
we're now abusing patch descriptions.  It may be the case that internally they
are stored in the same data structure, but I don't think that's a good argument
for considering them to be conceptually the same.

This makes me think about the bigger issue of changing patch formats.  If we
want to add new patch types how will we do this in a backwards compatible way? 
If we want to change hunk format or add more features to patch formats how will
we do this in a backwards compatible way?  Suppose we want to add user definable
attributes to patches, how will we do this in a backwards compatible way?

We have infrastructure for describing the repository format and features, but
patch transfer is not encompassed in this scheme.

I propose that we change the format of patchinfos to contain version information
now and then at a later date begin using that info.  The later date being some
point when we feel there are enough versions of darcs in the wild that it's
relatively safe to start using the functionality.  That is, I think we should
start now making darcs accept patchinfos but don't actually start using it until
later when we can be confident that it won't break older darcs.

The idea is that we give the older versions of darcs a chance to complain and
exit.  So this isn't true backwards compatibility, but it is a way to fail
gracefully on won't be understood.  I don't think true backwards compatibility
is attainable, but I'll entertain ideas for it.  Preferably simple ideas that
don't confuse users or make darcs overly complex internally.

I'm marking this as urgent because I believe the sooner we get the code in
place, the sooner we can flip the switch.
msg6104 (view) Author: simon Date: 2008-09-23.20:37:44
This seems like a good idea. 

A simpler change, just on the display level: I hadn't realised (until kowey
explained) that the new, non-useful Ignore-this output is displayed in all darcs
versions, if they happen to pull a patch with this info. It seems that we
display all a patch's info, including new fields which might not be appropriate
for display. I think a given darcs version should predictably display only the
patch info fields it knows about. I spent some time looking for this in the code
today but haven't found it.
msg6110 (view) Author: droundy Date: 2008-09-23.21:54:12
On Tue, Sep 23, 2008 at 2:19 PM, Jason Dagit <bugs@darcs.net> wrote:
> I propose that we change the format of patchinfos to contain version information
> now and then at a later date begin using that info.  The later date being some
> point when we feel there are enough versions of darcs in the wild that it's
> relatively safe to start using the functionality.  That is, I think we should
> start now making darcs accept patchinfos but don't actually start using it until
> later when we can be confident that it won't break older darcs.
>
> The idea is that we give the older versions of darcs a chance to complain and
> exit.  So this isn't true backwards compatibility, but it is a way to fail
> gracefully on won't be understood.  I don't think true backwards compatibility
> is attainable, but I'll entertain ideas for it.  Preferably simple ideas that
> don't confuse users or make darcs overly complex internally.

How is this different from the format information we've already got in place?

It has the same effect of allowing older versions of darcs to exit
gracefully, and has the major advantage of already being implemented.

David
msg6111 (view) Author: droundy Date: 2008-09-23.21:56:29
Regarding the issue27 fix, newer darcs won't display the junk, and can easily be
make to allow extra junk in the patchinfo, if we ever want to add any.

Marking as wishlist, since this doesn't sound like a well-defined request.

David.
msg6115 (view) Author: dagit Date: 2008-09-23.22:15:52
On Tue, Sep 23, 2008 at 2:54 PM, David Roundy <bugs@darcs.net> wrote:

>
> David Roundy <droundy@darcs.net> added the comment:
>
> On Tue, Sep 23, 2008 at 2:19 PM, Jason Dagit <bugs@darcs.net> wrote:
> > I propose that we change the format of patchinfos to contain version
> information
> > now and then at a later date begin using that info.  The later date being
> some
> > point when we feel there are enough versions of darcs in the wild that
> it's
> > relatively safe to start using the functionality.  That is, I think we
> should
> > start now making darcs accept patchinfos but don't actually start using
> it until
> > later when we can be confident that it won't break older darcs.
> >
> > The idea is that we give the older versions of darcs a chance to complain
> and
> > exit.  So this isn't true backwards compatibility, but it is a way to
> fail
> > gracefully on won't be understood.  I don't think true backwards
> compatibility
> > is attainable, but I'll entertain ideas for it.  Preferably simple ideas
> that
> > don't confuse users or make darcs overly complex internally.
>
> How is this different from the format information we've already got in
> place?

How can the format information that we have  apply to patches that are
sent?  I think I commented on this in my bug post, if not it was certainly
in an earlier draft of the ideas.

It has the same effect of allowing older versions of darcs to exit
> gracefully, and has the major advantage of already being implemented.

Already implemented for repositories but not patches.  Or at least, that's
my understanding.

Jason
Attachments
msg6117 (view) Author: dagit Date: 2008-09-23.22:22:04
On Tue, Sep 23, 2008 at 2:56 PM, David Roundy <bugs@darcs.net> wrote:

>
> David Roundy <droundy@darcs.net> added the comment:
>
> Regarding the issue27 fix, newer darcs won't display the junk, and can
> easily be
> make to allow extra junk in the patchinfo, if we ever want to add any.

My email client will still display the junk as part of the patch description
when I look at patch bundles but it isn't part of the user-editable
description.  I'm hoping that we can stop displaying it as part of the
user-editable data and move it to a place that looks more official.

Also, just wondering, how does amend-record --edit-long-comment work with
this ignorable field?  Does newer darcs hide it here too?  And what if I use
older darcs, am I able to change it if I'm not careful?  I guess it's evil
to amend-record someone else patch anyway.

> Marking as wishlist, since this doesn't sound like a well-defined request.

It is too bad we can't have bug/wishlist as separate from
urgent/critical/etc.  Ideally I could have marked it as an urgent wish.

Jason
Attachments
msg6118 (view) Author: droundy Date: 2008-09-23.22:25:25
On Tue, Sep 23, 2008 at 03:21:56PM -0700, Jason Dagit wrote:
> On Tue, Sep 23, 2008 at 2:56 PM, David Roundy <bugs@darcs.net> wrote:
> > Regarding the issue27 fix, newer darcs won't display the junk, and
> > can easily be make to allow extra junk in the patchinfo, if we
> > ever want to add any.
> 
> My email client will still display the junk as part of the patch description
> when I look at patch bundles but it isn't part of the user-editable
> description.  I'm hoping that we can stop displaying it as part of the
> user-editable data and move it to a place that looks more official.

I don't care.

> Also, just wondering, how does amend-record --edit-long-comment work with
> this ignorable field?  Does newer darcs hide it here too?  And what if I use
> older darcs, am I able to change it if I'm not careful?  I guess it's evil
> to amend-record someone else patch anyway.

Yes, amend-record creates new junk.

With an older darcs, you could make the mistake of not changing it
when using amend, but at least the date is likely to change, so it
shouldn't hurt, and certainly isn't a regression.

> > Marking as wishlist, since this doesn't sound like a well-defined request.
> 
> It is too bad we can't have bug/wishlist as separate from
> urgent/critical/etc.  Ideally I could have marked it as an urgent wish.

I still don't see why this is anything like urgent.

David
msg6119 (view) Author: droundy Date: 2008-09-23.22:27:10
On Tue, Sep 23, 2008 at 03:23:29PM -0700, Jason Dagit wrote:
> On Tue, Sep 23, 2008 at 3:22 PM, David Roundy <droundy@darcs.net> wrote:
> 
> > On Tue, Sep 23, 2008 at 03:15:41PM -0700, Jason Dagit wrote:
> > > > How is this different from the format information we've already got in
> > > > place?
> > >
> > > How can the format information that we have  apply to patches that are
> > > sent?  I think I commented on this in my bug post, if not it was
> > certainly
> > > in an earlier draft of the ideas.
> >
> > You mean patch bundles? Apart from bundles, patches are always in
> > repositories.
> 
> Exactly.

That's a valid concern, and we really ought to have a format field for
patch bundles, but it definitely shouldn't be part of the patches
themselves, and definitely should use the same set of format
identifiers that repositories use.

And I don't see what this has to do with the issue27 change.

David
msg6120 (view) Author: kowey Date: 2008-09-23.22:28:30
A possible case study might be the planned upgrade to the new on-disk
chunk format for hunks (i.e. marking whole regions of adds and deletes
instead of each line separately).  Currently, the sensible appoarch
seems to be to rely on the repository format for this.  The relationship
between chunky repositories and non-chunky ones could be akin to that
between hashed and old fashioned repositories.  Sending patches would
rely on the old format.  So in this particular case, we can get away
relying solely on the repository format compatibility mechanism, of
course, the trick being that we only modify the on-disk patch
representation and not that of patches we send back and forth in
bundles.

What we need for this discussion, it seems, is a compelling use case
where we would want to upgrade the patch format without incurring a
repository format incompatibility.

As for how we go about this introducing this change, I am in favour of a
more cautious approach to upgrading formats.  If we were to introduce a
more flexible patch format (e.g. one that included a notion of
patch-format-versions), we should probably use the the repository format
mechanism, adding a key like "flexible-patches".  This way we guarantee
that older darcs won't try to interact with repositories that use
flexible patches.  It seems like this would lead to more predictible
results than just waiting for old darcs to die and faster in a sense,
since we don't /have/ to wait.

There are cases where we have gone the route of implementing the read
functionality first, and then turning on the write functionality when
old darcs has died out... but I think we've only used it for very
straightforward and easy-to-work-around changes like the _darcs/current
to _darcs/pristine rename

Thanks!
msg6122 (view) Author: kowey Date: 2008-09-23.22:37:54
> > It is too bad we can't have bug/wishlist as separate from
> > urgent/critical/etc.  Ideally I could have marked it as an urgent wish.
> 
> I still don't see why this is anything like urgent.

I agree with both of you :-)

Yes, all three of us seem to think that this is not an important matter.

Jason meant 'urgent' in the sense 'needs action soon' (for example, we
need to drink the milk in the fridge urgently because it's past its due
date...  it's not important that we do so, and if we don't, tough
luck... it's just that doing it soon is vastly preferable to doing it
later)

Even in that light, I think we are overestimating the urgency (in the
unimportant but milk-past-its-date sense) of this bug, because we are
thinking of it in terms of implementing the change and waiting for older
darcs to die out, and then flipping the switch.  Rather than having a
switch-flip day to wait for, we should just use a repository format
change to herald the hypothetical new patch format... no waiting
involved, and therefore less urgency. :-)
msg6123 (view) Author: dagit Date: 2008-09-23.22:54:51
Closing this as the feature appears to be controversial/undesirable.
msg6124 (view) Author: dagit Date: 2008-09-23.22:55:20
And I selected the wrong status...trying again.
msg10418 (view) Author: kowey Date: 2010-03-22.16:20:31
I'm deferring this for Darcs 3, but if we were to un-defer it, it would
need to pop into need-action with the action involved being a design
discussion of some sort.
History
Date User Action Args
2008-09-23 18:19:48dagitcreate
2008-09-23 18:22:10dagitsetstatus: unread -> needs-reproduction
2008-09-23 20:37:46simonsetmessages: + msg6104
2008-09-23 21:54:15droundysetnosy: + droundy
messages: + msg6110
2008-09-23 21:56:31droundysetpriority: urgent -> wishlist
nosy: droundy, kowey, dagit, simon
messages: + msg6111
2008-09-23 22:15:54dagitsetfiles: + unnamed
nosy: droundy, kowey, dagit, simon
messages: + msg6115
2008-09-23 22:22:06dagitsetfiles: + unnamed
nosy: droundy, kowey, dagit, simon
messages: + msg6117
2008-09-23 22:25:27droundysetnosy: droundy, kowey, dagit, simon
messages: + msg6118
2008-09-23 22:27:12droundysetnosy: droundy, kowey, dagit, simon
messages: + msg6119
2008-09-23 22:28:32koweysetnosy: droundy, kowey, dagit, simon
messages: + msg6120
2008-09-23 22:38:00koweysetnosy: droundy, kowey, dagit, simon
messages: + msg6122
2008-09-23 22:54:52dagitsetnosy: droundy, kowey, dagit, simon
messages: + msg6123
2008-09-23 22:55:22dagitsetstatus: needs-reproduction -> wont-fix
nosy: droundy, kowey, dagit, simon
messages: + msg6124
2009-08-06 18:00:32adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, mornfall, tommy, beschmi, thorkilnaur, - droundy
2009-08-06 21:12:50adminsetnosy: - beschmi
2009-08-10 21:49:00adminsetnosy: - tommy, markstos, darcs-devel, zooko, jast, Serware, mornfall
2009-08-10 23:46:20adminsetnosy: - dagit
2009-08-25 17:25:25adminsetnosy: + darcs-devel, - simon
2009-08-27 14:12:15adminsetnosy: kowey, darcs-devel, thorkilnaur, dmitry.kurochkin
2010-03-22 16:10:42koweylinkissue1786 superseder
2010-03-22 16:20:36koweysetstatus: wont-fix -> deferred
topic: + Target-3.0
messages: + msg10418
2010-06-15 21:10:48adminsettopic: - Target-3.0
2010-06-15 21:10:48adminsetmilestone: 3.0.0
2012-03-16 20:35:24mndrixsetnosy: + mndrix
2017-07-31 00:39:19ghsetstatus: deferred -> given-up