Not for screened yet but I'd like some comments on the approach.

This is for maintaining backwards compatibility when I change
the rebase format again. As we discussed the new format is called
"darcs-2.16". Although I wonder if we might not call the new darcs
release 3.0, given V3 patches?

The code for dealing with different formats is spread around a
bit and would benefit from a bit more centralising, but I
couldn't summon up the energy to do that. But I am trying
to put the patch code for old rebase formats in a standard
place, namely Darcs.Patch.Rebase.Legacy.XXX

3 patches for repository darcs-unstable@darcs.net:screened:

patch e53634631290053be32cd48eee2684bc11ec31b0
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Mon Mar  2 22:21:22 GMT 2020
  * introduce new rebase format
  
  The new rebase format is named rebase-2.16 on the assumption that
  it will be the one used in Darcs 2.16.
  
  The "new-style" format is now legacy, but still supported for
  upgrades so that repositories created during the 2.16 development
  cycle can be rescued.
  
  As of this patch, darcs-2.16 rebases are exactly the same format
  as new-style, but this will change without preserving backwards
  compatibility.

patch 861ec4861fd46aad1f2d0959c0ceebcc5aab687d
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Mon Mar  2 22:45:12 GMT 2020
  * introduce shim for new-style rebase
  
  This is just a newtype wrapper for now but will evolve
  to contain more of the existing code as that gets refactored.
  

patch 744374c51edcbbc11779c056dca40a11d8f396b0
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Tue Mar  3 06:03:56 GMT 2020
  * move D.P.Named.Wrapped into D.P.Rebase.Legacy namespace

Attachments

• introduce-new-rebase-format.dpatch
• patch-preview.txt
• unnamed

This is the (probably) final version of this patch that I'd
like to screen soon.

The point of this set of patches is to setup the groundwork
for making incompatible changes to the rebase format whilst
making sure that "new-style" continues to be supported.

I apologise for the churn in the final patch, I first propagated
a copy and paste error I made several months ago, then
corrected it in both places. It seemed trivial enough
that rebasing to sort it out wasn't worthwhile, as I
have other pending changes that sit on top of these.

3 patches for repository darcs-unstable@darcs.net:screened:

patch 5bc9ce15457cf8e4c7a687684334c00683905aee
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:08:27 GMT 2020
  * introduce new rebase format
  
  The new rebase format is named rebase-2.16 on the assumption that
  it will be the one used in Darcs 2.16.
  
  The "new-style" format is now legacy, but still supported for
  upgrades so that repositories created during the 2.16 development
  cycle can be rescued.
  
  As of this patch, darcs-2.16 rebases are exactly the same format
  as new-style, but this will change without preserving backwards
  compatibility.

patch d21bec0e20ac76834ddebd17926431808c4a5f28
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:09:00 GMT 2020
  * introduce shim for new-style rebase
  
  This is just a newtype wrapper for now but will evolve
  to contain more of the existing code as that gets refactored.
  

patch ec5a8168c61acc7c35b289e737e951d2d0218307
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:09:29 GMT 2020
  * tests: remove some useless code resulting from copy+paste

Attachments

• introduce-new-rebase-format.dpatch
• patch-preview.txt
• unnamed

Adding one more patch that logically belongs in this set of
changes.

BTW I have followups ready that make the actual transition
of rebase to use prim patches, those will go in a separate
bundle.

4 patches for repository darcs-unstable@darcs.net:screened:

patch 5bc9ce15457cf8e4c7a687684334c00683905aee
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:08:27 GMT 2020
  * introduce new rebase format
  
  The new rebase format is named rebase-2.16 on the assumption that
  it will be the one used in Darcs 2.16.
  
  The "new-style" format is now legacy, but still supported for
  upgrades so that repositories created during the 2.16 development
  cycle can be rescued.
  
  As of this patch, darcs-2.16 rebases are exactly the same format
  as new-style, but this will change without preserving backwards
  compatibility.

patch d21bec0e20ac76834ddebd17926431808c4a5f28
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:09:00 GMT 2020
  * introduce shim for new-style rebase
  
  This is just a newtype wrapper for now but will evolve
  to contain more of the existing code as that gets refactored.
  

patch ec5a8168c61acc7c35b289e737e951d2d0218307
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:09:29 GMT 2020
  * tests: remove some useless code resulting from copy+paste

patch db7971d45509835f773bfb54a0ff6b9e2037c7a0
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Mar  5 12:09:00 GMT 2020
  * move D.P.Named.Wrapped into D.P.Rebase.Legacy namespace

Attachments

• introduce-new-rebase-format.dpatch
• patch-preview.txt
• unnamed

> Not for screened yet but I'd like some comments on the approach.
> 
> This is for maintaining backwards compatibility when I change
> the rebase format again. As we discussed the new format is called
> "darcs-2.16". Although I wonder if we might not call the new darcs
> release 3.0, given V3 patches?

I am still of the opinion that as long as things aren't settled and we
release 2.16, we should use darcs-2.15.3/4/5 etc. When we make the next
release we can decide whether to remove the intermediate formats,
including "new-style". I guess I am the only one who uses the current
head for daily work.

> The code for dealing with different formats is spread around a
> bit and would benefit from a bit more centralising, but I
> couldn't summon up the energy to do that. But I am trying
> to put the patch code for old rebase formats in a standard
> place, namely Darcs.Patch.Rebase.Legacy.XXX

Fine with me.

> 
> 3 patches for repository darcs-unstable@darcs.net:screened:
> 
> patch e53634631290053be32cd48eee2684bc11ec31b0
> Author: Ganesh Sittampalam <ganesh@earth.li>
> Date:   Mon Mar  2 22:21:22 GMT 2020
>   * introduce new rebase format
>   
>   The new rebase format is named rebase-2.16 on the assumption that
>   it will be the one used in Darcs 2.16.
>   
>   The "new-style" format is now legacy, but still supported for
>   upgrades so that repositories created during the 2.16 development
>   cycle can be rescued.
>   
>   As of this patch, darcs-2.16 rebases are exactly the same format
>   as new-style, but this will change without preserving backwards
>   compatibility.

The last sentence is unclear to me. Also see my initial remarks.

> patch 861ec4861fd46aad1f2d0959c0ceebcc5aab687d
> Author: Ganesh Sittampalam <ganesh@earth.li>
> Date:   Mon Mar  2 22:45:12 GMT 2020
>   * introduce shim for new-style rebase
>   
>   This is just a newtype wrapper for now but will evolve
>   to contain more of the existing code as that gets refactored.
>   
> 
> patch 744374c51edcbbc11779c056dca40a11d8f396b0
> Author: Ganesh Sittampalam <ganesh@earth.li>
> Date:   Tue Mar  3 06:03:56 GMT 2020
>   * move D.P.Named.Wrapped into D.P.Rebase.Legacy namespace

Skinnging the diffs everything looks plausible enough to me.

Attachments

• pEpkey.asc

> Skinnging the diffs everything looks plausible enough to me.

Ahem. "Skimming".

Attachments

• pEpkey.asc

On 06/03/2020 21:51, Ben Franksen wrote:

> I am still of the opinion that as long as things aren't settled and we
> release 2.16, we should use darcs-2.15.3/4/5 etc. When we make the next
> release we can decide whether to remove the intermediate formats,
> including "new-style". I guess I am the only one who uses the current
> head for daily work.

Adding a new format is quite a bit of boilerplate, in addition to
testing+legacy conversion code, so I am not too keen on proliferating
them. Though I do appreciate that you do actually test current darcs in
daily usage and breaking a format without giving you an upgrade path
isn't good. Not quite sure how to balance those two issues.

>>   As of this patch, darcs-2.16 rebases are exactly the same format
>>   as new-style, but this will change without preserving backwards
>>   compatibility.
> 
> The last sentence is unclear to me. Also see my initial remarks.

The point I was trying to get across is that whilst this patch fixes
"new-style" in stone and promises backwards compatibility for it,
"rebase-2.16" would be subject to change at least for a while. But the
final approach will depend on the discussion above.

Ganesh

>> I am still of the opinion that as long as things aren't settled and we
>> release 2.16, we should use darcs-2.15.3/4/5 etc. When we make the next
>> release we can decide whether to remove the intermediate formats,
>> including "new-style". I guess I am the only one who uses the current
>> head for daily work.
> 
> Adding a new format is quite a bit of boilerplate, in addition to
> testing+legacy conversion code, so I am not too keen on proliferating
> them. Though I do appreciate that you do actually test current darcs in
> daily usage and breaking a format without giving you an upgrade path
> isn't good. Not quite sure how to balance those two issues.

I think the right thing to do here, at least for the intermediate
formats (i.e. between official releases), is to provide automatic
upgrade. That is, if it finds an outdated rebase state, the user is
prompted to confirm and then it gets upgraded to the latest version.
This would be similar to the internal version for the index or the patch
index, where we also do automatic upgrades. (Though I concede that the
situation with the indexes is somewhat simpler since we can re-generate
index information from scratch if it is outdated, which is not true for
an existing rebase state.)

If we do it like that, we can use a simple number for the (intermediate)
rebase versions, independent of the darcs release. This number should be
embedded inside the rebase patch to make it independent from _darcs/format.

> The point I was trying to get across is that whilst this patch fixes
> "new-style" in stone and promises backwards compatibility for it,
> "rebase-2.16" would be subject to change at least for a while. But the
> final approach will depend on the discussion above.

Okay, understood. But I really need an upgrade path. There are lots of
errors I find (and usually fix, sooner or later) in my daily use of
darcs. This is invaluable to keep bugs from creeping in. And I usually
do have a number of open rebase states and loosing them would be bad.

Attachments

• pEpkey.asc

On 07/03/2020 10:29, Ben Franksen wrote:

> I think the right thing to do here, at least for the intermediate
> formats (i.e. between official releases), is to provide automatic
> upgrade. That is, if it finds an outdated rebase state, the user is
> prompted to confirm and then it gets upgraded to the latest version.
> This would be similar to the internal version for the index or the patch
> index, where we also do automatic upgrades. (Though I concede that the
> situation with the indexes is somewhat simpler since we can re-generate
> index information from scratch if it is outdated, which is not true for
> an existing rebase state.)
> 
> If we do it like that, we can use a simple number for the (intermediate)
> rebase versions, independent of the darcs release. This number should be
> embedded inside the rebase patch to make it independent from _darcs/format.

> [...] But I really need an upgrade path. There are lots of
> errors I find (and usually fix, sooner or later) in my daily use of
> darcs. This is invaluable to keep bugs from creeping in. And I usually
> do have a number of open rebase states and loosing them would be bad.

Using the version numbers inside the rebase patch for upgrading is a
good point I hadn't thought of. It actually already has a version number
there. If we silently update the format in ReadPatch, then it'll be a
lot easier.

I could actually switch back to the "new-style" name given the idea of
using the rebase version number instead. On the other hand I'd somewhat
like to drop that particular name, so maybe switching the format
specifier is a good idea anyway. Or maybe the new format specifier could
be some kind of alias rather than being a full blown separate one.

But regardless, supporting old formats does incur some overhead: both in
dragging around old code and in testing that the old format works.

My current submission goes through three phases:

- introduce rebase-2.16 format, that otherwise looks just like new-style
- switch to RebaseChange for storing suspendeds
- switch to prims instead of repo patches
   [technically this won't actually change the format because
    unconflicted repo patches are currently stored just like the
    underlying prim, and even before this patch we don't have any
    conflicts in rebase, but I'd rather avoid relying on this.]

I am keen on keeping the patches broken up because it really helps to
keep the changes logically separate, which they are. But I am also
unkeen on supporting those intermediate states :-)

In practice if we just push them to screened and then reviewed together,
I don't think you will actually encounter those states. But it is a bit
unprincipled.

How about if, during the intermediate states, darcs just printed out a
warning that when using rebase that the current state is temporary? So I
would introduce the message in the first patch, and remove it once I'd
finished making changes.

Removing it would also be a good point to add backwards compatibility
tests for the new format. Any future changes would then need a new
upgrade path.

> Using the version numbers inside the rebase patch for upgrading is a
> good point I hadn't thought of. It actually already has a version number
> there. If we silently update the format in ReadPatch, then it'll be a
> lot easier.

Good!

> I could actually switch back to the "new-style" name given the idea of
> using the rebase version number instead. On the other hand I'd somewhat
> like to drop that particular name,

I don't like the name too much either. But we should postpone this move
for two reasons: One is that we aren't sure what name the next release
will have. More importantly, I think this should /not/ be mixed with the
changes in the rebase format you are going to make. The move from
old-style to new-style rebase was of quite a different quality: it did
not concern the format of the rebase patch itself, but rather its
location and the way it is handled by the Patch and Repository code.

BTW, if this is just a name change, then the upgrade path is very
simple: just rename the flag in the _darcs/format file. I could even do
this manually or with a small script.

> so maybe switching the format
> specifier is a good idea anyway. Or maybe the new format specifier could
> be some kind of alias rather than being a full blown separate one.

I would rather this be a simple rename, as that is easier to handle.

> But regardless, supporting old formats does incur some overhead: both in
> dragging around old code and in testing that the old format works.
> 
> My current submission goes through three phases:
> 
> - introduce rebase-2.16 format, that otherwise looks just like new-style

See above.

> - switch to RebaseChange for storing suspendeds
> - switch to prims instead of repo patches
>    [technically this won't actually change the format because
>     unconflicted repo patches are currently stored just like the
>     underlying prim, and even before this patch we don't have any
>     conflicts in rebase, but I'd rather avoid relying on this.]
> 
> I am keen on keeping the patches broken up because it really helps to
> keep the changes logically separate, which they are. But I am also
> unkeen on supporting those intermediate states :-)
> 
> In practice if we just push them to screened and then reviewed together,
> I don't think you will actually encounter those states. But it is a bit
> unprincipled.

I don't see any problem with this approach. There is no rule that says
all intermediate states of our repo have to be compatible (or even
buildable, though of course we try to achieve that).

> How about if, during the intermediate states, darcs just printed out a
> warning that when using rebase that the current state is temporary? So I
> would introduce the message in the first patch, and remove it once I'd
> finished making changes.

I don't think this is necessary. It is also not very effective. If you
really want to be sure, you should disable the rebase state so that no
command touches it. But I think this is not justified.

> Removing it would also be a good point to add backwards compatibility
> tests for the new format. Any future changes would then need a new
> upgrade path.

I think these tests should be part of the bundle right from the beginning.

Attachments

• pEpkey.asc

On 07/03/2020 21:29, Ben Franksen wrote:

>> I could actually switch back to the "new-style" name given the idea of
>> using the rebase version number instead. On the other hand I'd somewhat
>> like to drop that particular name,
> 
> I don't like the name too much either. But we should postpone this move
> for two reasons: One is that we aren't sure what name the next release
> will have. More importantly, I think this should /not/ be mixed with the
> changes in the rebase format you are going to make. The move from
> old-style to new-style rebase was of quite a different quality: it did
> not concern the format of the rebase patch itself, but rather its
> location and the way it is handled by the Patch and Repository code.

OK.

> I don't see any problem with this approach. There is no rule that says
> all intermediate states of our repo have to be compatible (or even
> buildable, though of course we try to achieve that).

OK. You'll just need to be careful not to use those intermediate states.

> I don't think this is necessary. It is also not very effective. If you
> really want to be sure, you should disable the rebase state so that no
> command touches it. But I think this is not justified.

Well, that's not possible either as it breaks all the tests (and proving
they keep working in the intermediate states is important too.) But
never mind.

>> Removing it would also be a good point to add backwards compatibility
>> tests for the new format. Any future changes would then need a new
>> upgrade path.
> 
> I think these tests should be part of the bundle right from the beginning.

I can add them as part of the bundle, but obviously they have to be
logically after finalising the new format.

Given this discussion I'll probably reject this bundle as the key bits
with the new format don't make any sense any more, and send in an
updated version of the other bundle so it can be accepted and pushed
together.