Re: [Gnu-arch-users] [BUG] Fix merge commands to behave sanely for mixed versions

[Aaron Bentley]

David Allouche wrote:
> On Wed, Jun 09, 2004 at 09:45:12AM -0400, Aaron Bentley wrote:
>
> > > REPLAY should check for the error _before_ it starts modifying the tree.
> >
> > My rationale is that if the user does replay, they want the latest
> > revision.  If replay can't produce the lastest revision, it should
> > produce the latest revision that it can produce.  Users can then use
> > 'update' to get the true latest revision.
>
>
> The latest revision REPLAY can produce is generally useless to the user
> who has been trying to update to the latest revision.

Perhaps.  But you're really getting into DWIM behavour here.  Replay is 
pretty simple (aside from --skip-present), and this makes it harder to 
understand.

> And it's generally a bad thing to modify the tree in a way which is not
> always reversable when we are not sure to give the user what he wanted,
> because that can pollute uncommitted changes.

That implies that the user is trying to commit merges and other changes 
at the same time, which usually a bad idea.  I don't think we should go 
out of our way to help people do it wrong.

> It looks like some people only swear by "replay --skip-present" as a
> merge operator. That's a perfectly reasonable use case. And in that
> case, modifying the tree while the merge cannot be possible completed is
> not a desirable behaviour.

It depends which line of descent you're interested in.  If you're 
working with a tree related to tree base-0 was branched from, you may 
well want to apply these.  But if there are a series of continuations 
(e.g. updates to follow the main branch after the initial work was 
completed), you may not want to apply the absolute latest revision.

I think the problem is that --no-sanity doesn't help enough.  What we 
need is the ability to specify a range of revisions, and replay should 
error out if that range includes a continuation.


> Using --no-sanity, there will be no need for any additional network
> operation in the common case because continuation revisions can be
> indentified by examining the patchlog.

Theoretically true, though I don't think it's easy to read the patchlog 
from a changeset at present.  For the uncommon case, it requires 
downloading a patch in order to determine its type.  It will also break 
badly for import revisions.

I don't think the overhead of checking revision type is significant 
compared with the time it takes to download a typical changeset, and 
it's somthing I think can be optimized further with pipelining or 
parallel downloads.  It would also make the code simpler to do

stop_revision=scan_archive_forwards()

if (stop_revision != archive_latest)
  if (!no_sanity)
    //print error message
  else
    replay()


> There are currently three cases:
>
>    * the patch is missing, it must be fetched anyway before
>      application, so we have it handy to check wether it is a
>      continuation
>
>    * the patch is present, just check the tree patchlogs

Err.  If the patch is present, replay won't apply it.

>    * the patch is missing but won't be applied because of
>      --skip-present. We can only know the patch will not be applied
>      after retreiving the patchlog with cat-archive-log anyway

Typically, we check the revision type directly.  We don't cat-archive-log.

> My point is that the REPLAY CLI provides a merge tool, and the merge is
> only meaningful if all replayed patches belong to the same lineage.

Actually, no.  In some cases, you'd want to apply

patch-5 (hacking)
patch-6 (hacking)
not patch-7 (a commit --base continuation with no hacking)
patch-8 (more hacking)
patch-9...

Admittedly, that's not what I was arguing for, but I think it's a 
reasonable use case.

> Even if that is only a principled argument, I think it is strong enough
> to warrant the inclusion of a significant finger-guard.

The thing about merge commands is that you shouldn't apply them when you 
have uncommitted changes, so undo is your friend.

> For the (prospective, unlikely) case where the logic of the
> --frankenstein option would be needed, the desired behaviour can be
> obtained using an explicit patch list and some scripting. I think that's
> a good compromise between usability and functionality.

I'd like replay --list to work with stdin, mind you.  That way you could 
just do tla logs|tla replay --list -


> Nice to see that my line of thought is consistent with what tla already
> does :-) Let me update my proposal:
>
>     REPLAY $REVISION is meant to apply an archived changeset. It should
>     fail with an error message if the specified revision has no
>     associated changeset (which is distinct from a null changeset). That
>     is IMPORT revisions.
>                                                                                                      
>     Suggestion for the error message:
>                                                                                                      
>       replay does not make sense: revision has no changeset

This is reasonable.

>     Alternatively, one can consider that IMPORT revisions are
>     associated to a simple changeset which just adds the patchlog. It
>     would make sense since replay-reverse could then be used to remove
>     that patchlog. In that case, JOIN-BRANCH becomes essentially
>     obsolete.  That is a bigger change, but I think it improves the
>     consistence of the command set.

Actually, I think it's a disimprovement.  When tla replay REVISION 
succeeds, it produces the same result as tla apply-delta ANCESTOR 
REVISION.  When tla replay --reverse succeeds, it produces the same 
result as tla apply-delta REVISION ANCESTOR.  Your proposal would 
radically change the meaning.

> Storing imports as full reversible changesets is not a good idea. That
> would make IMPORT a slight variant of COMMIT which can work when there
> is not history-previous revision. In particular, descendant imports (not
> initial imports, that is) could not be distinguised from COMMIT
> revisions.

It would be possible to identify imports, but yeah, I tend to agree that 
it's not worth it.

> > > In that case, JOIN-BRANCH becomes essentially obsolete.
> >
> > It stops being obsolete again when commit --base is implemented.
>
>
> I do not see what useful functionality would JOIN-BRANCH provide that
> REPLAY cannot provide if imports are associated to trivial patchlog
> changesets.

Commit --base will produce continuations, not imports.    Those 
continuations will have associated changesets that do more than add a 
patchlog.

If commit --base base--0--patch-5 produces foo--0--patch-3, replay 
foo--0--patch-3 is equivalent to apply-delta base--0--patch-5 
foo--0--patch-3.

So what's this with imports?  tla prevents imports anywhere but base-0, 
but Tom says archives with imports elsewhere are still well-formed. 
larch could produce them, so some probably exist.

Aaron

--
Aaron Bentley
Director of Technology
Panometrics, Inc.