[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r980811 - in /subversion/trunk/subversion/libsvn_fs_fs: fs.h fs_fs.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sun, 1 Aug 2010 15:58:14 +0200

> -----Original Message-----
> From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> Sent: vrijdag 30 juli 2010 16:54
> To: commits_at_subversion.apache.org
> Subject: svn commit: r980811 - in
> /subversion/trunk/subversion/libsvn_fs_fs: fs.h fs_fs.c
>
> Author: stsp
> Date: Fri Jul 30 14:54:26 2010
> New Revision: 980811
>
> URL: http://svn.apache.org/viewvc?rev=980811&view=rev
> Log:
> Fix the "missing fsfs.conf" file situation in FSFS.
> This is a follow-up to the partial fix made in r905303.
>
> Make svnadmin upgrade create the fsfs.conf file, and make svnadmin
> hotcopy ask the user to run svnadmin upgrade if the file is missing.
>
> * subversion/libsvn_fs_fs/fs.h
> (SVN_FS_FS__MIN_CONFIG_FILE): New constant.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (upgrade_body): Try to create the fsfs.conf config file if it is not
> a file, and error out if it is a directory.
> (svn_fs_fs__hotcopy): Only try to copy fsfs.conf if the format is
> new enough to support fsfs.conf. If copying fsfs.conf fails because
> it does not exist, raise an error asking the user to run svnadmin
> upgrade.
>
> Review by: danielsh
> (but he saw a slightly older version of this diff)
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/fs.h
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/f
> s.h?rev=980811&r1=980810&r2=980811&view=diff
> =======================================================================
> =======
> --- subversion/trunk/subversion/libsvn_fs_fs/fs.h (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs.h Fri Jul 30 14:54:26
> 2010
> @@ -125,6 +125,9 @@ extern "C" {
> /* The minimum format number that supports packed revprop shards. */
> #define SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT 5
>
> +/* The minimum format number that supports a configuration file
> (fsfs.conf) */
> +#define SVN_FS_FS__MIN_CONFIG_FILE 4
> +
> /* Private FSFS-specific data shared between all svn_txn_t objects
> that
> relate to a particular transaction in a filesystem (as identified
> by transaction id and filesystem UUID). Objects of this type are
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/f
> s_fs.c?rev=980811&r1=980810&r2=980811&view=diff
> =======================================================================
> =======
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul 30
> 14:54:26 2010
> @@ -1275,11 +1275,32 @@ upgrade_body(void *baton, apr_pool_t *po
> fs_fs_data_t *ffd = fs->fsap_data;
> int format, max_files_per_dir;
> const char *format_path = path_format(fs, pool);
> + svn_node_kind_t kind;
>
> /* Read the FS format number and max-files-per-dir setting. */
> SVN_ERR(read_format(&format, &max_files_per_dir, format_path,
> pool));
>
> - /* If we're already up-to-date, there's nothing to be done here. */
> + /* If the config file does not exist, create one. */
> + SVN_ERR(svn_io_check_path(svn_dirent_join(fs->path, PATH_CONFIG,
> pool),
> + &kind, pool));
> + switch (kind)
> + {
> + case svn_node_none:
> + SVN_ERR(write_config(fs, pool));
> + break;
> + case svn_node_dir:
> + return svn_error_return(svn_error_createf(SVN_ERR_FS_GENERAL,
> NULL,
> + _("'%s' is a
> directory. "
> + "Please move it out
> of "
> + "the way and try
> again"),
> + svn_dirent_join(fs-
> >path,
> +
> PATH_CONFIG,
> +
> pool)));
> + default:
> + break;
> + }
> +
> + /* If we're already up-to-date, there's nothing else to be done
> here. */
> if (format == SVN_FS_FS__FORMAT_NUMBER)
> return SVN_NO_ERROR;
>
> @@ -1496,15 +1517,64 @@ svn_fs_fs__hotcopy(const char *src_path,
> pool));
> SVN_ERR(check_format(format));
>
> + /* Try to copy the config.
> + *
> + * ### We try copying the config file before doing anything else,
> + * ### because higher layers will abort the hotcopy if we throw
> + * ### an error from this function, and that renders the hotcopy
> + * ### unusable anyway. */
> + if (format >= SVN_FS_FS__MIN_CONFIG_FILE)
> + {
> + svn_error_t *err;
> +
> + err = svn_io_dir_file_copy(src_path, dst_path, PATH_CONFIG,
> pool);
> + if (err)
> + {
> + if (APR_STATUS_IS_ENOENT(err->apr_err))
> + {
> + /* 1.6.0 to 1.6.11 did not copy the configuration file
> during
> + * hotcopy. So if we're hotcopying a repository which
> has been
> + * created as a hotcopy itself, it's possible that
> fsfs.conf
> + * does not exist. Ask the user to re-create it.
> + *
> + * ### It would be nice to make this a non-fatal error,
> + * ### but this function does not get an svn_fs_t object
> + * ### so we have no way of just printing a warning via
> + * ### the fs->warning() callback. */
> +
> + const char *msg;
> + const char *src_abspath;
> + const char *dst_abspath;
> + const char *config_relpath;
> +
> + config_relpath = svn_dirent_join(src_path, PATH_CONFIG,
> pool);
> + SVN_ERR(svn_dirent_get_absolute(&src_abspath, src_path,
> pool));
> + SVN_ERR(svn_dirent_get_absolute(&dst_abspath, dst_path,
> pool));
> +
> + /* ### hack: strip off the 'db/' directory from paths so
> + * ### they make sense to the user */
> + src_abspath = svn_dirent_dirname(src_abspath, pool);
> + dst_abspath = svn_dirent_dirname(dst_abspath, pool);

I don't think we should remove the '/db' part here.

At the repos layer the '/db' doesn't make sense, but this is an implementation on the 'fs' layer and there the 'db' does make sense.

(And if we decide to remove /db here, we should remove it in at least a dozen other error messages defined in subversion/include/private/svn_fs_util.h. (I actually wrote a patch to do that a few days ago, but then I found the different layering).

        Bert

> +
> + msg = apr_psprintf(pool,
> + _("Failed to create hotcopy at '%s'.
> "
> + "The file '%s' is missing from the
> source "
> + "repository. Please create this
> file, for "
> + "instance by running 'svnadmin
> upgrade %s'"),
> + dst_abspath, config_relpath,
> src_abspath);
> + return svn_error_return(svn_error_quick_wrap(err, msg));
> + }
> + else
> + return svn_error_return(err);
> + }
> + }
> +
> /* Copy the 'current' file. */
> SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_CURRENT,
> pool));
>
> /* Copy the uuid. */
> SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_UUID, pool));
>
> - /* Copy the config. */
> - SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_CONFIG,
> pool));
> -
> /* Copy the rep cache before copying the rev files to make sure all
> cached references will be present in the copy. */
> src_subdir = svn_dirent_join(src_path, REP_CACHE_DB_NAME, pool);
>
Received on 2010-08-01 15:58:57 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.