[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [RFC] Issue #3322 - In tree conflict info, report left/right URL@REV of the incoming change

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 17 Nov 2008 18:04:26 +0000

On Mon, 2008-11-17 at 15:02 +0000, Julian Foad wrote:
> Tree conflicts usability: we should store the "left"/"old" and the
> "right"/"theirs" URL_at_REV of the incoming change, and show it in the tree
> conflict details in "svn info".
> <http://subversion.tigris.org/issues/show_bug.cgi?id=3322>
> I recently filed this issue at a low priority, but I'm wondering if in
> fact this should be almost as high priority as the other stuff we're
> currently considering.
> In a strict work environment, the user can discover or reconstruct these
> values, but that can involve having to remember what the last merge was,
> and/or finding the revision range using "svn mergeinfo" and/or "svn
> diff" on a target that was merged. In other words, it's tricky, and the
> last thing we want is to make users have to do tricky detective work
> when a conflict is already an event that is often perceived as a
> nuisance.
> I hate to add a non-bug-fix item to the mix but I'm now think it's
> effectively a usability bug fix.
> Considered opinions please.

This was prompted today by a discussion Stefan and I had in IRC:

<julianf> (2) must apply already to deletes and adds. Maybe the correct
recipe is something like: in parent, write "child is in an incomplete
state"; then modify the child; then write in the parent "child state is
X and is complete".
<stsp> i.e. what do they do in addition to clearing the victim's
conflicted state
<julianf> OK, resolve/revert...
<julianf> Revert, by definition, undoes all local scheduled changes. For
a merge that's easy to define: just revert to the base state.
<stsp> yes
<julianf> Revert after an update/switch is the same, but leads to
another question: what did the up/sw do to the base state when it raise
the TC?
<stsp> do we have an answer for that in all cases? is that what sbutler
was doing when he worked on "skipping"?
<julianf> Yes it's related to skipping. Yes I thought about what the
base should be. No I'm not sure we have a single answer or that
"skipping" was the answer.
<julianf> My old doc was notes/tree-conflicts/resolution.txt
<stsp> so the base we end up with is currently undefined, or what is it
<julianf> Undefined, to be blunt.
<stsp> that's baaaaaaaaaaaaad :)
<julianf> Sure.
<julianf> At one point I said "let's do the easy thing which is skip the
<julianf> I'd better go and check whether that's what we've currently
<stsp> if we skip the base update during update, we had better tell the
<julianf> It's "easy" in that it's easy to implement in "up/sw" code
(except that the up/sw reports "Updated to rXXX" which is misleading if
you don't know that tree conflict victims have not been updated.
<stsp> otherwise they can't commit even though they think they're
<julianf> Yes.
<julianf> However... Though skipping the base update is easy to
implement, it's really not what we want to be doing. It makes resolution
<stsp> is the problem really that we have no way currently of storing
more than one base per file?
<julianf> No,
<julianf> I don't think that's the problem. It's OK (IMO) to require
that the user go and contact the repository do do diffs or merges or to
fetch the old text if required.
<stsp> right, it's just that 'revert' will not undo the update, i.e. the
user cannot expect revert to go back to what they had before they ran
<julianf> The problem is that it's complex to update the base, because
that means we've got to change the scheduling of the working version to
match complement we did to the base.
<julianf> re. revert will not undo... yes.
<stsp> I don't get the "to match complement we did to the base" part...
<julianf> If the change is trying to delete the file but it's locally
modified, then if we update the base (i.e. remove the base), we have to
change the schedule from "normal" to "add" in order to not break the WC
and to keep the working file known to Subversion.
<stsp> and 'svn diff' will stop working as expected
<julianf> Doesn't sound too complex, but (a) there are several cases,
(b) we haven't thoroughly understood that's what we want and gone
through the consequences.
<julianf> No, I don't agree that "diff" will stop working as expected.
"diff" always shows working against base by default.
<julianf> I see what you mean thought,
<julianf> yes.
<julianf> OK, "expected" = "I want to see what I had changed"
<stsp> if they file is schedule add, won't svn diff consider it
completely new and print the whole file?
<julianf> yes
<stsp> right, I think that's a problem (unless we make sure people don't
expect to be able to see their local mods)
<stsp> however, making it hard to create a diff of local mods (e.g. for
quick temporary backup) is a bad idea
<stsp> I like what git does in this case
<julianf> So, what the user wants is for the working file to still be
(scheduled as) "my" change, i.e. the base is NOT updated yet. That make
some sense?
<stsp> it stores all bases in the index, and you can look at each one,
and diff against each, and check each out to a seperate file if you want
<stsp> I really think the problem is that we can only store one base
when we should be storing both and make them accessible
<julianf> yes, but as long as the "bases" are accessible somewhere for
diffs etc, the fact that they're stored locally is an optimisation. We
still can access the old/theirs/mine in Subversion by using repos
<julianf> (When someone's doing up/sw/merge, they're on line.)
<stsp> oh yes, but you have to know what the old base was then
<julianf> Yes. YES!
<stsp> so either the user tells us, or we store that information
<julianf> We should be storing the old base REV (and URL, in case of
<stsp> i.e. store at which rev a tc victim was at before the base was
<stsp> yes
<stsp> but if we do that, why not store the entire base? :)
<julianf> And for merge, store the "merge-left source" URL_at_REV and the
"merge-right source" URL_at_REV.
<stsp> in the tree-conflict-data in the entries file
<julianf> Only reasons not to store the entire base are practical
matters of the effort of design (API and UI how to access it) and
<stsp> then the user can look at the tc information, and will be told
"look, the file's base was updated, you can still get at it via URL_at_REV"
<julianf> It would be really good to store them, yes. Just harder than
just storing the URL_at_REV. And I think we'd definitely want to store the
URL_at_REV as well anyway.
<stsp> "... still get the old base ..."
<julianf> Yes.
<stsp> let's store URLS for now, and store all text bases once gstein
has support for that ready
<julianf> So, I nearly filed an enhancement for "store the URL_at_REV in
each conflict description" the other day.
<julianf> Checking...
<julianf> #3322.
<stsp> it's cumbersome to use URLs for this (note that with git, you can
use commands like "git cat-file <sha1 of base>" or "git diff
my/tree/conflicted/file <sha1 of base>"
<stsp> )
<stsp> and that's really neat
<stsp> julianf: yes, #3322 is spot on
<julianf> Sure. But it's a start. Ages ago I proposed "svn diff foo_at_OLD"
kind of syntax but got some negative comments. Maybe the commenters
didn't see the value.
<stsp> that would be handy
<julianf> (Revision keywords "OLD", "MINE", "THEIRS" would only be
usable while there's a conflict. They would work for tree, text and prop
<stsp> right, next lecture -- embedded systems, more fun :)
<julianf> It would be Great, I think.
<julianf> OK, you'd better go. Later...
<Bert> julianf: I still like that old plan of path_at_THEIRS :)
<Bert> (Maybe OLD / BASE needs some extra explanation)
<Bert> julianf: Your old @mine idea is in issue #3252
<julianf> Bert: thanks.
<Bert> http://svn.haxx.se/dev/archive-2008-07/0643.shtml has that patch
you sent
<Bert> I was first searching with the wrong keywords.. It was a
suggestion for handling property conflicts at the time

I checked whether we're updating the base, and we are not. This comment
in libsvn_wc/update_editor.c seems to be accurate:

 * [*3] For now, we skip the update, and require the user to:
 * - Modify the WC to be compatible with the incoming change;
 * - Mark the conflict as resolved;
 * - Repeat the update.
 * Ideally, it would be possible to resolve any conflict without
 * repeating the update. To achieve this, we would have to store the
 * necessary data at conflict detection time, and delay the update of
 * the Base until the time of resolving.

(While not great for usability, at least that is simple.)

- Julian

To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-17 19:04:54 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.