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/05/23 17:56:42 UTC

svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Author: julianfoad
Date: Mon May 23 15:56:42 2011
New Revision: 1126553

URL: http://svn.apache.org/viewvc?rev=1126553&view=rev
Log:
Fix the code added in r1125455 to add ".svn-base" to pristine file names,
add a test for it, and clean it up a bit.  The test is XFail because that
code will remain inactive until the WC format is bumped to 29.

Found by: gstein

* subversion/libsvn_wc/upgrade.c
  (PRISTINE_STORAGE_EXT, PRISTINE_BASENAME_OLD_LEN): New definitions.
  (rename_pristine_file): Check the length of the path basename, not the
    whole path. Rename path variables to indicate 'abspath'. Use a defined
    constant for the ".svn-base" extension.
  (bump_to_29): Rename path variables to indicate 'abspath'.

* subversion/tests/cmdline/upgrade_tests.py
  (upgrade_from_format_28): New test, marked XFail.
  (test_list): Add the new test.

* subversion/tests/cmdline/upgrade_tests_data/format_28.tar.bz2
  New file, containing a Greek tree checkout at format 28.

Added:
    subversion/trunk/subversion/tests/cmdline/upgrade_tests_data/format_28.tar.bz2   (with props)
Modified:
    subversion/trunk/subversion/libsvn_wc/upgrade.c
    subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/upgrade.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/upgrade.c?rev=1126553&r1=1126552&r2=1126553&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/upgrade.c (original)
+++ subversion/trunk/subversion/libsvn_wc/upgrade.c Mon May 23 15:56:42 2011
@@ -73,6 +73,9 @@
 
 /* New pristine location */
 #define PRISTINE_STORAGE_RELPATH "pristine"
+#define PRISTINE_STORAGE_EXT ".svn-base"
+/* Number of characters in a pristine file basename, in WC format <= 28. */
+#define PRISTINE_BASENAME_OLD_LEN 40
 #define SDB_FILE  "wc.db"
 
 
@@ -1213,7 +1216,9 @@ bump_to_28(void *baton, svn_sqlite__db_t
   return SVN_NO_ERROR;
 }
 
-/* If FINFO indicates that PATH names a file, rename it to '<PATH>.svn-base'.
+/* If FINFO indicates that ABSPATH names a file, rename it to
+ * '<ABSPATH>.svn-base'.
+ *
  * Ignore any file whose name is not the expected length, in order to make
  * life easier for any developer who runs this code twice or has some
  * non-standard files in the pristine directory.
@@ -1221,17 +1226,18 @@ bump_to_28(void *baton, svn_sqlite__db_t
  * A callback for bump_to_29(), implementing #svn_io_walk_func_t. */
 static svn_error_t *
 rename_pristine_file(void *baton,
-                     const char *path,
+                     const char *abspath,
                      const apr_finfo_t *finfo,
                      apr_pool_t *pool)
 {
-  const char *new_path;
-
-  /* The old pristine file name was 40 hex digits. */
-  if (finfo->filetype == APR_REG && strlen(path) == 40)
+  if (finfo->filetype == APR_REG
+      && (strlen(svn_dirent_basename(abspath, pool))
+          == PRISTINE_BASENAME_OLD_LEN))
     {
-      new_path = apr_pstrcat(pool, path, ".svn-base", (char *)NULL);
-      SVN_ERR(svn_io_file_rename(path, new_path, pool));
+      const char *new_abspath
+        = apr_pstrcat(pool, abspath, PRISTINE_STORAGE_EXT, (char *)NULL);
+
+      SVN_ERR(svn_io_file_rename(abspath, new_abspath, pool));
     }
   return SVN_NO_ERROR;
 }
@@ -1240,17 +1246,17 @@ static svn_error_t *
 bump_to_29(void *baton, svn_sqlite__db_t *sdb, apr_pool_t *scratch_pool)
 {
   const char *wcroot_abspath = ((struct bump_baton *)baton)->wcroot_abspath;
-  const char *pristine_dir;
+  const char *pristine_dir_abspath;
 
   /* ### Before enabling this code we should be able to upgrade existing
      ### file externals to their new location */
   SVN_ERR_MALFUNCTION();
 
   /* Rename all pristine files, adding a ".svn-base" suffix. */
-  pristine_dir = svn_dirent_join_many(scratch_pool, wcroot_abspath,
-                                      svn_wc_get_adm_dir(scratch_pool),
-                                      PRISTINE_STORAGE_RELPATH, NULL);
-  SVN_ERR(svn_io_dir_walk2(pristine_dir, APR_FINFO_MIN,
+  pristine_dir_abspath = svn_dirent_join_many(scratch_pool, wcroot_abspath,
+                                              svn_wc_get_adm_dir(scratch_pool),
+                                              PRISTINE_STORAGE_RELPATH, NULL);
+  SVN_ERR(svn_io_dir_walk2(pristine_dir_abspath, APR_FINFO_MIN,
                            rename_pristine_file, NULL, scratch_pool));
 
   /* Externals */

Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgrade_tests.py?rev=1126553&r1=1126552&r2=1126553&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Mon May 23 15:56:42 2011
@@ -935,6 +935,29 @@ def tree_replace2(sbox):
   #   C/g
   #   C/E
 
+@XFail()  # Requires WC format >= 29.
+def upgrade_from_format_28(sbox):
+  """upgrade from format 28: rename pristines"""
+
+  # Start with a format-28 WC that is a clean checkout of the Greek tree.
+  sbox.build(create_wc = False)
+  replace_sbox_with_tarfile(sbox, 'format_28.tar.bz2')
+
+  # Get the old and new pristine file paths for file 'iota'.
+  checksum = '2c0aa9014a0cd07f01795a333d82485ef6d083e2'
+  old_pristine_path = os.path.join(sbox.wc_dir, svntest.main.get_admin_name(),
+                                   'pristine', checksum[0:2], checksum)
+  new_pristine_path = old_pristine_path + '.svn-base'
+
+  assert os.path.exists(old_pristine_path)
+  assert not os.path.exists(new_pristine_path)
+
+  # Touch the WC to auto-upgrade it
+  svntest.actions.run_and_verify_svn(None, None, [], 'info', sbox.wc_dir)
+
+  assert not os.path.exists(old_pristine_path)
+  assert os.path.exists(new_pristine_path)
+
 ########################################################################
 # Run the tests
 
@@ -976,6 +999,7 @@ test_list = [ None,
               upgrade_with_scheduled_change,
               tree_replace1,
               tree_replace2,
+              upgrade_from_format_28,
              ]
 
 

Added: subversion/trunk/subversion/tests/cmdline/upgrade_tests_data/format_28.tar.bz2
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgrade_tests_data/format_28.tar.bz2?rev=1126553&view=auto
==============================================================================
Binary file - no diff available.

Propchange: subversion/trunk/subversion/tests/cmdline/upgrade_tests_data/format_28.tar.bz2
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream



Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-05-23 at 13:16 -0400, Greg Stein wrote:
> On May 23, 2011 12:48 PM, "Julian Foad" <ju...@wandisco.com> wrote:
> >
> > Greg Stein wrote:
> > > On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
> > > >...
> > > was 40 hex digits. */
> > > > -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> > > > +  if (finfo->filetype == APR_REG
> > > > +      && (strlen(svn_dirent_basename(abspath, pool))
> > > > +          ==
> > >
> > > On my phone, so I can't tell, but ... can basename() take NULL for the
> pool?
> >
> > Yes it can.  You thinking we should pass NULL and so squeeze a few more
> > microseconds out of the developer-only upgrade scenario?
> 
> The basename is completely transient, so there's no reason to alloc and
> copy.
> 
> That said: you're right. It was a reflexive suggestion, which is kinda moot
> given the larger context.

No sweat.  I got as far as being about to change it when I thought, Hold
on, this just isn't worth it this time.

- Julian



Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Greg Stein <gs...@gmail.com>.
On May 23, 2011 12:48 PM, "Julian Foad" <ju...@wandisco.com> wrote:
>
> Greg Stein wrote:
> > On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
> > >...
> > was 40 hex digits. */
> > > -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> > > +  if (finfo->filetype == APR_REG
> > > +      && (strlen(svn_dirent_basename(abspath, pool))
> > > +          ==
> >
> > On my phone, so I can't tell, but ... can basename() take NULL for the
pool?
>
> Yes it can.  You thinking we should pass NULL and so squeeze a few more
> microseconds out of the developer-only upgrade scenario?

The basename is completely transient, so there's no reason to alloc and
copy.

That said: you're right. It was a reflexive suggestion, which is kinda moot
given the larger context.

Cheers,
-g

Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Julian Foad <ju...@wandisco.com>.
Greg Stein wrote:
> On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
> >...
> was 40 hex digits. */
> > -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> > +  if (finfo->filetype == APR_REG
> > +      && (strlen(svn_dirent_basename(abspath, pool))
> > +          ==
> 
> On my phone, so I can't tell, but ... can basename() take NULL for the pool?

Yes it can.  You thinking we should pass NULL and so squeeze a few more
microseconds out of the developer-only upgrade scenario?

- Julian



Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Greg Stein <gs...@gmail.com>.
On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
>...
was 40 hex digits. */
> -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> +  if (finfo->filetype == APR_REG
> +      && (strlen(svn_dirent_basename(abspath, pool))
> +          ==

On my phone, so I can't tell, but ... can basename() take NULL for the pool?