when running arc diff to send a review, each time a new file is in the diff then the said fail it shown in differential as if the patch was applied twice
see https://phabric.freebsd.org/D282 for example
Description
Description
Revisions and Commits
Revisions and Commits
Related Objects
Related Objects
Event Timeline
Comment Actions
I'm not sure I understand, that links looks fine to me. Which file is showing incorrectly?
Comment Actions
the file devel/pkg-gmake/Makefile is wrong
if you have a look you see it appears twice (the numbering of line restart to 1 after line 29)
Comment Actions
What can I provide more to help diagnose this, or I could probably fix it myself if one can give me a direction where to look in the code?
Comment Actions
I can't reproduce this:
$ svn st A new-file.txt $ arc diff --only --conduit-uri=https://secure.phabricator.com/ Created a new Differential diff: Diff URI: https://secure.phabricator.com/differential/diff/23679/ Included changes: A new-file.txt
Produces this: https://secure.phabricator.com/differential/diff/23679/
Some things that might be helpful:
- What version of SVN do you have?
- Does this happen if you arc diff --only, too?
- If you arc diff --only --trace, you'll see all the commands arc is running. In particular, there should be a line like this:
>>> [6] <exec> $ svn diff --depth empty --diff-cmd '/INSECURE/devtools/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'new-file.txt'
- If you run your local version of that command in isolation, does it produce a reasonable-looking diff? Can you show me a copy?
- If you pipe the diff that command produces into arc diff --raw, does that produce a normal or broken diff?
Comment Actions
So I'm using svn 1.8.9 and I can reproduce on every svn 1.8 version (I haven't tried earlier version)
arc diff --only is giving this result: https://phabric.freebsd.org/differential/diff/651/ which is broken
with --trace I got the following:
<<< [4] <exec> 766,773 us
>>> [6] <exec> $ svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/Makefile'
>>> [7] <exec> $ svn info '/home/bapt/dev/src/usr.bin/Makefile'@
>>> [8] <exec> $ svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/timeout'
>>> [9] <exec> $ svn info '/home/bapt/dev/src/usr.bin/timeout'@
>>> [10] <exec> $ svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/timeout/Makefile'
>>> [11] <exec> $ svn info '/home/bapt/dev/src/usr.bin/timeout/Makefile'@
>>> [12] <exec> $ svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/timeout/timeout.1'
>>> [13] <exec> $ svn info '/home/bapt/dev/src/usr.bin/timeout/timeout.1'@
<<< [7] <exec> 14,540 us
>>> [14] <exec> $ svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/timeout/timeout.c'
<<< [11] <exec> 16,535 us
>>> [15] <exec> $ svn info '/home/bapt/dev/src/usr.bin/timeout/timeout.c'@
<<< [13] <exec> 16,676 us
<<< [6] <exec> 23,517 us
<<< [9] <exec> 24,467 us
<<< [12] <exec> 22,453 us
<<< [10] <exec> 26,674 us
<<< [8] <exec> 28,347 us
<<< [14] <exec> 18,868 us
<<< [15] <exec> 15,341 us
>>> [16] <exec> $ svn info .
<<< [16] <exec> 9,011 us
>>> [17] <conduit> arcanist.projectinfo() <bytes = 181>
>>> [18] <http> https://phabric.freebsd.org/api/arcanist.projectinfo
<<< [18] <http> 235,660 us
<<< [17] <conduit> 235,788 us
>>> [19] <conduit> repository.query() <bytes = 192>
>>> [20] <http> https://phabric.freebsd.org/api/repository.query
<<< [20] <http> 240,972 us
<<< [19] <conduit> 241,080 us
>>> [21] <conduit> differential.creatediff() <bytes = 43916>
>>> [22] <http> https://phabric.freebsd.org/api/differential.creatediff
<<< [22] <http> 990,618 us
<<< [21] <conduit> 990,733 us
>>> [23] <event> diff.wasCreated <listeners = 0>
<<< [23] <event> 37 us
Created a new Differential diff:
Diff URI: https://phabric.freebsd.org/differential/diff/652/
Included changes:
M usr.bin/Makefile
A (dir) usr.bin/timeout
A usr.bin/timeout/Makefile
A usr.bin/timeout/timeout.1
A usr.bin/timeout/timeout.cThis following command is giving me a normal diff
svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/timeout/timeout.c'
this is also getting a normal diff:
svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/binary_safe_diff.sh' -x -U32767 'usr.bin/timeout/timeout.c' | arc diff --raw
see https://phabric.freebsd.org/differential/diff/653/
The following is also giving me a normal diff
arc diff --only usr.bin/timeout/timeout.c
see https://phabric.freebsd.org/differential/diff/654/
I also get a normal diff with:
arc diff --only usr.bin/timeout/timeout.c usr.bin/timeout/timeout.1
see https://phabric.freebsd.org/differential/diff/655/
but arc diff --only usr.bin/timeout/
gives me a wrong diff
https://phabric.freebsd.org/differential/diff/656/
So this seems to be related to when a new directory is added
Comment Actions
We pass --depth empty to prevent svn diff subdir/ from including changes to files in the subdirectory, but apparently this doesn't do anything if the directory is new:
D9921 should fix this.