Re: [PATCH] Fix for issue 1797 case two. A non-recursive commit of a deleted directory fails post-commit.

makl <makl@tigris.org> writes:

> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c (revision 9193)
> +++ subversion/libsvn_client/commit.c (working copy)
> @@ -1016,6 +1016,29 @@
> return SVN_NO_ERROR;
> }
>
> +/* Check if a directory must be locked even in non recursive mode.
> + Currently this is the case for deleted directories. */
> +static svn_error_t *
> +check_for_recursive_locking(svn_boolean_t *is_deleted,
> + const char *path,
> + apr_pool_t *pool)
> +{
> + *is_deleted = FALSE;
> +
> + const svn_wc_entry_t *tmp_entry;
> + svn_wc_adm_access_t *tmp_adm_access;
> +
> + SVN_ERR (svn_wc_adm_open2 (&tmp_adm_access, NULL, path, FALSE, 0, pool));
> + SVN_ERR (svn_wc_entry (&tmp_entry, path, tmp_adm_access, TRUE, pool));
> +
> + if (!tmp_entry || (tmp_entry->schedule == svn_wc_schedule_delete))
> + *is_deleted = TRUE;
> +
> + SVN_ERR (svn_wc_adm_close (tmp_adm_access));
> +
> + return SVN_NO_ERROR;
> +}

It's unfortunate that a temporary adm_open is required as that means
reading the entries file twice. It's also a race since it's making a
decision based on the read-only access baton, and that may not be
valid once the write baton is taken. I don't think either of these is
a serious problem, but I wonder if there is a better way to do it.

How about using the existing code to open the access batons, i.e. the
loop over dirs_to_lock and dirs_to_lock_recursive, and then doing
something like

   for dir in dirs_to_lock
     get entry for dir
       if schedule delete
          for each entry in dir
             if entry is dir
                try adm_retrieve for entry
                if not locked
                   take recursive lock for entry

I suppose that would fail if dirs_to_lock contained an item that was
within another schedule delete item:

   svn commit -N foo foo/bar/baz

where foo is schedule delete. Personally I'd accept that as a valid
way to fail, but others may not agree.

This is really another case for svn_wc_adm_extend, a function that was
never written. This mythical function would have an interface like
svn_wc_adm_open, but would not produce an error if the associated
access baton already contained any of the required locks. It's quite
possible this should be the default behaviour for svn_wc_adm_open.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org