Ping.
This patch submission has received no new comments.
On 09/03/2011, at 4:18 PM, Noorul Islam K M wrote:
> Julian Foad <julian.foad_at_wandisco.com> writes:
>
>> On Thu, 2011-03-03 at 22:48 +0530, Noorul Islam K M wrote:
>>
>>> Julian Foad <julian.foad_at_wandisco.com> writes:
>>>
>>>> On Wed, 2011-03-02, Noorul Islam K M wrote:
>>>>
>>>>> Please find attached patch for issue 3826. All tests pass using 'make
>>>>> check'
>>>> [...]
>>>>> Index: subversion/svn/diff-cmd.c
>>>>> ===================================================================
>>>>> --- subversion/svn/diff-cmd.c (revision 1076214)
>>>>> +++ subversion/svn/diff-cmd.c (working copy)
>>>>> @@ -324,8 +324,11 @@
>>>>> return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
>>>>> _("Path '%s' not relative to base URLs"),
>>>>> path);
>>>>> + if (! svn_dirent_is_absolute(path))
>>>>> + path = svn_relpath_canonicalize(path, iterpool);
>>>>> + else
>>>>> + path = svn_dirent_canonicalize(path, iterpool);
>>
>> Your new code here expects that 'path' could be either an absolute local
>> path (know as a 'dirent'), or a 'relpath' ...
>>
>>>>>
>>>>> - path = svn_relpath_canonicalize(path, iterpool);
>>>>> if (svn_path_is_url(old_target))
>>>>> target1 = svn_path_url_add_component2(old_target, path, iterpool);
>>
>> ... but here (if old_target is a URL) the code requires that 'path' is a
>> relpath. So what happens if 'path' is a 'dirent'? Does this
>> 'url_add_component' line get executed (and if so it will produce wrong
>> results), or does it not get executed (and if not, why not)?
>>
>
> From 'svn help diff'
>
> 2. Display the differences between OLD-TGT as it was seen in OLDREV and
> NEW-TGT as it was seen in NEWREV. PATHs, if given, are relative to
> OLD-TGT and NEW-TGT and restrict the output to differences for those
> paths. OLD-TGT and NEW-TGT may be working copy paths or URL[@REV].
> NEW-TGT defaults to OLD-TGT if not specified. -r N makes OLDREV default
> to N, -r N:M makes OLDREV default to N and NEWREV default to M.
>
> I modified the patch a bit so that conversion to relpath happens only
> when user specified new-tgt or old-tgt. In other cases I think it can be
> absolute path.
>
> Attached is the modified patch.
>
> Thanks and Regards
> Noorul
>
> Index: subversion/tests/cmdline/diff_tests.py
> ===================================================================
> --- subversion/tests/cmdline/diff_tests.py (revision 1079662)
> +++ subversion/tests/cmdline/diff_tests.py (working copy)
> @@ -3761,7 +3761,6 @@
> '-c2', '--git')
> os.chdir(was_cwd)
>
> -_at_XFail()
> @Issue(3826)
> def diff_abs_localpath_from_wc_folder(sbox):
> "diff absolute localpath from wc folder"
> @@ -3770,9 +3769,10 @@
>
> A_path = os.path.join(wc_dir, 'A')
> B_path = os.path.join(wc_dir, 'A', 'B')
> + B_abspath = os.path.abspath(B_path)
> os.chdir(os.path.abspath(A_path))
> svntest.actions.run_and_verify_svn(None, None, [], 'diff',
> - os.path.abspath(B_path))
> + B_abspath)
>
> ########################################################################
> #Run the tests
> Index: subversion/svn/diff-cmd.c
> ===================================================================
> --- subversion/svn/diff-cmd.c (revision 1079662)
> +++ subversion/svn/diff-cmd.c (working copy)
> @@ -325,7 +325,11 @@
> _("Path '%s' not relative to base URLs"),
> path);
>
> - path = svn_relpath_canonicalize(path, iterpool);
> + if ((strcmp(old_target, "") != 0) || (strcmp(new_target, "") != 0))
> + path = svn_relpath_canonicalize(path, iterpool);
> + else
> + path = svn_dirent_canonicalize(path, iterpool);
> +
> if (svn_path_is_url(old_target))
> target1 = svn_path_url_add_component2(old_target, path, iterpool);
> else
Received on 2011-03-28 02:21:27 CEST