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

Re: wc-ng patch review

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 09 Nov 2009 14:11:24 +0000

Neels J Hofmeyr wrote:
> Remove use of svn_wc_entry_t from tree-conflict detection during update
> (wc-ng).
>
> * subversion/libsvn_wc/update_editor.c
> (SVN_WC_CONFLICT_REASON_NONE): New local helper #define.
> (get_node_working_state, get_node_uri):
> New helper functions for check_tree_conflict().
> (check_tree_conflict): Replace svn_wc_entry_t use with calls to the WC DB
> via the new helper functions. Remove obsolete check for duplicate tree
> conflict involving an add of a file external (cannot reproduce).
> --This line, and those below, will be ignored--
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 40372)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -1516,6 +1516,176 @@ tree_has_local_mods(svn_boolean_t *modif
> return SVN_NO_ERROR;
> }
>
> +
> +#define SVN_WC_CONFLICT_REASON_NONE (svn_wc_conflict_reason_t)(-1)
> +
> +/* Query a node's local status found in DB at LOCAL_ABSPATH.
> + *
> + * Return POSSIBLE_CONFLICT_REASON, which is the current status / schedule /
> + * difference-between-BASE-and-WORKING of the node, paraphrased into an
> + * svn_wc_conflict_reason_t. For example, if the node is replaced by
> + * WORKING, this returns svn_wc_conflict_reason_replaced. If this node is
> + * not changed by WORKING, this returns SVN_WC_CONFLICT_REASON_NONE. If it's
> + * not SVN_WC_CONFLICT_REASON_NONE, POSSIBLE_CONFLICT_REASON can be plugged
> + * directly into a tree-conflict descriptor struct (i.e.
> + * svn_wc_conflict_description2_t) once this reason is found to conflict
> + * with an incoming action.
> + *
> + * Return LOCAL_ABSPATH's KIND in the BASE tree, which is
> + * - svn_node_file for a file or symlink,
> + * - svn_node_dir for a directory and
> + * - svn_node_none if not found in BASE;
> + * - svn_node_unknown in all other cases. ### TODO: check each of those
> + * "other cases" (e.g. DB reports "unknown", ...) and make this
> + * svn_node_none if possible.
> + */
> +static svn_error_t*
> +get_node_working_state(svn_wc_conflict_reason_t *possible_conflict_reason,
> + svn_node_kind_t *kind,

So this is supposed to get the BASE kind and the WORKING "reason"? Than
call the "kind" parameter "base_kind", or call the function
get_base_kind_and_working_state().

> + svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool)
> +{
> + svn_error_t *err;
> + svn_wc__db_status_t status;
> + svn_wc__db_kind_t db_kind;
> + svn_boolean_t base_shadowed;
> +
> + *possible_conflict_reason = SVN_WC_CONFLICT_REASON_NONE;
> + *kind = svn_node_unknown;
> +
> + err = svn_wc__db_read_info(&status,
> + &db_kind,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL,
> + &base_shadowed,
> + NULL, NULL,
> + db,
> + local_abspath,
> + scratch_pool,
> + scratch_pool);
> +
> + if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> + {
> + svn_error_clear(err);
> + *possible_conflict_reason = svn_wc_conflict_reason_missing;
> + *kind = svn_node_none;
> + }
> + else
> + {
> + SVN_ERR(err);
> +
> + /* Find the "reason" (local schedule) of the potential conflict. */
> + if (status == svn_wc__db_status_added
> + || status == svn_wc__db_status_obstructed_add)
> + {
> + *possible_conflict_reason = svn_wc_conflict_reason_added;
> + /* Is it a replace? */
> + if (base_shadowed)
> + {
> + svn_wc__db_status_t base_status;
> + SVN_ERR(svn_wc__db_base_get_info(&base_status, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL,
> + db, local_abspath,
> + scratch_pool,
> + scratch_pool));
> + if (base_status != svn_wc__db_status_not_present)
> + *possible_conflict_reason = svn_wc_conflict_reason_replaced;

If the base is shadowed, then the node kind needs to be retrieved here,
doesn't it? We don't want to return the WORKING kind.

> @@ -1660,35 +1808,34 @@ check_tree_conflict(svn_wc_conflict_desc
[...]
>
> /* Source-left repository root URL and path in repository.
> * The Source-right ones will be the same for update.
> * For switch, only the path in repository will differ, because
> * a cross-repository switch is not possible. */

The comment talks about getting two bits of data. The call beneath this
comment now gets the revision number as well. The comment says "only the
path will differ" but the revision will also differ. This is a bit
confusing. Please update the comment to match the code.

> - repos_url = entry->repos;
> - path_in_repos = svn_uri_is_child(repos_url, entry->url, pool);
> - if (path_in_repos == NULL)
> - path_in_repos = "";
> + SVN_ERR(get_node_uri(&revision, &path_in_repos, &repos_url,
> + eb->db, local_abspath, pool, pool));

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415792
Received on 2009-11-09 15:11:52 CET

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