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 2010/04/16 09:08:15 UTC
Re: svn commit: r934627 - in /subversion/trunk: ./ subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
stsp@apache.org writes:
> Author: stsp
> Date: Thu Apr 15 22:18:59 2010
> New Revision: 934627
>
> URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> Log:
> Reintegrate the svn-patch-improvements branch into trunk.
> + /* Rebuild the empty dirs list, replacing empty dirs which have
> + * an empty parent with their parent. */
> + again = FALSE;
> + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> + apr_array_clear(empty_dirs);
> + for (i = 0; i < empty_dirs_copy->nelts; i++)
apr_array_clear is only available in APR 1.3.x and later.
We could write svn_array_clear, or perhaps even apr_array_clear, or we
could simply reset empty_dirs->nelts directly.
--
Philip
Re: svn commit: r934627 - in /subversion/trunk: ./ subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> On Fri, Apr 16, 2010 at 11:38:41AM +0100, Philip Martin wrote:
>> Julian Foad <ju...@wandisco.com> writes:
>> > Just write "arr->nelts = 0;" (which is the entire body of
>> > apr_array_clear()) and don't introduce an ifdef.
>
> I'm a bit sceptical.
> Do we really want to rely on implementation details like
> "arr->nelts = 0" in the APR code?
Two examples where we already set nelts:
$ grep nelts\ =[^=] ../src/subversion/libsvn_*/*c
../src/subversion/libsvn_fs_base/dag.c: copy_ids->nelts = i_out;
../src/subversion/libsvn_fs_fs/caching.c: manifest->nelts = data_len / sizeof(apr_off_t);
--
Philip
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 16, 2010 at 10:50:22AM -0500, Hyrum K. Wright wrote:
> This was causing some of the buildbots not to be able to build, so I went
> ahead and committed a fix in r934969.
Thanks!
Stefan
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Fri, Apr 16, 2010 at 7:39 AM, Stefan Sperling <st...@elego.de> wrote:
> On Fri, Apr 16, 2010 at 11:38:41AM +0100, Philip Martin wrote:
> > Julian Foad <ju...@wandisco.com> writes:
> > > Just write "arr->nelts = 0;" (which is the entire body of
> > > apr_array_clear()) and don't introduce an ifdef.
>
> I'm a bit sceptical.
> Do we really want to rely on implementation details like
> "arr->nelts = 0" in the APR code? We don't even do this in our own code.
>
> > I agree that we don't want #if like that in the main code. Put it in
> > one of our private header files if you want to use it.
>
> I can make a private macro or function for this, sure.
>
This was causing some of the buildbots not to be able to build, so I went
ahead and committed a fix in r934969. Feel free to review/update.
-Hyrum
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 16, 2010 at 11:38:41AM +0100, Philip Martin wrote:
> Julian Foad <ju...@wandisco.com> writes:
> > Just write "arr->nelts = 0;" (which is the entire body of
> > apr_array_clear()) and don't introduce an ifdef.
I'm a bit sceptical.
Do we really want to rely on implementation details like
"arr->nelts = 0" in the APR code? We don't even do this in our own code.
> I agree that we don't want #if like that in the main code. Put it in
> one of our private header files if you want to use it.
I can make a private macro or function for this, sure.
Stefan
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Fri, Apr 16, 2010 at 5:38 AM, Philip Martin
<ph...@wandisco.com>wrote:
> Julian Foad <ju...@wandisco.com> writes:
>
> > Stefan Sperling wrote:
> >> On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
> >> > stsp@apache.org writes:
> >> >
> >> > > Author: stsp
> >> > > Date: Thu Apr 15 22:18:59 2010
> >> > > New Revision: 934627
> >> > >
> >> > > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> >> > > Log:
> >> > > Reintegrate the svn-patch-improvements branch into trunk.
> >> >
> >> > > + /* Rebuild the empty dirs list, replacing empty dirs which
> have
> >> > > + * an empty parent with their parent. */
> >> > > + again = FALSE;
> >> > > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> >> > > + apr_array_clear(empty_dirs);
> >> > > + for (i = 0; i < empty_dirs_copy->nelts; i++)
> >> >
> >> > apr_array_clear is only available in APR 1.3.x and later.
> >> >
> >> > We could write svn_array_clear, or perhaps even apr_array_clear, or we
> >> > could simply reset empty_dirs->nelts directly.
> >>
> >> Hmmm...
> >>
> >> I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
>
> APR_VERSION_AT_LEAST
>
When using this macro, don't forget to include svn_dep_compat.h.
APR_VERSION_AT_LEAST wasn't introduced into APR itself until 1.3.0.
> >> array manually in the #else clause.
> >
> > Just write "arr->nelts = 0;" (which is the entire body of
> > apr_array_clear()) and don't introduce an ifdef.
>
> I agree that we don't want #if like that in the main code. Put it in
> one of our private header files if you want to use it.
>
+1
-Hyrum
Re: svn commit: r934627 - in /subversion/trunk: ./ subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@wandisco.com> writes:
> Stefan Sperling wrote:
>> On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
>> > stsp@apache.org writes:
>> >
>> > > Author: stsp
>> > > Date: Thu Apr 15 22:18:59 2010
>> > > New Revision: 934627
>> > >
>> > > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
>> > > Log:
>> > > Reintegrate the svn-patch-improvements branch into trunk.
>> >
>> > > + /* Rebuild the empty dirs list, replacing empty dirs which have
>> > > + * an empty parent with their parent. */
>> > > + again = FALSE;
>> > > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
>> > > + apr_array_clear(empty_dirs);
>> > > + for (i = 0; i < empty_dirs_copy->nelts; i++)
>> >
>> > apr_array_clear is only available in APR 1.3.x and later.
>> >
>> > We could write svn_array_clear, or perhaps even apr_array_clear, or we
>> > could simply reset empty_dirs->nelts directly.
>>
>> Hmmm...
>>
>> I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
APR_VERSION_AT_LEAST
>> array manually in the #else clause.
>
> Just write "arr->nelts = 0;" (which is the entire body of
> apr_array_clear()) and don't introduce an ifdef.
I agree that we don't want #if like that in the main code. Put it in
one of our private header files if you want to use it.
--
Philip
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Julian Foad <ju...@wandisco.com>.
Stefan Sperling wrote:
> On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
> > stsp@apache.org writes:
> >
> > > Author: stsp
> > > Date: Thu Apr 15 22:18:59 2010
> > > New Revision: 934627
> > >
> > > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> > > Log:
> > > Reintegrate the svn-patch-improvements branch into trunk.
> >
> > > + /* Rebuild the empty dirs list, replacing empty dirs which have
> > > + * an empty parent with their parent. */
> > > + again = FALSE;
> > > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> > > + apr_array_clear(empty_dirs);
> > > + for (i = 0; i < empty_dirs_copy->nelts; i++)
> >
> > apr_array_clear is only available in APR 1.3.x and later.
> >
> > We could write svn_array_clear, or perhaps even apr_array_clear, or we
> > could simply reset empty_dirs->nelts directly.
>
> Hmmm...
>
> I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
> array manually in the #else clause.
Just write "arr->nelts = 0;" (which is the entire body of
apr_array_clear()) and don't introduce an ifdef.
- Julian
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
> stsp@apache.org writes:
>
> > Author: stsp
> > Date: Thu Apr 15 22:18:59 2010
> > New Revision: 934627
> >
> > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> > Log:
> > Reintegrate the svn-patch-improvements branch into trunk.
>
> > + /* Rebuild the empty dirs list, replacing empty dirs which have
> > + * an empty parent with their parent. */
> > + again = FALSE;
> > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> > + apr_array_clear(empty_dirs);
> > + for (i = 0; i < empty_dirs_copy->nelts; i++)
>
> apr_array_clear is only available in APR 1.3.x and later.
>
> We could write svn_array_clear, or perhaps even apr_array_clear, or we
> could simply reset empty_dirs->nelts directly.
Hmmm...
I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
array manually in the #else clause.
Thanks for catching this,
Stefan
--
printf("Eh???/n");