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

Re: Tree-conflicts branch - log message / review

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 22 Aug 2008 16:24:51 +0100

On Wed, 2008-08-20 at 10:58 +0100, Julian Foad wrote:
> Here is a log message for the changes currently sitting on the
> tree-conflicts branch.

And here's the public API part of it with some review comments from
myself.

This patch is, roughly, part of /branches/tree-conflicts_at_32555 relative
to its last catch-up point, /trunk_at_32485.

# PUBLIC API

* subversion/include/svn_client.h
  (svn_info_t): Add a field to hold the tree conflicts info for this
node.

* subversion/include/svn_wc.h
  (svn_wc_conflict_reason_t): Add a value representing 'added'.
  (svn_wc_conflict_kind_t): Add a value representing 'tree conflict'.
  (svn_wc_operation_t): New enumeration.
  (svn_wc_conflict_description_t): Add fields representing the operation
    resulting in a conflict, and the victim of a tree conflict.
  (svn_wc_entry_t): Add a 'tree_conflict_data' field.
  (svn_wc_conflicted_p2): New function to support tree conflicts,
    superseding svn_wc_conflicted_p() which becomes deprecated.
  ### Has '###' comments.
  (svn_wc_status2_t): Add a flag indicating 'dir contains tree
conflicts'.
  ### Want a flag indicating 'this item is a tree conflict victim'.
  (svn_wc_resolved_conflict4): New function to support tree conflicts,
    superseding svn_wc_resolved_conflict3() which becomes deprecated.
  (svn_wc_read_tree_conflicts_from_entry): New function.
  (svn_wc_add_tree_conflict_data): New function.

> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (.../trunk) (revision 32485)
> +++ subversion/include/svn_client.h (.../branches/tree-conflicts) (revision 32555)
> @@ -4229,12 +4229,20 @@ typedef struct svn_info_t
> * The size of the file after being translated into its local
> * representation, or @c SVN_INFO_SIZE_UNKNOWN if
> * unknown. Not applicable for directories.
> * @since New in 1.5.
> */
> apr_size_t working_size;
> +
> + /**
> + * For a directory only, all tree-conflicted children, stored
> + * in an array of @c svn_wc_conflict_description_t,
> + * @since New in 1.6.
> + */
> + apr_array_header_t *tree_conflicts;

For backward compatibility, must move the new field to end of struct, or
rev the struct.

> /** @} */
>
> /**
> * The size of the file in the repository (untranslated,
> * e.g. without adjustment of line endings and keyword
> * expansion). Only applicable for file -- not directory -- URLs.
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (.../trunk) (revision 32485)
> +++ subversion/include/svn_wc.h (.../branches/tree-conflicts) (revision 32555)
> @@ -848,14 +848,14 @@ typedef enum svn_wc_notify_lock_state_t
> svn_wc_notify_lock_state_unlocked
> } svn_wc_notify_lock_state_t;
>
> /**
> * Structure used in the @c svn_wc_notify_func2_t function.
> *
> - * @c kind, @c content_state, @c prop_state and @c lock_state are from
> - * after @c action, not before.
> + * @c kind, @c content_state, @c prop_state, @c tree_state and
> + * @c lock_state are from after @c action, not before.
> *
> * @note If @c action is @c svn_wc_notify_update, then @c path has
> * already been installed, so it is legitimate for an implementation of
> * @c svn_wc_notify_func2_t to examine @c path in the working copy.
> *
> * @note The purpose of the @c kind, @c mime_type, @c content_state, and
> @@ -1064,12 +1064,13 @@ typedef enum svn_wc_conflict_action_t
> */
> typedef enum svn_wc_conflict_reason_t
> {
> svn_wc_conflict_reason_edited, /* local edits are already present */
> svn_wc_conflict_reason_obstructed, /* another object is in the way */
> svn_wc_conflict_reason_deleted, /* object is already schedule-delete */
> + svn_wc_conflict_reason_added, /* object is already added or schedule-add */
> svn_wc_conflict_reason_missing, /* object is unknown or missing */
> svn_wc_conflict_reason_unversioned /* object is unversioned */
>
> } svn_wc_conflict_reason_t;
>
>
> @@ -1078,19 +1079,31 @@ typedef enum svn_wc_conflict_reason_t
> *
> * @since New in 1.5.
> */
> typedef enum svn_wc_conflict_kind_t
> {
> svn_wc_conflict_kind_text, /* textual conflict (on a file) */
> - svn_wc_conflict_kind_property /* property conflict (on a file or dir) */
> -
> - /* ### Add future kinds here that represent "tree" conflicts. */
> + svn_wc_conflict_kind_property, /* property conflict (on a file or dir) */
> + svn_wc_conflict_kind_tree /* tree conflict (on a dir) */
>
> } svn_wc_conflict_kind_t;
>
>
> +/** The user operation that exposed a conflict.
> + *
> + * @since New in 1.6.
> + */
> +typedef enum svn_wc_operation_t
> +{
> + svn_wc_operation_update,
> + svn_wc_operation_switch,
> + svn_wc_operation_merge,
> +
> +} svn_wc_operation_t;

Why is 'operation' to be used only for tree conflicts? Can we make it
univeral and a separate enhancement?

> +
> +
> /** A struct that describes a conflict that has occurred in the
> * working copy. Passed to @c svn_wc_conflict_resolver_func_t.
> *
> * @note Fields may be added to the end of this structure in future
> * versions. Therefore, to preserve binary compatibility, users
> * should not directly allocate structures of this type.
> @@ -1153,12 +1166,28 @@ typedef struct svn_wc_conflict_descripti
> /** my locally-edited version of the file */
> const char *my_file;
>
> /** merged version; may contain conflict markers */
> const char *merged_file;
>
> + /** The operation that exposed the conflict.
> + * Used only for tree conflicts.
> + *
> + * @since New in 1.6.
> + */
> + svn_wc_operation_t operation;
> +
> + /** The path to the victim of a tree conflict.
> + *
> + * See the notes at the top of subversion/libsvn_wc/tree_conflicts.h
> + * for the definition of a tree conflict victim.
> + *
> + * @since New in 1.6.
> + */
> + const char *victim_path;

'victim_path' needs clearer documentation. If this can only represent
one victim, then why the separate field at all? How do other fields
(esp. base_file etc.) behave if this represents a tree conflict?

> +
> } svn_wc_conflict_description_t;
>
>
> /** The way in which the conflict callback chooses a course of action.
> *
> * @since New in 1.5.
> @@ -1859,12 +1888,17 @@ typedef struct svn_wc_entry_t
> * ### all entries. Maybe some future extensibility would make this
> * ### field meaningful on entries besides this_dir.
> *
> * @since New in 1.5. */
> svn_depth_t depth;
>
> + /** Serialized data for all of the tree conflicts detected in this_dir.
> + *
> + * @since New in 1.6. */
> + const char *tree_conflict_data;
> +
> /* IMPORTANT: If you extend this structure, check the following functions in
> * subversion/libsvn_wc/entries.c, to see if you need to extend them as well.
> *
> * svn_wc__atts_to_entry()
> * svn_wc_entry_dup()
> * alloc_entry()
> @@ -1955,16 +1989,37 @@ svn_wc_entry_t *
> svn_wc_entry_dup(const svn_wc_entry_t *entry,
> apr_pool_t *pool);
>
>
> /** 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.
> + * @a text_conflicted_p, @a prop_conflicted_p and @a tree_conflicted_p.
> + * @a tree_conflicted_p refers to @a entry being the victim of a conflict.
> *
> - * (If the entry mentions that a .rej or .prej exist, but they are
> - * both removed, assume the conflict has been resolved by the user.)
> + * Only files can be text conflicted.
> + * Only directories can be tree conflicted.
> + * Property conflicts apply to both.
> + *
> + * If the entry mentions that a text conflict file, property conflicts
> + * file, or a tree conflict report file (### obsolete) exist, but they are all
> + * removed, assume the conflict has been resolved by the user. ### and adjust the entry?
> + * ### Shouldn't this WC function just report what the entry says, and let the client
> + * clear the conflict if files are gone (etc.) if that's what it wants to do?
> + *
> + * @since New in 1.6.
> + */
> +svn_error_t *
> +svn_wc_conflicted_p2(svn_boolean_t *text_conflicted_p,
> + svn_boolean_t *prop_conflicted_p,
> + svn_boolean_t *tree_conflicted_p,
> + const char *dir_path,
> + const svn_wc_entry_t *entry,
> + apr_pool_t *pool);
> +
> +/** Like svn_wc_conflicted_p2, but without the capability to
> + * detect tree conflicts.
> */
> 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,
> @@ -2352,12 +2407,17 @@ typedef struct svn_wc_status2_t
> * @since New in 1.3
> */
> const char *ood_last_cmt_author;
>
> /** @} */
>
> + /** Set @c TRUE if the entry is a directory containing tree conflicts.
> + * @since New in 1.6
> + */
> + svn_boolean_t tree_conflicted;
> +
> /* NOTE! Please update svn_wc_dup_status2() when adding new fields here. */
> } svn_wc_status2_t;
>
>
>
> /**
> @@ -2963,13 +3023,14 @@ svn_wc_remove_from_revision_control(svn_
>
>
> /**
> * Assuming @a path is under version control and in a state of conflict,
> * then take @a path *out* of this state. If @a resolve_text is TRUE then
> * any text conflict is resolved, if @a resolve_props is TRUE then any
> - * property conflicts are resolved.
> + * property conflicts are resolved, if @a resolve_tree is TRUE then any
> + * tree conflicts are resolved.
> *
> * If @a depth is @c svn_depth_empty, act only on @a path; if
> * @c svn_depth_files, resolve @a path and its conflicted file
> * children (if any); if @c svn_depth_immediates, resolve @a path and
> * all its immediate conflicted children (both files and directories,
> * if any); if @c svn_depth_infinity, resolve @a path and every
> @@ -3000,17 +3061,38 @@ svn_wc_remove_from_revision_control(svn_
> *
> * If @a path is not under version control, return @c SVN_ERR_ENTRY_NOT_FOUND.
> * If @a path isn't in a state of conflict to begin with, do nothing, and
> * return @c SVN_NO_ERROR.
> *
> * If @c path was successfully taken out of a state of conflict, report this
> - * information to @c notify_func (if non-_at_c NULL.) If only text or only
> - * property conflict resolution was requested, and it was successful, then
> - * success gets reported.
> + * information to @c notify_func (if non-_at_c NULL.) If only text, only
> + * property, or only tree conflict resolution was requested, and it was
> + * successful, then success gets reported.
> *
> - * @since New in 1.5.
> + * @since New in 1.6.
> + */
> +svn_error_t *
> +svn_wc_resolved_conflict4(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + svn_boolean_t resolve_text,
> + svn_boolean_t resolve_props,
> + svn_boolean_t resolve_tree,
> + svn_depth_t depth,
> + svn_wc_conflict_choice_t conflict_choice,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool);
> +
> +
> +/**
> + * Similar to svn_wc_resolved_conflict4(), but without tree-conflict
> + * resolution support.
> + *
> + * @deprecated Provided for backward compatibility with the 1.5 API.
> */
> svn_error_t *
> svn_wc_resolved_conflict3(const char *path,
> svn_wc_adm_access_t *adm_access,
> svn_boolean_t resolve_text,
> svn_boolean_t resolve_props,
> @@ -4857,12 +4939,36 @@ svn_wc_set_changelist(const char *path,
> svn_wc_notify_func2_t notify_func,
> void *notify_baton,
> apr_pool_t *pool);
>
> /** @} */
>
> +/**
> + * Read tree conflict descriptions from @a dir_entry.
> + * Append pointers to newly allocated svn_wc_conflict_description_t
> + * objects to the array pointed to by @a conflicts.
> + * Do all allocations in @a pool.
> + *
> + * @since New in 1.6.
> + */
> +svn_error_t *
> +svn_wc_read_tree_conflicts_from_entry(apr_array_header_t *conflicts,
> + const svn_wc_entry_t *dir_entry,
> + apr_pool_t *pool);
> +
> +/**
> + * Add a tree conflict to the directory entry belonging to @a adm_access.
> + * Pass a description of the new tree conflict in @a conflict.
> + * Do all allocations in @a pool.
> + *
> + * @since New in 1.6.
> + */
> +svn_error_t *
> +svn_wc_add_tree_conflict_data(const svn_wc_conflict_description_t *conflict,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> ^L
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
>
> #endif /* SVN_WC_H */

- Julian

---------------------------------------------------------------------
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-22 17:25:30 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.