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

Re: [PATCH] Re: "svn diff" doesn't work correctly if a file is replaced with a symlink locally

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Thu, 19 Jul 2018 18:51:18 +0000

Dmitry Pavlenko wrote on Thu, Jul 19, 2018 at 19:03:30 +0200:
> I'm attaching a reproducing test.
>

Thanks for the test!

> I'm not 100% sure that
>
> [
> 'Index: %s\n' % sbox.path('iota'),
> '===================================================================\n',
> '--- %s\t(revision 1)\n' % sbox.path('iota'),
> '+++ %s\t(working copy)\n' % sbox.path('iota'),
> '@@ -1 +1 @@\n',
> '-This is the file \'iota\'.\n',
> '+link iota\n',
> '\ No newline at end of file\n',
> '\n',
> 'Property changes on: iota\n',
> '___________________________________________________________________\n',
> 'Added: svn:special\n',
> '## -0,0 +1 ##\n',
> '+*\n',
> '\ No newline at end of property\n',
> ]
>
> is the expected output but definitely the diff command shouldn't fail with
> exit code 1 as it does now.

There isn't a patch flying around that produces this output, right?

In this case, I suggest that we add a regression test that simply expects any
output and exit code zero — that's «run_and_verify_svn(svntest.verify.AnyOutput,
[], 'diff', wc_dir)» — and add a comment reminding us to write a more explicit
expectation once the issue is fixed. This would make sure the patch start
passing as soon as we change the behaviour, even if the expected output
predicted is a little off.

> +++ subversion/tests/cmdline/diff_tests.py (working copy)
> @@ -5201,7 +5201,36 @@ def diff_summary_repo_wc_local_copy_unmodified(sbo
> '--old=' + sbox.ospath('iota') + '@HEAD',
> '--new=' + sbox.ospath('iota2'))
>
> +def diff_file_replaced_by_symlink(sbox):

There should be an "@XFail()" decorator here, so `make test` (and
`./diff_tests.py`) still exit 0 despite this test (X)FAILing.

> + "diff base vs working: symlink replaces a file"
> + sbox.build()

Since this test doesn't commit, it can pass read_only=True, then build() would
be cheaper.

> + svntest.actions.run_and_verify_svn([
> + '\ No newline at end of file\n',

Please don't add new instances of backslash-space; it's an undefined/deprecated
syntax that generates warnings in newer Pythons. See r1834787.

Cheers,

Daniel
Received on 2018-07-19 20:51:35 CEST

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