officially freezing svn:mergeinfo syntax

[Stefan Sperling <st...@elego.de>]

The recent fix for issue #3895 ("repos layer does not verify mergeinfo
syntax") implies that the syntax of svn:mergeinfo is frozen and cannot
be changed in future release without breaking compatibility of newer
clients with 1.7 servers.

We also backported this change to 1.6.17.

I am pointing this out on dev@ since it might not be obvious to those
who haven't reviewed the change.

Do people think that this might cause problems?

I would say it is not a problem because current clients will already
error out hard if the syntax is changed (see also issue #3896) so we're
effectively already stuck with the current syntax.

We can always add a new property (e.g. svn:mergeinfo2) if we want
to add information or use a new syntax.

Thoughts?

Re: officially freezing svn:mergeinfo syntax

[Paul Burba <pt...@gmail.com>]

On Mon, May 23, 2011 at 9:45 AM, Paul Burba <pt...@gmail.com> wrote:
> On Mon, May 23, 2011 at 8:29 AM, Greg Stein <gs...@gmail.com> wrote:
>> I've been assuming the syntax is frozen for a long time now, so yeah...
>> freeze it more :-)
>> On May 23, 2011 8:12 AM, "Stefan Sperling" <st...@elego.de> wrote:
>>> The recent fix for issue #3895 ("repos layer does not verify mergeinfo
>>> syntax") implies that the syntax of svn:mergeinfo is frozen and cannot
>>> be changed in future release without breaking compatibility of newer
>>> clients with 1.7 servers.
>>>
>>> We also backported this change to 1.6.17.
>>>
>>> I am pointing this out on dev@ since it might not be obvious to those
>>> who haven't reviewed the change.
>>>
>>> Do people think that this might cause problems?
>>>
>>> I would say it is not a problem because current clients will already
>>> error out hard if the syntax is changed (see also issue #3896) so we're
>>> effectively already stuck with the current syntax.
>
> Agreed, either *potentially* break future clients with 1.6.17-1.7.x
> repositories or *definitely* break existing clients exposed to invalid
> mergeinfo that got into <1.6.16 repositories.  We need to pick our
> poison at this point and protecting existing clients against a
> demonstrated problem strikes me as a better choice than worrying about
> potential mergeinfo syntax changes.
>
> FWIW I read issue #3896 to describe the problems with a client when
> *invalid* mergeinfo enters the repository; though the same problems
> hold for older clients if we purposefully change the syntax.

I'm taking a look at issue #3896 today.  It's something we should do
sooner rather than later.  If users encounter issue #3895 we want a
better answer (i.e. upgrade to 1.7 client) than what we can provide
right now: "Dump/edit/load your repository, with a lot of hand-waving
on the edit part".

Paul

>>> We can always add a new property (e.g. svn:mergeinfo2) if we want
>>> to add information or use a new syntax.
>>>
>>> Thoughts?
>
> In an ideal world we would have laid the groundwork in 1.5 clients for
> this, with separate mergeinfo syntax checking for repository-provided
> mergeinfo vs. client/user provided/generated mergeinfo (disable merge
> tracking given the former, error out given the latter).  Or ever
> better, the repository could have provided the syntax it supports.
> Regardless, we didn't foresee the need and so we are stuck with what
> we have.
>
> Paul
>

Re: officially freezing svn:mergeinfo syntax

[Paul Burba <pt...@gmail.com>]

On Mon, May 23, 2011 at 8:29 AM, Greg Stein <gs...@gmail.com> wrote:
> I've been assuming the syntax is frozen for a long time now, so yeah...
> freeze it more :-)
> On May 23, 2011 8:12 AM, "Stefan Sperling" <st...@elego.de> wrote:
>> The recent fix for issue #3895 ("repos layer does not verify mergeinfo
>> syntax") implies that the syntax of svn:mergeinfo is frozen and cannot
>> be changed in future release without breaking compatibility of newer
>> clients with 1.7 servers.
>>
>> We also backported this change to 1.6.17.
>>
>> I am pointing this out on dev@ since it might not be obvious to those
>> who haven't reviewed the change.
>>
>> Do people think that this might cause problems?
>>
>> I would say it is not a problem because current clients will already
>> error out hard if the syntax is changed (see also issue #3896) so we're
>> effectively already stuck with the current syntax.

Agreed, either *potentially* break future clients with 1.6.17-1.7.x
repositories or *definitely* break existing clients exposed to invalid
mergeinfo that got into <1.6.16 repositories.  We need to pick our
poison at this point and protecting existing clients against a
demonstrated problem strikes me as a better choice than worrying about
potential mergeinfo syntax changes.

FWIW I read issue #3896 to describe the problems with a client when
*invalid* mergeinfo enters the repository; though the same problems
hold for older clients if we purposefully change the syntax.

>> We can always add a new property (e.g. svn:mergeinfo2) if we want
>> to add information or use a new syntax.
>>
>> Thoughts?

In an ideal world we would have laid the groundwork in 1.5 clients for
this, with separate mergeinfo syntax checking for repository-provided
mergeinfo vs. client/user provided/generated mergeinfo (disable merge
tracking given the former, error out given the latter).  Or ever
better, the repository could have provided the syntax it supports.
Regardless, we didn't foresee the need and so we are stuck with what
we have.

Paul

Re: officially freezing svn:mergeinfo syntax

[Greg Stein <gs...@gmail.com>]

I've been assuming the syntax is frozen for a long time now, so yeah...
freeze it more :-)
On May 23, 2011 8:12 AM, "Stefan Sperling" <st...@elego.de> wrote:
> The recent fix for issue #3895 ("repos layer does not verify mergeinfo
> syntax") implies that the syntax of svn:mergeinfo is frozen and cannot
> be changed in future release without breaking compatibility of newer
> clients with 1.7 servers.
>
> We also backported this change to 1.6.17.
>
> I am pointing this out on dev@ since it might not be obvious to those
> who haven't reviewed the change.
>
> Do people think that this might cause problems?
>
> I would say it is not a problem because current clients will already
> error out hard if the syntax is changed (see also issue #3896) so we're
> effectively already stuck with the current syntax.
>
> We can always add a new property (e.g. svn:mergeinfo2) if we want
> to add information or use a new syntax.
>
> Thoughts?

Re: officially freezing svn:mergeinfo syntax

[Daniel Shahaf <d....@daniel.shahaf.name>]

Stefan Sperling wrote on Mon, May 23, 2011 at 14:11:44 +0200:
> We can always add a new property (e.g. svn:mergeinfo2) if we want
> to add information or use a new syntax.

Err... not easily.

The filesystem layer has certain optimizations for efficiently finding
paths with svn:mergeinfo set on them (e.g., minfo-count in fsfs), so if
we introduce a new property then we won't be able to efficiently find it
in old servers.

Unless we encode its location into the existing svn:mergeinfo properties
in a format that 1.6.17 would consider valid but wouldn't affect its
mergeinfo calculations.... but that's impractical.