You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2011/10/05 12:09:31 UTC

svn commit: r1179135 - in /subversion/branches/showing-merge-info/subversion: include/private/svn_client_private.h include/svn_client.h libsvn_client/prop_commands.c libsvn_client/url.c svn/main.c svn/mergeinfo-cmd.c

Author: julianfoad
Date: Wed Oct  5 10:09:30 2011
New Revision: 1179135

URL: http://svn.apache.org/viewvc?rev=1179135&view=rev
Log:
On the showing-merge-info branch: Check in some work in progress.

Make 'svn mergeinfo' print a more friendly description of the merging
situation.

* subversion/svn/main.c
  (svn_cl__cmd_table): Describe the source branch argument to 'svn mergeinfo'
    as optional, defaulting to 'the parent branch'.

* subversion/svn/mergeinfo-cmd.c
  (has_merge_prop_change): New function.
  (print_log_rev): Show whether the rev is operative, no-op or a merge.
  (location_is_resolved, target_for_display, targets_are_same_branch,
   find_source_branch): New functions.
  (svn_cl__mergeinfo): If source branch is not specified, find the parent
    branch. Check whether source and target are related. Print some high
    level info about source and target branches. Print both merged and
    eligible revs.

* subversion/include/private/svn_client_private.h
  (SVN_PROP_BRANCHING_ROOT): New property name.
  (svn_client__check_branch_root_marker): New function.

* subversion/libsvn_client/url.c
  (get_branch_root_marker, svn_client__check_branch_root_marker): New
    functions.

Define a new 'target' type encapsulating URL or local path, peg rev,
and operative rev.

* subversion/include/svn_client.h
  (svn_client_target_t): New type.
  (svn_client__target, svn_client__resolve_location,
   svn_client__resolve_target_location, svn_client_resolve_repo_location):
    New functions.
  (svn_client_propget5, svn_client_proplist4, svn_client_export6,
   svn_client_list3, svn_client_cat3, svn_client_info4): Revised APIs using
    svn_client_target_t instead of separate arguments.

* subversion/libsvn_client/prop_commands.c
  (svn_client_propget5): Revised API, wrapping svn_client_propget4().

* subversion/libsvn_client/url.c
  (svn_client__target, svn_client__resolve_location,
   svn_client__resolve_target_location): New functions.

Modified:
    subversion/branches/showing-merge-info/subversion/include/private/svn_client_private.h
    subversion/branches/showing-merge-info/subversion/include/svn_client.h
    subversion/branches/showing-merge-info/subversion/libsvn_client/prop_commands.c
    subversion/branches/showing-merge-info/subversion/libsvn_client/url.c
    subversion/branches/showing-merge-info/subversion/svn/main.c
    subversion/branches/showing-merge-info/subversion/svn/mergeinfo-cmd.c

Modified: subversion/branches/showing-merge-info/subversion/include/private/svn_client_private.h
URL: http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/subversion/include/private/svn_client_private.h?rev=1179135&r1=1179134&r2=1179135&view=diff
==============================================================================
--- subversion/branches/showing-merge-info/subversion/include/private/svn_client_private.h (original)
+++ subversion/branches/showing-merge-info/subversion/include/private/svn_client_private.h Wed Oct  5 10:09:30 2011
@@ -59,6 +59,20 @@ svn_client__create_status(svn_client_sta
                           apr_pool_t *result_pool,
                           apr_pool_t *scratch_pool);
 
+/* This property marks a branch root. Branches with the same value of this
+ * property are mergeable. */
+#define SVN_PROP_BRANCHING_ROOT "svn:ignore" /* ### should be "svn:branching-root" */
+
+/* Set *MARKER to the project-root marker that is common to SOURCE and
+ * TARGET, or to NULL if neither has such a marker.
+ * If only one has such a marker or they are different, throw an error. */
+svn_error_t *
+svn_client__check_branch_root_marker(const char **marker,
+                                     svn_client_target_t *source,
+                                     svn_client_target_t *target,
+                                     svn_client_ctx_t *ctx,
+                                     apr_pool_t *pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/branches/showing-merge-info/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/subversion/include/svn_client.h?rev=1179135&r1=1179134&r2=1179135&view=diff
==============================================================================
--- subversion/branches/showing-merge-info/subversion/include/svn_client.h (original)
+++ subversion/branches/showing-merge-info/subversion/include/svn_client.h Wed Oct  5 10:09:30 2011
@@ -1009,6 +1009,50 @@ svn_client_create_context(svn_client_ctx
  * @{
  */
 
+/* */
+typedef struct svn_client_target_t
+  {
+    const char *path_or_url;
+    const char *abspath_or_url;
+    svn_opt_revision_t peg_revision;
+    svn_opt_revision_t revision;
+
+    /* The following fields are the resolved location after contacting
+     * the repository, else NULL or SVN_INVALID_REVNUM. */
+    const char *repos_root_url;
+    const char *repos_uuid;
+    const char *repos_relpath;
+    svn_revnum_t repos_revnum;
+
+    /* The pool in which to allocate new fields */
+    apr_pool_t *pool;
+  } svn_client_target_t;
+
+/* Allocate a svn_client_target_t structure. Initialize pool and
+ * abspath_or_url fields. */
+svn_error_t *
+svn_client__target(svn_client_target_t **target,
+                   const char *path_or_url,
+                   apr_pool_t *pool);
+
+/* */
+svn_error_t *
+svn_client__resolve_location(const char **repo_root_url_p,
+                             const char **repo_uuid_p,
+                             svn_revnum_t *repo_revnum_p,
+                             const char **repo_relpath_p,
+                             const char *path_or_url,
+                             const svn_opt_revision_t *peg_revision,
+                             const svn_opt_revision_t *revision,
+                             svn_client_ctx_t *ctx,
+                             apr_pool_t *pool);
+
+/* */
+svn_error_t *
+svn_client__resolve_target_location(svn_client_target_t *target,
+                                    svn_client_ctx_t *ctx,
+                                    apr_pool_t *pool);
+
 /**
  * Pull remaining target arguments from @a os into @a *targets_p,
  * converting them to UTF-8, followed by targets from @a known_targets
@@ -4495,7 +4539,9 @@ svn_client_revprop_set(const char *propn
  * Don't store any path, not even @a target, if it does not have a
  * property named @a propname.
  *
- * If @a revision->kind is #svn_opt_revision_unspecified, then: get
+ * @a target ...
+ *
+ * ###? If @a revision->kind is #svn_opt_revision_unspecified, then: get
  * properties from the working copy if @a target is a working copy
  * path, or from the repository head if @a target is a URL.  Else get
  * the properties as of @a revision.  The actual node revision
@@ -4525,7 +4571,23 @@ svn_client_revprop_set(const char *propn
  * This function returns SVN_ERR_UNVERSIONED_RESOURCE when it is called on
  * unversioned nodes.
  *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_propget5(apr_hash_t **props,
+                    const char *propname,
+                    svn_client_target_t *target,
+                    svn_depth_t depth,
+                    const apr_array_header_t *changelists,
+                    svn_client_ctx_t *ctx,
+                    apr_pool_t *result_pool,
+                    apr_pool_t *scratch_pool);
+
+/* Like svn_client_propget5() but using separate path and revision arguments
+ * instead of svn_client_target_t.
+ *
  * @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
  */
 svn_error_t *
 svn_client_propget4(apr_hash_t **props,
@@ -4653,7 +4715,22 @@ svn_client_revprop_get(const char *propn
  *
  * If @a target is not found, return the error #SVN_ERR_ENTRY_NOT_FOUND.
  *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_proplist4(svn_client_target_t *target,
+                     svn_depth_t depth,
+                     const apr_array_header_t *changelists,
+                     svn_proplist_receiver_t receiver,
+                     void *receiver_baton,
+                     svn_client_ctx_t *ctx,
+                     apr_pool_t *pool);
+
+/* Like svn_client_proplist4() but using separate path and revision arguments
+ * instead of svn_client_target_t.
+ *
  * @since New in 1.5.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
  */
 svn_error_t *
 svn_client_proplist3(const char *target,
@@ -4790,7 +4867,24 @@ svn_client_revprop_list(apr_hash_t **pro
  *
  * All allocations are done in @a pool.
  *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_export6(svn_client_target_t *target,
+                   const char *to_path,
+                   svn_boolean_t overwrite,
+                   svn_boolean_t ignore_externals,
+                   svn_boolean_t ignore_keywords,
+                   svn_depth_t depth,
+                   const char *native_eol,
+                   svn_client_ctx_t *ctx,
+                   apr_pool_t *pool);
+
+/* Like svn_client_export6() but using separate path and revision arguments
+ * instead of svn_client_target_t.
+ *
  * @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
  */
 svn_error_t *
 svn_client_export5(svn_revnum_t *result_rev,
@@ -4952,7 +5046,23 @@ typedef svn_error_t *(*svn_client_list_f
  * otherwise simply bitwise OR together the combination of @c SVN_DIRENT_
  * fields you care about.
  *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_list3(svn_client_target_t *target,
+                 svn_depth_t depth,
+                 apr_uint32_t dirent_fields,
+                 svn_boolean_t fetch_locks,
+                 svn_client_list_func_t list_func,
+                 void *baton,
+                 svn_client_ctx_t *ctx,
+                 apr_pool_t *pool);
+
+/* Like svn_client_list3() but using separate path and revision arguments
+ * instead of svn_client_target_t.
+ *
  * @since New in 1.5.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
  */
 svn_error_t *
 svn_client_list2(const char *path_or_url,
@@ -5080,12 +5190,24 @@ svn_client_ls(apr_hash_t **dirents,
  *         determined. <br>
  *         If no error occurred, return #SVN_NO_ERROR.
  *
- * @since New in 1.2.
+ * @since New in 1.8.
  *
  * @see #svn_client_ctx_t <br> @ref clnt_revisions for
  *      a discussion of operative and peg revisions.
  */
 svn_error_t *
+svn_client_cat3(svn_stream_t *out,
+                svn_client_target_t *target,
+                svn_client_ctx_t *ctx,
+                apr_pool_t *pool);
+
+/* Like svn_client_cat3() but using separate path and revision arguments
+ * instead of svn_client_target_t.
+ *
+ * @since New in 1.2.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
+ */
+svn_error_t *
 svn_client_cat2(svn_stream_t *out,
                 const char *path_or_url,
                 const svn_opt_revision_t *peg_revision,
@@ -5093,7 +5215,6 @@ svn_client_cat2(svn_stream_t *out,
                 svn_client_ctx_t *ctx,
                 apr_pool_t *pool);
 
-
 /**
  * Similar to svn_client_cat2() except that the peg revision is always
  * the same as @a revision.
@@ -5521,6 +5642,9 @@ typedef svn_error_t *(*svn_client_info_r
  * or @c NULL, then information will be pulled solely from the working copy;
  * no network connections will be made.
  *
+ * ### Should not process svn_opt_revision_working as a repo request. Should
+ * error, or process it as a WC request?
+ *
  * Otherwise, information will be pulled from a repository.  The
  * actual node revision selected is determined by the @a abspath_or_url
  * as it exists in @a peg_revision.  If @a peg_revision->kind is
@@ -5556,7 +5680,24 @@ typedef svn_error_t *(*svn_client_info_r
  * it's a member of one of those changelists.  If @a changelists is
  * empty (or altogether @c NULL), no changelist filtering occurs.
  *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_info4(svn_client_target_t *target,
+                 svn_depth_t depth,
+                 svn_boolean_t fetch_excluded,
+                 svn_boolean_t fetch_actual_only,
+                 const apr_array_header_t *changelists,
+                 svn_client_info_receiver2_t receiver,
+                 void *receiver_baton,
+                 svn_client_ctx_t *ctx,
+                 apr_pool_t *scratch_pool);
+
+/* Like svn_client_info4() but using separate path and revision arguments
+ * instead of svn_client_target_t.
+ *
  * @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
  */
 svn_error_t *
 svn_client_info3(const char *abspath_or_url,
@@ -5784,6 +5925,12 @@ svn_client_url_from_path(const char **ur
                          apr_pool_t *pool);
 
 
+/* */
+svn_error_t *
+svn_client_resolve_repo_location(svn_client_target_t *target,
+                                 svn_client_ctx_t *ctx,
+                                 apr_pool_t *pool);
+
 /** Set @a *url to the repository root URL of the repository in which
  * @a path_or_url is versioned (or scheduled to be versioned),
  * allocated in @a pool.  @a ctx is required for possible repository

Modified: subversion/branches/showing-merge-info/subversion/libsvn_client/prop_commands.c
URL: http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/subversion/libsvn_client/prop_commands.c?rev=1179135&r1=1179134&r2=1179135&view=diff
==============================================================================
--- subversion/branches/showing-merge-info/subversion/libsvn_client/prop_commands.c (original)
+++ subversion/branches/showing-merge-info/subversion/libsvn_client/prop_commands.c Wed Oct  5 10:09:30 2011
@@ -799,6 +799,25 @@ get_prop_from_wc(apr_hash_t *props,
 
 /* Note: this implementation is very similar to svn_client_proplist. */
 svn_error_t *
+svn_client_propget5(apr_hash_t **props,
+                    const char *propname,
+                    svn_client_target_t *target,
+                    svn_depth_t depth,
+                    const apr_array_header_t *changelists,
+                    svn_client_ctx_t *ctx,
+                    apr_pool_t *result_pool,
+                    apr_pool_t *scratch_pool)
+{
+  SVN_ERR(svn_client_propget4(props, propname,
+                              target->abspath_or_url,
+                              &target->peg_revision, &target->revision,
+                              &target->repos_revnum,
+                              depth, changelists,
+                              ctx, result_pool, scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
 svn_client_propget4(apr_hash_t **props,
                     const char *propname,
                     const char *target,

Modified: subversion/branches/showing-merge-info/subversion/libsvn_client/url.c
URL: http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/subversion/libsvn_client/url.c?rev=1179135&r1=1179134&r2=1179135&view=diff
==============================================================================
--- subversion/branches/showing-merge-info/subversion/libsvn_client/url.c (original)
+++ subversion/branches/showing-merge-info/subversion/libsvn_client/url.c Wed Oct  5 10:09:30 2011
@@ -34,6 +34,7 @@
 #include "svn_dirent_uri.h"
 #include "svn_path.h"
 
+#include "private/svn_client_private.h"
 #include "private/svn_wc_private.h"
 #include "client.h"
 #include "svn_private_config.h"
@@ -76,3 +77,148 @@ svn_client_root_url_from_path(const char
            svn_client__get_repos_root(url, NULL, path_or_url,
                                       ctx, pool, pool));
 }
+
+svn_error_t *
+svn_client__target(svn_client_target_t **target_p,
+                   const char *path_or_url,
+                   apr_pool_t *pool)
+{
+  *target_p = apr_pcalloc(pool, sizeof(**target_p));
+
+  (*target_p)->pool = pool;
+  (*target_p)->path_or_url = path_or_url;
+  if (svn_path_is_url(path_or_url))
+    (*target_p)->abspath_or_url = path_or_url;
+  else
+    svn_error_clear(svn_dirent_get_absolute(&(*target_p)->abspath_or_url,
+                                    path_or_url, pool));
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__resolve_location(const char **repo_root_url_p,
+                             const char **repo_uuid_p,
+                             svn_revnum_t *repo_revnum_p,
+                             const char **repo_relpath_p,
+                             const char *path_or_url,
+                             const svn_opt_revision_t *peg_revision,
+                             const svn_opt_revision_t *revision,
+                             svn_client_ctx_t *ctx,
+                             apr_pool_t *pool)
+{
+  const char *abspath_or_url;
+  const char *repos_root_url;
+
+  if (svn_path_is_url(path_or_url))
+    abspath_or_url = path_or_url;
+  else
+    SVN_ERR(svn_dirent_get_absolute(&abspath_or_url, path_or_url, pool));
+
+  SVN_ERR(svn_client__get_repos_root(&repos_root_url, repo_uuid_p,
+                                     abspath_or_url,
+                                     ctx, pool, pool));
+  if (repo_root_url_p)
+    *repo_root_url_p = repos_root_url;
+
+  if (repo_relpath_p)
+    {
+      const char *url;
+
+      SVN_ERR(svn_client_url_from_path2(&url, path_or_url, ctx, pool, pool));
+      if (! url)
+        return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
+                                 _("Path '%s' has no URL in the repository"),
+                                 path_or_url);
+      *repo_relpath_p = svn_uri_skip_ancestor(repos_root_url, url, pool);
+    }
+
+  if (repo_revnum_p)
+    {
+      svn_ra_session_t *ra_session = NULL;
+
+      SVN_ERR(svn_client__get_revision_number(repo_revnum_p,
+                                              NULL /* youngest */,
+                                              ctx->wc_ctx,
+                                              abspath_or_url,
+                                              ra_session,
+                                              peg_revision,
+                                              pool));
+    }
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__resolve_target_location(svn_client_target_t *target,
+                                    svn_client_ctx_t *ctx,
+                                    apr_pool_t *pool)
+{
+  SVN_ERR(svn_client__resolve_location(&target->repos_root_url,
+                                       &target->repos_uuid,
+                                       &target->repos_revnum,
+                                       &target->repos_relpath,
+                                       target->path_or_url,
+                                       &target->peg_revision,
+                                       &target->revision,
+                                       ctx, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Set *MARKER to the value of the branch-root-identifier of TARGET. */
+static svn_error_t *
+get_branch_root_marker(const char **marker,
+                       svn_client_target_t *target,
+                       svn_client_ctx_t *ctx,
+                       apr_pool_t *pool)
+{
+  apr_hash_t *props;
+  const char *propname = SVN_PROP_BRANCHING_ROOT;
+  svn_string_t *propval;
+
+  SVN_ERR(svn_client_propget5(&props, propname, target, svn_depth_empty,
+                              NULL, ctx, pool, pool));
+  propval = apr_hash_get(props, target->abspath_or_url, APR_HASH_KEY_STRING);
+  *marker = propval ? propval->data : NULL;
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__check_branch_root_marker(const char **marker,
+                                     svn_client_target_t *source,
+                                     svn_client_target_t *target,
+                                     svn_client_ctx_t *ctx,
+                                     apr_pool_t *pool)
+{
+  const char *source_marker, *target_marker;
+
+  /* Check the source's and target's branch-marker properties. */
+  SVN_ERR(get_branch_root_marker(&target_marker, target, ctx, pool));
+  SVN_ERR(get_branch_root_marker(&source_marker, source, ctx, pool));
+  if (! source_marker && ! target_marker)
+    {
+      /* Old-style branches, not marked as such. Marker will be NULL. */
+    }
+  else if (! source_marker || ! target_marker)
+    {
+      if (source_marker)
+        return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
+                                 _("Source is marked as a branch of "
+                                   "project '%s', but target is not marked"),
+                                 source_marker);
+      else
+        return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
+                                 _("Target is marked as a branch of "
+                                   "project '%s', but source is not marked"),
+                                 target_marker);
+    }
+  else if (strcmp(source_marker, target_marker) != 0)
+    {
+      /* ### The '.99' is just for display tidiness when I'm messing about
+       * with using 'svn:ignore' as the branch marker property. */
+      return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
+                               _("error: Source is marked as branch of project '%.99s' "
+                                 "but target is marked as branch of project '%.99s'"),
+                               source_marker, target_marker);
+    }
+  *marker = source_marker;
+  return SVN_NO_ERROR;
+}

Modified: subversion/branches/showing-merge-info/subversion/svn/main.c
URL: http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/subversion/svn/main.c?rev=1179135&r1=1179134&r2=1179135&view=diff
==============================================================================
--- subversion/branches/showing-merge-info/subversion/svn/main.c (original)
+++ subversion/branches/showing-merge-info/subversion/svn/main.c Wed Oct  5 10:09:30 2011
@@ -926,10 +926,11 @@ const svn_opt_subcommand_desc2_t svn_cl_
 
   { "mergeinfo", svn_cl__mergeinfo, {0}, N_
     ("Display merge-related information.\n"
-     "usage: mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
+     "usage: mergeinfo [SOURCE[@REV]] [TARGET[@REV]]\n"
      "\n"
      "  Display information related to merges (or potential merges) between\n"
-     "  SOURCE and TARGET (default: '.').  Display the type of information\n"
+     "  SOURCE and TARGET.  The default SOURCE is the parent branch, and\n"
+     "  the default TARGET is '.'.  Display the type of information\n"
      "  specified by the --show-revs option.  If --show-revs isn't passed,\n"
      "  it defaults to --show-revs='merged'.\n"
      "\n"

Modified: subversion/branches/showing-merge-info/subversion/svn/mergeinfo-cmd.c
URL: http://svn.apache.org/viewvc/subversion/branches/showing-merge-info/subversion/svn/mergeinfo-cmd.c?rev=1179135&r1=1179134&r2=1179135&view=diff
==============================================================================
--- subversion/branches/showing-merge-info/subversion/svn/mergeinfo-cmd.c (original)
+++ subversion/branches/showing-merge-info/subversion/svn/mergeinfo-cmd.c Wed Oct  5 10:09:30 2011
@@ -36,21 +36,138 @@
 #include "svn_types.h"
 #include "cl.h"
 
+#include "private/svn_client_private.h"
+#include "private/svn_opt_private.h"
 #include "svn_private_config.h"
 
 
 /*** Code. ***/
 
+/* Set *CONTENT_MODIFIED if so, else set *MERGEINFO_CHANGED if so, else set
+ * both to FALSE. */
+static svn_error_t *
+has_merge_prop_change(svn_boolean_t *content_modified,
+                      svn_boolean_t *mergeinfo_changed,
+                      apr_hash_t *changed_paths,
+                      apr_pool_t *pool)
+{
+  apr_hash_index_t *hi;
+
+  *content_modified = FALSE;
+  *mergeinfo_changed = FALSE;
+
+  for (hi = apr_hash_first(pool, changed_paths); hi; hi = apr_hash_next(hi))
+    {
+      const char *path = svn__apr_hash_index_key(hi);
+      svn_log_changed_path2_t *cp = svn__apr_hash_index_val(hi);
+
+      SVN_DBG(("%c %s%s %c %s\n", cp->action, svn_tristate__to_word(cp->text_modified), svn_tristate__to_word(cp->props_modified), cp->node_kind == svn_node_dir ? 'D' : 'f', path));
+      if (cp->action == 'A' || cp->action == 'D'
+          || cp->text_modified == svn_tristate_true)
+        {
+          *content_modified = TRUE;
+          return SVN_NO_ERROR;
+        }
+    }
+  return SVN_NO_ERROR;
+}
+
 /* Implements the svn_log_entry_receiver_t interface. */
 static svn_error_t *
 print_log_rev(void *baton,
               svn_log_entry_t *log_entry,
               apr_pool_t *pool)
 {
-  if (log_entry->non_inheritable)
-    SVN_ERR(svn_cmdline_printf(pool, "r%ld*\n", log_entry->revision));
+  svn_boolean_t content_modified;
+  svn_boolean_t mergeinfo_changed;
+  const char *kind;
+
+  /* Identify this source-rev as "original change" or "no-op" or "a merge" */
+  SVN_ERR(has_merge_prop_change(&content_modified, &mergeinfo_changed,
+                                log_entry->changed_paths2, pool));
+  if (content_modified)
+    {
+      kind = "operative (at least on some paths)";
+    }
+  else if (mergeinfo_changed)
+    {
+      kind = "merge";
+    }
   else
-    SVN_ERR(svn_cmdline_printf(pool, "r%ld\n", log_entry->revision));
+    {
+      /* ### No-op revs aren't currently sent to this callback function at
+       * all, but later we may use this function on such revs. */
+      kind = "no-op";
+    }
+  SVN_ERR(svn_cmdline_printf(pool, "r%ld%s%s%s -- %s\n", log_entry->revision,
+                             log_entry->non_inheritable ? "*" : " ",
+                             log_entry->subtractive_merge ? " (reverse)" : "",
+                             log_entry->has_children ? " (has children)" : "",
+                             kind));
+  return SVN_NO_ERROR;
+}
+
+/* */
+static svn_boolean_t
+location_is_resolved(const svn_client_target_t *location)
+{
+  return (location->repos_uuid != NULL);
+}
+
+/* */
+static const char *
+target_for_display(const svn_client_target_t *target,
+                   apr_pool_t *pool)
+{
+  SVN_ERR_ASSERT_NO_RETURN(location_is_resolved(target));
+  if (target->revision.kind == svn_opt_revision_working)
+    {
+      SVN_ERR_ASSERT_NO_RETURN(target->peg_revision.kind == svn_opt_revision_working);
+      return apr_psprintf(pool, "^/%s (working copy)",
+                          target->repos_relpath);
+    }
+  if (target->revision.kind == svn_opt_revision_base)
+    {
+      SVN_ERR_ASSERT_NO_RETURN(target->peg_revision.kind == svn_opt_revision_base);
+      return apr_psprintf(pool, "^/%s (wc base = r%ld)",
+                          target->repos_relpath, target->repos_revnum);
+    }
+  return apr_psprintf(pool, "^/%s (r%ld)",
+                      target->repos_relpath, target->repos_revnum);
+}
+
+/* Return TRUE iff SOURCE and TARGET refer to the same repository branch. */
+static svn_boolean_t
+targets_are_same_branch(svn_client_target_t *source,
+                        svn_client_target_t *target,
+                        apr_pool_t *pool)
+{
+  return (strcmp(source->repos_relpath, target->repos_relpath) == 0);
+}
+
+/* Find the preferred "parent" branch.  At the moment, returns the
+ * copyfrom path (at peg rev r1170000). */
+static svn_error_t *
+find_source_branch(svn_client_target_t **source_p,
+                   svn_client_target_t *target,
+                   svn_client_ctx_t *ctx,
+                   apr_pool_t *pool)
+{
+  apr_array_header_t *suggestions;
+  const char *copyfrom_url;
+
+  /* This isn't properly doc'd, but the first result it gives is the
+   * copyfrom source URL. */
+  SVN_ERR(svn_client_suggest_merge_sources(&suggestions,
+                                           target->path_or_url,
+                                           &target->peg_revision,
+                                           ctx, pool));
+  copyfrom_url = APR_ARRAY_IDX(suggestions, 0, const char *);
+
+  SVN_ERR(svn_client__target(source_p, copyfrom_url, pool));
+  (*source_p)->peg_revision.kind = svn_opt_revision_number;
+  (*source_p)->peg_revision.value.number = 1170000;
+  (*source_p)->revision.kind = svn_opt_revision_unspecified;
 
   return SVN_NO_ERROR;
 }
@@ -64,72 +181,111 @@ svn_cl__mergeinfo(apr_getopt_t *os,
   svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
   svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
   apr_array_header_t *targets;
-  const char *source, *target;
-  svn_opt_revision_t src_peg_revision, tgt_peg_revision;
+  svn_client_target_t *source;
+  svn_client_target_t *target;
+  const char *marker;
   /* Default to depth empty. */
   svn_depth_t depth = opt_state->depth == svn_depth_unknown
-    ? svn_depth_empty : opt_state->depth;
+    ? svn_depth_infinity : opt_state->depth;
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
                                                       ctx, FALSE, pool));
 
-  /* We expect a single source URL followed by a single target --
-     nothing more, nothing less. */
-  if (targets->nelts < 1)
-    return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
-                            _("Not enough arguments given"));
   if (targets->nelts > 2)
     return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
                             _("Too many arguments given"));
 
-  /* Parse the SOURCE-URL[@REV] argument. */
-  SVN_ERR(svn_opt_parse_path(&src_peg_revision, &source,
-                             APR_ARRAY_IDX(targets, 0, const char *), pool));
-
-  /* Parse the TARGET[@REV] argument (if provided). */
+  /* Locate the target branch: the second argument or this dir. */
   if (targets->nelts == 2)
     {
-      SVN_ERR(svn_opt_parse_path(&tgt_peg_revision, &target,
+      SVN_ERR(svn_client__target(&target, "", pool));
+      SVN_ERR(svn_opt_parse_path(&target->peg_revision, &target->path_or_url,
                                  APR_ARRAY_IDX(targets, 1, const char *),
                                  pool));
+      target->revision.kind = svn_opt_revision_unspecified;
     }
   else
     {
-      target = "";
-      tgt_peg_revision.kind = svn_opt_revision_unspecified;
+      SVN_ERR(svn_client__target(&target, "", pool));
+      target->peg_revision.kind = svn_opt_revision_working;
+      target->revision.kind = svn_opt_revision_working;
     }
 
-  /* If no peg-rev was attached to the source URL, assume HEAD. */
-  if (src_peg_revision.kind == svn_opt_revision_unspecified)
-    src_peg_revision.kind = svn_opt_revision_head;
+  SVN_ERR(svn_client__resolve_target_location(target, ctx, pool));
 
-  /* If no peg-rev was attached to a URL target, then assume HEAD; if
-     no peg-rev was attached to a non-URL target, then assume BASE. */
-  if (tgt_peg_revision.kind == svn_opt_revision_unspecified)
+  /* Locate the source branch: the first argument or automatic. */
+  if (targets->nelts >= 1)
     {
-      if (svn_path_is_url(target))
-        tgt_peg_revision.kind = svn_opt_revision_head;
-      else
-        tgt_peg_revision.kind = svn_opt_revision_base;
+      const char *path_or_url;
+      svn_opt_revision_t peg_revision;
+
+      SVN_ERR(svn_opt_parse_path(&peg_revision, &path_or_url,
+                                 APR_ARRAY_IDX(targets, 0, const char *), pool));
+      SVN_ERR(svn_client__target(&source, path_or_url, pool));
+      source->peg_revision = peg_revision;
+      source->revision.kind = svn_opt_revision_unspecified;
+
+      /* If no peg-rev was attached to the source URL, assume HEAD. */
+      if (source->peg_revision.kind == svn_opt_revision_unspecified)
+        source->peg_revision.kind = svn_opt_revision_head;
+    }
+  else
+    {
+      printf("Assuming source branch is copied-from source of target branch.\n");
+      SVN_ERR(find_source_branch(&source, target, ctx, pool));
     }
 
-  /* Do the real work, depending on the requested data flavor. */
-  if (opt_state->show_revs == svn_cl__show_revs_merged)
+  SVN_ERR(svn_client__resolve_target_location(source, ctx, pool));
+
+  printf("Source branch: %s\n", target_for_display(source, pool));
+  printf("Target branch: %s\n", target_for_display(target, pool));
+
+  if (targets_are_same_branch(source, target, pool))
+    {
+      return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
+                              _("Source and target are the same branch"));
+    }
+  SVN_ERR(svn_client__check_branch_root_marker(&marker, source, target,
+                                               ctx, pool));
+  if (marker == NULL)
+    {
+      printf("warning: Source and target are not marked as branches.\n");
+    }
+  else
     {
-      SVN_ERR(svn_client_mergeinfo_log(TRUE, target, &tgt_peg_revision,
-                                       source, &src_peg_revision,
-                                       print_log_rev, NULL,
-                                       TRUE, depth, NULL, ctx,
-                                       pool));
+      printf("Source and target are marked as branches of the same project.\n");
     }
-  else if (opt_state->show_revs == svn_cl__show_revs_eligible)
+
+  /* If no peg-rev was attached to a URL target, then assume HEAD; if
+     no peg-rev was attached to a non-URL target, then assume BASE. */
+  if (target->peg_revision.kind == svn_opt_revision_unspecified)
     {
-      SVN_ERR(svn_client_mergeinfo_log(FALSE, target, &tgt_peg_revision,
-                                       source, &src_peg_revision,
-                                       print_log_rev, NULL,
-                                       TRUE, depth, NULL, ctx,
-                                       pool));
+      if (svn_path_is_url(target->path_or_url))
+        target->peg_revision.kind = svn_opt_revision_head;
+      else
+        target->peg_revision.kind = svn_opt_revision_base;
     }
+
+  printf(_("Merged revisions:\n"));
+  SVN_ERR(svn_client_mergeinfo_log(TRUE /* finding_merged */,
+                                   target->path_or_url,
+                                   &target->peg_revision,
+                                   source->path_or_url,
+                                   &source->peg_revision,
+                                   print_log_rev, NULL,
+                                   TRUE, depth, NULL, ctx,
+                                   pool));
+
+  printf(_("Eligible revisions:\n"));
+  SVN_ERR(svn_client_mergeinfo_log(FALSE /* finding_merged */,
+                                   target->path_or_url,
+                                   &target->peg_revision,
+                                   source->path_or_url,
+                                   &source->peg_revision,
+                                   print_log_rev, NULL,
+                                   TRUE, depth, NULL, ctx,
+                                   pool));
+
   return SVN_NO_ERROR;
 }



Re: Identifying branch roots

Posted by Trent Nelson <tr...@snakebite.org>.
On 07-Oct-11 6:59 AM, Julian Foad wrote:
> On Fri, 2011-10-07 at 11:29 +0100, Julian Foad wrote:
>> Stefan Sperling wrote:
>>> julianfoad wrote:
>>>> +/* This property marks a branch root. Branches with the same value of this
>>>> + * property are mergeable. */
>>>> +#define SVN_PROP_BRANCHING_ROOT "svn:ignore" /* ### should be "svn:branching-root" */
>>
>> Hi Stefan. Thanks for picking up on this.
>>
>>> I think your addition of a 'branch root' property is quite a significant
>>> step. Is this really necessary in order to improve the output of
>>> 'svn mergeinfo' or do you have additional steps planned that go beyond
>>> tuning output?
>>
>> Both.  I think knowing whether the (requested) merge source and target
>> are branch roots (and indeed branches of the *same* "project" or tree)
>> is important for improving the output and diagnostics of "svn mergeinfo"
>> and "svn merge" commands.
>>
>> It could of course enable other new behaviours relating to branches, and
>> I don't know what those are yet (apart from trivial UI things like
>> answering "is this a branch?").
>>
>> So I'm working on the idea that it would be useful to have branch roots
>> identifiable by some mechanism, so I'll add "some mechanism" (currently
>> this property, but I'm totally open to a different mechanism such as
>> branch points being defined in a config file) and see what useful
>> behaviours I can come up with.
>>
>>> There has been some discussion about adding a property for this
>>> and similar purposes in the past, see
>>> http://svn.haxx.se/dev/archive-2009-09/0156.shtml
>>> (there are probably more threads about this topic)
>>
>> Yes, and it's time to figure out what we can usefully do with such
>> information and then we'll know exactly what branch configuration
>> information we need and what's a good way to store it.
>>
>> I'll reply to the rest in a further email.
>>
>> - Julian
Welp, I'm never going to get a better lead in than that, so, hi, folks!
Freelance SCM consultant here; used to specialise in ClearQuest, of all
things, but my last two gigs ended up revolving around Subversion.

Specifically, Subversion merges, in the enterprise, and the, uh, quirks
involved.  Each client had different requirements, and thus, the
solution I ended up delivering to each one differed a bit.  The first
solution was neat, and did all kinds of funky ClearQuest integration
and merge validation, but the second one is more applicable to this
discussion, so I'll describe that first.

In essence, it's a hook framework that attempts to enforce Subversion
best-practices by blocking* incoming commits if it detects one or more
of the following:

         (*) Sometimes it'll block, but phrase the error message
             along the lines of "if you *really* want to do this,
             re-try your commit with the phrase 'CONFIRM MULTI-ROOT
             RENAME' somewhere in your commit message".

     TagCopied
     TagRenamed
     TagRemoved
     TagModified
     TagReplaced
     TagSubtreeCopied
     TagSubtreeRenamed
     TagSubtreeRemoved
     TagSubtreeModified
     TagSubtreeReplaced
     MultipleUnknownAndKnownRootsModified
     MixedRootNamesInMultiRootCommit
     MixedRootTypesInMultiRootCommit
     SubversionRepositoryCheckedIn
     MergeinfoAddedToRepositoryRoot
     MergeinfoModifiedOnRepositoryRoot
     SubtreeMergeinfoAdded
     RootMergeinfoRemoved
     DirectoryReplacedDuringMerge
     EmptyMergeinfoCreated
     TagDirectoryCreatedManually
     BranchDirectoryCreatedManually
     BranchRenamedToTrunk
     TrunkRenamedToBranch
     TrunkRenamedToTag
     BranchRenamedToTag
     BranchRenamedOutsideRootBaseDir
     TagSubtreePathRemoved
     RenameAffectsMultipleRoots
     UncleanRenameAffectsMultipleRoots
     MultipleRootsCopied
     UncleanCopy
     FileRemovedFromTag
     CopyKnownRootSubtreeToValidAbsRootPath
     MixedRootsNotClarifiedByExternals
     CopyKnownRootToIncorrectlyNamedRootPath
     CopyKnownRootSubtreeToIncorrectlyNamedRootPath
     RenamedKnownRootToIncorrectlyNamedRootPath
     MixedChangeTypesInMultiRootCommit
     CopyKnownRootToKnownRootSubtree
     UnknownPathCopiedToIncorrectlyNamedNewRootPath
     RenamedKnownRootToKnownRootSubtree
     FileUnchangedAndNoParentCopyOrRename
     DirUnchangedAndNoParentCopyOrRename
     EmptyChangeSet
     CopyKnownRootToUnknownPath
     CopyKnownRootSubtreeToInvalidRootPath
     NewRootCreatedByRenamingUnknownPath
     UnknownPathCopiedToKnownRootSubtree
     NewRootCreatedByCopyingUnknownPath
     PathCopiedFromOutsideRootDuringNonMerge
     UnknownDirReplacedViaCopyDuringNonMerge
     DirReplacedViaCopyDuringNonMerge
     DirectoryReplacedDuringNonMerge
     PreviousPathNotMatchedToPathsInMergeinfo
     PreviousRevDiffersFromParentCopiedFromRev
     PreviousPathDiffersFromParentCopiedFromPath
     PreviousRevDiffersFromParentRenamedFromRev
     PreviousPathDiffersFromParentRenamedFromPath
     KnownRootPathReplacedViaCopy
     BranchesDirShouldBeCreatedManuallyNotCopied
     TagsDirShouldBeCreatedManuallyNotCopied
     CopiedFromPathNotMatchedToPathsInMergeinfo
     InvariantViolatedModifyContainsMismatchedPreviousPath
     InvariantViolatedModifyContainsMismatchedPreviousRev
     InvariantViolatedCopyNewPathInRootsButNotReplace
     MultipleRootsAffectedByRemove
     AbsoluteRootOfRepositoryCopied
     PropertyChangedButOldAndNewValuesAreSame
     CopiedOrRenamedUnknownPathToIncorrectlyNamedNewRootPath
     UnknownPathRenamedViaReplaceToExistingKnownRoot
     UnknownPathCopiedViaReplaceToExistingKnownRoot
     UnknownPathRenamedToKnownRootSubtree
     UnknownPathCopiedToKnownRootSubtree
     KnownRootSubtreeRenamedViaReplaceToExistingKnownRoot
     UncleanRenameOfRootAncestorPath
     RenamedKnownRootViaReplaceToExistingKnownRoot
     RootPathAncestorRenamedViaReplaceToExistingKnownRoot
     RenamedKnownRootViaReplaceToRootAncestorPath
     RenamedKnownRootViaReplaceToRootAncestorPath
     RootPathAncestorRenamedToValidAbsoluteRootPath
     RootPathAncestorRenamedToValidRootPathSubtree
     RootPathAncestorRenamedToKnownRootSubtree
     RootPathAncestorRenamedViaReplaceToRootAncestorPath
     RenamedKnownRootToUnknownPath
     RenamedKnownRootSubtreeToUnknownPath
     RenamedKnownRootSubtreeToValidRootPath
     RenamedKnownRootSubtreeToIncorrectlyNamedRootPath
     UncleanRename
     RenameRelocatedPathOutsideKnownRoot

         (There's probably room for another e-mail thread just
          discussing all of these conditions; let's just say,
          Subversion repositories in the enterprise rarely look
          like their usually-well-laid out open source repository
          brethren.  What was the Blade Runner line?  "I've seen
          things you people wouldn't believe."? ;-)  My personal
          favorite: 'SubversionRepositoryCheckedIn'.)

So, as you can see, most of these conditions involve the concept of a
root.  Thus, the ability to accurately discern what constitutes a root
took up a large portion of my time.

Hard-coding regexes and forcing all repositories to confirm to a pre-
defined repository layout worked like a charm for my first client, as
I was coming in before they had any Subversion repositories rolled out
into production.  (Well, sort of.)

That unfortunately wasn't feasible for my second client.  They were a
*huge* Subversion shop.  At the time I came in they had something like
960 production repositories, and I wouldn't be surprised if they were
well over 1,000 by now.  There was no standard layout between repos,
and a lot of repos used non-standard branches/tags/trunks paths so
trying to manage 'root detection' via regexes was a non-starter.

For example, a number of repos had layouts like this:

     /foo/trunk
     /foo/branches/1.0.x
     /foo/branches/bugzilla/1081

i.e. 'bugzilla' was just some random directory they created to hold
developer branches related to bugs.  A regex approach would have
matched 'bugzilla' as the branch root, whereas, in fact, the branch
root would have been 1081.

The other non-starter was requiring the admin staff to have to go in
and manually specify what constituted a branch, i.e. setting a 'branch
root' property on relevant paths.  The overhead that would have been
required to do that for ~1,000 repositories (with hundreds, if not
thousands of differently named branches/roots (i.e. not particularly
easy to automate reliably)) was not acceptable (for many enterprisey
reasons mainly surrounding cost).

So, I needed to design the branch detection logic in such a way that
it didn't require any hand-holding from the admins or support staff.

It took two attempts.

For the first attempt, I played around with the notion of a root *base*
directory, i.e. /branches and /tags.  The first thing the framework
would do when processing a pre-commit was create a 'RepositoryRoots'
class (the framework was written in Python FWIW), which would recurse
through the repo up to N-levels deep in order to determine the valid
root base directories.  Except for trunk, which was special, if a
directory had subdirectories that were created by copying another path
(i.e. how tags or branches are created), then the directory would be
considered a root base dir.

That lasted... about a day or two.  It was a leaky abstraction at best,
and broke when I encountered repos with the more non-standard layouts.
(I'm not even sure if I've described it accurately above; but eh, who
cares, it's gone now.)

The problem with the regex and base-root-dir discovery approaches was
that they were essentially heuristic based.  "This directory features
lots of subdirectories that were copies of other paths, therefore, it's
a good chance it's a valid root base directory."

In most cases, yes, that was a valid assumption, but not always.  The
root detection logic was the most critical piece of my solution -- I
wasn't getting paid to correctly detect roots 70% of the time in 60% of
the repos.  It needed to be 100% in 100%.

So, I thought to myself, how can I correctly and autonomously identify
a root with 100% accuracy?  What one property did valid roots share
that I could interrogate?  Heck, what even constitutes a root? A branch
is a root, so is a tag, so is trunk.

....and then it dawned on me.  It seems so simple now, in retrospect:

     In the beginning, there was one root: trunk.  Then it was copied
     elsewhere, and became a branch, or maybe a tag.  These copies are
     also roots, and copies of them should also be considered roots.

Ah, so simple!  I just need to start at revision 0 and work my way up to
HEAD, whilst keeping a record of roots I encounter along the way.  And
that's pretty much it ;-)

Turns out, that approach has worked surprisingly well.  It's been in
production at the second client's site for nearly a year now.  They just
run the 'repo analysis' part of the code against new repositories before
enabling the hooks, and wallah, they get instant root detection and
prevention of some 80-something erroneous conditions.

Here are some techie' details about the implementation.  So, the script
stores root information in a revision property called 'evn:roots' (set
against the root of the repository).  The value of evn:roots at any
given revision will list all of the known roots in the repo at that
revision:

% svn pg --revprop -r26503 evn:roots svn://client.com/repos/foo
{'/build/branches/3.0.1/': {'created': 22323},
  '/build/branches/3.0.2/': {'created': 23129},
  '/build/branches/3.1.0/': {'created': 25804},
  '/build/branches/cvs/0.0.1/': {'created': 26389},
  '/build/branches/bugzilla/4144/': {'created': 22121},
  '/build/branches/bugzilla/6952/': {'created': 17661},
  '/build/release//3.0.0/': {'created': 20774},
  '/build/release/paris/3.0.0/': {'created': 20307},
  '/build/release/rome/3.0.1/': {'created': 22473},
  '/build/trunk/': {'created': 2919},
  '/src/trunk/': {'created': 9353},
  ...

The 'created' revision refers to the revision that the root was created
in.  That's important, 'cause we store special metadata against the root
in the revprop for the revision it was created in:

% svn pg --revprop -r9353 svn://client.com/repos/foo
  ...
  '/src/trunk/': {
     'copies': {
         9834:  [('/src/branches/2.1/', 9835)],
         9997:  [('/src/branches/bugzilla/2800/', 9998)],
         10211: [('/src/branches/bugzilla/3326/', 10212)],
         10252: [('/src/branches/bugzilla/2160/', 10253)],
         10468: [('/src/branches/2.2/', 10469)],
         11148: [('/src/branches/2.3/', 11149)],
         11420: [('/src/branches/bugzilla/3720/', 11421)]},
     'created': 9353,
     'creation_method': 'created'},
  ...

i.e. we store all the subsequent forward-copies of this root, as well as
details of how it was created (which isn't very interesting in this
example, as it's trunk and was created via mkdir, but if it were a
branch or tag, it would contain details about where it was copied from).

Let's say I delete /src/trunk in r26504.  The entry for it in evn:roots
in that revision will be gone; but a note will be made against the r9353
creation revprop to indicate which rev it was deleted in.

The importance of storing data like this becomes apparent when you deal
with situations like this:

  *hooks are turned off*
     r2:     svn cp ^/trunk ^/branches/foo
     r3:     svn rm ^/branches/foo
     r4:     svn mkdir ^/branches/foo
  *repo is analysed, evn:roots are set, hooks are turned on*

An attempt to do the following would be blocked, because r4/HEAD of
/branches/foo was not created correctly (i.e. wasn't copied from an
existing root), and thus, isn't considered a root either:

     svn cp /branches/foo /branches/bar

However, the following *would* work, because /branches/foo *was* a valid
root in r2:

     svn cp -r2 /branches/foo /branches/bar


Thoughts?

     Trent.


Re: Identifying branch roots

Posted by Julian Foad <ju...@wandisco.com>.
C. Michael Pilato wrote:
[...]
> The thing you are describing is a branch root, so just call it what it is.
> ("svn:branch-root" is what I suggested a couple of years ago for this.)
[...]

I guess, name "svn:branch-root" => value "subversion" can make sense
when we get used to the semantics being "this is *a* branch-root of the
<thing> called 'subversion'" rather than "the branch-root is named ...".

- Julian



Re: Identifying branch roots

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Fri, Oct 07, 2011 at 11:08:20 -0400:
> On 10/07/2011 06:59 AM, Julian Foad wrote:
> >>> I think this is an exciting feature with lots of potential but
> >>> it has a lot more inherent complexity than improving 'svn mergeinfo'
> >>> output. Could we split improving the output of 'svn mergeinfo' and
> >>> identifying branch roots into two distinct feature branches?
> > 
> > Splitting it wouldn't work for me, because the branch marker information
> > has no purpose and no defined requirements without the mergeinfo user
> > interface work; the existence and meaning of the info goes hand-in-hand
> > with developing some ways in which that info can be used.
> > 
> > When we come to integrate some of this work to trunk, by then the
> > specification of the branching info will have been worked out, and so
> > then it may be best to create a feature branch specifically for adding
> > that (without the mergeinfo changes) if it is complex enough to be
> > worthwhile doing so.
> 
> Seems sane to me.  If all you need is an property whose mere presence
> matters, we can add meaning to specific values later.

Will the meaning be added before or after the code is released?  There
are compatiblity implications in the latter case.

Re: Identifying branch roots

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/07/2011 06:59 AM, Julian Foad wrote:
>>> I think this is an exciting feature with lots of potential but
>>> it has a lot more inherent complexity than improving 'svn mergeinfo'
>>> output. Could we split improving the output of 'svn mergeinfo' and
>>> identifying branch roots into two distinct feature branches?
> 
> Splitting it wouldn't work for me, because the branch marker information
> has no purpose and no defined requirements without the mergeinfo user
> interface work; the existence and meaning of the info goes hand-in-hand
> with developing some ways in which that info can be used.
> 
> When we come to integrate some of this work to trunk, by then the
> specification of the branching info will have been worked out, and so
> then it may be best to create a feature branch specifically for adding
> that (without the mergeinfo changes) if it is complex enough to be
> worthwhile doing so.

Seems sane to me.  If all you need is an property whose mere presence
matters, we can add meaning to specific values later.

>>>> +/* Set *MARKER to the project-root marker that is common to SOURCE and
>>>> + * TARGET, or to NULL if neither has such a marker.
>>>
>>> Why do you need to introduce a new term "project"?
> 
> That's not a very good term, and I started by calling it a "branch-root
> marker", because it does indeed mark a branch root, but I wanted a name
> that says the value of this property is the name of the "code base" or
> "project tree" that has been branched, rather than the name of the
> branch.  I'm open to better ideas.

Yeah, it's not a very good term at all because "project root" is an
established term in Subversion already (and has been for a decade) -- and
not the same thing you're trying to describe here.

The thing you are describing is a branch root, so just call it what it is.
("svn:branch-root" is what I suggested a couple of years ago for this.)
Even when you set the property the first time on, say, your trunk, you are
declaring the existence of a conceptual branch of the
codebase-as-a-tree-metaphor, if the only such branch extant at the time.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: Identifying branch roots

Posted by Julian Foad <ju...@wandisco.com>.
On Fri, 2011-10-07 at 11:29 +0100, Julian Foad wrote:
> Stefan Sperling wrote:
> > julianfoad wrote:
> > > +/* This property marks a branch root. Branches with the same value of this
> > > + * property are mergeable. */
> > > +#define SVN_PROP_BRANCHING_ROOT "svn:ignore" /* ### should be "svn:branching-root" */
> 
> Hi Stefan. Thanks for picking up on this.
> 
> > I think your addition of a 'branch root' property is quite a significant
> > step. Is this really necessary in order to improve the output of
> > 'svn mergeinfo' or do you have additional steps planned that go beyond
> > tuning output?
> 
> Both.  I think knowing whether the (requested) merge source and target
> are branch roots (and indeed branches of the *same* "project" or tree)
> is important for improving the output and diagnostics of "svn mergeinfo"
> and "svn merge" commands.
> 
> It could of course enable other new behaviours relating to branches, and
> I don't know what those are yet (apart from trivial UI things like
> answering "is this a branch?").
> 
> So I'm working on the idea that it would be useful to have branch roots
> identifiable by some mechanism, so I'll add "some mechanism" (currently
> this property, but I'm totally open to a different mechanism such as
> branch points being defined in a config file) and see what useful
> behaviours I can come up with.
> 
> 
> > There has been some discussion about adding a property for this
> > and similar purposes in the past, see
> > http://svn.haxx.se/dev/archive-2009-09/0156.shtml
> > (there are probably more threads about this topic)
> 
> Yes, and it's time to figure out what we can usefully do with such
> information and then we'll know exactly what branch configuration
> information we need and what's a good way to store it.
> 
> I'll reply to the rest in a further email.
> 
> - Julian
> 
> 
> > One idea I've had in mind which relates to the notion of a branch root
> > is to have a property which specifies the merge policy for the branch,
> > [...]
> > 
> > Simple sketch:
> >  - feature-branch (can only receive catch-up merges from its direct parent
> >                    branch (copyfrom-source), or be reintegrated)
> >     => svn:mergepolicy on branch root: 'sync ^/trunk
> >                                         reintegrate ^/trunk'
> > 
> >  - release-branch (cannot be reintegrated into its parent)
> >     => svn:mergepolicy on branch root: 'cherry-pick ^/trunk'

Yes, this could be useful, and is the kind of policy configuration I'm
talking about here:
<http://wiki.apache.org/subversion/BranchingMergingTerminology>.

> > I haven't though this through yet.
> > I think your approach of "Branches with the same value of this
> > property are mergeable" is a design decision which has a similar
> > kind of impact on how a branch-root property would be used.
> > How do we really want it to be used?
> > 
> > I think this is an exciting feature with lots of potential but
> > it has a lot more inherent complexity than improving 'svn mergeinfo'
> > output. Could we split improving the output of 'svn mergeinfo' and
> > identifying branch roots into two distinct feature branches?

Splitting it wouldn't work for me, because the branch marker information
has no purpose and no defined requirements without the mergeinfo user
interface work; the existence and meaning of the info goes hand-in-hand
with developing some ways in which that info can be used.

When we come to integrate some of this work to trunk, by then the
specification of the branching info will have been worked out, and so
then it may be best to create a feature branch specifically for adding
that (without the mergeinfo changes) if it is complex enough to be
worthwhile doing so.


> > > +/* Set *MARKER to the project-root marker that is common to SOURCE and
> > > + * TARGET, or to NULL if neither has such a marker.
> > 
> > Why do you need to introduce a new term "project"?

That's not a very good term, and I started by calling it a "branch-root
marker", because it does indeed mark a branch root, but I wanted a name
that says the value of this property is the name of the "code base" or
"project tree" that has been branched, rather than the name of the
branch.  I'm open to better ideas.

[...]
> > Or are you suggesting Subversion should have a "project" concept?
> > If so, what is your definition of this term?

It's the "code base" or "tree of files" that gets branched.  I expect
(but am not yet certain) that if we introduce "branch" as a concept then
we will need a name for the generic "thing" that is being branched,
otherwise we will never be able to talk clearly about branching.
"Tree" (as in a tree of files) is one possible suggestion, but a rather
poor choice because the "branches" are not branches in that tree but are
rather branches *of* that tree in (conceptually) another dimension.

If I were to drop the idea of identifying the "project" and just have a
property "svn:is-branch-root" with value "*" meaning "yes", then the
need to find a term for the "thing that is branched" would be postponed
for a short time more.

- Julian



Identifying branch roots [was: svn commit: r1179135 - in /subversion/branches/showing-merge-info/...]

Posted by Julian Foad <ju...@wandisco.com>.
Stefan Sperling wrote:
> julianfoad wrote:
> > +/* This property marks a branch root. Branches with the same value of this
> > + * property are mergeable. */
> > +#define SVN_PROP_BRANCHING_ROOT "svn:ignore" /* ### should be "svn:branching-root" */

Hi Stefan. Thanks for picking up on this.

> I think your addition of a 'branch root' property is quite a significant
> step. Is this really necessary in order to improve the output of
> 'svn mergeinfo' or do you have additional steps planned that go beyond
> tuning output?

Both.  I think knowing whether the (requested) merge source and target
are branch roots (and indeed branches of the *same* "project" or tree)
is important for improving the output and diagnostics of "svn mergeinfo"
and "svn merge" commands.

It could of course enable other new behaviours relating to branches, and
I don't know what those are yet (apart from trivial UI things like
answering "is this a branch?").

So I'm working on the idea that it would be useful to have branch roots
identifiable by some mechanism, so I'll add "some mechanism" (currently
this property, but I'm totally open to a different mechanism such as
branch points being defined in a config file) and see what useful
behaviours I can come up with.


> There has been some discussion about adding a property for this
> and similar purposes in the past, see
> http://svn.haxx.se/dev/archive-2009-09/0156.shtml
> (there are probably more threads about this topic)

Yes, and it's time to figure out what we can usefully do with such
information and then we'll know exactly what branch configuration
information we need and what's a good way to store it.

I'll reply to the rest in a further email.

- Julian


> One idea I've had in mind which relates to the notion of a branch root
> is to have a property which specifies the merge policy for the branch,
> called 'svn:mergepolicy' (or maybe even use the 'branch root' property
> itself for this purpose). It could use keywords such as "cherry-pick, 
> sync, reintegrate" to define the set of allowed types of merges.
> 
> Simple sketch:
>  - feature-branch (can only receive catch-up merges from its direct parent
>                    branch (copyfrom-source), or be reintegrated)
>     => svn:mergepolicy on branch root: 'sync ^/trunk
>                                         reintegrate ^/trunk'
> 
>  - release-branch (cannot be reintegrated into its parent)
>     => svn:mergepolicy on branch root: 'cherry-pick ^/trunk'
> 
> 
> I haven't though this through yet.
> I think your approach of "Branches with the same value of this
> property are mergeable" is a design decision which has a similar
> kind of impact on how a branch-root property would be used.
> How do we really want it to be used?
> 
> I think this is an exciting feature with lots of potential but
> it has a lot more inherent complexity than improving 'svn mergeinfo'
> output. Could we split improving the output of 'svn mergeinfo' and
> identifying branch roots into two distinct feature branches?
> 
> > +/* Set *MARKER to the project-root marker that is common to SOURCE and
> > + * TARGET, or to NULL if neither has such a marker.
> 
> Why do you need to introduce a new term "project"?
> 
> > +  /* Check the source's and target's branch-marker properties. */
> > +  SVN_ERR(get_branch_root_marker(&target_marker, target, ctx, pool));
> > +  SVN_ERR(get_branch_root_marker(&source_marker, source, ctx, pool));
> > +  if (! source_marker && ! target_marker)
> > +    {
> > +      /* Old-style branches, not marked as such. Marker will be NULL. */
> > +    }
> > +  else if (! source_marker || ! target_marker)
> > +    {
> > +      if (source_marker)
> > +        return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> > +                                 _("Source is marked as a branch of "
> > +                                   "project '%s', but target is not marked"),
> 
> Same here. This terminology can confuse users because Subversion
> doesn't have a 'project' concept.
> 
> Why don't you just say "marked as a branch of '%s'"?
> 
> Or are you suggesting Subversion should have a "project" concept?
> If so, what is your definition of this term?
> 
> More instances of this term are below:
> 
> > +                                 source_marker);
> > +      else
> > +        return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> > +                                 _("Target is marked as a branch of "
> > +                                   "project '%s', but source is not marked"),
> > +                                 target_marker);
> > +    }
> > +  else if (strcmp(source_marker, target_marker) != 0)
> > +    {
> > +      /* ### The '.99' is just for display tidiness when I'm messing about
> > +       * with using 'svn:ignore' as the branch marker property. */
> > +      return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> > +                               _("error: Source is marked as branch of project '%.99s' "
> > +                                 "but target is marked as branch of project '%.99s'"),
> > +                               source_marker, target_marker);
> > +    }
> > +  *marker = source_marker;
> > +  return SVN_NO_ERROR;
> > +}



Re: svn commit: r1179135 - in /subversion/branches/showing-merge-info/subversion: include/private/svn_client_private.h include/svn_client.h libsvn_client/prop_commands.c libsvn_client/url.c svn/main.c svn/mergeinfo-cmd.c

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Oct 05, 2011 at 10:09:31AM -0000, julianfoad@apache.org wrote:
> Author: julianfoad
> Date: Wed Oct  5 10:09:30 2011
> New Revision: 1179135
> 
> URL: http://svn.apache.org/viewvc?rev=1179135&view=rev
> Log:
> On the showing-merge-info branch: Check in some work in progress.
> 
> Make 'svn mergeinfo' print a more friendly description of the merging
> situation.

> * subversion/include/private/svn_client_private.h
>   (SVN_PROP_BRANCHING_ROOT): New property name.
>   (svn_client__check_branch_root_marker): New function.

[...]

> +/* This property marks a branch root. Branches with the same value of this
> + * property are mergeable. */
> +#define SVN_PROP_BRANCHING_ROOT "svn:ignore" /* ### should be "svn:branching-root" */
> +

I think your addition of a 'branch root' property is quite a significant
step. Is this really necessary in order to improve the output of
'svn mergeinfo' or do you have additional steps planned that go beyond
tuning output?

There has been some discussion about adding a property for this
and similar purposes in the past, see
http://svn.haxx.se/dev/archive-2009-09/0156.shtml
(there are probably more threads about this topic)

One idea I've had in mind which relates to the notion of a branch root
is to have a property which specifies the merge policy for the branch,
called 'svn:mergepolicy' (or maybe even use the 'branch root' property
itself for this purpose). It could use keywords such as "cherry-pick, 
sync, reintegrate" to define the set of allowed types of merges.

Simple sketch:
 - feature-branch (can only receive catch-up merges from its direct parent
                   branch (copyfrom-source), or be reintegrated)
    => svn:mergepolicy on branch root: 'sync ^/trunk
                                        reintegrate ^/trunk'

 - release-branch (cannot be reintegrated into its parent)
    => svn:mergepolicy on branch root: 'cherry-pick ^/trunk'


I haven't though this through yet.
I think your approach of "Branches with the same value of this
property are mergeable" is a design decision which has a similar
kind of impact on how a branch-root property would be used.
How do we really want it to be used?

I think this is an exciting feature with lots of potential but
it has a lot more inherent complexity than improving 'svn mergeinfo'
output. Could we split improving the output of 'svn mergeinfo' and
identifying branch roots into two distinct feature branches?

> +/* Set *MARKER to the project-root marker that is common to SOURCE and
> + * TARGET, or to NULL if neither has such a marker.

Why do you need to introduce a new term "project"?

> +  /* Check the source's and target's branch-marker properties. */
> +  SVN_ERR(get_branch_root_marker(&target_marker, target, ctx, pool));
> +  SVN_ERR(get_branch_root_marker(&source_marker, source, ctx, pool));
> +  if (! source_marker && ! target_marker)
> +    {
> +      /* Old-style branches, not marked as such. Marker will be NULL. */
> +    }
> +  else if (! source_marker || ! target_marker)
> +    {
> +      if (source_marker)
> +        return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> +                                 _("Source is marked as a branch of "
> +                                   "project '%s', but target is not marked"),

Same here. This terminology can confuse users because Subversion
doesn't have a 'project' concept.

Why don't you just say "marked as a branch of '%s'"?

Or are you suggesting Subversion should have a "project" concept?
If so, what is your definition of this term?

More instances of this term are below:

> +                                 source_marker);
> +      else
> +        return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> +                                 _("Target is marked as a branch of "
> +                                   "project '%s', but source is not marked"),
> +                                 target_marker);
> +    }
> +  else if (strcmp(source_marker, target_marker) != 0)
> +    {
> +      /* ### The '.99' is just for display tidiness when I'm messing about
> +       * with using 'svn:ignore' as the branch marker property. */
> +      return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> +                               _("error: Source is marked as branch of project '%.99s' "
> +                                 "but target is marked as branch of project '%.99s'"),
> +                               source_marker, target_marker);
> +    }
> +  *marker = source_marker;
> +  return SVN_NO_ERROR;
> +}