You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/07/14 12:22:54 UTC

[RFC] Property conflicts should save old/theirs/mine versions

Howdy folks.

I was talking to Mike a couple of weeks ago.

When we get a property conflict, we store a message in "<filename>.prej"
that says:

> Trying to change property 'p' from 'v1' to 'v2',
> but property has been locally changed from 'v1' to 'wcval'.

That's OK for a short plain-text value, but is not human-readable for a
very long value or a "binary" value, and not machine-parseable if the
value contains a single-quote character.

Mike and I agree it's totally obvious - "d'uh" - that we should store
the "old" and "theirs" and "mine" values in a more rigorous,
programmatically-accessible form, as we do for the file text in a file
text conflict. This would enable tools built on top of Subversion to
offer proper property conflict resolution.

I see two solutions (Mike, I only thought of (1) until after talking to
you last week, and then I realised you were probably thinking of (2)):

(1) Save the item's "old", "theirs" and "mine" properties in the admin
area if there is any property conflict on the item.

  * Save each of these three sets of properties as a property hash file,
just like the "base" properties file.

  * Provide APIs (and possibly a UI in "svn") to read these properties
in a way that is very similar to reading the base properties.

(2) For each property that conflicts, create "old", "theirs" and "mine"
files holding the bare value of that property in the user's WC
directory. (This is where we write the files for text conflicts.)

  * Provide a way to access the names of these files. (They can't be
quite as simple as "<filename>.old" as there can be multiple properties
conflicting on one item.)


With (1), we store all the properties if any of them conflict. If a
property that conflicts is closely related to another property that
doesn't, say "svnmerge-integrated" and "svn:mergeinfo", then both
properties' values will be available to help the user in the resolution.

Solution (1) requires a libsvn_wc API to retrieve the old/theirs/mine
properties, and a UI extension if we want "svn" users to benefit.
Solution (2) requires exposing the file names through svn_wc_entry_t,
and optionally exposing them to the user through "svn info", or perhaps
through text in the existing "<filename>.prej".

I have implemented the basics of (1): that is, storing the properties,
an API to retrieve them, and a suggestion for a UI. The "retrieve" API
looks something like this:

> Index: subversion/include/svn_wc.h
> ===================================================================
>  svn_error_t *
>  svn_wc_prop_list(apr_hash_t **props,
>                   const char *path,
>                   svn_wc_adm_access_t *adm_access,
>                   apr_pool_t *pool);
> 
> +/* A type to specify which of the versions held in the WC. Types 'pristine'
> + * and 'working' are always meaningful (though they may be in the
> + * "nonexistent" state), whereas types 'conflict_*' are only valid when the WC
> + * item is in a state of conflict. */
> +typedef enum svn_wc_rev_kind_t
> +{
> +  svn_wc_rev_kind_pristine,      /* the 'pristine' or 'base' version */
> +  svn_wc_rev_kind_working,       /* the version that can be locally edited */
> +  svn_wc_rev_kind_conflict_old,  /* the incoming change's 'old' or 'left' side */
> +  svn_wc_rev_kind_conflict_new,  /* the incoming change's 'new' or 'right' side */
> +  svn_wc_rev_kind_conflict_mine  /* what was 'working' before the conflict */
> +} svn_wc_rev_kind_t;
> +
> +/** Set @a *props to a hash table mapping <tt>char *</tt> names onto
> + * <tt>svn_string_t *</tt> values for all the regular properties of
> + * @a path.  Allocate the table, names, and values in @a pool.  If
> + * the node has no properties, or does not exist in the working copy,
> + * then an empty hash is returned.  @a adm_access is an access baton
> + * set that contains @a path.
> + * @a rev_kind specified whether to access the pristine or working or
> + * one of the conflict artifact versions of this WC item.
> + */
> +svn_error_t *
> +svn_wc_prop_list2(apr_hash_t **props,
> +                  const char *path,
> +                  svn_wc_rev_kind_t rev_kind,
> +                  svn_wc_adm_access_t *adm_access,
> +                  apr_pool_t *pool);
> +

I can then see a command-line UI something like

  svn propget --conflict-artifact=old|theirs|mine TARGET

The sort of ultimate solution I would like to see at some point in the future would be one where all svn commands could access these pseudo-versions just as easily as they can access BASE or WORKING:

  svn propget -r mine TARGET

... and have the file content conflict versions handled the same way, so:

  svn cat -r theirs TARGET
  svn diff -r old:theirs TARGET
  svn merge -r123:theirs TARGET wc

Maybe pie-in-the-sky unrealistic for this season, but I think a move towards it are good. This kind of capability is what we will the user will eventually need to properly resolve large and complex tree conflicts. It is also something that I expect WC-ng will make much easier to implement.

Comments?

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [RFC] Property conflicts should save old/theirs/mine versions

Posted by "C. Michael Pilato" <cm...@collab.net>.

Karl Fogel wrote:
> Stefan Sperling <st...@elego.de> writes:
>> So Julian's approach to storing property conflicts data in the auth
>> is consistent with what we have already done on the tree conflicts
>> branch.
>>
>> While maintaining consistency between the way text conflicts and
>> other types of conflicts are handled is certainly desirable, I don't
>> think it's viable. The way we currently deal with text conflicts is fine.
>> But it is also much simpler than how we want to deal with tree and property
>> conflicts. And the way our current trunk code is dealing with either
>> of those is, well, suboptimal.
> 
> *nod*  Okay, that all sounds reasonable to me.  (Assuming you mean
> "admin area" where you write "auth area").

Man, I hope so.  If "auth area" was really what was intended, I'll have to 
hit the local sporting goods shop to purchase enough red flags to wave.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: [RFC] Property conflicts should save old/theirs/mine versions

Posted by Stefan Sperling <st...@elego.de>.

On Thu, Jul 24, 2008 at 12:33:31PM -0400, Karl Fogel wrote:
> Stefan Sperling <st...@elego.de> writes:
> > So Julian's approach to storing property conflicts data in the auth
> > is consistent with what we have already done on the tree conflicts
> > branch.
> >
> > While maintaining consistency between the way text conflicts and
> > other types of conflicts are handled is certainly desirable, I don't
> > think it's viable. The way we currently deal with text conflicts is fine.
> > But it is also much simpler than how we want to deal with tree and property
> > conflicts. And the way our current trunk code is dealing with either
> > of those is, well, suboptimal.
> 
> *nod*  Okay, that all sounds reasonable to me.  (Assuming you mean
> "admin area" where you write "auth area").

Your assumption is correct, I mixed that up, sorry.

Don't worry, no one wants to store conflict meta data in the
authentication cache :)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [RFC] Property conflicts should save old/theirs/mine versions

Posted by Karl Fogel <kf...@red-bean.com>.

Stefan Sperling <st...@elego.de> writes:
> So Julian's approach to storing property conflicts data in the auth
> is consistent with what we have already done on the tree conflicts
> branch.
>
> While maintaining consistency between the way text conflicts and
> other types of conflicts are handled is certainly desirable, I don't
> think it's viable. The way we currently deal with text conflicts is fine.
> But it is also much simpler than how we want to deal with tree and property
> conflicts. And the way our current trunk code is dealing with either
> of those is, well, suboptimal.

*nod*  Okay, that all sounds reasonable to me.  (Assuming you mean
"admin area" where you write "auth area").

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [RFC] Property conflicts should save old/theirs/mine versions

Posted by Stefan Sperling <st...@elego.de>.

On Tue, Jul 22, 2008 at 06:37:38PM +0100, Julian Foad wrote:
> On Tue, 2008-07-22 at 17:56 +0100, Julian Foad wrote:
> > On Tue, 2008-07-22 at 14:52 +0100, Julian Foad wrote:
> > > Now, that's still quite a bit of work and may well be too much to bite
> > > off now (though large chunks of it are temptingly easy). But I think we
> > > need to be considering moves like this, and to something in this
> > > direction sooner or later.
> > 
> > For anyone interested, the attached patch contains my work in progress
> > on this.
> 
> Of course, I know how unfriendly is a patch without a log message.

Julian,

I read through the whole thread and I totally agree with the
general idea. Being smarter about property conflicts is a
very good thing.

I haven't looked at the patch in detail. But looking at the size
of it, I think you're already close to having enough code to justify
a new branch. If you decided to keep the scope of the branch limited
to the first steps needed towards your long-term goal, and named the
branch accordingly, it might even help preventing the diff from growing
too many new features prematurely. Given the size of what you're aiming
for, I guess anything that helps drawing a line somewhere is probably
a good thing :)

Karl,

even though I totally agree that the API is more important than
where the data ends up being stored, I have a comment on this:

On the tree conflicts branch, we already store conflict data in
the auth area. Here's why:

There may be multiple tree conflicts in a directory, just as there
may be multiple property conflicts on a file or directory.

We want to be able to block commits from tree-conflicted directories
if any of their multiple tree conflicts were not resolved yet.
This is important because the entire point of tree conflict handling
is preventing users from shooting themselves in the foot during
complex merges. Commits that could cause damage without the user
being aware of the potential damage need to be blocked.

Also, we want to be as supportive as we can, and enabling users to
deal with the conflicts one after another is one part of this goal.
With each tree conflict, we associate a ``victim'', i.e. a file or
directory which may be lost or damaged if the conflict is not resolved.
We can then explain each tree conflict (e.g. "The update tried to delete
file foo.c, which you edited") to the user separately.

Users should then be able to tell us "OK, I've resolved the tree
conflict which affects foo.c". We then clear the meta data
corresponding to that tree conflict.

(Note that this is currently not implemented as described.
In the current implementation, running "svn resolved" on a
tree-conflicted directory will clear all tree conflict information.
But the long-term goal is the above behaviour. See issue #3145.)

We first toyed with the idea of storing tree conflict information
in a user-visible file. The file would explain to the user what kinds
of tree conflicts had happened in its containing directory.
There are two possible ways to block commits in such a setup:

1) Don't care about the file's content. Check if the file is still
   present. If it is, disallow commits. If it is not, allow commits.
   This means that we do not have more fine-grained information other
   than "this directory contains one or more tree conflicts".

   Note that this approach is used for text conflicts, by checking
   the presence of .mine, .rX, and .rY and such files.
   It works there because a binary logic is fine in this case:
   The entire file is either text-conflicted or not. We don't
   care about further details, so we don't need more information.

2) Make the file somewhat machine parsable while keeping it
   human-readable, and get per-conflict data from the file.

We (Steve and myself) were inclined to go with 2), but then realised
that any change made by the user to that data could cause us to lose
information about what kind of tree conflicts we had detected during
updates/merges. So we could be tricked into forgetting about some tree
conflicts when the user deleted wrong parts of that file, for example.
We did not like this. Such sensitive data should be in the auth area.
Then it's pretty clear that if users are messing with the data directly,
it's their fault.

So we decided to hide plain tree conflict meta data from users, and put
it in the auth area, and provided an API that translates the meta data
into a human readable form. This is already implemented.
This way, we have the best of both worlds: We control the meta data,
and users are still presented with human-reable information about
tree conflicts.

So Julian's approach to storing property conflicts data in the auth
is consistent with what we have already done on the tree conflicts
branch.

While maintaining consistency between the way text conflicts and
other types of conflicts are handled is certainly desirable, I don't
think it's viable. The way we currently deal with text conflicts is fine.
But it is also much simpler than how we want to deal with tree and property
conflicts. And the way our current trunk code is dealing with either
of those is, well, suboptimal.

Stefan