subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From julianf...@apache.org
Subject svn commit: r1683390 - in /subversion/branches/move-tracking-2: BRANCH-README subversion/include/private/svn_editor3e.h subversion/libsvn_delta/editor3e.c subversion/svnmover/svnmover.c
Date Wed, 03 Jun 2015 16:44:26 GMT
Author: julianfoad
Date: Wed Jun  3 16:44:26 2015
New Revision: 1683390

URL: http://svn.apache.org/r1683390
Log:
On the 'move-tracking-2' branch: Change detection better than recording
whether any operation was done. Avoids no-op commits.

There is a small change in behaviour: 'revert' no longer reports 'There are
no changes to be reverted', simply because I don't particularly want that
behaviour and so have not re-implemented it in the new way.

* BRANCH-README
  Remove the to-do item.
  Note that 'revert' is implemented.

* subversion/include/private/svn_editor3e.h,
  subversion/libsvn_delta/editor3e.c
  (svn_editor3__change_detection_editor): New.
  (change_detection_baton_t,
   change_detection_*): New.

* subversion/svnmover/svnmover.c
  (svnmover_wc_t): Delete 'changes_made'; remove all assignments to it.
  (wc_commit): Detect changes while preparing a commit; avoid a no-op commit;
    return an indication of whether a commit was made.
  (do_commit): Adapt to this way of working. Only check out a new WC txn if
    a commit was made.
  (do_revert): Instead of checking out a new WC txn, replay the inverse of
    the changes from the current edit txn into itself. This doesn't actually
    detect whether any changes were reverted, but brings the code into a
    form where it is clear how we could do so if we wanted.
  (execute,
   final_commit): Adjust callers.

Modified:
    subversion/branches/move-tracking-2/BRANCH-README
    subversion/branches/move-tracking-2/subversion/include/private/svn_editor3e.h
    subversion/branches/move-tracking-2/subversion/libsvn_delta/editor3e.c
    subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c

Modified: subversion/branches/move-tracking-2/BRANCH-README
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/BRANCH-README?rev=1683390&r1=1683389&r2=1683390&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/BRANCH-README (original)
+++ subversion/branches/move-tracking-2/BRANCH-README Wed Jun  3 16:44:26 2015
@@ -46,9 +46,6 @@ BUGS:
     branch-and-delete, never by copy-and-delete. Is that right? If so,
     delete the 'if' and the 'else' code block.
 
-  * svnmover's 'made_changes' flag is fallible: undoing all changes doesn't
-    reset the flag. Instead, query whether the txn has any changes.
-
 
 Work on this branch:
 
@@ -89,6 +86,7 @@ Work on this branch:
       merge
       commit
       update
+      revert
 
     UI things to do:
       add a 'parallel mode' UI as an alternative to the sequential mode

Modified: subversion/branches/move-tracking-2/subversion/include/private/svn_editor3e.h
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/include/private/svn_editor3e.h?rev=1683390&r1=1683389&r2=1683390&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/include/private/svn_editor3e.h (original)
+++ subversion/branches/move-tracking-2/subversion/include/private/svn_editor3e.h Wed Jun
 3 16:44:26 2015
@@ -1032,6 +1032,15 @@ svn_editor3__get_debug_editor(svn_editor
                               apr_pool_t *result_pool);
 #endif
 
+/* After driving this editor, *CHANGE_DETECTED will be true iff an editor
+ * method was called that can make a change. This does not necessarily mean
+ * a non-empty change was actually made.
+ */
+svn_error_t *
+svn_editor3__change_detection_editor(svn_editor3_t **editor_p,
+                                     svn_boolean_t *change_detected,
+                                     svn_editor3_t *wrapped_editor,
+                                     apr_pool_t *result_pool);
 
 /** Callback to retrieve a node's kind and content.  This is
  * needed by the various editor shims in order to effect backwards

Modified: subversion/branches/move-tracking-2/subversion/libsvn_delta/editor3e.c
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/libsvn_delta/editor3e.c?rev=1683390&r1=1683389&r2=1683390&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/libsvn_delta/editor3e.c (original)
+++ subversion/branches/move-tracking-2/subversion/libsvn_delta/editor3e.c Wed Jun  3 16:44:26
2015
@@ -633,6 +633,195 @@ svn_editor3__get_debug_editor(svn_editor
 
 /*
  * ===================================================================
+ */
+
+typedef struct change_detection_baton_t
+{
+  svn_editor3_t *wrapped_editor;
+
+  svn_boolean_t *change_detected;
+
+} change_detection_baton_t;
+
+static svn_error_t *
+change_detection_new_eid(void *baton,
+             svn_branch_eid_t *eid_p,
+             svn_branch_state_t *branch,
+             apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  SVN_ERR(svn_editor3_new_eid(eb->wrapped_editor,
+                              eid_p, branch));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_add(void *baton,
+         svn_branch_state_t *branch,
+         svn_branch_eid_t eid,
+         svn_branch_eid_t new_parent_eid,
+         const char *new_name,
+         const svn_element_payload_t *new_payload,
+         apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  *eb->change_detected = TRUE;
+  SVN_ERR(svn_editor3_add(eb->wrapped_editor,
+                          branch, eid,
+                          new_parent_eid, new_name, new_payload));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_copy_one(void *baton,
+              const svn_branch_el_rev_id_t *src_el_rev,
+              svn_branch_state_t *branch,
+              svn_branch_eid_t local_eid,
+              svn_branch_eid_t new_parent_eid,
+              const char *new_name,
+              const svn_element_payload_t *new_payload,
+              apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  *eb->change_detected = TRUE;
+  SVN_ERR(svn_editor3_copy_one(eb->wrapped_editor,
+                               src_el_rev,
+                               branch, local_eid,
+                               new_parent_eid, new_name, new_payload));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_copy_tree(void *baton,
+               const svn_branch_el_rev_id_t *src_el_rev,
+               svn_branch_state_t *branch,
+               svn_branch_eid_t new_parent_eid,
+               const char *new_name,
+               apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  *eb->change_detected = TRUE;
+  SVN_ERR(svn_editor3_copy_tree(eb->wrapped_editor,
+                                src_el_rev,
+                                branch, new_parent_eid, new_name));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_delete(void *baton,
+            svn_branch_state_t *branch,
+            svn_branch_eid_t eid,
+            apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  *eb->change_detected = TRUE;
+  SVN_ERR(svn_editor3_delete(eb->wrapped_editor,
+                             branch, eid));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_alter(void *baton,
+           svn_branch_state_t *branch,
+           svn_branch_eid_t eid,
+           svn_branch_eid_t new_parent_eid,
+           const char *new_name,
+           const svn_element_payload_t *new_payload,
+           apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  *eb->change_detected = TRUE;
+  SVN_ERR(svn_editor3_alter(eb->wrapped_editor,
+                            branch, eid,
+                            new_parent_eid, new_name, new_payload));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_payload_resolve(void *baton,
+                     svn_element_payload_t **payload_p,
+                     const svn_branch_el_rev_content_t *element,
+                     apr_pool_t *result_pool,
+                     apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  SVN_ERR(svn_editor3_payload_resolve(eb->wrapped_editor,
+                                      payload_p,
+                                      element,
+                                      result_pool));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_sequence_point(void *baton,
+                    apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  SVN_ERR(svn_editor3_sequence_point(eb->wrapped_editor));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_complete(void *baton,
+              apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  SVN_ERR(svn_editor3_complete(eb->wrapped_editor));
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+change_detection_abort(void *baton,
+           apr_pool_t *scratch_pool)
+{
+  change_detection_baton_t *eb = baton;
+
+  SVN_ERR(svn_editor3_abort(eb->wrapped_editor));
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_editor3__change_detection_editor(svn_editor3_t **editor_p,
+                                     svn_boolean_t *change_detected,
+                                     svn_editor3_t *wrapped_editor,
+                                     apr_pool_t *result_pool)
+{
+  static const svn_editor3_cb_funcs_t wrapper_funcs = {
+    change_detection_new_eid,
+    change_detection_add,
+    change_detection_copy_one,
+    change_detection_copy_tree,
+    change_detection_delete,
+    change_detection_alter,
+    change_detection_payload_resolve,
+    change_detection_sequence_point,
+    change_detection_complete,
+    change_detection_abort
+  };
+  change_detection_baton_t *eb = apr_palloc(result_pool, sizeof(*eb));
+
+  eb->wrapped_editor = wrapped_editor;
+  eb->change_detected = change_detected;
+  *change_detected = FALSE;
+
+  *editor_p = svn_editor3_create(&wrapper_funcs, eb,
+                                 NULL, NULL, /* cancellation */
+                                 result_pool);
+
+  return SVN_NO_ERROR;
+}
+
+/*
+ * ===================================================================
  * Branch functionality
  * ===================================================================
  */

Modified: subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c?rev=1683390&r1=1683389&r2=1683390&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c (original)
+++ subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c Wed Jun  3 16:44:26
2015
@@ -124,7 +124,6 @@ typedef struct svnmover_wc_t
   svn_branch_revision_root_t *edit_txn;
   svn_client_ctx_t *ctx;
 
-  svn_boolean_t made_changes;
 } svnmover_wc_t;
 
 /* Update the WC to revision BASE_REVISION (SVN_INVALID_REVNUM means HEAD).
@@ -214,7 +213,6 @@ wc_create(svnmover_wc_t **wc_p,
 
   wc->pool = wc_pool;
   wc->ctx = ctx;
-  wc->made_changes = FALSE;
 
   SVN_ERR(svn_client_open_ra_session2(&wc->ra_session, anchor_url,
                                       NULL /* wri_abspath */, ctx,
@@ -420,10 +418,16 @@ display_diff_of_commit(const commit_call
  *
  * Open a new commit txn to the repo. Replay the changes from WC into it.
  *
- * Set WC->head_revision to the committed revision number.
+ * Set WC->head_revision and *NEW_REV_P to the committed revision number.
+ *
+ * If there are no changes to commit, set *NEW_REV_P to SVN_INVALID_REVNUM
+ * and do not make a commit and do not change WC->head_revision.
+ *
+ * NEW_REV_P may be null if not wanted.
  */
 static svn_error_t *
-wc_commit(svnmover_wc_t *wc,
+wc_commit(svn_revnum_t *new_rev_p,
+          svnmover_wc_t *wc,
           apr_hash_t *revprops,
           apr_pool_t *scratch_pool)
 {
@@ -434,6 +438,7 @@ wc_commit(svnmover_wc_t *wc,
   svn_branch_revision_root_t *commit_txn;
   svn_editor3_t *commit_editor;
   commit_callback_baton_t ccbb;
+  svn_boolean_t change_detected;
 
   /* Choose whether to store branching info in a local dir or in revprops.
      (For now, just to exercise the options, we choose local files for
@@ -455,6 +460,10 @@ wc_commit(svnmover_wc_t *wc,
                                        NULL /*lock_tokens*/, FALSE /*keep_locks*/,
                                        branch_info_dir,
                                        scratch_pool));
+  SVN_ERR(svn_editor3__change_detection_editor(&commit_editor,
+                                               &change_detected,
+                                               commit_editor,
+                                               scratch_pool));
   ccbb.edit_txn = commit_txn;
   ccbb.editor = commit_editor;
   /*SVN_ERR(svn_editor3__get_debug_editor(&wc->editor, wc->editor, scratch_pool));*/
@@ -463,12 +472,22 @@ wc_commit(svnmover_wc_t *wc,
                  left_txn,
                  right_txn,
                  scratch_pool));
-  SVN_ERR(svn_editor3_complete(commit_editor));
+  if (change_detected)
+    {
+      SVN_ERR(svn_editor3_complete(commit_editor));
+      SVN_ERR(display_diff_of_commit(&ccbb, scratch_pool));
 
-  SVN_ERR(display_diff_of_commit(&ccbb, scratch_pool));
+      wc->head_revision = ccbb.revision;
+      if (new_rev_p)
+        *new_rev_p = ccbb.revision;
+    }
+  else
+    {
+      SVN_ERR(svn_editor3_abort(commit_editor));
+      if (new_rev_p)
+        *new_rev_p = SVN_INVALID_REVNUM;
+    }
 
-  wc->made_changes = FALSE;
-  wc->head_revision = ccbb.revision;
   return SVN_NO_ERROR;
 }
 
@@ -2044,22 +2063,37 @@ display_diff_of_commit(const commit_call
 }
 
 /* Commit and update WC.
+ *
+ * Set *NEW_REV_P to the committed revision number, and update the WC to
+ * that revision.
+ *
+ * If there are no changes to commit, set *NEW_REV_P to SVN_INVALID_REVNUM
+ * and do not make a commit and do not update the WC.
+ *
+ * NEW_REV_P may be null if not wanted.
  */
 static svn_error_t *
-do_commit(svnmover_wc_t *wc,
+do_commit(svn_revnum_t *new_rev_p,
+          svnmover_wc_t *wc,
           apr_hash_t *revprops,
           apr_pool_t *scratch_pool)
 {
+  svn_revnum_t new_rev;
+
   /* Complete the old edit drive (into the 'WC') */
   SVN_ERR(svn_editor3_complete(wc->editor));
 
   /* Commit */
-  SVN_ERR(wc_commit(wc, revprops, scratch_pool));
+  SVN_ERR(wc_commit(&new_rev, wc, revprops, scratch_pool));
 
-  /* Check out a new WC */
-  SVN_ERR(wc_checkout(wc, SVN_INVALID_REVNUM /*=head*/,
-                      scratch_pool));
+  /* Check out a new WC if a commit was performed */
+  if (SVN_IS_VALID_REVNUM(new_rev))
+    {
+      SVN_ERR(wc_checkout(wc, new_rev, scratch_pool));
+    }
 
+  if (new_rev_p)
+    *new_rev_p = new_rev;
   return SVN_NO_ERROR;
 }
 
@@ -2069,8 +2103,14 @@ static svn_error_t *
 do_revert(svnmover_wc_t *wc,
           apr_pool_t *scratch_pool)
 {
-  SVN_ERR(wc_checkout(wc, wc->base_revision, scratch_pool));
-  wc->made_changes = FALSE;
+  svn_branch_revision_root_t *base_txn
+    = svn_array_get(wc->edit_txn->repos->rev_roots, (int)wc->base_revision);
+
+  /* Replay the inverse of the current edit txn, into the current edit txn */
+  SVN_ERR(replay(wc->editor, wc->edit_txn->root_branch,
+                 wc->edit_txn,
+                 base_txn,
+                 scratch_pool));
 
   return SVN_NO_ERROR;
 }
@@ -2314,7 +2354,6 @@ execute(svnmover_wc_t *wc,
             notify("A+   %s%s", action->relpath[1],
                    branch_str(new_branch, iterpool));
           }
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_BRANCH_INTO:
@@ -2329,7 +2368,6 @@ execute(svnmover_wc_t *wc,
                                            iterpool));
             notify("A+   %s (subtree)", action->relpath[1]);
           }
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_MKBRANCH:
@@ -2349,7 +2387,6 @@ execute(svnmover_wc_t *wc,
             notify("A    %s%s", action->relpath[0],
                    branch_str(new_branch, iterpool));
           }
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_MERGE:
@@ -2363,7 +2400,6 @@ execute(svnmover_wc_t *wc,
                                      arg[2]->el_rev /*yca*/,
                                      iterpool));
           }
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_MV:
@@ -2385,7 +2421,6 @@ execute(svnmover_wc_t *wc,
           SVN_ERR(do_move(editor, arg[0]->el_rev, arg[1]->parent_el_rev, arg[1]->path_name,
                           iterpool));
           notify("V    %s (from %s)", action->relpath[1], action->relpath[0]);
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_CP:
@@ -2400,7 +2435,6 @@ execute(svnmover_wc_t *wc,
                                         arg[1]->parent_el_rev->branch,
                                         arg[1]->parent_el_rev->eid, arg[1]->path_name));
           notify("A+   %s (from %s)", action->relpath[1], action->relpath[0]);
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_RM:
@@ -2416,7 +2450,6 @@ execute(svnmover_wc_t *wc,
           SVN_ERR(svn_editor3_delete(editor,
                                      arg[0]->el_rev->branch, arg[0]->el_rev->eid));
           notify("D    %s", action->relpath[0]);
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_MKDIR:
@@ -2437,7 +2470,6 @@ execute(svnmover_wc_t *wc,
                                     payload));
           }
           notify("A    %s", action->relpath[0]);
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_PUT_FILE:
@@ -2505,14 +2537,15 @@ execute(svnmover_wc_t *wc,
               }
           }
           notify("A    %s", action->relpath[1]);
-          wc->made_changes = TRUE;
           break;
 
         case ACTION_COMMIT:
           {
-            if (wc->made_changes)
+            svn_revnum_t new_rev;
+
+            SVN_ERR(do_commit(&new_rev, wc, revprops, iterpool));
+            if (SVN_IS_VALID_REVNUM(new_rev))
               {
-                SVN_ERR(do_commit(wc, revprops, iterpool));
                 editor = wc->editor;
               }
             else
@@ -2534,15 +2567,7 @@ execute(svnmover_wc_t *wc,
 
         case ACTION_REVERT:
           {
-            if (wc->made_changes)
-              {
-                SVN_ERR(do_revert(wc, iterpool));
-                editor = wc->editor;
-              }
-            else
-              {
-                printf("There are no changes to revert.\n");
-              }
+            SVN_ERR(do_revert(wc, iterpool));
           }
           break;
 
@@ -2562,18 +2587,11 @@ final_commit(svnmover_wc_t *wc,
 {
   svn_error_t *err;
 
-  if (wc->made_changes)
-    {
-      /* Complete the old edit drive (into the 'WC') */
-      SVN_ERR(svn_editor3_complete(wc->editor));
+  /* Complete the old edit drive (into the 'WC') */
+  SVN_ERR(svn_editor3_complete(wc->editor));
 
-      /* Commit */
-      err = wc_commit(wc, revprops, scratch_pool);
-    }
-  else
-    {
-      err = svn_editor3_abort(wc->editor);
-    }
+  /* Commit, if there are any changes */
+  err = wc_commit(NULL, wc, revprops, scratch_pool);
 
   svn_pool_destroy(wc->pool);
 



Mime
View raw message