[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: 1.8.x backport r1619380 - diff locally copied dir with props

From: Bert Huijben <bert_at_qqmail.nl>
Date: Mon, 16 Feb 2015 18:52:19 +0100

> -----Original Message-----
> From: Julian Foad [mailto:julianfoad_at_btopenworld.com]
> Sent: maandag 16 februari 2015 17:41
> To: dev_at_subversion.apache.org
> Cc: Bert Huijben; Stefan Sperling; Stefan Fuhrmann
> Subject: 1.8.x backport r1619380 - diff locally copied dir with props
>
> 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_at_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.

The Windows GUIs don't care about assert() as those are compiled away when
compiling in release mode on Windows. (See old mails from Stefan Kung and
me)

assert() is typically used as maintainer mode helper, while SVN_ERR_ASSERT()
is used for assertions that also happen at runtime in production code.

All assertions in diff_editor.c are just checks for preconditions that are
later also checked via if statements.

The incomplete state itself says that you can't trust the list of children
nor the list of properties on the affected nodes... So you can't trust the
diff of those either. Producing a proper error message is probably the best
thing we can do here.

        Bert
Received on 2015-02-16 18:53:42 CET

This is an archived mail posted to the Subversion Dev mailing list.