On Fri, May 20, 2011 at 12:38, <julianfoad_at_apache.org> wrote:
> Author: julianfoad
> Date: Fri May 20 16:38:24 2011
> New Revision: 1125455
>
> URL: http://svn.apache.org/viewvc?rev=1125455&view=rev
> Log:
> Prepare to give pristine text files a filename extension of ".svn-base"
> after the string of 40 hex digits. This will help users who search the disk
> using a tool that can easily exclude matches to a given filename extension
> but cannot easily exclude pristine files any other way.
>
> This change will not be active until SVN_WC__VERSION is bumped to 29, at
> which time svntest/wc.py:text_base_path() will need to be adjusted as
> indicated in its comment.
Did you actually test this by bumping your local Subversion to 29? I
don't see how this code can possibly work. See below.
>...
> +++ subversion/trunk/subversion/libsvn_wc/upgrade.c Fri May 20 16:38:24 2011
> @@ -1213,13 +1213,47 @@ 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'.
> + * 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.
> + *
> + * 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 apr_finfo_t *finfo,
> + apr_pool_t *pool)
> +{
> + const char *new_path;
Please use abspath in these variable names for clarity. The walk began
with an abspath, so the svn_dirent_join() calls in svn_io_dir_walk2()
will also product abspaths.
> +
> + /* The old pristine file name was 40 hex digits. */
> + if (finfo->filetype == APR_REG && strlen(path) == 40)
Because you have abspath values, this strlen() condition can never be true.
> + {
> + new_path = apr_pstrcat(pool, path, ".svn-base", (char *)NULL);
> + SVN_ERR(svn_io_file_rename(path, new_path, pool));
If they were NOT abspaths, then the above rename would never function
because the walk does not change the current directory. But
whatever... we know they are abspaths, but the above condition doesn't
work trigger anyways.
I would also suggest using a #define for the ".svn-base" extension.
upgrade.c has plenty of these at the top. (I wouldn't bother to try
and share it from wc_db.c)
> + }
> + return SVN_NO_ERROR;
> +}
> +
> 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;
Having abspath in the pristine_dir localvar would be nice. That would
increase clarity which would find problems like the strlen() above.
>...
Cheers,
-g
Received on 2011-05-22 15:30:43 CEST