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

svn commit: r1127864 - in /subversion/trunk/subversion: libsvn_wc/adm_ops.c libsvn_wc/wc_db.c tests/cmdline/authz_tests.py

Author: philip
Date: Thu May 26 10:58:22 2011
New Revision: 1127864

URL: http://svn.apache.org/viewvc?rev=1127864&view=rev
Log:
Fix issue 3900, wc delete of presence=absent nodes

* subversion/libsvn_wc/wc_db.c
  (op_delete_txn): Check for absent nodes.

* subversion/libsvn_wc/adm_ops.c
  (svn_wc_delete4): No need to check for absent node.

* subversion/tests/cmdline/authz_tests.py
  (wc_delete): New test.
  (test_list): Add new test.

Modified:
    subversion/trunk/subversion/libsvn_wc/adm_ops.c
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/tests/cmdline/authz_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127864&r1=1127863&r2=1127864&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Thu May 26 10:58:22 2011
@@ -632,9 +632,7 @@ svn_wc_delete4(svn_wc_context_t *wc_ctx,
 
   switch (status)
     {
-      /* ### Move this check into svn_wc__db_op_delete so that we
-             check the whole tree? */
-      case svn_wc__db_status_absent:
+      /* svn_wc__db_status_absent handled by svn_wc__db_op_delete */
       case svn_wc__db_status_excluded:
       case svn_wc__db_status_not_present:
         return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1127864&r1=1127863&r2=1127864&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu May 26 10:58:22 2011
@@ -6057,7 +6057,7 @@ op_delete_txn(void *baton,
 {
   struct op_delete_baton_t *b = baton;
   svn_wc__db_status_t status;
-  svn_boolean_t op_root;
+  svn_boolean_t have_row, op_root;
   svn_boolean_t add_work = FALSE;
   svn_sqlite__stmt_t *stmt;
   const char *like_arg;
@@ -6078,6 +6078,28 @@ op_delete_txn(void *baton,
       || status == svn_wc__db_status_not_present)
     return SVN_NO_ERROR;
 
+  like_arg = construct_like_arg(local_relpath, scratch_pool);
+
+  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                    STMT_SELECT_ABSENT_NODES));
+  SVN_ERR(svn_sqlite__bindf(stmt, "iss",
+                            wcroot->wc_id, local_relpath, like_arg));
+  SVN_ERR(svn_sqlite__step(&have_row, stmt));
+  if (have_row)
+    {
+      const char *absent_path
+        = svn_dirent_local_style(svn_sqlite__column_text(stmt, 0, scratch_pool),
+                                 scratch_pool);
+        
+      return svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS,
+                               svn_sqlite__reset(stmt),
+                           _("Cannot delete '%s' as '%s'is excluded by server"),
+                               svn_dirent_local_style(local_relpath,
+                                                      scratch_pool),
+                               absent_path);
+    }
+  SVN_ERR(svn_sqlite__reset(stmt));
+
   if (op_root)
     {
       svn_boolean_t below_base;
@@ -6104,8 +6126,6 @@ op_delete_txn(void *baton,
       SVN_ERR(op_depth_of(&select_depth, wcroot, local_relpath));
     }
 
-  like_arg = construct_like_arg(local_relpath, scratch_pool);
-
   /* ### Put actual-only nodes into the list? */
   SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
                                     STMT_INSERT_DELETE_LIST));

Modified: subversion/trunk/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/authz_tests.py?rev=1127864&r1=1127863&r2=1127864&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/authz_tests.py Thu May 26 10:58:22 2011
@@ -1206,6 +1206,38 @@ def authz_tree_conflict(sbox):
                                         None, None, None, None, 0,
                                         '-r', '1', wc_dir)
   
+@Issue(3900)
+@Skip(svntest.main.is_ra_type_file)
+def wc_delete(sbox):
+  "wc delete with absent nodes"
+
+  sbox.build(create_wc = False)
+  local_dir = sbox.wc_dir
+  write_restrictive_svnserve_conf(sbox.repo_dir)
+
+  write_authz_file(sbox, {'/'       : '* = r',
+                          '/A/B/E'  : '* =', })
+
+  expected_output = svntest.main.greek_state.copy()
+  expected_output.wc_dir = local_dir
+  expected_output.tweak(status='A ', contents=None)
+  expected_output.remove('A/B/E', 'A/B/E/alpha', 'A/B/E/beta')
+  expected_wc = svntest.main.greek_state.copy()
+  expected_wc.remove('A/B/E', 'A/B/E/alpha', 'A/B/E/beta')
+
+  svntest.actions.run_and_verify_checkout(sbox.repo_url, local_dir,
+                                          expected_output,
+                                          expected_wc)
+
+  expected_status = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
+
+  expected_err = ".*svn: E155035: .*excluded by server*"
+  svntest.actions.run_and_verify_svn(None, None, expected_err,
+                                     'rm', sbox.ospath('A/B/E'))
+  svntest.actions.run_and_verify_svn(None, None, expected_err,
+                                     'rm', sbox.ospath('A'))
+
+  expected_status = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
 
 
 ########################################################################
@@ -1234,6 +1266,7 @@ test_list = [ None,
               authz_recursive_ls,
               case_sensitive_authz,
               authz_tree_conflict,
+              wc_delete,
              ]
 serial_only = True
 



Re: svn commit: r1127864 - in /subversion/trunk/subversion: libsvn_wc/adm_ops.c libsvn_wc/wc_db.c tests/cmdline/authz_tests.py

Posted by Greg Stein <gs...@gmail.com>.
On May 26, 2011 9:13 AM, "Philip Martin" <ph...@wandisco.com> wrote:
>
> Greg Stein <gs...@gmail.com> writes:
>
> > On May 26, 2011 6:58 AM, <ph...@apache.org> wrote:
> >>
> >> @@ -6078,6 +6078,28 @@ op_delete_txn(void *baton,
> >>       || status == svn_wc__db_status_not_present)
> >>     return SVN_NO_ERROR;
> >>
> >> +  like_arg = construct_like_arg(local_relpath, scratch_pool);
> >> +
> >> +  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> >> +                                    STMT_SELECT_ABSENT_NODES));
> >> +  SVN_ERR(svn_sqlite__bindf(stmt, "iss",
> >> +                            wcroot->wc_id, local_relpath, like_arg));
> >> +  SVN_ERR(svn_sqlite__step(&have_row, stmt));
> >> +  if (have_row)
> >
> > Maybe switch to a statement that has a LIMIT 1? That's gotta be way
easier
> > on SQLite.
>
> STMT_SELECT_ABSENT_NODES has LIMIT 1.

Eek. Then I'd say it is named poorly. Maybe something like
STMT_HAS_ABSENT_NODES, since you're not really selecting them (I've used
that naming elsewhere).

>
> >
> >> +    {
> >> +      const char *absent_path
> >> +        = svn_dirent_local_style(svn_sqlite__column_text(stmt, 0,
> > scratch_pool),
> >> +                                 scratch_pool);
> >
> > The column fetch can use NULL for the pool.
>
> svn_dirent_local_style can return the input path, I'd need to generate
> the error message before calling svn_sqlite__reset.

Yikes. Didn't know that. Thanks for the Clue!

Cheers,
-g

Re: svn commit: r1127864 - in /subversion/trunk/subversion: libsvn_wc/adm_ops.c libsvn_wc/wc_db.c tests/cmdline/authz_tests.py

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> On May 26, 2011 6:58 AM, <ph...@apache.org> wrote:
>>
>> @@ -6078,6 +6078,28 @@ op_delete_txn(void *baton,
>>       || status == svn_wc__db_status_not_present)
>>     return SVN_NO_ERROR;
>>
>> +  like_arg = construct_like_arg(local_relpath, scratch_pool);
>> +
>> +  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> +                                    STMT_SELECT_ABSENT_NODES));
>> +  SVN_ERR(svn_sqlite__bindf(stmt, "iss",
>> +                            wcroot->wc_id, local_relpath, like_arg));
>> +  SVN_ERR(svn_sqlite__step(&have_row, stmt));
>> +  if (have_row)
>
> Maybe switch to a statement that has a LIMIT 1? That's gotta be way easier
> on SQLite.

STMT_SELECT_ABSENT_NODES has LIMIT 1.

>
>> +    {
>> +      const char *absent_path
>> +        = svn_dirent_local_style(svn_sqlite__column_text(stmt, 0,
> scratch_pool),
>> +                                 scratch_pool);
>
> The column fetch can use NULL for the pool.

svn_dirent_local_style can return the input path, I'd need to generate
the error message before calling svn_sqlite__reset.

>
>> +
>> +      return svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS,
>> +                               svn_sqlite__reset(stmt),
>> +                           _("Cannot delete '%s' as '%s'is excluded by
> server"),
>
> Needs a space before "is"

Ah, yes.

-- 
Philip

Re: svn commit: r1127864 - in /subversion/trunk/subversion: libsvn_wc/adm_ops.c libsvn_wc/wc_db.c tests/cmdline/authz_tests.py

Posted by Greg Stein <gs...@gmail.com>.
On May 26, 2011 6:58 AM, <ph...@apache.org> wrote:
>
> @@ -6078,6 +6078,28 @@ op_delete_txn(void *baton,
>       || status == svn_wc__db_status_not_present)
>     return SVN_NO_ERROR;
>
> +  like_arg = construct_like_arg(local_relpath, scratch_pool);
> +
> +  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> +                                    STMT_SELECT_ABSENT_NODES));
> +  SVN_ERR(svn_sqlite__bindf(stmt, "iss",
> +                            wcroot->wc_id, local_relpath, like_arg));
> +  SVN_ERR(svn_sqlite__step(&have_row, stmt));
> +  if (have_row)

Maybe switch to a statement that has a LIMIT 1? That's gotta be way easier
on SQLite.

> +    {
> +      const char *absent_path
> +        = svn_dirent_local_style(svn_sqlite__column_text(stmt, 0,
scratch_pool),
> +                                 scratch_pool);

The column fetch can use NULL for the pool.

> +
> +      return svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS,
> +                               svn_sqlite__reset(stmt),
> +                           _("Cannot delete '%s' as '%s'is excluded by
server"),

Needs a space before "is"

Cheers,
-g