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

On Thu, Feb 28, 2008 at 12:40 PM, Julian Foad
<julianfoad_at_btopenworld.com> wrote:
>
> David Glasser wrote:
> > On Thu, Feb 28, 2008 at 10:40 AM, Julian Foad
> > <julianfoad_at_btopenworld.com> wrote:
> >
> >>David Glasser wrote:
> >> > On Thu, Feb 28, 2008 at 9:17 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> >> >
> >> >>David Glasser wrote:
> >> >>
> >> >>
> >> >>>Julian Foad wrote:
> >> >>
> >> >> >> Therefore I'd propose just adding [@PEGREV] support to the target URL in all
> >> >> >> commands that are to support this feature:
> >> >> >>
> >> >> >> svn pdel URL[@PEGREV]
> >> >> >> svn pset URL[@PEGREV]
> >> >> >> svn pedit URL[@PEGREV]
> >> >> >> svn copy SRC[@PEGREV]... DSTURL[@PEGREV]
> >> >> >> ...
> >> >> >>
> >> >> >> Notice that "pedit" reads and writes a single object, and the notion of both
> >> >> >> reading from "URL_at_PEG" and writing a new revision based on the same "URL_at_PEG"
> >> >> >> makes total sense.
> >> >> >>
> >> >> >> I'm still willing to hear any counter-proposals. The main ones I can think of are:
> >> >> >>
> >> >> >> [--base-rev=BASEREV] URL[@PEGREV]
> >> >> >> (as I sketched above, where BASEREV and PEGREV are usually the same, and
> >> >> >> BASEREV should not be specified without PEGREV because it would be ambiguous)
> >> >> >>
> >> >> >> [--base-rev=BASEREV] URL
> >> >> >> (where BASEREV defaults to HEAD and is used both as the Base Revision and
> >> >> >> as the Peg Revision for URL, instead of "@" syntax)
> >> >> >>
> >> >> >> (And "--base-rev" would probably need a better name.)
> >> >> >>
> >> >> >> Can anyone see a strong reason for or against any of these?
> >> >> >
> >> >> > I think your proposal is good, but for the specific case of "propset"
> >> >> > I think the pegrev should be *required*.
> >> >>
> >> >> Why, specifically?
> >> >
> >> > Um, for the same reason that we historically haven't supported propset
> >> > on URLs --- so you don't clobber somebody else's changes in the
> >> > meantime? (Would you support letting "svn import" (with no special
> >> > "--replace"/"--force"/etc flags or baserev-type argument) overwriting
> >> > file contents by default?
> >>
> >> It's not the case that only "propset" could clobber other people's changes. So
> >> can "svn rm URL" and "svn mv URL URL" already, and propdel, import, merge, etc.
> >> also could if we were to allow them to. So that's not the sole reason we
> >> haven't supported propset on URLs.
> >>
> >> I don't see anything special about "propset" compared to the other commands.
> >>
> >> - Julian
> >
> >
> > I do think there's a difference between "erase this" and "change this
> > to that". Consider something like svn:ignore or svn:externals, for
> > example. One could assume for "erase svn:ignore", the user wouldn't
> > care what the previous value is; but for "add a line" or "change this
> > particular line" or "delete a line", the race condition is nastier.
> >
> > That is, anywhere where a "merge" (not in the 'svn merge' sense, but
> > in the textual sense) is necessary shouldn't allow the race condition.
> > "rm" and "mv" are different here. And you say "import" and "merge"
> > could, but I would be opposed to that as well!
> >
> > If people really want to make themselves suspectable to race
> > conditions, our API allows it, and they can use svnput or the like.
> > But let's not put it in the default client.
>
> It's not that some of these commands are susceptible to race conditions and
> others are not: they all are. It's just a question of degree and likelihood.
>
> Example:
> [[[
> INITIALLY
> svn:ignore = *.tmp
>
> USER1
> # Temporary files don't need to be ignored any more:
> svn propdel svn:ignore
>
> USER2
> # Adds "*.bak":
> svn propset svn:ignore "*.tmp
> *.bak"
>
> RESULT
> Same race condition as if USER1 had been using "propset".
> ]]]

Well, actually, yeah: it's the same race condition, and thus would be
prevented by the same requirement to use baserev for propset :-)

--dave

> This not-unlikely scenario shows that "propdel" is not unlike "propset" in its
> ability to participate in race conditions, only more limited in the variety of
> changes it can make.
>
> Also consider that often the user will be pretty certain that the path
> specified is not being changed by other people in any way that matters. E.g.
> "svn propset p v /branches/julian1": it's my branch so I'm pretty sure no-one
> else is touching it. I don't want to be forced to specify the revision number
> when I don't have any practical need to do so.
>
> Anyway, forcing the user to specify a peg revision doesn't force the user to
> note and specify the correct revision. He can just specify "HEAD", or, if we
> disallow that, any recent revision number.
>
> So please let's keep the syntax consistent across all commands.
>
> - Julian
>

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org