subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julianf...@btopenworld.com>
Subject 1.8.x backport r1619380 - diff locally copied dir with props
Date Mon, 16 Feb 2015 16:41:08 GMT
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>

Mime
View raw message