Re: [PATCH] Re: "svn diff" doesn't work correctly if a file is replaced with a symlink locally
For git style patches there is some magic that also allows creating
symlinks via the file mode. I think we have some test cases on that
behavior. you might be able to use this for building a regression test for
this case.
Bert
On Thu, Jul 19, 2018 at 8:51 PM, Daniel Shahaf
wrote:
> 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
>
Re: [PATCH] Re: "svn diff" doesn't work correctly if a file is replaced with a symlink locally
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
[PATCH] Re: Directory becomes file when applying patch
Hello, Julian!
I've checked that the latest trunk version skips the directory in this case
(much better than making it a file). So I'm not sure if it is possible to turn
it into directory addition at all.
Anyway I've created a test for the case. I'm not 100% sure in the expected
output/disk/status of the test but from the first glance it looks ok.
The test fails because it expects a directory with a property to be created
but it is skipped.
--
Dmitry Pavlenko,
TMate Software,
http://subgit.com/ - git-svn bridge
> Hello!
> When I apply Git format patch that adds a directory with properties, a file
> is added instead.
>
> Here I provide a reproducing script and also add a file for comparison.
>
> I think the origin of the problem in the fact that SVN patch doesn't keep
> "node kind". But a patch in "svn diff --git" format does. Here's the example
> of the patch. Notice
>
> a) "Property changes on: dir" and "Property changes on: file" lines;
> b) "new file mode 10644" for file and no such line for directory;
>
> allowing to distinguish between files and directories.
>
> I don't know whether it's a known issue or not but I think it would be nice
> to infer "node kind" from that information and also (if this is not already
> done) by the following logic: if patch changes "dir" and "dir/someFile",
> then "dir" is probably a directory.
>
>
> Index: /tmp/wc/dir
> ===
> diff --git a/dir b/dir
> --- a/dir (nonexistent)
> +++ b/dir (working copy)
>
> Property changes on: dir
> ___
> Added: propName
> ## -0,0 +1 ##
> +propValue
> \ No newline at end of property
> Index: /tmp/wc/file
> ===
> diff --git a/file b/file
> new file mode 10644
>
> Property changes on: file
> ___
> Added: propName
> ## -0,0 +1 ##
> +propValue
> \ No newline at end of property
>
>
>
> The reproducing script:
>
>
> #!/bin/sh
>
> SVN=svn
>
> #1. Create an empty SVN repository.
>
> REPOSITORY_PATH="$PWD/svn.repo"
>
> svnadmin create "$REPOSITORY_PATH"
>
> # 2. Add a file with properties and a directory with properties to the
> repository.
>
> WC_PATH="/tmp/wc"
> REPOSITORY_URL="file://$REPOSITORY_PATH"
>
> $SVN co $REPOSITORY_URL $WC_PATH
>
> touch $WC_PATH/file
>
> $SVN add $WC_PATH/file
> $SVN mkdir $WC_PATH/dir
>
> $SVN propset propName propValue $WC_PATH/file
> $SVN propset propName propValue $WC_PATH/dir
>
> # 3. Create diff between repository HEAD and working copy:
>
> PATCH_FILE=/tmp/patch
>
> $SVN diff --git $REPOSITORY_URL $WC_PATH > $PATCH_FILE
>
> # 4. Cleanup the working copy
>
> $SVN revert $WC_PATH/file
> $SVN revert $WC_PATH/dir
>
> rm $WC_PATH/file
> rmdir $WC_PATH/dir
>
> # 5. Apply the patch back:
>
> $SVN patch $PATCH_FILE $WC_PATH
>
> # 6. Make sure that the file is file and that dir is directory
>
> FILE_TYPE=`stat -c "%F" $WC_PATH/file`
> DIRECTORY_TYPE=`stat -c "%F" $WC_PATH/dir`
>
> if [ "$FILE_TYPE" != "regular empty file" ] ; then
> echo ""
> echo "File is not file but $FILE_TYPE"
> echo ""
> fi
>
> if [ "$DIRECTORY_TYPE" != "directory" ] ; then
> echo ""
> echo "Dir is not directory but $DIRECTORY_TYPE"
> echo ""
> fi
Index: subversion/tests/cmdline/patch_tests.py
===
--- subversion/tests/cmdline/patch_tests.py (revision 1836275)
+++ subversion/tests/cmdline/patch_tests.py (working copy)
@@ -7913,6 +7913,56 @@ def patch_empty_prop(sbox):
os.chdir(was_cwd)
+def patch_git_empty_directory_with_prop(sbox):
+ "empty directory addition confused with file"
+ sbox.build(empty=True)
+ wc_dir = sbox.wc_dir
+
+ # This patch is generated for empty directory addition with property by version 1.10
+ git_patch = [ "Index: dir\n",
+"===\n",
+"diff --git a/dir b/dir\n",
+"--- a/dir(nonexistent)\n",
+"+++ b/dir(working copy)\n",
+"\n",
+"Property changes on: dir\n",
+"___\n",
+"Added: p\n",
+"## -0,0 +1 ##\n",
+"+v\n",
+"\ No newline at end of property\n",
+ ]
+ value = 'v'
+
+ patch_file_path = sbox.get_tempname('my.patch')
+ svntest.main.file_write(patch_file_path, ''.join(git_patch), 'wb')
+
+ dir_path = sbox.path('dir')
+
+ expected_output = wc.State(wc_dir, {
+'dir' : Item(status='A '),
+ })
+ expected_disk = svntest.wc.State('', {})
+ expected_disk.add({'dir': Item(props={'p' : value })})
+ expected_status = svntest.wc.State(wc_
[PATCH] Re: "svn diff" doesn't work correctly if a file is replaced with a symlink locally
Hello again,
I'm attaching a reproducing 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.
--
Dmitry Pavlenko,
TMate Software,
http://subgit.com/ - git-svn bridge
On среда, 11 июля 2018 г. 16:13:06 CEST Dmitry Pavlenko wrote:
> Hello.
> The following scenario fails for me: delete a file, create a symlink with
> the same name, run "svn diff". Instead of displaying the diff, SVN tries to
> read the link content. If "NOWHERE" variable is set to the file name, the
> symlink refers to itself, showing another error.
>
> Instead I would expect something with "-" and "+" markers. If you uncomment
> #echo "link $NOWHERE" > $WC_PATH/trunk/file
>
> and comment out "ln -s" call, the script will display approximately what I
> would expect (but without svn:special).
>
> I'm not sure if this is a known bug, sorry for reporting something known if
> it is.
>
> Locally I also have similar scenario failing in another way: before reading
> the symlink it displays deletion of "trunk/trunk/symlink" (yes, unexpectedly
> double "trunk", in my local test the file is called "symlink"). But I think
> it's the same issue. If it will still fail after this one is fixed, I'll
> analyze why.
>
>
>
> #!/bin/sh
>
> SVN=svn
>
> #1. Create an empty SVN repository.
>
> REPOSITORY_PATH="$PWD/svn.repo"
>
> svnadmin create "$REPOSITORY_PATH"
>
> # 2. Add a file to the repository.
>
> WC_PATH="/tmp/wc"
> REPOSITORY_URL="file://$REPOSITORY_PATH"
>
> $SVN co $REPOSITORY_URL $WC_PATH
> $SVN mkdir $WC_PATH/trunk
>
> echo "content" > $WC_PATH/trunk/file
>
> $SVN add $WC_PATH/trunk/file
> $SVN commit -m "A file added." $WC_PATH
>
> # 3. Replace the file with a symlink pointing to nowhere (or to itself ---
> for another kind of error)
>
> NOWHERE="changed/path"
> #NOWHERE="file"
>
> rm $WC_PATH/trunk/file
>
> ln -s $NOWHERE $WC_PATH/trunk/file
>
> # By the way: uncomment the following line and comment out the previous one
> for comparison
> #echo "link $NOWHERE" > $WC_PATH/trunk/file
>
> # 4. Create diff between repository HEAD and working copy:
>
> $SVN diff --git $REPOSITORY_URL $WC_PATH
>
> echo ""
> echo ""
> echo "-"
> echo "If you see something like"
> echo "-content"
> echo "+link $NOWHERE"
> echo "And svn:special property set to '*'"
> echo "Then consider test passed"
> echo "-"
Index: subversion/tests/cmdline/diff_tests.py
===
--- subversion/tests/cmdline/diff_tests.py (revision 1836275)
+++ 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):
+ "diff base vs working: symlink replaces a file"
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ iota_path = sbox.ospath('iota')
+ os.remove(iota_path)
+
+ # create a symlink pointing to itself
+ # alternatively it could point to a non-existing path
+ sbox.simple_symlink('iota', 'iota')
+
+ svntest.actions.run_and_verify_svn([
+'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',
+ ], [], 'diff', wc_dir)
+
#Run the tests
@@ -5300,6 +5329,7 @@ test_list = [ None,
diff_unversioned_files_git,
diff_summary_repo_wc_local_copy,
diff_summary_repo_wc_local_copy_unmodified,
+ diff_file_replaced_by_symlink,
]
if __name__ == '__main__':
Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format
Hello Julian,
Here's a failing test for "no dot" case. After removing the code as I wrote
before the test passes.
--
Dmitry Pavlenko,
TMate Software,
http://subgit.com/ - git-svn bridge
> Dmitry Pavlenko wrote:
> > > ... would you mind making a regression test? ...
> >
> > OK, I could do that, but much later, in 2 weeks maybe...
>
> Thanks. I think we should commit your fix anyway, but better if we can also
> fix the 'svn diff' output and also add a test or two.
>
> I am thinking two tests would make sense:
> 1. test that "svn patch" can correctly parse and apply the present form of
> input where there is no dot 2. test that the round-trip of "svn diff"
> followed by "svn patch" works
>
> Maybe I will do some or all of that, soon, but I would love it if anyone
> else wants to take care of it before I get to it.
>
> I have filed this issue as https://issues.apache.org/jira/browse/SVN-4763
>
> - Julian
Index: subversion/tests/cmdline/patch_tests.py
===
--- subversion/tests/cmdline/patch_tests.py (revision 1836275)
+++ subversion/tests/cmdline/patch_tests.py (working copy)
@@ -7913,6 +7913,49 @@ def patch_empty_prop(sbox):
os.chdir(was_cwd)
+def patch_git_wcroot(sbox):
+ "patch working copy root"
+ sbox.build(empty=True)
+ wc_dir = sbox.wc_dir
+
+ git_patch = [ "Index: .\n",
+"===\n",
+"diff --git a/ b/\n",
+"--- a/ (revision 0)\n",
+"+++ b/ (working copy)\n",
+"Property changes on: \n",
+"___\n",
+"Added: p\n",
+"## -0,0 +1 ##\n",
+"+v\n",
+"\ No newline at end of property\n",
+ ]
+ value = 'v'
+
+ patch_file_path = sbox.get_tempname('my.patch')
+ svntest.main.file_write(patch_file_path, ''.join(git_patch), 'wb')
+
+ expected_output = wc.State(wc_dir, {
+'.' : Item(status=' U'),
+ })
+ expected_disk = svntest.wc.State('', {})
+ expected_disk.add({'': Item(props={'p' : value })})
+ expected_status = svntest.wc.State(wc_dir, {})
+ expected_status.add({'': Item(status=' M', wc_rev='0')})
+ expected_skip = wc.State('', { })
+
+ svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
+ expected_output,
+ expected_disk,
+ expected_status,
+ expected_skip,
+ None, # expected err
+ True, # check-props
+ False, # dry-run
+ )
+
+ svntest.actions.check_prop('p', wc_dir, [value.encode()])
+
#Run the tests
@@ -7998,6 +8041,7 @@ test_list = [ None,
patch_merge,
patch_mergeinfo_in_regular_prop_format,
patch_empty_prop,
+ patch_git_wcroot,
]
if __name__ == '__main__':
