Page MenuHomePhabricator

New files in svn not properly sent to differential
Closed, ResolvedPublic


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 for example

Event Timeline

bapt raised the priority of this task from to Needs Triage.
bapt updated the task description. (Show Details)
bapt added projects: Arcanist, Differential.
bapt added a subscriber: bapt.

I'm not sure I understand, that links looks fine to me. Which file is showing incorrectly?

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)

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?

I can't reproduce this:

$ svn st
A       new-file.txt
$ arc diff --only --conduit-uri=
Created a new Differential diff:
        Diff URI:

Included changes:
  A       new-file.txt

Produces this:

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/' -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?

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: 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/' -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/' -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/' -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/' -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/' -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>
<<< [18] <http> 235,660 us
<<< [17] <conduit> 235,788 us
>>> [19] <conduit> repository.query() <bytes = 192>
>>> [20] <http>
<<< [20] <http> 240,972 us
<<< [19] <conduit> 241,080 us
>>> [21] <conduit> differential.creatediff() <bytes = 43916>
>>> [22] <http>
<<< [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:

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.c

This following command is giving me a normal diff
svn diff --depth empty --diff-cmd '/usr/local/lib/php/arcanist/scripts/repository/' -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/' -x -U32767 'usr.bin/timeout/timeout.c' | arc diff --raw
The following is also giving me a normal diff
arc diff --only usr.bin/timeout/timeout.c

I also get a normal diff with:

arc diff --only usr.bin/timeout/timeout.c usr.bin/timeout/timeout.1

but arc diff --only usr.bin/timeout/
gives me a wrong diff

So this seems to be related to when a new directory is added

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.

I can confirm it fixes it, thank you very much!

epriestley claimed this task.