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

Re: A proposed solution for svn admin directory names

From: Branko Čibej <brane_at_xbc.nu>
Date: 2005-07-12 04:58:22 CEST

Branko Čibej wrote:

> All of which convinced me that we should deprecate
> SVN_WC_ADM_DIR_NAME, and replace all its uses outside libsvn_wc
> (which, not surprisingly, are all string compares against this
> constant) with calls to a new function in libsvn_wc, e.g.,
> svn_wc_is_adm_dir(const char*).
>
> Then we can deal with using different admin dir names privately inside
> libsvn_wc, where such decisions belong.

The initial patch is below. It's a quick mock-up, but it removes the
need for a public symbol for the adm dir name -- except for one place in
libsvn_subr, where I've left the current code in because we'd get a
circular dependency between libsvn_wc and libsvn_subr, which I don't
quite know how to handle yet.

The point is that if/when this is in, we can keep the knowledge about
the actual admin directory name inside libsvn_wc/adm_files.c. If we
decide that getenv is appropriate for 1.x, we can still cache the result
in the local variable (using apr_atomc_casptr to change the value; we
seem to have forgotten that APR has had atomics since at least 0.9.5, if
not earlier, so this kind of local caching can be entirely safe).

Tests pass on Windows with this patch.

[[[
Remove knowledge about the admin directory name from the public
API, preparing the way for run-time parameterisation.

* subversion/include/svn_wc.h (svn_wc_is_adm_dir): New prototype.
  (SVN_WC_ADM_DIR_NAME): Deprecate.

* subversion/libsvn_wc/adm_files.c (DEFAULT_ADM_DIR_NAME): New constant.
  (adm_dir_name) New file-scope variable.
  (svn_wc_is_adm_dir): Implement.
  (v_extend_with_adm_name): Use adm_dir_name instead of SVN_WC_ADM_DIR_NAME.

* subversion/libsvn_wc/copy.c (post_copy_cleanup): Call svn_wc__adm_path
   to construct the path to the admin directory.

* subversion/libsvn_wc/adm_ops.c (erase_from_wc),
  subversion/libsvn_wc/status.c (get_dir_status),
  subversion/libsvn_wc/update_editor.c (add_directory),
  subversion/libsvn_client/add.c (add_dir_recursive),
  subversion/libsvn_client/commit.c (import_dir, svn_client_import2):
   Call svn_wc_is_adm_dir instead of strcmp'ing with SVN_WC_ADM_DIR_NAME.

* subversion/libsvn_subr/opt.c (svn_opt_args_to_target_array2):
   Mark the place where a similar change should be made.
]]]

Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 15318)
+++ subversion/include/svn_wc.h (working copy)
@@ -337,6 +337,15 @@
                             apr_pool_t *pool);
 
 
+/**
+ * Return @c TRUE if @a name is the name of the WC administrative
+ * directory (usually ".svn"). Use @a pool for any temporary allocations.
+ *
+ * @since New in 1.3.
+ */
+svn_boolean_t svn_wc_is_adm_dir (const char *name, apr_pool_t *pool);
+
+
 
 /** Traversal information is information gathered by a working copy
  * crawl or update. For example, the before and after values of the
@@ -1023,6 +1032,8 @@
  * who knew the adm subdir's name). However, import wants to protect
  * against importing administrative subdirs, so now the name is a
  * matter of public record.
+ *
+ * @deprecated Provided for backward compatibility with the 1.2 API.
  */
 #define SVN_WC_ADM_DIR_NAME ".svn"
 
Index: subversion/libsvn_wc/copy.c
===================================================================
--- subversion/libsvn_wc/copy.c (revision 15318)
+++ subversion/libsvn_wc/copy.c (working copy)
@@ -291,7 +291,7 @@
      hidden. */
 #ifdef APR_FILE_ATTR_HIDDEN
   {
- const char *adm_dir = svn_path_join (path, SVN_WC_ADM_DIR_NAME, pool);
+ const char *adm_dir = svn_wc__adm_path (path, FALSE, pool, NULL);
     const char *path_apr;
     apr_status_t status;
     SVN_ERR (svn_path_cstring_from_utf8 (&path_apr, adm_dir, pool));
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 15318)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -770,7 +770,7 @@
             name = key;
 
             /* The admin directory will show up, we don't want to delete it */
- if (!strcmp (name, SVN_WC_ADM_DIR_NAME))
+ if (svn_wc_is_adm_dir (name, pool))
               continue;
 
             /* Versioned directories will show up, don't delete those either */
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 15318)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -869,8 +869,8 @@
         
       /* Skip versioned things, and skip the administrative
          directory. */
- if ((apr_hash_get (entries, key, klen))
- || (strcmp (key, SVN_WC_ADM_DIR_NAME) == 0))
+ if (apr_hash_get (entries, key, klen)
+ || svn_wc_is_adm_dir (key, subpool))
         continue;
 
       /* Clear the iteration subpool. */
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c (revision 15318)
+++ subversion/libsvn_wc/adm_files.c (working copy)
@@ -42,7 +42,25 @@
 
 /*** File names in the adm area. ***/
 
+/* The default name of the WC admin directory. This name is always
+ checked by svn_wc_is_adm_dir. */
+#define DEFAULT_ADM_DIR_NAME ".svn"
 
+/* The name that is actually used for the WC admin directory. The
+ commonest case where this won't be the default is in Windows
+ ASP.NET development environments, which choke on ".svn". */
+static const char *adm_dir_name = DEFAULT_ADM_DIR_NAME;
+
+
+svn_boolean_t
+svn_wc_is_adm_dir (const char *name, apr_pool_t *pool)
+{
+ (void)pool; /* Silence compiler warnings about unused parameter */
+ return (0 == strcmp (name, adm_dir_name)
+ || 0 == strcmp (name, DEFAULT_ADM_DIR_NAME));
+}
+
+
 /* Return the path to something in PATH's administrative area.
  *
  * First, the adm subdir is appended to PATH as a component, then the
@@ -66,7 +84,7 @@
   const char *this;
 
   /* Tack on the administrative subdirectory. */
- path = svn_path_join (path, SVN_WC_ADM_DIR_NAME, pool);
+ path = svn_path_join (path, adm_dir_name, pool);
 
   /* If this is a tmp file, name it into the tmp area. */
   if (use_tmp)
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 15318)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -1050,7 +1050,7 @@
        svn_path_local_style (db->path, pool));
 
   /* It may not be named the same as the administrative directory. */
- if (strcmp (svn_path_basename (path, pool), SVN_WC_ADM_DIR_NAME) == 0)
+ if (svn_wc_is_adm_dir (svn_path_basename (path, pool), pool))
     return svn_error_createf
       (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
        _("Failed to add directory '%s': object of the same name as the "
Index: subversion/libsvn_subr/opt.c
===================================================================
--- subversion/libsvn_subr/opt.c (revision 15318)
+++ subversion/libsvn_subr/opt.c (working copy)
@@ -654,6 +654,10 @@
              target a SVN admin dir unless svn_wc_check_wc passes on
              the target, too? */
           base_name = svn_path_basename (target, pool);
+ /* FIXME: How to avoid the circular dependency between
+ linsvn_wc and libsvn_subr here?
+ if (svn_wc_is_adm_dir (base_name, pool))
+ */
           if (! strcmp (base_name, SVN_WC_ADM_DIR_NAME))
             continue;
         }
Index: subversion/libsvn_client/add.c
===================================================================
--- subversion/libsvn_client/add.c (revision 15318)
+++ subversion/libsvn_client/add.c (working copy)
@@ -327,7 +327,7 @@
         SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
       
       /* Skip over SVN admin directories. */
- if (strcmp (this_entry.name, SVN_WC_ADM_DIR_NAME) == 0)
+ if (svn_wc_is_adm_dir (this_entry.name, subpool))
         continue;
 
       /* Skip entries for this dir and its parent. */
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 15318)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -335,7 +335,7 @@
       if (ctx->cancel_func)
         SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
 
- if (strcmp (filename, SVN_WC_ADM_DIR_NAME) == 0)
+ if (svn_wc_is_adm_dir (filename, subpool))
         {
           /* If someone's trying to import a directory named the same
              as our administrative directories, that's probably not
@@ -748,14 +748,15 @@
   /* The repository doesn't know about the reserved administrative
      directory. */
   if (new_entries->nelts &&
- (strcmp (APR_ARRAY_IDX (new_entries,
- new_entries->nelts - 1,
- const char *), SVN_WC_ADM_DIR_NAME) == 0))
+ svn_wc_is_adm_dir (temp = APR_ARRAY_IDX (new_entries,
+ new_entries->nelts - 1,
+ const char *),
+ pool))
     return svn_error_createf
       (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
        _("'%s' is a reserved name and cannot be imported"),
        /* ### Is svn_path_local_style() really necessary for this? */
- svn_path_local_style (SVN_WC_ADM_DIR_NAME, pool));
+ svn_path_local_style (temp, pool));
 
 
   /* If an error occurred during the commit, abort the edit and return

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 12 05:00:06 2005

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