> -----Original Message-----
> From: Greg Stein [mailto:gstein_at_gmail.com]
> Sent: donderdag 23 juni 2011 22:30
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1132968 - in
> /subversion/trunk/subversion/include: svn_types.h svn_version.h
>
> On Thu, Jun 23, 2011 at 15:47, Bert Huijben <bert_at_qqmail.nl> wrote:
> >> -----Original Message-----
> >> From: Greg Stein [mailto:gstein_at_gmail.com]
> >> Sent: donderdag 23 juni 2011 21:01
> >> To: dev_at_subversion.apache.org
> >> Subject: Re: svn commit: r1132968 - in
> >> /subversion/trunk/subversion/include: svn_types.h svn_version.h
> >>
> >> On Tue, Jun 7, 2011 at 08:14, <rhuijben_at_apache.org> wrote:
> >> > Author: rhuijben
> >> > Date: Tue Jun 7 12:14:14 2011
> >> > New Revision: 1132968
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1132968&view=rev
> >> > Log:
> >> > Following up on r1132965, just move the type. This matches how we
> >> handled the
> >> > problem for svn_error_t.
> >> >
> >> > * subversion/include/svn_types.h
> >> > (svn_version_t): Add full definition here.
> >> >
> >> > * subversion/include/svn_version.h
> >> > (svn_version_t): And remove it here.
> >>
> >> I've been thinking more on this change and absolutely hate it.
> >>
> >> We have a header DEDICATED to this structure and its concepts. The
> >> structure should be in that header file. It makes no sense to have a
> >> dedicated header, yet to move its key structure somewhere else.
> >>
> >> Please revert this change.
> >
> > We also have a header file dedicated to svn_error_t and yet it is
defined in
> > svn_types.h.
>
> Yes. We wanted to avoid recursive includes, per the comment attached
> to svn_error_t. That is because svn_error_t is used within
> svn_types.h.
>
> svn_version_t is NOT used within svn_types.h, so there is no need to
> disentangle recursive #includes.
>
> > The fact that you personally hate it doesn't add any weight to your
other
> > arguments.
> >
> > I don't see any other strong opinions on this and as you try to teach
> > everyone on this list Apache doesn't have per project dictators who say
> what
> > can, can't and must be done. With a veto we ask for a different solution
in
> > order not to stall the project.
>
> I'm not a dictator, and I didn't attempt to veto this. I'm asking you
> to revert a change that myself and a few others disagree with.
>
> I dunno what a "different solution" would be because the move of the
> structure didn't solve any problems.
>
> > What solution do you suggest for having a header included everywhere
> that
> > changes on every tag?
>
> Huh? My svn_version.h hasn't changed since June 9th, when I pulled
> down this change. It never changes for us developers.
>
> > Can we move the defines that change to a different header that isn't
> > included everywhere?
> > What kind of forward (typedef) would work to allow keeping the reduced
> set
> > of includes?
>
> I really don't know what defines you're talking about that change.
>
> > I just did what you did in early 1.7 development: reduce the number of
> > recursive header includes and this one really helps in the build time:
> > Especially for third party projects building on top of Subversion.
(Which we
> > currently ask to follow trunk)
>
> I don't understand the third party angle here. What does that have to
> do with the placement of the svn_version_t structure?
The defines I'm talking about are
(I'm not interested in these as these don't change that often, but I add
them here for completeness)
#define SVN_VER_MAJOR 1
#define SVN_VER_MINOR 7
#define SVN_VER_PATCH 0
But most importantly
#define SVN_VER_TAG " (under development)"
#define SVN_VER_NUMTAG "-dev"
#define SVN_VER_REVISION 0
These are redefined by third party distributions that somehow influence how
Subversion is built to make it visible to their users that they are not 100%
the official build.
These defines are in svn_version.h while only the files that currently
explicitly include svn_version.h really need these defines.
All our svn_*.h files except for the very global ones implemented in
libsvn_subr have at least one function that returns a svn_version_t * to
allow runtime version checks, so these headers used to include svn_version.h
just to get this typedef.
(r1132968 made it possible just to include just svn_types.h, which any
header file has to do anyway)
Changing the defines as part of an incremental build (after an svn update)
with svn_version.h included everywhere, makes it a full build.
And not only of Subversion, but also of all the third party code that
includes a header like svn_client.h.
Bert
Received on 2011-06-23 23:00:17 CEST