You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/12/05 18:36:58 UTC

svn commit: r1548214 - in /subversion/trunk/subversion: libsvn_wc/externals.c libsvn_wc/update_editor.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h libsvn_wc/wc_db_update_move.c tests/libsvn_wc/op-depth-test.c

Author: rhuijben
Date: Thu Dec  5 17:36:57 2013
New Revision: 1548214

URL: http://svn.apache.org/r1548214
Log:
Apply some minor preparing and correctness changes for the move logic in
libsvn_wc.

This patch adds support for telling the wc_db revision bump whether an actual
update was performed. With this knowledge a lot of move handling logic can
be simplified for the generic case in future patches.

Some parts of this patch are to make sure some move behavior patches I have
kept local for some time don't have to keep in sync with more files than
necessary.

* subversion/libsvn_wc/externals.c
  (close_edit): Update caller.

* subversion/libsvn_wc/update_editor.c
  (edit_baton): Add variable.
  (close_directory): Update editor baton on edit actions.
  (close_edit): Pass edited value.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_SELECT_LOCAL_RELPATH_OP_DEPTH): Add column.

* subversion/libsvn_wc/wc_db.c
  (bump_revisions_post_update): Add argument. Add comment.
  (svn_wc__db_op_bump_revisions_post_update): Pass argument

* subversion/libsvn_wc/wc_db.h
  (svn_wc__db_op_bump_revisions_post_update): Add argument.

* subversion/libsvn_wc/wc_db_update_move.c
  (replace_moved_layer): Properly extend parent delete.

* subversion/tests/libsvn_wc/op-depth-test.c
  (check_tree_conflict_repos_path): Turn possible segfault in proper
    test error.

Modified:
    subversion/trunk/subversion/libsvn_wc/externals.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
    subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c

Modified: subversion/trunk/subversion/libsvn_wc/externals.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/externals.c?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/externals.c (original)
+++ subversion/trunk/subversion/libsvn_wc/externals.c Thu Dec  5 17:36:57 2013
@@ -963,6 +963,7 @@ close_edit(void *edit_baton,
                                                        *eb->target_revision,
                                                        apr_hash_make(pool),
                                                        wcroot_iprops,
+                                                       TRUE /* empty update */,
                                                        eb->notify_func,
                                                        eb->notify_baton,
                                                        pool));

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Thu Dec  5 17:36:57 2013
@@ -258,6 +258,9 @@ struct edit_baton
   /* Absolute path of the working copy root or NULL if not initialized yet */
   const char *wcroot_abspath;
 
+  /* After closing the root directory a copy of its edited value */
+  svn_boolean_t edited;
+
   apr_pool_t *pool;
 };
 
@@ -2960,6 +2963,9 @@ close_directory(void *dir_baton,
       eb->notify_func(eb->notify_baton, notify, scratch_pool);
     }
 
+  if (db->edited)
+    eb->edited = db->edited;
+
   /* We're done with this directory, so remove one reference from the
      bump information. */
   SVN_ERR(maybe_release_dir_info(db));
@@ -4733,6 +4739,7 @@ close_edit(void *edit_baton,
                                                        *(eb->target_revision),
                                                        eb->skipped_trees,
                                                        eb->wcroot_iprops,
+                                                       ! eb->edited,
                                                        eb->notify_func,
                                                        eb->notify_baton,
                                                        eb->pool));

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Thu Dec  5 17:36:57 2013
@@ -250,7 +250,7 @@ WHERE wc_id = ?1 
   AND op_depth > ?3
 
 -- STMT_SELECT_LOCAL_RELPATH_OP_DEPTH
-SELECT local_relpath
+SELECT local_relpath, kind
 FROM nodes
 WHERE wc_id = ?1
   AND (local_relpath = ?2 OR IS_STRICT_DESCENDANT_OF(local_relpath, ?2))

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Dec  5 17:36:57 2013
@@ -11625,6 +11625,7 @@ bump_revisions_post_update(svn_wc__db_wc
                            svn_revnum_t new_revision,
                            apr_hash_t *exclude_relpaths,
                            apr_hash_t *wcroot_iprops,
+                           svn_boolean_t empty_update,
                            svn_wc_notify_func2_t notify_func,
                            void *notify_baton,
                            apr_pool_t *scratch_pool)
@@ -11676,6 +11677,7 @@ bump_revisions_post_update(svn_wc__db_wc
                              TRUE /* is_root */, FALSE, db,
                              scratch_pool));
 
+  /* ### TODO: Use empty_update flag for change knowledge */
   SVN_ERR(svn_wc__db_bump_moved_away(wcroot, local_relpath, depth, db,
                                      scratch_pool));
 
@@ -11696,6 +11698,7 @@ svn_wc__db_op_bump_revisions_post_update
                                          svn_revnum_t new_revision,
                                          apr_hash_t *exclude_relpaths,
                                          apr_hash_t *wcroot_iprops,
+                                         svn_boolean_t empty_update,
                                          svn_wc_notify_func2_t notify_func,
                                          void *notify_baton,
                                          apr_pool_t *scratch_pool)
@@ -11718,7 +11721,7 @@ svn_wc__db_op_bump_revisions_post_update
     bump_revisions_post_update(wcroot, local_relpath, db,
                                depth, new_repos_relpath, new_repos_root_url,
                                new_repos_uuid, new_revision,
-                               exclude_relpaths, wcroot_iprops,
+                               exclude_relpaths, wcroot_iprops, empty_update,
                                notify_func, notify_baton, scratch_pool),
     wcroot);
 

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Thu Dec  5 17:36:57 2013
@@ -2539,6 +2539,9 @@ svn_wc__db_global_update(svn_wc__db_t *d
    for pathnames contained in EXCLUDE_RELPATHS are not touched by this
    function.  These pathnames should be paths relative to the wcroot.
 
+   If EMPTY_UPDATE is TRUE then no nodes at or below LOCAL_ABSPATH have been
+   affected by the update/switch yet.
+
    If WCROOT_IPROPS is not NULL it is a hash mapping const char * absolute
    working copy paths to depth-first ordered arrays of
    svn_prop_inherited_item_t * structures.  If LOCAL_ABSPATH exists in
@@ -2555,6 +2558,7 @@ svn_wc__db_op_bump_revisions_post_update
                                          svn_revnum_t new_revision,
                                          apr_hash_t *exclude_relpaths,
                                          apr_hash_t *wcroot_iprops,
+                                         svn_boolean_t empty_update,
                                          svn_wc_notify_func2_t notify_func,
                                          void *notify_baton,
                                          apr_pool_t *scratch_pool);

Modified: subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c Thu Dec  5 17:36:57 2013
@@ -1627,6 +1627,7 @@ replace_moved_layer(const char *src_relp
       svn_error_t *err;
       svn_sqlite__stmt_t *stmt2;
       const char *src_cp_relpath = svn_sqlite__column_text(stmt, 0, NULL);
+      svn_node_kind_t kind = svn_sqlite__column_token(stmt, 1, kind_map);
       const char *dst_cp_relpath
         = svn_relpath_join(dst_relpath,
                            svn_relpath_skip_ancestor(src_relpath,
@@ -1643,6 +1644,11 @@ replace_moved_layer(const char *src_relp
                                                     scratch_pool));
       if (!err)
         err = svn_sqlite__step_done(stmt2);
+
+      if (!err && strlen(dst_cp_relpath) > strlen(dst_relpath))
+        err = svn_wc__db_extend_parent_delete(wcroot, dst_cp_relpath, kind,
+                                              dst_op_depth, scratch_pool);
+
       if (err)
         return svn_error_compose_create(err, svn_sqlite__reset(stmt));
 

Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1548214&r1=1548213&r2=1548214&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Thu Dec  5 17:36:57 2013
@@ -5772,6 +5772,8 @@ check_tree_conflict_repos_path(svn_test_
                                    sbox_wc_path(b, wc_path),
                                    b->pool, b->pool));
 
+  SVN_TEST_ASSERT(conflict != NULL);
+
   SVN_ERR(svn_wc__conflict_read_info(&operation, &locations,
                                      &text_conflicted, &prop_conflicted,
                                      &tree_conflicted,



Re: svn commit: r1548214 - in /subversion/trunk/subversion: libsvn_wc/externals.c libsvn_wc/update_editor.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h libsvn_wc/wc_db_update_move.c tests/libsvn_wc/op-depth-test.c

Posted by Stefan Sperling <st...@elego.de>.
On Fri, Dec 06, 2013 at 03:18:27PM +0100, Bert Huijben wrote:
> It is a check if dst_cp_relpath is the root node or any of its descendants.
> The root of a move can never be shadowed (as the maximum op_depth of a node
> is its own op depth... which is the move)

Then why not use something like svn_relpath_skip_ancestor()?
I think using strcmp() in this context is misleading.

> A string comparision is quite heavy if we really only need this check. (The
> db query already assures that there will only be nodes at or below
> dst_relpath)
> 
> 	Bert
> 
> > 
> > > @@ -1643,6 +1644,11 @@ replace_moved_layer(const char *src_relp
> > >                                                      scratch_pool));
> > >        if (!err)
> > >          err = svn_sqlite__step_done(stmt2);
> > > +
> > > +      if (!err && strlen(dst_cp_relpath) > strlen(dst_relpath))
> > > +        err = svn_wc__db_extend_parent_delete(wcroot, dst_cp_relpath,
> > kind,
> > > +                                              dst_op_depth,
> scratch_pool);
> > > +
> > >        if (err)
> > >          return svn_error_compose_create(err, svn_sqlite__reset(stmt));
> > >
> > >

RE: svn commit: r1548214 - in /subversion/trunk/subversion: libsvn_wc/externals.c libsvn_wc/update_editor.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h libsvn_wc/wc_db_update_move.c tests/libsvn_wc/op-depth-test.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: vrijdag 6 december 2013 12:25
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1548214 - in /subversion/trunk/subversion:
> libsvn_wc/externals.c libsvn_wc/update_editor.c libsvn_wc/wc-queries.sql
> libsvn_wc/wc_db.c libsvn_wc/wc_db.h libsvn_wc/wc_db_update_move.c
> tests/libsvn_wc/op-depth-test.c
> 
> On Thu, Dec 05, 2013 at 05:36:58PM -0000, rhuijben@apache.org wrote:
> > Author: rhuijben
> > Date: Thu Dec  5 17:36:57 2013
> > New Revision: 1548214
> >
> > URL: http://svn.apache.org/r1548214
> > Log:
> > Apply some minor preparing and correctness changes for the move logic in
> > libsvn_wc.
> >
> > This patch adds support for telling the wc_db revision bump whether an
> actual
> > update was performed. With this knowledge a lot of move handling logic
> can
> > be simplified for the generic case in future patches.
> >
> > Some parts of this patch are to make sure some move behavior patches I
> have
> > kept local for some time don't have to keep in sync with more files than
> > necessary.
> 
> > * subversion/libsvn_wc/wc_db_update_move.c
> >   (replace_moved_layer): Properly extend parent delete.
> 
> Hi Bert,
> 
> Can you please explain why the strlen() check is the right thing
> to do here? The log message doesn't make this clear, and there
> is no code comment either. It looks rather odd.

It is a check if dst_cp_relpath is the root node or any of its descendants.
The root of a move can never be shadowed (as the maximum op_depth of a node
is its own op depth... which is the move)

A string comparision is quite heavy if we really only need this check. (The
db query already assures that there will only be nodes at or below
dst_relpath)

	Bert

> 
> > @@ -1643,6 +1644,11 @@ replace_moved_layer(const char *src_relp
> >                                                      scratch_pool));
> >        if (!err)
> >          err = svn_sqlite__step_done(stmt2);
> > +
> > +      if (!err && strlen(dst_cp_relpath) > strlen(dst_relpath))
> > +        err = svn_wc__db_extend_parent_delete(wcroot, dst_cp_relpath,
> kind,
> > +                                              dst_op_depth,
scratch_pool);
> > +
> >        if (err)
> >          return svn_error_compose_create(err, svn_sqlite__reset(stmt));
> >
> >


Re: svn commit: r1548214 - in /subversion/trunk/subversion: libsvn_wc/externals.c libsvn_wc/update_editor.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h libsvn_wc/wc_db_update_move.c tests/libsvn_wc/op-depth-test.c

Posted by Stefan Sperling <st...@elego.de>.
On Thu, Dec 05, 2013 at 05:36:58PM -0000, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Thu Dec  5 17:36:57 2013
> New Revision: 1548214
> 
> URL: http://svn.apache.org/r1548214
> Log:
> Apply some minor preparing and correctness changes for the move logic in
> libsvn_wc.
> 
> This patch adds support for telling the wc_db revision bump whether an actual
> update was performed. With this knowledge a lot of move handling logic can
> be simplified for the generic case in future patches.
> 
> Some parts of this patch are to make sure some move behavior patches I have
> kept local for some time don't have to keep in sync with more files than
> necessary.

> * subversion/libsvn_wc/wc_db_update_move.c
>   (replace_moved_layer): Properly extend parent delete.

Hi Bert,

Can you please explain why the strlen() check is the right thing
to do here? The log message doesn't make this clear, and there
is no code comment either. It looks rather odd.

> @@ -1643,6 +1644,11 @@ replace_moved_layer(const char *src_relp
>                                                      scratch_pool));
>        if (!err)
>          err = svn_sqlite__step_done(stmt2);
> +
> +      if (!err && strlen(dst_cp_relpath) > strlen(dst_relpath))
> +        err = svn_wc__db_extend_parent_delete(wcroot, dst_cp_relpath, kind,
> +                                              dst_op_depth, scratch_pool);
> +
>        if (err)
>          return svn_error_compose_create(err, svn_sqlite__reset(stmt));
>  
>