Gavin,

This is fixed in r1081799 by Mike, so this patch can be ignored.

Thanks and Regards
Noorul

-----Original Message-----
From: Gavin Baumanis [mailto:gav...@thespidernet.com]
Sent: Mon 3/28/2011 5:50 AM
To: noorul Islam. Kamal Malmiyoda
Cc: Julian Foad; Subversion
Subject: Re: [PATCH] Fix for issue 3826
 
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.f...@wandisco.com> writes:
> 
>> On Thu, 2011-03-03 at 22:48 +0530, Noorul Islam K M wrote:
>> 
>>> Julian Foad <julian.f...@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)
> 
> -@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


Reply via email to