You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/10/02 12:56:22 UTC
Re: svn commit: r1389851 - /subversion/trunk/subversion/libsvn_client/commit.c
stsp@apache.org writes:
> Author: stsp
> Date: Tue Sep 25 13:10:00 2012
> New Revision: 1389851
>
> URL: http://svn.apache.org/viewvc?rev=1389851&view=rev
> Log:
> Fix commit from multiple working copies which are nested within an unrelated
> working copy.
>
> This patch fixes commit_test 26 on the 1.7.x branch when run within
> a format 30 working copy (backport nomination will follow).
> + /* When committing from multiple WCs, get the RA editor from
> + * the first WC, rather than the BASE_ABSPATH. The BASE_ABSPATH
> + * might be an unrelated parent of nested working copies.
> + * We don't support commits to multiple repositories so using
> + * the first WC to get the RA session is safe. */
> + if (lock_targets->nelts > 1)
> + ra_session_wc = APR_ARRAY_IDX(lock_targets, 0, const char *);
> + else
> + ra_session_wc = base_abspath;
> +
Is that more complicated than necessary? I think we can use
APR_ARRAY_IDX(lock_targets, 0, const char *) all the time.
--
Join us this October at Subversion Live 2012
http://www.wandisco.com/svn-live-2012
Re: svn commit: r1389851 -
/subversion/trunk/subversion/libsvn_client/commit.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Oct 02, 2012 at 11:56:22AM +0100, Philip Martin wrote:
> stsp@apache.org writes:
>
> > Author: stsp
> > Date: Tue Sep 25 13:10:00 2012
> > New Revision: 1389851
> >
> > URL: http://svn.apache.org/viewvc?rev=1389851&view=rev
> > Log:
> > Fix commit from multiple working copies which are nested within an unrelated
> > working copy.
> >
> > This patch fixes commit_test 26 on the 1.7.x branch when run within
> > a format 30 working copy (backport nomination will follow).
>
> > + /* When committing from multiple WCs, get the RA editor from
> > + * the first WC, rather than the BASE_ABSPATH. The BASE_ABSPATH
> > + * might be an unrelated parent of nested working copies.
> > + * We don't support commits to multiple repositories so using
> > + * the first WC to get the RA session is safe. */
> > + if (lock_targets->nelts > 1)
> > + ra_session_wc = APR_ARRAY_IDX(lock_targets, 0, const char *);
> > + else
> > + ra_session_wc = base_abspath;
> > +
>
> Is that more complicated than necessary? I think we can use
> APR_ARRAY_IDX(lock_targets, 0, const char *) all the time.
It was using base_abspath before my change, and I kept the existing
code path as it was. But thinking about it, yes, if there's only
one target it will be in lock_targets[0].
I suppose we can tweak this on trunk but leave 1.7 as it is.
It's not very elegant but it isn't wrong.