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