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

Re: [PATCH] Add URL support for propset and propdel and fix URL support for propedit

From: <kmradke_at_rockwellcollins.com>
Date: Fri, 22 Feb 2008 16:20:43 -0600

james82_at_gmail.com wrote on 02/22/2008 04:03:04 PM:
> On Fri, Feb 22, 2008 at 11:44 AM, <kmradke_at_rockwellcollins.com> wrote:
> > "Mark Phippard" <markphip_at_gmail.com> wrote on 02/22/2008 01:34:23 PM:
> >
> > > 2008/2/22 Kevin Radke <kmradke_at_gmail.com>:
> > > > This is my first attempt at a patch to add URL support to the
propset
> > > > and propdel subcommands of svn. In doing so, I also noticed
that
> > > > propedit did not support multiple URL arguments correctly.
> > > >
> > > > I needed URL functionality for propdel, since we had a
repository
> > which
> > > > had the svn:special property set for a file that was not
special. I
> > was
> > > > unable to create a working copy with this file due to this
error, and
> > > > I was also unable to delete the property since you previously
needed
> > > > a working copy to delete properties. I was able to use this
modified
> > > > propdel to delete the incorrect property directly in the
repository.
> > >
> > > What is the stuff about the -r parameter for? We would never allow
> > > you to go back in time and delete or edit a property, only HEAD. Is
> > > it being used for some other purpose?
> > >
> > > I am just going by the email, have not looked at the patch.
> >
> > Needed a way to specify a "base revision", because of the possible
> > race condition between looking at the properties and then
> > performing a command directly on the repository.
> >
> > We kicked around a --base-rev parameter in another thread, but
> > it was easier for me to "overload" the meaning of -r here.
> >
> > Not using -r and using some other parameter is definitely a
> > (probably less confusing) option.
> >
> > We could also assume "HEAD" (others didn't like this)
> >
> > Is peg revision syntax appropriate? (no idea on this)
>
> I think that 'svn propdel' and 'svn propset' should assume the
> "base-rev" is HEAD. Here's why:
> - If the file currently exists, svn rm already assumes that the
> base-rev is HEAD. So svn propdel should do the same when the property
> exists.
> - If the file does not exist, svn import already assumes that the
> base-rev is HEAD. So svn propset should do the same when the property
> does not exist.
>
> Based on the above, there is only one situation where you need to
> specify a base revision -- if you are trying to modify (but not
> delete) a property which is already present in the repository. In
> other cases, you can specify a base revision if you want to have
> additional safety constraints, but there is no need to do so.

The argument against defaulting to HEAD was this use case:

1) User A looks for all files with a property x with a value of y.
2) User B then modifies a property x to a value of z on a file.
3) User A then deletes all properties found in #1.
4) User B wonders why User A deleted the property on his file because
   it didn't have a value of y.

This is hopefully a very rare case, but it could happen.

> Some more comments:
> - If a pegrev is explicitly specified, the base rev should default
> to the pegrev.
> - If you leave out the revision, or explicitly specify "-r HEAD" or
> file_at_HEAD for 'svn propset', svn propset should check to see if the
> property already exists in the repository. If the property already
> exists, an error message should be printed which explains that a
> base-rev is required.

Since you can't set a property on an old revision, I'm not convinced
a peg revision specification should even be possible...

> I can also think of a few places outside of the 'svn prop*" commands
> where base revs might be useful:
> - In future, I think we should extend the 'svn rm', and 'svn import'
> commands to also support base revisions. In both cases, this would
> allow for additional safety when you want to be sure you aren't
> overwriting someone else's changes. In the case of 'svn import' if you
> specify a base-rev, you can actually overwrite an existing file in the
> repository with a new version.
> - In future, we might also want to extend 'svn cp' and 'svn mv' to
> support base revs and a --replace option. This would allow, in theory,
> for users to overwrite a file or directory which is already present in
> the repository with a new version in a single commit.
>
> Based on the fact that base revs might be useful in future in commands
> which already accept the '-r' option, I think that we should use a
> different option for 'base-rev', such as --base-rev.

If there is a true need for a --base-rev parameter for other
commands, it probably would be best to use it now instead of
overloading the meaning of -r.

Kevin R.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-22 23:20:57 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.