subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <danie...@elego.de>
Subject Re: svn commit: r1508170 - in /subversion/trunk: build/ac-macros/svn-macros.m4 configure.ac subversion/include/svn_hash.h
Date Tue, 30 Jul 2013 14:46:44 GMT
Stefan Fuhrmann wrote on Tue, Jul 30, 2013 at 16:30:00 +0200:
> On Tue, Jul 30, 2013 at 3:57 PM, Daniel Shahaf <danielsh@elego.de> wrote:
> 
> > Stefan Fuhrmann wrote on Tue, Jul 30, 2013 at 01:04:43 +0200:
> > > On Mon, Jul 29, 2013 at 9:06 PM, Daniel Shahaf <danielsh@elego.de>
> > wrote:
> > >
> > > > danielsh@apache.org wrote on Mon, Jul 29, 2013 at 18:35:26 -0000:
> > > > > Author: danielsh
> > > > > Date: Mon Jul 29 18:35:25 2013
> > > > > New Revision: 1508170
> > > > >
> > > > > URL: http://svn.apache.org/r1508170
> > > > > Log:
> > > > > Follow-up to r1507366: svn_hash_gets: compute the length of
> > > > > string literal keys (common case) at compile-time, without
> > > > > multiply-evaluating dynamically-computed keys.
> > > > >
> > > > > Review by: philip
> > > > >            breser
> > > >
> > > > Oddly enough, the optimization doesn't seem to kick in for me
...
> > > >
> > >
> > > A two things have been broken here:
> > >
> >
> > Thanks for fixing this.  Collective review of your commits:
> >
> > r1508222: the #ifndef seems to be the wrong solution to whatever problem
> > it is trying to fix.  If it's trying to generate a compiler warning, it
> > should use the #warning preprocessor directive; if it's trying to
> > silence one about use of an undefined macro, how about doing
> >   #ifndef SVN_HAS_DUNDER_BUILTINS
> >   #error "Build broken (do you need to include svn_private_config.h earlier?)"
> >   #endif
> >
> 
> No go. This is a public API header. People will not define
> SVN_HAS_DUNDER_BUILTINS in their applications.
> 

It would be nice if we can propagate the fix to C API consumers too.

How can we do that?  Do we just document the macro (say, in an #ifdef
DOXYGEN block) along with an example of how to detect the setting for
it in various build systems?  Or do try to sniff it at compile time?

> > r1508225: changing the include order //only in places that currently
> > include svn_hash.h// seems like a bad idea; we should change it
> > _everywhere_ (since other places may grow svn_hash.h includes in the
> > future).
> >
> 
> Well, I got kind of fed up after manually patching
> ~180 files (about the same number of files left to
> check). Even the commit took longer than 4 minutes.
> 
> So, feel free to do some search/replace magic to
> handle the remaining files.

What makes you think I'm not as lazy as you are? :-)

Index: build/ac-macros/compiler.m4
===================================================================
--- build/ac-macros/compiler.m4	(revision 1508471)
+++ build/ac-macros/compiler.m4	(working copy)
@@ -74,6 +74,7 @@ AC_DEFUN([SVN_CC_MODE_SETUP],
 
   CNOWARNFLAGS="$CFLAGS"
   CFLAGS="$CFLAGS_KEEP"
+  CFLAGS="-DSVN__CORE $CFLAGS"
 
   AC_SUBST(CMODEFLAGS)
   AC_SUBST(CNOWARNFLAGS)
Index: build/generator/gen_win.py
===================================================================
--- build/generator/gen_win.py	(revision 1508471)
+++ build/generator/gen_win.py	(working copy)
@@ -714,7 +714,8 @@ class WinGeneratorBase(gen_win_dependencies.GenDep
     fakedefines = ["WIN32","_WINDOWS","alloca=_alloca",
                    "_CRT_SECURE_NO_DEPRECATE=",
                    "_CRT_NONSTDC_NO_DEPRECATE=",
-                   "_CRT_SECURE_NO_WARNINGS="]
+                   "_CRT_SECURE_NO_WARNINGS=",
+                   "SVN__CORE"]
 
     if cfg == 'Debug':
       fakedefines.extend(["_DEBUG","SVN_DEBUG"])
Index: subversion/include/svn_hash.h
===================================================================
--- subversion/include/svn_hash.h	(revision 1508471)
+++ subversion/include/svn_hash.h	(working copy)
@@ -248,7 +248,11 @@ svn_hash_from_cstring_keys(apr_hash_t **hash,
  * below will usually generate a compiler warning if your definition is
  * being encountered _after_ #including this header. 
  */
-#define SVN_HAS_DUNDER_BUILTINS 0
+# ifdef SVN__CORE
+#  error "Build broken (should you include svn_private_config.h earlier?)"
+# else
+#  define SVN_HAS_DUNDER_BUILTINS 0
+# endif
 #endif
 
 /** Shortcut for apr_hash_get() with a const char * key.

Untested, but I'll add it to my queue for next week.

> > Also, let's take this opportunity to document our preferred order of
> > #include's in HACKING (The relative order of: stdlib, apr, mod_dav, bdb,
> > serf, sqlite, svn_*.h, svn_*_private.h, ./*.h, ../libsvn_{fs|ra}/*.h,
> > subversion_config_private.h, and APR_WANT_FOO) --- I always end
> > up copying the order from some existing file.

Mime
View raw message