You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2013/01/03 21:47:37 UTC

svn commit: r1428585 - in /subversion/trunk/subversion: libsvn_client/status.c libsvn_wc/adm_ops.c libsvn_wc/diff_local.c svn/svn.c tests/cmdline/changelist_tests.py

Author: cmpilato
Date: Thu Jan  3 20:47:37 2013
New Revision: 1428585

URL: http://svn.apache.org/viewvc?rev=1428585&view=rev
Log:
Per discussion on IRC, and in compliance with one of the suggestions
found at http://subversion.tigris.org/issues/show_bug.cgi?id=3348#desc10,
revert the changes (r1159286, r1159299, and r1160691) which caused the
empty-string changelist name (as applied to the --changelist option)
to be specially handled.

Modified:
    subversion/trunk/subversion/libsvn_client/status.c
    subversion/trunk/subversion/libsvn_wc/adm_ops.c
    subversion/trunk/subversion/libsvn_wc/diff_local.c
    subversion/trunk/subversion/svn/svn.c
    subversion/trunk/subversion/tests/cmdline/changelist_tests.py

Modified: subversion/trunk/subversion/libsvn_client/status.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/status.c?rev=1428585&r1=1428584&r2=1428585&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/status.c (original)
+++ subversion/trunk/subversion/libsvn_client/status.c Thu Jan  3 20:47:37 2013
@@ -85,23 +85,12 @@ tweak_status(void *baton,
   /* If the status item has an entry, but doesn't belong to one of the
      changelists our caller is interested in, we filter out this status
      transmission.  */
-  /* ### duplicated in ../libsvn_wc/diff_local.c */
-  if (sb->changelist_hash)
+  if (sb->changelist_hash
+      && (! status->changelist
+          || ! apr_hash_get(sb->changelist_hash, status->changelist,
+                            APR_HASH_KEY_STRING)))
     {
-      if (status->changelist)
-        {
-          /* Skip unless the caller requested this changelist. */
-          if (! apr_hash_get(sb->changelist_hash, status->changelist,
-                             APR_HASH_KEY_STRING))
-            return SVN_NO_ERROR;
-        }
-      else
-        {
-          /* Skip unless the caller requested changelist-lacking items. */
-          if (! apr_hash_get(sb->changelist_hash, "",
-                             APR_HASH_KEY_STRING))
-            return SVN_NO_ERROR;
-        }
+      return SVN_NO_ERROR;
     }
 
   /* If we know that the target was deleted in HEAD of the repository,

Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1428585&r1=1428584&r2=1428585&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Thu Jan  3 20:47:37 2013
@@ -2676,11 +2676,9 @@ svn_wc__internal_changelist_match(svn_wc
       return FALSE;
     }
 
-  /* The empty changelist name is special-cased. */
   return (changelist
-          ? apr_hash_get((apr_hash_t *)clhash, changelist, APR_HASH_KEY_STRING)
-          : apr_hash_get((apr_hash_t *)clhash, "", APR_HASH_KEY_STRING)
-         ) != NULL;
+            && apr_hash_get((apr_hash_t *)clhash, changelist,
+                            APR_HASH_KEY_STRING) != NULL);
 }
 
 

Modified: subversion/trunk/subversion/libsvn_wc/diff_local.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/diff_local.c?rev=1428585&r1=1428584&r2=1428585&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/diff_local.c (original)
+++ subversion/trunk/subversion/libsvn_wc/diff_local.c Thu Jan  3 20:47:37 2013
@@ -446,25 +446,11 @@ diff_status_callback(void *baton,
         break; /* Go check other conditions */
     }
 
-  /* Filter items by changelist. */
-  /* ### duplicated in ../libsvn_client/status.c */
-  if (eb->changelist_hash)
-    {
-      if (status->changelist)
-        {
-          /* Skip unless the caller requested this changelist. */
-          if (! apr_hash_get(eb->changelist_hash, status->changelist,
-                             APR_HASH_KEY_STRING))
-            return SVN_NO_ERROR;
-        }
-      else
-        {
-          /* Skip unless the caller requested changelist-lacking items. */
-          if (! apr_hash_get(eb->changelist_hash, "",
-                             APR_HASH_KEY_STRING))
-            return SVN_NO_ERROR;
-        }
-    }
+  if (eb->changelist_hash != NULL
+      && (!status->changelist
+          || ! apr_hash_get(eb->changelist_hash, status->changelist,
+                            APR_HASH_KEY_STRING)))
+    return SVN_NO_ERROR; /* Filtered via changelist */
 
   /* ### The following checks should probably be reversed as it should decide
          when *not* to show a diff, because generally all changed nodes should

Modified: subversion/trunk/subversion/svn/svn.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/svn.c?rev=1428585&r1=1428584&r2=1428585&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/svn.c (original)
+++ subversion/trunk/subversion/svn/svn.c Thu Jan  3 20:47:37 2013
@@ -2096,6 +2096,12 @@ sub_main(int argc, const char *argv[], a
         break;
       case opt_changelist:
         opt_state.changelist = apr_pstrdup(pool, opt_arg);
+        if (opt_state.changelist[0] == '\0')
+          {
+            err = svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                                   _("Changelist names must not be empty"));
+            return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+          }
         apr_hash_set(changelists, opt_state.changelist,
                      APR_HASH_KEY_STRING, (void *)1);
         break;

Modified: subversion/trunk/subversion/tests/cmdline/changelist_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/changelist_tests.py?rev=1428585&r1=1428584&r2=1428585&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/changelist_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/changelist_tests.py Thu Jan  3 20:47:37 2013
@@ -1189,57 +1189,6 @@ def readd_after_revert(sbox):
   svntest.actions.run_and_verify_svn(None, None, [],
                                      'add', dummy)
 
-def empty_pseudo_changelist(sbox):
-  "the empty pseudo-changelist"
-
-  # Boilerplate.
-  sbox.build(read_only = True)
-  wc_dir = sbox.wc_dir
-
-  # Helper functions.
-
-  def found_nodes(*args):
-    # Extract the Greek-tree-relative paths.
-    return set(map(lambda info: info['Path'][len(wc_dir)+1:],
-                    svntest.actions.run_and_parse_info(*args)))
-
-  def find_nodes(nodeset, *args):
-    assert isinstance(nodeset, set)
-    foundset = found_nodes(*args)
-    nodeset = set(map(lambda path: path.replace('/', os.path.sep), nodeset))
-    if nodeset != foundset:
-      raise svntest.Failure("Expected nodeset %s but found %s"
-                            % (nodeset, foundset))
-
-  # Convenience variables.
-  E_path = sbox.ospath('A/B/E')
-  alpha_path = sbox.ospath('A/B/E/alpha')
-  beta_path = sbox.ospath('A/B/E/beta')
-  iota_path = sbox.ospath('iota')
-
-  # Can't add an item to the empty changelist.
-  expected_err = 'svn: E125014: .*'
-  svntest.actions.run_and_verify_svn(None, [], expected_err,
-                                     'changelist', '', iota_path)
-
-  # Modify alpha and beta
-  svntest.main.file_append(alpha_path, "More stuff in alpha\n")
-  svntest.main.file_append(beta_path, "More stuff in beta\n")
-
-  # Add beta to 'testlist'.
-  svntest.actions.run_and_verify_svn(None, None, [],
-                                     'changelist', 'testlist', beta_path)
-
-  # Convenience variables.
-  changelist = {
-    'testlist' : set(['A/B/E/beta']),
-    '' : set(['A/B/E', 'A/B/E/alpha']),
-  }
-
-  # Some basic validations.
-  find_nodes(changelist['testlist'] | changelist[''], '-R', E_path)
-  find_nodes(changelist['testlist'], '--cl', 'testlist', '-R', E_path)
-  find_nodes(changelist[''], '--cl', '', '-R', E_path)
 
 ########################################################################
 # Run the tests
@@ -1263,7 +1212,6 @@ test_list = [ None,
               add_remove_non_existent_target,
               add_remove_unversioned_target,
               readd_after_revert,
-              empty_pseudo_changelist,
              ]
 
 if __name__ == '__main__':