sussman@tigris.org writes:
> Modified: trunk/subversion/libsvn_client/client.h
> ==============================================================================
> --- trunk/subversion/libsvn_client/client.h (original)
> +++ trunk/subversion/libsvn_client/client.h Wed Apr 14 14:26:43 2004
> @@ -78,15 +78,75 @@
>
> [...]
>
> +
> +/* Given the CHANGED_PATHS and REVISION from an instance of a
> + svn_log_message_receiver_t function, determine at which location
> + PATH may be expected in the next log message, and set *PREV_PATH_P
> + to that value. KIND is the node kind of PATH. Perform all
> + allocations in POOL.
> +
> + This is useful for tracking the various changes in location a
> + particular resource has undergone when performing an RA->get_logs()
> + operation on that resource. */
> +svn_error_t *svn_client__prev_log_path (const char **prev_path_p,
> + apr_hash_t *changed_paths,
> + const char *path,
> + svn_node_kind_t kind,
> + svn_revnum_t revision,
> + apr_pool_t *pool);
> +
I had some comments about this doc string, and the semantics of the
function, but resolved them in private conversation with Mike. Here's
the new doc string (it's not yet committed), as my other comments in
this mail depend on it for context:
/* Set *PREV_PATH_P to the "predecessor location" for PATH from
REVISION, allocated in POOL. CHANGED_PATHS is all the changed
paths for REVISION, using the same representation accepted by
svn_log_message_receiver_t, and KIND is the node kind of PATH in
REVISION.
The "predecessor location" is the path at which the next older
change to the resource represented by PATH in REVISION occurred.
In other words:
If PATH was deleted or simply modified in REVISION (but not
copied nor added), then the predecessor location is just PATH,
though reallocated in POOL.
Else if PATH was copied into REVISION, then the predecessor
location is the path of the copy source.
Else if PATH was added into REVISION, the predecessor location
is NULL.
Else if PATH was not affected in REVISION at all, according to
CHANGED_PATHS, then return SVN_ERR_ILLEGAL_TARGET.
### Can't find a better error, maybe we should invent one? ###
(Of course, PATH might be affected in REVISION by being
underneath a copied path, so just because PATH doesn't appear
directly in CHANGED_PATHS doesn't mean it's not "included" in
CHANGED_PATHS.)
This is useful for tracking the location changes a particular
resource has undergone, by calling RA->get_log() on that resource.
Note that the 'start' and 'end' parameters to RA->get_log() must be
in reverse chronological order for this to work: start >= end. */
Okay, continuing...
> +/** Set @a *start_url and @a *start_revision (and maybe @a *end_url
> + * and @a *end_revision) to the revisions and repository URLs of one
> + * (or two) points of interest along a particular versioned resource's
> + * line of history. @a path as it exists in "peg revision" @a
> + * revision identifies that line of history, and @a start and @a end
> + * specify the point(s) of interest (typically the revisions referred
> + * to as the "operative range" for a given operation) along that history.
> + *
> + * @a end may be of kind svn_opt_revision_unspecified (in which case
> + * @a end_url and @a end_revision are not touched by the function);
> + * @a start and @a revision may not.
> + *
> + * A NOTE ABOUT FUTURE REPORTING:
> + *
> + * If either @a start or @end are greater than @a revision, then do a
> + * sanity check (since we cannot search future history yet): verify
> + * that @a path in the future revision(s) is the "same object" as the
> + * one pegged by @a revision. In other words, all three objects must
> + * be connected by a single line of history which exactly passes
> + * through @a path at @a revision. If this sanity check fails, return
> + * SVN_ERR_CLIENT_UNRELATED_RESOURCES.
> + *
> + * @a ctx is the client context baton.
> + *
> + * @a ra_lib and @a ra_session are required; they represent an
> + * already-open RA session to @a path's url.
> + *
> + * Use @a pool for all allocations.
> + */
> +svn_error_t *
> +svn_client__repos_locations (const char **start_url,
> + svn_opt_revision_t **start_revision,
> + const char **end_url,
> + svn_opt_revision_t **end_revision,
> + const char *path,
> + const svn_opt_revision_t *revision,
> + const svn_opt_revision_t *start,
> + const svn_opt_revision_t *end,
> + svn_ra_plugin_t *ra_lib,
> + void *ra_session,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
Am also tweaking this doc string a bit, for clarity. One question is,
why does it take two args ("start" and "end") instead of just one arg
("other_rev") or N args ("array_of_inquiry_revs")? Mike Pilato
answered that question for me: because a survey of callers showed that
no one would be passing more than two. But, as Mike also pointed out,
this is an internal interface, so we're free to change it if we want.
And the underlying implementation (the ra protocol) will allow for N
args, so as not to constrain this interface via bad performance.
[The new doc string isn't ready yet, so not included here.]
> Modified: trunk/subversion/libsvn_client/ra.c
> ==============================================================================
> --- trunk/subversion/libsvn_client/ra.c (original)
> +++ trunk/subversion/libsvn_client/ra.c Wed Apr 14 14:26:43 2004
> @@ -24,6 +24,7 @@
> #include "svn_error.h"
> #include "svn_pools.h"
> #include "svn_string.h"
> +#include "svn_sorts.h"
> #include "svn_ra.h"
> #include "svn_wc.h"
> #include "svn_client.h"
> @@ -320,5 +321,330 @@
> SVN_ERR (svn_client_uuid_from_url (uuid, entry->url, ctx, pool));
> }
>
> + return SVN_NO_ERROR;
> +}
> +
> +
> +struct log_message_baton
> +{
> + /* The kind of the path we're tracing. */
> + svn_node_kind_t kind;
> +
> + /* The path at which we are trying to find our versioned resource in
> + the log output. */
> + const char *last_path;
> +
> + /* Input revisions and output paths; the whole point of this little game. */
> + svn_revnum_t start_revision;
> + const char *start_path;
> + svn_revnum_t end_revision;
> + const char *end_path;
> + svn_revnum_t peg_revision;
> + const char *peg_path;
> +
> + /* Client context baton. */
> + svn_client_ctx_t *ctx;
> +
> + /* A pool from which to allocate stuff stored in this baton. */
> + apr_pool_t *pool;
> +};
Since the log message is a part of the log data that we're completely
ignoring here, it might be better to name this struct something other
than "log_message_baton" :-). Perhaps 'log_receiver_baton' or
'log_thang_baton' or whatever?
> +svn_error_t *
> +svn_client__prev_log_path (const char **prev_path_p,
> + apr_hash_t *changed_paths,
> + const char *path,
> + svn_node_kind_t kind,
> + svn_revnum_t revision,
> + apr_pool_t *pool)
> +{
> + svn_log_changed_path_t *change;
> + const char *prev_path = NULL;
> +
> + /* If PATH was explicitly changed in this revision, that makes
> + things easy -- we keep the path (but check to see). If so,
> + we'll either use the path, or, if was copied, use its
> + copyfrom_path. */
> + change = apr_hash_get (changed_paths, path, APR_HASH_KEY_STRING);
> + if (change)
> + {
> + if (change->copyfrom_path)
> + prev_path = apr_pstrdup (pool, change->copyfrom_path);
> + else
> + prev_path = path;
> + }
We have to look harder at the type of the change -- if it is a
deletion, than prev_path should really be set to NULL, not to path.
> + else if (apr_hash_count (changed_paths))
> + {
> + /* The path was not explicitly changed in this revision. The
> + fact that we're hearing about this revision implies, then,
> + that the path was a child of some copied directory. We need
> + to find that directory, and effectively "re-base" our path on
> + that directory's copyfrom_path. */
> + int i;
> + apr_array_header_t *paths;
> +
> + /* Build a sorted list of the changed paths. */
> + paths = svn_sort__hash (changed_paths,
> + svn_sort_compare_items_as_paths, pool);
> +
> + /* Now, walk the list of paths backwards, looking a parent of
> + our path that has copyfrom information. */
> + for (i = paths->nelts; i > 0; i--)
> + {
> + svn_sort__item_t item = APR_ARRAY_IDX (paths,
> + i - 1, svn_sort__item_t);
> + const char *ch_path = item.key;
> + int len = strlen (ch_path);
> +
> + /* See if our path is the child of this change path. If
> + not, keep looking. */
> + if (! ((strncmp (ch_path, path, len) == 0) && (path[len] == '/')))
> + continue;
> +
> + /* Okay, our path *is* a child of this change path. If
> + this change was copied, we just need to apply the
> + portion of our path that is relative to this change's
> + path, to the change's copyfrom path. Otherwise, this
> + change isn't really interesting to us, and our search
> + continues. */
> + change = apr_hash_get (changed_paths, ch_path, len);
> + if (change->copyfrom_path)
> + {
> + prev_path = svn_path_join (change->copyfrom_path,
> + path + len + 1, pool);
> + break;
> + }
> + }
> + }
> +
> + /* If we didn't find what we expected to find, return an error.
> + (Because directories bubble-up, we get a bunch of logs we might
> + not want. Be forgiving in that case.) */
> + if (! prev_path)
> + {
> + if (kind == svn_node_dir)
> + prev_path = apr_pstrdup (pool, path);
> + else
> + return svn_error_createf (APR_EGENERAL, NULL,
> + "Missing changed-path information for "
> + "'%s' in revision %" SVN_REVNUM_T_FMT,
> + path, revision);
> + }
We should use some Subversion error here, and document it (or I
suppose we could at least document that it's APR_EGENERAL).
For now, the new doc string promises 'SVN_ERR_ILLEGAL_TARGET'.
Thoughts?
> + *prev_path_p = prev_path;
> + return SVN_NO_ERROR;
> +}
> +
> +
> +/* Implements svn_log_message_receiver_t; helper for
> + svn_client_repos_locations. */
Rather, "svn_client__repos_locations". (two underscores)
> +static svn_error_t *
> +log_receiver (void *baton,
> + apr_hash_t *changed_paths,
> + svn_revnum_t revision,
> + const char *author,
> + const char *date,
> + const char *message,
> + apr_pool_t *pool)
We should document the type of BATON and the effects this has inside
BATON.
> +{
> + struct log_message_baton *lmb = baton;
> + const char *current_path = lmb->last_path;
> + const char *prev_path;
> +
> + /* See if the user is fed up with this time-consuming process yet. */
> + if (lmb->ctx->cancel_func)
> + SVN_ERR (lmb->ctx->cancel_func (lmb->ctx->cancel_baton));
> +
> + /* If we've already determined all of our paths, then frankly, why
> + are we here? Oh well, just do nothing. */
> + if (lmb->start_path && lmb->peg_path && lmb->end_path)
> + return SVN_NO_ERROR;
> +
> + /* Determine the paths for any of the revisions for which we haven't
> + gotten paths already. */
> + if ((! lmb->start_path) && (revision <= lmb->start_revision))
> + lmb->start_path = apr_pstrdup (lmb->pool, current_path);
> + if ((! lmb->end_path) && (revision <= lmb->end_revision))
> + lmb->end_path = apr_pstrdup (lmb->pool, current_path);
> + if ((! lmb->peg_path) && (revision <= lmb->peg_revision))
> + lmb->peg_path = apr_pstrdup (lmb->pool, current_path);
> +
> + /* Figure out at which repository path our object of interest lived
> + in the previous revision. */
> + SVN_ERR (svn_client__prev_log_path (&prev_path, changed_paths,
> + current_path, lmb->kind,
> + revision, pool));
> +
> + /* Squirrel away our "next place to look" path (suffer the strcmp
> + hit to save on allocations). */
> + if (strcmp (prev_path, current_path) != 0)
> + lmb->last_path = apr_pstrdup (lmb->pool, prev_path);
> +
> + return SVN_NO_ERROR;
> +}
I had to stop reviewing here, unfortunately :-(.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 15 22:28:30 2004