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

Re: [PATCH] 'svn' should not silently ignore a peg revision

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 29 Jun 2011 15:55:30 +0300

Julian Foad wrote on Wed, Jun 29, 2011 at 13:07:01 +0100:
> My question is about this case:
> $ svn copy SOURCE TARGET_at_HEAD
> This case falls under the last sentence of point (C) above. Issue #3651
> [3] claims that the 'HEAD' peg should be ignored in this case, and that
> claim is explicitly tested by copy_tests 62 [4]. (It is the only test
> that fails with the attached patch.)
> But is 'HEAD' supposed to be a special case on the basis that it is
> redundant but still correct? If so, then we should also allow '123' iff
> that is currently the head, and 'BASE' for a local path iff that is
> currently the head, and so on. In other words I don't think we have
> reason to claim that 'HEAD' is a special case.

- 'svn cp ^/foo ^/bar_at_123'

  + when youngest is 123 or more: error out

  + when youngest is 122 or less:
    This could mean 'copy foo to bar and only commit that if the commit
    would create r123'. But we don't support that at the FS, so this
    should cause an error too.

- 'svn cp ^/foo ^/bar_at_HEAD'

  Need to decide:

  + if 'HEAD' means "'HEAD' before the commit":
    Error out, this doesn't make any sense. (Well, /technically/ it's
    a reverse obliteration --- a constructive alteration of a committed
    revision --- but let's not go there ☺)

  + if 'HEAD' means "'HEAD' after the commit":

    Semantically it's unneeded, so we could either treat '^/bar' and
    '^/bar_at_HEAD' identically or error out on the latter.

So, just erroring out seems fine (unless someone comes up with a use
case for one of the combinations I'd ruled out).

Makes sense?

> The logic gets very messy if we think we must support redundant pegs.
> Even if we only implement a subset of the messy logic (such as just
> allowing 'HEAD') the conceptual logic is messy which is a bad thing for
> documentation and learning. So my conclusion is that we should reject
> any peg revision specifier, even 'HEAD', in places where we're not
> prepared to use it.

+1 to erroring out when pegs are provide in places where they are
semantically meaningless.

> If that's agreeable, I'll make it happen. (The attached patch is not
> quite finished; for example it has no doc string for one of the new
> functions.)

I haven't yet read the attached patch in detail.

> - Julian
> [1] Issue #3416 "Cannot add or commit 'dir/@file.txt'",
> <http://subversion.tigris.org/issues/show_bug.cgi?id=3416>.
> [2] The only ways I can imagine the second example could make sense are
> if it were to mean, "Make this directory, assuming that HEAD is 123, and
> fail if not" or, "Make this directory, assuming that HEAD is 122 so that
> the committed revision will be 123, and fail if not." But we don't have
> any plans to implement such semantics, and the syntax does not make
> sense for the semantics we have implemented.
> [3] Issue #3651 "svn copy does not eat peg revision within copy target
> path", <http://subversion.tigris.org/issues/show_bug.cgi?id=3651>.
> [4] <http://svn.apache.org/viewvc?revision=952992&view=revision>
Received on 2011-06-29 14:56:14 CEST

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.