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

[Noorul Islam K M] Re: [PATCH] Fix for issue 3826

From: Noorul Islam K M <noorul_at_collab.net>
Date: Wed, 16 Mar 2011 07:45:14 +0530

I wonder, why this patch was not considered for fixing issue 3826. I
could see a fix 1081799 which I see very similar to what I have
done. Did I miss anything here?

Thanks and Regards
Noorul

attached mail follows:


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-16 03:16:03 CET

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