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

Re: Info needed in 1.6.x STATUS file

From: Paul Burba <ptburba_at_gmail.com>
Date: Tue, 15 Jun 2010 14:05:46 -0400

On Tue, Jun 15, 2010 at 1:21 PM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> Hi devs.  Going through the 1.6.x. STATUS file, I am finding some items
> that could do with a better description or justification to make it
> easier to evaluate them.  Can anyone provide information on the
> following:
>
>
>>  * r934599, r934603
>>    Fix a concurrency issue in the FSFS rep-cache code.  This indirectly
>>    concerns issue #3506.
>>    Justification:
>>      Opening the DB twice (and overwriting the handle) is undesired.
>
> Why is that undesired?  I mean that as a serious practical question -
> obviously it's theoretically redundant but that in itself doesn't
> necessarily matter.  What is the user-visible effect of the problem, and
> of the proposed change?
>
>>    Notes:
>>      r934599 updates the svn_atomic__init_once() interface.
>>      r934603 is the actual fix.
>>    Notes:
>>      Depends on the r879966 group, which does not merge cleanly.
>>    Branch:
>>      ^/subversion/branches/1.6.x-issue3506-init_once
>>      (based on ^/subversion/branches/1.6.x-issue3506)
>>    Votes:
>>      +1: danielsh
>
>
>>  * r878590, r878607, r878625, r878626, r878627
>>    In trunk we optimized the common case of 'find-and-replace with same uri'
>>    of proxied content thanks to issue 3445 "WebDAV proxy code munging user
>>    data".
>
> What is the purpose and effect of the change proposed for back-port?
>
>>    r878590 is just a change that adds FIXME marker to code comment. We take it
>>    to avoid spurious conflicts with other revisions.
>>    r878607 Special cases no-op find and replace.
>>    "r878625, r878626, r878627" are follow-up to r878607 and the other long
>>    pending Master info leak to the clients.
>>    Justification:
>>      1. This group has the most common 'optimization' fix of *not* groking the
>>         proxied response to find and replace with same string.
>>      2. Fixes the master information leak via the Location header.
>>      3. Need this to be ported for the other defect to be ported
>>         without conflict.
>>    Votes:
>>      +1: kameshj, cmpilato
>>      -0: julianfoad (seem to be multiple changes here for different reasons -
>>            at least issue '3445 and an optimization and an information leak;
>>            r878607 log msg says it fixes bugs but it's not clear what bugs;
>>            don't know how to tell whether justification 1 is significant;
>>            justifications don't seem to refer to issue #3445. Please can we
>>            separate these changes and clearly describe each one? And update
>>            the r878607 log msg.)
>
> Ah, I already put my concerns here.
>
>
>>  * r916286, r917512
>>    ???
>
> What is the purpose and effect of the change proposed for back-port?
>
> (It was me who added the question marks, some time ago.)
>
>>    Notes:
>>      This backport depends on the r878590 group only for the conflict free port.
>>    Justification:
>>      Vague commit error when server has been configured with <Location /svn/>
>>      and write through proxy.
>
> How bad is the old error message?  How much better is the new one?  An
> example might help.  Does the change have any other effect?
>
>>    Votes:
>>      +1: kameshj
>
>
>>  * r917523
>>    ???
>
> Ditto.
>
>>    Notes:
>>      This backport depends on the r878590 and r916286 groups only for the
>>      conflict free port.
>>    Justification:
>>      If the configured slave url has the *encodable* characters *write through*
>>      is not happening rather commit happens in slave itself.
>
> Commit happens in the slave?  That's certainly and obviously wrong.  But
> I don't understand what you mean by "the configured slave url has the
> *encodable* characters".
>
>>    Votes:
>>      +1: kameshj
>
>
>>  * r935631
>>    Fix one of the sanity checks in the 'svn merge --reintegrate' logic.
>
> What check?  What did it do?  What will it do after this change?
>
> OK, I've looked at this one, and it appears that the check to ensure the
> source and target are in the same repository was not being checked (it
> was comparing target-repos with target-repos), and this change will make
> it return an error if they are in different repositories, as was
> intended.  I'm not sure exactly what would happen if this condition were
> not caught.

Hi Julian,

Fortunately even without r935631 any attempt to reintegrate a source
from a foreign repository fails. So this change is really about
providing a (much) better error mesage:

1.6.11:

>svn merge %REPOS_A_ROOT_URL%file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/merge_tests-54.other/A_COPY A --reintegrate
svn: URL 'file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/merge_tests-54.other/A_COPY'
is not a child of repository root URL
'file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/merge_tests-54'

1.6.12 with Mike's patch:

>svn merge file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/merge_tests-54.other/A_COPY A --reintegrate
svn: 'file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/merge_tests-54.other/A_COPY'
must be from the same repository as 'A'

I made a note of the above in STATUS.

Paul

>>    Justification:
>>      Sanity checks should themselves be sane.
>>    Branch:
>>      ^/subversion/branches/1.6.x-r935631
>>    Votes:
>>      +1: cmpilato, pburba
>
>
>>  * r939375-939376
>>    Fix issue #3623, a bug in foreign repository merges that causes properties
>>    on merge-added files to be dropped from commits of those merges.
>>    Justification:
>>      This can actually result in what is arguably a corrupt working copy,
>>      one that thinks it is in sync with the repository but actually is not.
>
> (Here's an example of a good description and justification.)
>
>
>>  * r950445, 950468
>>    Fix for issue #2591 "'svnadmin hotcopy' does not replicate symlinks".
>>    Justification:
>>      This issue is waiting for resolution for more than 3 years.
>
> Length of time is not really a justification.
>
>>    Notes:
>>      r950445 fixes the issue and r950468 is a test for this issue.
>>    Votes:
>>      +1: stylesen
>
>
>>  * r952973, r952992
>>    Fix for issue #3651 "svn copy does not eat peg revision within copy target
>>    path"
>>    Justification:
>>      This issue exists in 1.6.x.
>
> All of these issues exist in 1.6.x, that's why the fixes are proposed
> for back-port.
>
>>    Notes:
>>      r952973 fixes the issue and r952992 is a test for this issue. Both r952973
>>      and r952992 are merged into the backport branch 1.6.x-issue3651.
>>      Use the private API in the 1.6.x branch since the API was made public in
>>      1.7. The backport branch exists in order to resolve this.
>>    Branch:
>>      ^/subversion/branches/1.6.x-issue3651
>>    Votes:
>>      +1: stylesen, pburba
>
>
> - Julian
>
>
>
Received on 2010-06-15 20:06:29 CEST

This is an archived mail posted to the Subversion Dev mailing list.