You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/09/02 00:15:39 UTC

Re: svn commit: r991619 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/commit.c libsvn_client/commit_util.c libsvn_client/merge.c libsvn_wc/entries.c libsvn_wc/node.c

On Wed, Sep 1, 2010 at 13:44,  <ju...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/include/private/svn_wc_private.h Wed Sep  1 17:44:41 2010
> @@ -394,6 +394,7 @@ svn_wc__node_is_status_deleted(svn_boole
>                                const char *local_abspath,
>                                apr_pool_t *scratch_pool);
>
> +#if 0  /* not required with SINGLE_DB */

Shouldn't this be #ifdef SVN_WC__SINGLE_DB ?  I thought the idea was
to keep using that define for a while so that developers can compile
multi-db for testing purposes.

Now... I recognize that symbol is not available to libsvn_client which
then means that multi-db isn't possible any more? Maybe a more
appropriate approach would be to just leave the functions as stubs?
And then some ### markers to fully remove them when we strip out all
the multi-db code? (in a couple weeks?)

>...

Cheers,
-g

RE: svn commit: r991619 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/commit.c libsvn_client/commit_util.c libsvn_client/merge.c libsvn_wc/entries.c libsvn_wc/node.c

Posted by Julian Foad <ju...@wandisco.com>.
Bert Huijben wrote:
> From: Greg Stein:
> > On Wed, Sep 1, 2010 at 13:44,  <ju...@apache.org> wrote:
> > > +#if 0  /* not required with SINGLE_DB */
> > 
> > Shouldn't this be #ifdef SVN_WC__SINGLE_DB ?  I thought the idea was
> > to keep using that define for a while so that developers can compile
> > multi-db for testing purposes.
> > 
> > Now... I recognize that symbol is not available to libsvn_client which
> > then means that multi-db isn't possible any more? Maybe a more

OK, first things first.  To improve this, I added a duplicate #define of
SVN_WC__SINGLE_DB in svn_wc_private.h so it can be used here and in the
client code.  A multi-DB build now requires commenting out that define
in only those two places rather than changing the "#if 0" in multiple
places.

r991887.  A bit ugly but one step better.

I considered moving the #define to some single place from where it would
be available everywhere - either in "include/svn_wc.h" or in the
configuration script (controlled by "configure --disable-single-db") -
but neither seems worth the effort.

> > appropriate approach would be to just leave the functions as stubs?
> > And then some ### markers to fully remove them when we strip out all
> > the multi-db code? (in a couple weeks?)

Not really, as I want the whole block of client code that uses these
functions to be disabled rather than just the functionality of the
functions themselves.

> Julian:
> Any constructs like this break the shared library build on Windows.

Oops.

> We have a python script somewhere in the tree (extractor.py if I remember
> correctly) that scans all the header files defined in build.conf to generate
> .def files for the Windows DLLs. And these functions will be made public for
> library users. 
> 
> But in this case the scripts finds functions that aren't defined, but makes
> them part of the public interface anyway. (Resulting in a linker error)
> 
> For a few specific cases (like mac only function) we applied some exceptions
> to the extractor, but in this case I think we should just start removing
> multi-db support.

I suppose deleting or commenting out just the function prototypes from
the header file is a possible intermediate solution.  Do you think so?

- Julian


RE: svn commit: r991619 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/commit.c libsvn_client/commit_util.c libsvn_client/merge.c libsvn_wc/entries.c libsvn_wc/node.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: donderdag 2 september 2010 2:16
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r991619 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/commit.c
> libsvn_client/commit_util.c libsvn_client/merge.c libsvn_wc/entries.c
> libsvn_wc/node.c
> 
> On Wed, Sep 1, 2010 at 13:44,  <ju...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/include/private/svn_wc_private.h Wed
> Sep  1 17:44:41 2010
> > @@ -394,6 +394,7 @@ svn_wc__node_is_status_deleted(svn_boole
> >                                const char *local_abspath,
> >                                apr_pool_t *scratch_pool);
> >
> > +#if 0  /* not required with SINGLE_DB */
> 
> Shouldn't this be #ifdef SVN_WC__SINGLE_DB ?  I thought the idea was
> to keep using that define for a while so that developers can compile
> multi-db for testing purposes.
> 
> Now... I recognize that symbol is not available to libsvn_client which
> then means that multi-db isn't possible any more? Maybe a more
> appropriate approach would be to just leave the functions as stubs?
> And then some ### markers to fully remove them when we strip out all
> the multi-db code? (in a couple weeks?)

Julian:
Any constructs like this break the shared library build on Windows.

We have a python script somewhere in the tree (extractor.py if I remember
correctly) that scans all the header files defined in build.conf to generate
.def files for the Windows DLLs. And these functions will be made public for
library users. 

But in this case the scripts finds functions that aren't defined, but makes
them part of the public interface anyway. (Resulting in a linker error)


For a few specific cases (like mac only function) we applied some exceptions
to the extractor, but in this case I think we should just start removing
multi-db support.

	Bert