This issue is proposed for backport:

 * r1619380, r1619393 
   Fix diff of a locally copied directory with props: it showed all props
   as added instead of a diff against the copy-from props.
   Justification:
     Behaviour regression introduced in 1.8.0.
   Notes:
     r1619380 is the fix; r1619393 a test for it.
     The test on trunk@1619393 is tweaked to account for a trunk bug in the 
     display of diff headers; the backport branch provides the correct
     version for 1.8.x.
   Branch:
     ^/subversion/branches/1.8.x-r1619380
   Votes:
     +1: rhuijben, stefan2
     -0: julianfoad (assertion failure on incomplete dir -- see email)


But the code it introduces contains an assertion that can fail, as Stefan 
Sperling reported recently[1], on an 'incomplete' directory such as can result 
from an aborted update.

> assertion "kind == svn_node_dir && (status == 
svn_wc__db_status_normal || status == svn_wc__db_status_added || (status == 
svn_wc__db_status_deleted && diff_pristine))" failed: file 
"subversion/libsvn_wc/diff_editor.c", line 1057, function 
"svn_wc__diff_local_only_dir"

To reproduce the conditions, svn_wc__db_read_info() needs to return 
status=incomplete for a directory that exists in the WC at one end of the 
requested diff's revision range, but doesn't exist in the repo at the other end 
of the revision range. (That's the case these "local-only" functions are 
handling.)

Do we want to:

  * backport first, then investigate and fix this case at leisure (on trunk)
  * hold off the backport until this is investigated and fixed?

?

For the command-line client, I think fixing the currently wrong output 
outweighs the possible crash in obscure 'incomplete' cases, but for GUIs that 
priority is probably reversed.

As for a fix, I was the one who inserted that code, but I did it by copying it 
from the similar ..._local_only_file function just above and didn't know I 
needed to adjust it. I don't know what should be done with it now, so I need 
someone else to step in. In any case, whatever is done here, please consider 
keeping ..._local_only_file() in sync with it.

- Julian


[1] <http://colabti.org/irclogger/irclogger_log/svn-dev?date=2015-02-13#l109>

Reply via email to