Bert Huijben wrote on Thu, Jun 23, 2011 at 22:59:47 +0200: > > > > -----Original Message----- > > From: Greg Stein [mailto:gst...@gmail.com] > > Sent: donderdag 23 juni 2011 22:30 > > To: Bert Huijben > > Cc: dev@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 <b...@qqmail.nl> wrote: > > >> -----Original Message----- > > >> From: Greg Stein [mailto:gst...@gmail.com] > > >> Sent: donderdag 23 juni 2011 21:01 > > >> To: dev@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, <rhuij...@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 ? > 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 >
