You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Dmitry Pavlenko <pa...@tmatesoft.com> on 2018/07/09 15:10:26 UTC

[PATCH] can't "svn patch" working copy root if the patch is in --git format

Hello!

I've found a bug when applying a patch in Git format to the working copy root.
Steps to reproduce:

#!/bin/sh

SVN=svn

#1. Create an empty SVN repository.

REPOSITORY_PATH="$PWD/svn.repo"

svnadmin create "$REPOSITORY_PATH"

# 2. Create a patch that sets some property on a root directory. E.g.

WC_PATH="/tmp/wc"
REPOSITORY_URL="file://$REPOSITORY_PATH"
PATCH_PATH=/tmp/patch
EXPECTED_PROPERTY_VALUE=value

$SVN co $REPOSITORY_URL $WC_PATH
svn propset svn:ignore $EXPECTED_PROPERTY_VALUE $WC_PATH

$SVN diff --git $WC_PATH > $PATCH_PATH

# 3. Create another clean working copy for that repository
# E.g. we can just clean an existing one:

$SVN revert $WC_PATH

# 4. Apply the patch

$SVN patch $PATCH_PATH $WC_PATH

# 5. Check the property of $WC_PATH

ACTUAL_PROPERTY_VALUE=`svn propget svn:ignore $WC_PATH`

echo "===="
if [ x"$EXPECTED_PROPERTY_VALUE" != x"$ACTUAL_PROPERTY_VALUE" ] ; then
  echo "Bug!"
  echo "Expected value: $EXPECTED_PROPERTY_VALUE"
  echo "Actual value: $ACTUAL_PROPERTY_VALUE"
else
  echo "Test passed!"
fi
echo "===="  


Note that the patch looks like

Index: /tmp/wc
===================================================================
diff --git a/ b/
--- a/  (revision 0)
+++ b/  (working copy)

Property changes on: 
___________________________________________________________________
Added: svn:ignore
## -0,0 +1 ##
+value


The problem happens in parse-diff.c in git_start() function when parsing

diff --git a/ b/

line in 2 places. The first one is:

 if (! *(new_path_marker + 3))
   {
     *new_state = state_start;
     return SVN_NO_ERROR;
   }

We get inside the check as there's no continuation after "b/" and SVN thinks 
the patch is invalid.

Another place is:

     /* No path after the marker. */
     if (! *new_path_start)
       break;

The logic is the same. When I comment out both check the code works correctly 
even though now there's an asymmetry between "old" and "new" path processing 
code.
[[[
   Fix "patch" command: allow empty path when patch is in Git format.

   * subversion/libsvn_diff/parse-diff.c
     (git_start): Remove empty path checks.
]]]
[[[
Index: subversion/libsvn_diff/parse-diff.c
===================================================================
--- subversion/libsvn_diff/parse-diff.c (revision 1835430)
+++ subversion/libsvn_diff/parse-diff.c (working copy)
@@ -1587,12 +1587,6 @@ git_start(enum parse_state *new_state, char *line,
       return SVN_NO_ERROR;
     }
 
-  if (! *(new_path_marker + 3))
-    {
-      *new_state = state_start;
-      return SVN_NO_ERROR;
-    }
-
   /* By now, we know that we have a line on the form '--git diff a/.+ b/.+'
    * We only need the filenames when we have deleted or added empty
    * files. In those cases the old_path and new_path is identical on the
@@ -1616,10 +1610,6 @@ git_start(enum parse_state *new_state, char *line,
       old_path_end = new_path_marker;
       new_path_start = new_path_marker + STRLEN_LITERAL(" b/");
 
-      /* No path after the marker. */
-      if (! *new_path_start)
-        break;
-
       len_old = old_path_end - old_path_start;
       len_new = new_path_end - new_path_start;

]]]
-- 
Dmitry Pavlenko,
TMate Software,
http://subgit.com/ - git-svn bridge


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

Posted by Julian Foad <ju...@apache.org>.
Thanks for the report. It would be great if you could file an issue and write a test for this problem.

- Julian


Dmitry Pavlenko wrote on 2018-07-11:
> 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 "-----------------------------------------"

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

Posted by Branko Čibej <br...@apache.org>.
On 20.07.2018 12:46, Julian Foad wrote:
> Julian Foad wrote:
>> Daniel Shahaf wrote:
>>> Dmitry Pavlenko wrote on Thu, Jul 19, 2018 at 19:03:30 +0200:
>>>> I'm not 100% sure that [...] is the expected output [...]
> [...]
>>> I suggest that we add a regression test that simply expects any output [...]
> But on Windows the buildbots have discovered that this 'diff' command does not error out, and so this test gives an XPASS result (which we choose to treat as an overall failure of the test suite).
>
> What to do? Add in some reasonable expectation?
>
> We need to design the expectation.
>
> Intuitively I expect a replacement of file with symlink to be presented as file deleted and symlink added, not as a content-modification (from normal file content to the 'special' representation of the symlink).
>
> Does that make sense, anyone? Does that fit with our architecture whereby knowledge of 'special files' is client-side?
>
> Presumably we would want a diff requested by the client to behave the same way, regardless whether the two items being diffed are located on the server and/or on the client. In contrast, if we request a diff through a server-layer tool such as 'svnadmin' then presumably it must display a content-modification and property-modification, because it is looking at the server-side representation of a symlink (using a 'special file').
>
> How does this fit with our ideas of object lifelines? Was that a replacement, starting a new lifeline, or not?

That's a tricky question. You can always add an svn:special property to
a file and modify its contents to the correct format. It's still the
same object; the server won't care.

It becomes a new object lifeline if the equivalent of the following
happened on the client:

   svn rm foo
   ln -s /some/thing foo
   svn add foo

Then it's just a replacement of file by file as far a the server is
concerned.

(Could be we have checks in the client that forbids manually adding
svn:special, but I can't recall).

-- Brane

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

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> Daniel Shahaf wrote:
> > Dmitry Pavlenko wrote on Thu, Jul 19, 2018 at 19:03:30 +0200:
> > > I'm not 100% sure that [...] is the expected output [...]
[...]
> > I suggest that we add a regression test that simply expects any output [...]

But on Windows the buildbots have discovered that this 'diff' command does not error out, and so this test gives an XPASS result (which we choose to treat as an overall failure of the test suite).

What to do? Add in some reasonable expectation?

We need to design the expectation.

Intuitively I expect a replacement of file with symlink to be presented as file deleted and symlink added, not as a content-modification (from normal file content to the 'special' representation of the symlink).

Does that make sense, anyone? Does that fit with our architecture whereby knowledge of 'special files' is client-side?

Presumably we would want a diff requested by the client to behave the same way, regardless whether the two items being diffed are located on the server and/or on the client. In contrast, if we request a diff through a server-layer tool such as 'svnadmin' then presumably it must display a content-modification and property-modification, because it is looking at the server-side representation of a symlink (using a 'special file').

How does this fit with our ideas of object lifelines? Was that a replacement, starting a new lifeline, or not?

-- 
- Julian

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

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> Dmitry Pavlenko wrote on Thu, Jul 19, 2018 at 19:03:30 +0200:
> > I'm not 100% sure that [...] is the expected output [...]
> 
> There isn't a patch flying around that produces this output, right?

No, there isn't yet.

> I suggest that we add a regression test that simply expects any output [...]
> There should be an "@XFail()" decorator [...]
> Since this test doesn't commit, it can pass read_only=True [...]
> Please don't add new instances of backslash-space [...]

I made those tweaks and committed it:
  r1836336

Thanks for the patch!

-- 
- Julian

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

Posted by Bert Huijben <be...@qqmail.nl>.
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 <d....@daniel.shahaf.name>
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

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
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: "svn diff" doesn't work correctly if a file is replaced with a symlink locally

Posted by Dmitry Pavlenko <pa...@tmatesoft.com>.
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 "-----------------------------------------"


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

Posted by Dmitry Pavlenko <pa...@tmatesoft.com>.
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 "-----------------------------------------"
-- 
Dmitry Pavlenko,
TMate Software,
http://subgit.com/ - git-svn bridge


Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format

Posted by Julian Foad <ju...@apache.org>.
Dmitry Pavlenko wrote:
> Here's a failing test for "no dot" case. After removing the code as I wrote 
> before the test passes.

Thank you. Committed in r1836339.

I also made a 'round-trip' version of the test and committed that in r1836340.

- Julian

Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format

Posted by Dmitry Pavlenko <pa...@tmatesoft.com>.
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


Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format

Posted by Julian Foad <ju...@apache.org>.
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

Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format

Posted by Dmitry Pavlenko <pa...@tmatesoft.com>.
Hello Julian,
OK, I could do that, but much later, in 2 weeks maybe. Now I'm busy testing 
patch functionality for other cases.
-- 
Dmitry Pavlenko,
TMate Software,
http://subgit.com/ - git-svn bridge

> Dmitry, the other thing is, would you mind making a regression test for this
> issue? That would be really good to have.
> 
> - Julian
> 
> Julian Foad wrote on 2018-07-09:
> > Dmitry Pavlenko wrote on 2018-07-09:
> > > Note that the patch looks like
> > > 
> > > Index: /tmp/wc
> > > ===================================================================
> > > diff --git a/ b/
> > > --- a/  (revision 0)
> > > +++ b/  (working copy)
> > > 
> > > Property changes on:
> > > [...]
> > 
> > Thanks for the bug report. I like removing code. Before we commit this,
> > I have a few questions (for anybody who can answer) about how this
> > should be working.
> > 
> > I think the "Property changes on: " line is not right: it should show a
> > dot for the path: "Property changes on: ."
> > 
> > Maybe the "diff --git" line should say "diff --git a/. b/." for
> > consistency (other directory paths printed there do not end with a
> > slash) and clarity. (Would that even make the "svn patch" operation work
> > correctly?)
> > 
> > We could do both fixes: make "svn diff" add a dot, to be more
> > conservative; and also make "svn patch" not require a dot, to be more
> > lenient, and to work with already-generated patches.
> > 
> > - Julian



Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format

Posted by Julian Foad <ju...@apache.org>.
Dmitry, the other thing is, would you mind making a regression test for this issue? That would be really good to have.

- Julian


Julian Foad wrote on 2018-07-09:
> Dmitry Pavlenko wrote on 2018-07-09:
> > Note that the patch looks like
> > 
> > Index: /tmp/wc
> > ===================================================================
> > diff --git a/ b/
> > --- a/  (revision 0)
> > +++ b/  (working copy)
> > 
> > Property changes on: 
> > [...]
> 
> Thanks for the bug report. I like removing code. Before we commit this, 
> I have a few questions (for anybody who can answer) about how this 
> should be working.
> 
> I think the "Property changes on: " line is not right: it should show a 
> dot for the path: "Property changes on: ."
> 
> Maybe the "diff --git" line should say "diff --git a/. b/." for 
> consistency (other directory paths printed there do not end with a 
> slash) and clarity. (Would that even make the "svn patch" operation work 
> correctly?)
> 
> We could do both fixes: make "svn diff" add a dot, to be more 
> conservative; and also make "svn patch" not require a dot, to be more 
> lenient, and to work with already-generated patches.
> 
> - Julian


-- 
- Julian

Re: [PATCH] can't "svn patch" working copy root if the patch is in --git format

Posted by Julian Foad <ju...@apache.org>.
Dmitry Pavlenko wrote on 2018-07-09:
> Note that the patch looks like
> 
> Index: /tmp/wc
> ===================================================================
> diff --git a/ b/
> --- a/  (revision 0)
> +++ b/  (working copy)
> 
> Property changes on: 
> [...]

Thanks for the bug report. I like removing code. Before we commit this, I have a few questions (for anybody who can answer) about how this should be working.

I think the "Property changes on: " line is not right: it should show a dot for the path: "Property changes on: ."

Maybe the "diff --git" line should say "diff --git a/. b/." for consistency (other directory paths printed there do not end with a slash) and clarity. (Would that even make the "svn patch" operation work correctly?)

We could do both fixes: make "svn diff" add a dot, to be more conservative; and also make "svn patch" not require a dot, to be more lenient, and to work with already-generated patches.

- Julian