Daniel Berlin wrote (earlier):
>>
>> $ svn diff -r0 wc
>> Index: /tmp/tmp
>> ===================================================================
>> Index: wc/foo
>> ===================================================================
>> --- wc/foo (revision 0)
>> +++ wc/foo (working copy)
>> @@ -1 +0,0 @@
>> -xxx
>>
>> It's that really the correct output?
>
> That's not the output i get, but i've got an external diff configured :(
Well, what output do you get? Philip replied "External diff makes no
difference as far as I can see." If you got something similar to this, don't
you agree it's wrong? If you got something very different, what's up?
Daniel Berlin wrote:
>> deleted perhaps I should get an empty diff:
>>
>> $ svn diff -r0 wc
>> Index: wc/foo
>> ===================================================================
>>
>> In all our diffs there is the issue about whether the header should be
>> suppressed if there is no difference, but given that we currently show
>> the header I would expect a single empty diff with the correct header.
>
> Okay.
> Try the attached, it should give the output you are looking for
> If so, i'll work up a changelog and commit it.
Daniel, it's great that you're so quick to come up with a new fix, but please
repost the patch with its log message (and "[PATCH]" in the subject line) and
hold off committing until tomorrow to give the weekday workers a chance to
review it and check whether it's the right fix.
> Index: subversion/libsvn_wc/diff.c
> ===================================================================
> --- subversion/libsvn_wc/diff.c (revision 17206)
> +++ subversion/libsvn_wc/diff.c (working copy)
> @@ -855,8 +855,6 @@ delete_entry (const char *path,
> the empty file against the current working copy. If
> 'reverse_order' is set, then show a deletion. */
>
> - if (entry->schedule == svn_wc_schedule_delete)
> - SVN_ERR (get_empty_file (pb->edit_baton, &full_path));
> SVN_ERR (get_local_mimetypes (&pristine_mimetype, &working_mimetype,
> NULL, adm_access, full_path, pool));
>
> @@ -866,6 +864,8 @@ delete_entry (const char *path,
> const char *textbase = svn_wc__text_base_path (full_path,
> FALSE, pool);
>
> + if (entry->schedule == svn_wc_schedule_delete)
> + SVN_ERR (get_empty_file (pb->edit_baton, &textbase));
> SVN_ERR (svn_wc_get_prop_diffs (NULL, &baseprops, full_path,
> adm_access, pool));
> SVN_ERR (pb->edit_baton->callbacks->file_deleted
> @@ -879,12 +879,15 @@ delete_entry (const char *path,
> }
> else
> {
> - /* Or normally, show the working file being added. */
> + const char *secondpath = full_path;
> + if (entry->schedule == svn_wc_schedule_delete)
> + secondpath = empty_file;
> + /* Or normally, show the working file being added. */
> /* ### Show the properties as well. */
> SVN_ERR (pb->edit_baton->callbacks->file_added
> (NULL, NULL, NULL, full_path,
> empty_file,
> - full_path,
> + secondpath,
> 0, entry->revision,
> NULL,
> working_mimetype,
> Index: subversion/tests/clients/cmdline/diff_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/diff_tests.py (revision 17206)
> +++ subversion/tests/clients/cmdline/diff_tests.py (working copy)
> @@ -1952,6 +1952,20 @@ def diff_property_changes_to_base(sbox):
> finally:
> os.chdir(current_dir)
>
> +def diff_schedule_delete(sbox):
> + "scheduled deleted"
> + sbox.build()
> + wc_dir = sbox.wc_dir
> + was_cwd = os.getcwd()
> + os.chdir(wc_dir)
> + svntest.main.file_append('foo', "xxx")
> + svntest.main.run_svn(None, 'add', 'foo')
> + diff_output, err_output = svntest.main.run_svn(None, 'ci', '-m', 'log msg')
There's no need to create and add a file: the default test repository already
has one, called "iota", in order to save test writers from this chore.
> + if err_output: raise svntest.Failure
> + svntest.main.run_svn(None, 'rm', 'foo')
> + diff_output, err = svntest.actions.run_and_verify_svn(None, None, [],
> + 'diff', '-r', '0' )
> + if err: raise svntest.Failure
Although lack of stderr output is sufficient to distinguish the fix from the
original behaviour, it would be much better to test the contents of
"diff_output", like most of the other tests in that file do.
> ########################################################################
> #Run the tests
> @@ -1987,6 +2001,7 @@ test_list = [ None,
> diff_force,
> XFail(diff_renamed_dir),
> XFail(diff_property_changes_to_base),
> + diff_schedule_delete
We prefer to keep the trailing comma on the last item so that each new test
just requires one new line added here.
> ]
Don't forget to update branches/1.3.x/STATUS now that r17190 is not a suitable
candidate for back-porting on its own.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 7 01:08:44 2005