On Wed, Jul 10, 2002 at 07:05:30AM -0500, philip@tigris.org wrote:
> Author: philip
> Date: Wed, 10 Jul 2002 07:05:13 -0500
> New Revision: 2452
>
> Modified:
> trunk/subversion/include/svn_error_codes.h
> trunk/subversion/include/svn_wc.h
> trunk/subversion/libsvn_client/commit.c
> trunk/subversion/libsvn_client/commit_util.c
> trunk/subversion/libsvn_client/copy.c
> trunk/subversion/libsvn_wc/adm_files.c
> trunk/subversion/libsvn_wc/adm_files.h
> trunk/subversion/libsvn_wc/lock.c
> trunk/subversion/libsvn_wc/log.c
> trunk/subversion/libsvn_wc/props.c
> trunk/subversion/libsvn_wc/update_editor.c
> trunk/subversion/libsvn_wc/wc.h
> Log:
> First step for issue 749. Introduce an access baton and open/close
> functions. Replace lock/unlock calls with open/close calls and start
> passing an access baton around. Check for a write lock when creating
> adm things.
First of all let me say. Good work Phillip! You are the 4th person to try
this, but the only one who got anything into a commitable state.
>
> +/* ### Should this type be opaque in the public interface? */
> +typedef struct svn_wc_adm_access_t
IMHO: Yes.
> +{
> + /* PATH to directory which contains the administrative area */
> + const char *path;
> +
> + enum svn_wc_adm_access_type {
> +
> + /* SVN_WC_ADM_ACCESS_UNLOCKED indicates no lock is held allowing
> + read-only access without cacheing. */
> + svn_wc_adm_access_unlocked,
> +
> +#if 0
It'd be nice to have a comment indicating why this is #if 0'd out.
> + /* ### If read-only operations are allowed sufficient write access to
> + ### create read locks (did you follow that?) then entries cacheing
> + ### could apply to read-only operations as well. This would
> + ### probably want to fall back to unlocked access if the
> + ### filesystem permissions prohibit writing to the administrative
> + ### area (consider running svn_wc_status on some other user's
> + ### working copy). */
> +
> + /* SVN_WC_ADM_ACCESS_READ_LOCK indicates that read-only access and
> + cacheing are allowed. */
> + svn_wc_adm_access_read_lock,
> +#endif
> +
> + /* SVN_WC_ADM_ACCESS_WRITE_LOCK indicates that read-write access and
> + cacheing are allowed. */
> + svn_wc_adm_access_write_lock
> +
> + } type;
> +
> + /* LOCK_EXISTS is set TRUE when the write lock exists */
> + svn_boolean_t lock_exists;
> +
> +#if 0
> + /* ENTRIES_MODIFED is set TRUE when the entries cached in ENTRIES have
> + been modified from the original values read from the file. */
> + svn_boolean_t entries_modified;
> +
> + /* Once the 'entries' file has been read, ENTRIES will cache the
> + contents if this access baton has an appropriate lock. Otherwise
> + ENTRIES will be NULL. */
> + apr_hash_t *entries;
> +#endif
> +
> + /* POOL is used to allocate cached items, they need to persist for the
> + lifetime of this access baton */
> + apr_pool_t *pool;
> +
> +} svn_wc_adm_access_t;
> +
> +/* Return an access baton in ADM_ACCESS for the working copy administrative
> + area associated with the directory PATH. If WRITE_LOCK is set the baton
> + will include a write lock, otherwise the baton can only be used for read
> + access. POOL will be used to allocate the baton and any subsequently
> + cached items. */
> +svn_error_t *svn_wc_adm_open (svn_wc_adm_access_t **adm_access,
> + const char *path,
> + svn_boolean_t write_lock,
> + apr_pool_t *pool);
What I determined when trying to cache entries files is that numerous places
need to also have access to a parent directory's entries file. (i.e. removing
a directory). This was getting pretty hairy in my patch, and is part of the
reason I scrapped it. I was thinking something along the lines of:
svn_wc_open (void **wc_baton,
const char *path,
svn_boolean_t write_lock,
void *parent_baton,
apr_pool_t *pool)
Where parent_baton is NULL for the anchor directory of an operation, or the
result of a previous svn_wc_open call for any lower dir. This would allow us
to do things like removing the parent dir.
Other than that, I think it would be much more of a performance benefit to
re-write most of the wc using code so that svn_wc_open was only called by
consumers of the libsvn_wc API, rather than internally. Then if the prototype
of (almost?) every wc function was changed to take a wc baton, we could
enforce that all operations happened loggily, and that all entry modifications
were done through the cache. The way you have it now, anyone maintaining
libsvn_wc could just as easily return the code to its old, half logged/half
unlogged state.
I didn't volunteer to take on this task, because it would be a much bigger
change than the one you are working on, but I definitely think it is the way
to go. It might be best to create branches/libsvn_wc_rewrite and work on it
there.
Sorry I didn't get to post this before you started working on this :(.
snip rest.
--
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson http://www.pilch-bisson.net
"Historically speaking, the presences of wheels in Unix
has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- application/pgp-signature attachment: stored
Received on Wed Jul 10 14:37:31 2002