On Thu, Apr 10, 2008 at 10:19 AM, Kamesh Jayachandran <kamesh_at_collab.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> > + SVN_ERR(svn_mergeinfo_to_string(&filtered_mergeinfo_str,
> > + filtered_mergeinfo,
> > pool));
> > adjusted_prop->name = SVN_PROP_MERGEINFO;
> > - adjusted_prop->value =
> > - svn_string_create(apr_pstrcat(pool, source_path, ":",
> > - adjusted_rangelist_s->data,
> > - NULL),
> > - pool);
> > + adjusted_prop->value = filtered_mergeinfo_str;
> > APR_ARRAY_PUSH(adjusted_props, svn_prop_t) = *adjusted_prop;
> > }
> > - } /* mergeinfo hash iteration */
>
>
> What about 'un-filtered' mergeinfo? Then we are not adding anything to
> adjusted_props so may loose un-filtered mergeinfo.
Kamesh,
If we don't filter anything then filtered_mergeinfo is equivalent to
the incoming svn:mergeinfo property addition. Notice that in the
'else /* Non-empty mergeinfo; filter self-referential mergeinfo out.
*/' block we *always* push unfiltered revisions into the
adjusted_rangelist array.
Does your local test case actually show a problem? If so could you post it.
> The code(even before this commit) looks buggy in this aspect.
> Following patch may fix it.
Despite what I said above I think your patch is actually needed to be
technically correct in one case:
If the incoming svn:mergeinfo prop add is simply empty mergeinfo, then
the empty mergeinfo would be filtered out from the property additions!
This is because that is the only case (that I can see) where the
for (hi = apr_hash_first(NULL, mergeinfo);
hi; hi = apr_hash_next(hi))
block is skipped entirely and we don't enter the 'if
(filtered_mergeinfo)' block. The result being the empty mergeinfo is
lost.
Now I don't think there is any practical effect from this because we
ultimately set mergeinfo describing the merge on PATH
merge_props_changed() is operating on and the empty mergeinfo will
seamlessly merge with that.
Paul
> I am testing the same with custom testcase.
>
> Index: subversion/libsvn_client/merge.c
> ===================================================================
> - --- subversion/libsvn_client/merge.c (revision 30479)
> +++ subversion/libsvn_client/merge.c (working copy)
> @@ -485,6 +485,8 @@
>
> adjusted_prop->value = filtered_mergeinfo_str;
> APR_ARRAY_PUSH(adjusted_props, svn_prop_t) = *adjusted_prop;
> }
> + else
> + APR_ARRAY_PUSH(adjusted_props, svn_prop_t) = *prop;
>
>
> /* If we reparented MERGE_B->RA_SESSION2 above, put it back
> to the original URL. */
>
>
>
> With regards
> Kamesh Jayachandran
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFH/iHX3WHvyO0YTCwRAtOhAJ9/s+cbwnv74P4y63okvrMPqjaYbgCfUFA0
> 00XyzHjV/AzDYvqeU/9eGcs=
> =hyj4
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-10 16:56:42 CEST