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
--- Begin Message ---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 NoorulIndex: 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
--- End Message ---
