2008/8/24 Neels Hofmeyr <neels_at_elego.de>:
> Hi all,
>
> We've had a recent commit clarifying the comment of
> subversion/include/svn_wc.h svn_wc_conflicted_p()
>
> It describes how svn_wc_conflicted_p() makes assumptions when conflict files
> that are expected to be there cannot be found:
> [[[
>
> /** Given a @a dir_path under version control, decide if one of its
> * entries (@a entry) is in state of conflict; return the answers in
> * @a text_conflicted_p and @a prop_conflicted_p.
> *
> * If the @a entry mentions that a text conflict file (.rej suffix)
> * exists, but it cannot be found, assume the text conflict has been
> * resolved by the user and return FALSE in @a *text_conflicted_p.
> *
> * Similarly, if the @a entry mentions that a property conflicts file
> * (.prej suffix) exists, but it cannot be found, assume the property
> * conflicts have been resolved by the user and return FALSE in
> * @a *prop_conflicted_p.
> *
> * The @a entry is not updated.
> */
> svn_error_t *
> svn_wc_conflicted_p(svn_boolean_t *text_conflicted_p,
> svn_boolean_t *prop_conflicted_p,
> const char *dir_path,
> const svn_wc_entry_t *entry,
> apr_pool_t *pool);
>
> ]]]
> (Implementation in subversion/libsvn_wc/questions.c)
>
>
> I see a very simple function made very complicated by the assumptions it makes.
>
> I also see potential problems: what if the conflict markers were set
> correctly, but writing the conflict files failed? This function would assume
> that the user removed the files and would mysteriously ignore conflicts.
>
> Plus, there doesn't seem to be a strong will behind this, because it doesn't
> even update the entries file if a conflict is assumed resolved.
>
>
> I invite anyone to name good reasons why this function changes its return
> values if it cannot find certain files.
>
> Are there good usability reasons behind this? Then maybe this should be
> redesigned to be a separate function, e.g. svn_wc_auto_resolve(), which also
> updates ENTRY once and for all?
>
>
> IMHO, it would be better to just return whatever flags are set in the ENTRY.
> It would make this function clearer and safer, and would enforce an explicit
> `svn resolve[d]' by the user, and ensure that she knows what's going on.
>
> BTW, I checked, and implementing the simpler version of this function does
> not break any tests, except the two tests that explicitly check for this
> automation (revert_tests.py 9 and 10). My diff for testing is attached. In
> revert_tests.py 9 and 10, removing conflict files is called "resolving
> manually".
>
>
> Have I got this wrong? Please comment!
In concept I'm very much in favor of removing this checking of file
state in the non-administrative region of the working copy.
(Especially if the entry is not updated to the current state).
I can't think of a reason to keep the old behavior in any of my
personal use cases and removing this test would make it easier for the
tree-conficts support and the new working copy database to integrate.
But: changing this side-effect, like introducing the concept of tree
conficts next to the original conflicts are both behavior changes.
In both cases an application previously linked to 1.5 can get into
conflicts it does not understand and can't resolve in the way it used
to be. (But I think no application should ever assume it can really
understand and/or resolve 100% of all conflicts it sees via
subversion).
So I am waiting on comments of others to see if we should see this as
a breaking change for their use cases or if we can declare it an old
implementation detail that we can change.
(I can imagine the original reason for this behavior was that we
couldn't show any information on the conflict when the .rej and .prej
files were lost.. That would most certainly be an implementation
detail of the current (or perhaps previous?) working copy version)
Bert
>
> ~Neels
>
> --
> Neels Hofmeyr -- elego Software Solutions GmbH
> Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
> phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
> http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
> Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-24 18:11:32 CEST