Hyrum K Wright wrote on Tue, Feb 08, 2011 at 16:59:13 +0000:
> On Tue, Feb 8, 2011 at 3:03 AM, Daniel Shahaf <d...@daniel.shahaf.name> wrote:
> > hwri...@apache.org wrote on Mon, Feb 07, 2011 at 22:09:15 -0000:
> >> Author: hwright
> >> Date: Mon Feb 7 22:09:15 2011
> >> New Revision: 1068169
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1068169&view=rev
> >> Log:
> >> * tools/server-side/svn-populate-node-origins-index.c
> >> (index_revision_adds): Update a deprecated function call, simplifying the
> >> code a bit.
> >>
> >> Modified:
> >> subversion/trunk/tools/server-side/svn-populate-node-origins-index.c
> >>
> >> Modified:
> >> subversion/trunk/tools/server-side/svn-populate-node-origins-index.c
> >> URL:
> >> http://svn.apache.org/viewvc/subversion/trunk/tools/server-side/svn-populate-node-origins-index.c?rev=1068169&r1=1068168&r2=1068169&view=diff
> >> ==============================================================================
> >> --- subversion/trunk/tools/server-side/svn-populate-node-origins-index.c
> >> (original)
> >> +++ subversion/trunk/tools/server-side/svn-populate-node-origins-index.c
> >> Mon Feb 7 22:09:15 2011
> >> @@ -77,7 +77,7 @@ index_revision_adds(int *count, svn_fs_t
> >>
> >> *count = 0;
> >> SVN_ERR(svn_fs_revision_root(&root, fs, revision, pool));
> >> - SVN_ERR(svn_fs_paths_changed(&changes, root, pool));
> >> + SVN_ERR(svn_fs_paths_changed2(&changes, root, pool));
> >>
> >> /* No paths changed in this revision? Nothing to do. */
> >> if (apr_hash_count(changes) == 0)
> >> @@ -88,7 +88,7 @@ index_revision_adds(int *count, svn_fs_t
> >> {
> >> const void *path;
> >> void *val;
> >> - svn_fs_path_change_t *change;
> >> + svn_fs_path_change2_t *change;
> >>
> >> svn_pool_clear(subpool);
> >> apr_hash_this(hi, &path, NULL, &val);
> >> @@ -96,12 +96,8 @@ index_revision_adds(int *count, svn_fs_t
> >> if ((change->change_kind == svn_fs_path_change_add)
> >> || (change->change_kind == svn_fs_path_change_replace))
> >> {
> >> - const char *copyfrom_path;
> >> - svn_revnum_t copyfrom_rev;
> >> -
> >> - SVN_ERR(svn_fs_copied_from(©from_rev, ©from_path,
> >> - root, path, subpool));
> >> - if (! (copyfrom_path && SVN_IS_VALID_REVNUM(copyfrom_rev)))
> >> + if (! (change->copyfrom_path
> >
> > The doc string for that parameter says:
> >
> > /** Copyfrom revision and path; this is only valid if copyfrom_known
> > * is true. */
>
> Since we're not actually using the copyfrom_path or copyfrom_rev, just
> checking their validity, would the following patch then make sense?
>
IMO, no. This is the idiom in libsvn_repos:
if (! change->copyfrom_known)
{
SVN_ERR(svn_fs_copied_from(&(change->copyfrom_rev),
&(change->copyfrom_path),
root, edit_path, pool));
change->copyfrom_known = TRUE;
}
In other words, I believe that the _known flag is "Did I fetch the
information", not "Does the FS have non-NULL values for this
information".
Daniel
(who learnt this the hard way)
> [[[
>
> Index: tools/server-side/svn-populate-node-origins-index.c
> ===================================================================
> --- tools/server-side/svn-populate-node-origins-index.c (revision
> 1068475)
> +++ tools/server-side/svn-populate-node-origins-index.c (working copy)
> @@ -96,8 +96,7 @@
> if ((change->change_kind == svn_fs_path_change_add)
> || (change->change_kind == svn_fs_path_change_replace))
> {
> - if (! (change->copyfrom_path
> - && SVN_IS_VALID_REVNUM(change->copyfrom_rev)))
> + if (!change->copyfrom_known)
> {
> svn_revnum_t origin;
> SVN_ERR(svn_fs_node_origin_rev(&origin, root, path, subpool));
> ]]]
>
> -Hyrum
