> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
> Sent: donderdag 23 juni 2011 23:26
> To: Bert Huijben
> Cc: 'Greg Stein'; dev_at_subversion.apache.org
> Subject: Re: svn commit: r1132968 - in
> /subversion/trunk/subversion/include: svn_types.h svn_version.h
>
> Bert Huijben wrote on Thu, Jun 23, 2011 at 22:59:47 +0200:
> >
> >
> > > -----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)
>
> #ifndef SVN_VER_TAG
> #define SVN_VER_TAG " (under development)"
> #endif
This would work if I had a way to build just the .c files that need this
define.
To get the same result I would have to rebuild all .c files to get the
defines in the right place, or the new value would just be ignored in an
incremental build.
Bert
Received on 2011-06-23 23:32:13 CEST