subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bert Huijben <b...@qqmail.nl>
Subject RE: svn commit: r1508170 - in /subversion/trunk:
Date Tue, 30 Jul 2013 16:03:58 GMT
 build/ac-macros/svn-macros.m4 configure.ac subversion/include/svn_hash.h
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

If this really makes a measurable difference for the average user
(which I doubt it even is for our use) this belongs in Apr.

Until then I think we are wasting a lot of development time in
optimizing a non measurable piece of our code, with
unconfirmed 'improvements' that time after time cause new problems.

Bert From: Daniel Shahaf
Sent: =E2=80=8E30/=E2=80=8E07/=E2=80=8E2013 16:47
To: Stefan Fuhrmann
Cc: Subversion Development; commits@subversion.apache.org
Subject: Re: svn commit: r1508170 - in /subversion/trunk:
build/ac-macros/svn-macros.m4 configure.ac subversion/include/svn_hash.h
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:
>=20
> > 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 proble=
m
> > 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 ear=
lier?)"
> >   #endif
> >
>=20
> No go. This is a public API header. People will not define
> SVN_HAS_DUNDER_BUILTINS in their applications.
>=20

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).
> >
>=20
> 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.
>=20
> 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- build/ac-macros/compiler.m4=09(revision 1508471)
+++ build/ac-macros/compiler.m4=09(working copy)
@@ -74,6 +74,7 @@ AC_DEFUN([SVN_CC_MODE_SETUP],
=20
   CNOWARNFLAGS=3D"$CFLAGS"
   CFLAGS=3D"$CFLAGS_KEEP"
+  CFLAGS=3D"-DSVN__CORE $CFLAGS"
=20
   AC_SUBST(CMODEFLAGS)
   AC_SUBST(CNOWARNFLAGS)
Index: build/generator/gen_win.py
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- build/generator/gen_win.py=09(revision 1508471)
+++ build/generator/gen_win.py=09(working copy)
@@ -714,7 +714,8 @@ class WinGeneratorBase(gen_win_dependencies.GenDep
     fakedefines =3D ["WIN32","_WINDOWS","alloca=3D_alloca",
                    "_CRT_SECURE_NO_DEPRECATE=3D",
                    "_CRT_NONSTDC_NO_DEPRECATE=3D",
-                   "_CRT_SECURE_NO_WARNINGS=3D"]
+                   "_CRT_SECURE_NO_WARNINGS=3D",
+                   "SVN__CORE"]
=20
     if cfg =3D=3D 'Debug':
       fakedefines.extend(["_DEBUG","SVN_DEBUG"])
Index: subversion/include/svn_hash.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- subversion/include/svn_hash.h=09(revision 1508471)
+++ subversion/include/svn_hash.h=09(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.=20
  */
-#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
=20
 /** 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