Best-practice for AM_MAINTAINER_MODE is to use AM_MAINTAINER_MODE([enable]), as Javier suggested on Bug #32245, so people compiling the package normally get the correct Makefile rules to rebuild things, but distribution autobuilders doing a one-off build (or people working around timestamp skew) can easily turn it off. However, in D-Bus it has the side-effect of changing the default settings tests ("embedded tests" if my branch from Bug #34570 is merged), assertions, and other "debug mode" things that should be off in a "production" build. The desired situation is: * If I obtain a release tarball, untar it, ./configure && make I get a "production" build. Rationale: this is best for distribution packagers, and for people who're installing from source for whatever reason (Linux-from-Scratch or whatever) * If I obtain a random snapshot from git, ./autogen.sh && make I get a "debug" build Rationale: this is what we want D-Bus developers to be running, to spot bugs quicker * If I obtain a release tag from git, either way could make sense? In telepathy-glib we achieve a similar result for -Werror by making the default depend on the "nano version", instead of on whether you ran autogen.sh or used a tarball: if the nano version is present, we use -Werror ("debug mode"), if absent, we don't ("release mode"). The equivalent in libdbus would be to default to "release mode" if the micro version is even, or "debug mode" if the micro version is odd.
Created attachment 51624 [details] [review] Set configure defaults from odd/even micro, not Automake maintainer mode Automake maintainer mode isn't about whether you're a maintainer or not (although its name would suggest that), it's about whether files that are normally distributed in the tarball get regenerated. As such, it's not really appropriate to use it to drive defaults for things like assertions and extra test code. Basing those defaults on whether you're building a random snapshot from git (odd micro version) or a tagged release (even micro version) gives the desired effect: developers building from git normally get tests and assertions, distribution packagers don't.
Created attachment 51625 [details] [review] [2/2] Enable Automake maintainer mode by default, but let distros disable it See http://blogs.gnome.org/desrt/2011/09/08/am_maintainer_mode-is-not-cool/ for more information.
Quite frankly I think a logic that binds building of tests to the version number is broken. If I check out a stable branch to test things I think we should enable the tests unconditionally, and disable them only if the user asked for that at configure time, which is then something the distro folks can do. It's a simple fact that distros sometimes put development versions of packages in the distro to get testng for it, hence using the micro version to figure that out is a really bad idea. If you really want an automatism here (and again, I think we shouldn't and just have distros pass --disable-tests or so), then bind this to whether $(top_srcdir)/.git exists or so. Also, there are more even numbers than just 0, 2, 4, and 8. What about 10, 12, 14, and so on?
(In reply to comment #3) > Quite frankly I think a logic that binds building of tests to the version > number is broken. For the "standalone tests" I agree - but those are on by default (if you have GLib) anyway. This bug is about the "embedded tests" and verbose-mode, which are compiled into the shared libdbus and are claimed to have security implications (I haven't checked exactly what that means, but it's a worrying thing to see). We have two conflicting goals here: we want everyone who develops D-Bus to build with all tests and verbose mode enabled by default, so they can "make check" meaningfully; but we want people who *install* D-Bus (distributions, pseudo-distributions like Linux from Scratch, and occasionally even end users on non-Linux OSs) to have the secure and efficient version, without the embedded test/verbosity code. In this branch I'm approximating "developer" by "using something that isn't a tagged release", since distros and end users should use a tagged release (and if distribution developers want releases to be made more frequently, they're welcome to ask for one, or join in with upstream maintenance and make one themselves). The other way we could achieve my goal on this bug would be to add --enable-developer-defaults (or something), make *that* change the defaults, and have autogen.sh pass that in instead of, or in addition to, --enable-maintainer-mode - then "maintainer mode" would purely mean the Automake thing. > It's a simple fact that distros sometimes put development versions of packages > in the distro to get testng for it, hence using the micro version to figure > that out is a really bad idea. This isn't triggered by an odd minor version (dbus-1.5), it's the micro version (the 7 in 1.5.7). dbus versions with an odd micro version aren't a release, they're some random snapshot from git; all stable and development releases (tarballs, tags) have an even micro version. Do people really put non-releases in distributions without paying careful attention to what that means? In Debian we currently put odd-minor-version releases like 1.5.8 in experimental, which is the "worse than unstable" repository; a more bleeding-edge distribution might put odd-minor releases in its equivalent of Debian unstable (e.g. Fedora rawhide), but even then, I think it should be an even-micro release, not some random snapshot. Alternative versioning schemes with the same effect: in Telepathy we use ".1" as a fourth version component (nano version) for non-releases, rather than an odd micro version - so 0.14.3 is a stable release, 0.13.7 is a development release, but 0.14.3.1 and 0.13.7.1 are non-release snapshots. In telepathy-mission-control, for a while we used "+" as a nano version, so a snapshot newer than 5.1.2 would be "version 5.1.2+". > If you really want an automatism here (and again, I think we shouldn't and just > have distros pass --disable-tests or so), then bind this to whether > $(top_srcdir)/.git exists or so. I think building the dbus-1.5.8 tag from git should be the same as building it from a tarball: conceptually, the tree is momentarily a supported release (then goes back to being only for developers 1 commit later). > Also, there are more even numbers than just 0, 2, 4, and 8. What about 10, 12, > 14, and so on? The cases are (*0|*2|...) so they'll match anything with an even last digit.
Created attachment 56858 [details] [review] [alternative 1/2] Set configure defaults from --enable-developer, not Automake maintainer mode Automake maintainer mode isn't about whether you're a maintainer or not (although its name would suggest that), it's about whether files that are normally distributed in the tarball get regenerated. As such, it's not really appropriate to use it to drive defaults for things like assertions and extra test code. The desired effect is that developers building from git normally get tests and assertions, while distribution packagers don't. --- I still think making "developer mode" automatic, based on whether this is a snapshot or a real release, is better... but if you insist, here's a version with an explicit --enable-developer.
Created attachment 56859 [details] [review] [alternative 2/2] Enable Automake maintainer mode by default, but let distros disable it See http://blogs.gnome.org/desrt/2011/09/08/am_maintainer_mode-is-not-cool/ for more information. --- A version of Attachment #51625 [details] suitable to apply after Attachment #56858 [details].
(In reply to comment #3) > I think we should enable the tests unconditionally, and disable them only > if the user asked for that at configure time, which is then something the > distro folks can do. Not every D-Bus user is a distribution: local installations shouldn't generally have the embedded tests enabled either. The standalone tests, sure, but those are already on-by-default (well, actually, they're "auto by default", so they'll be skipped if you don't have GLib).
Ping? I'd really like to be able to enable maintainer mode by default (for safer/more reproducible builds), but at the moment that results in the library getting extra test-related goo marked as "may reduce security" linked into it, which I don't think is an acceptable default.
Comment on attachment 56858 [details] [review] [alternative 1/2] Set configure defaults from --enable-developer, not Automake maintainer mode Review of attachment 56858 [details] [review]: ----------------------------------------------------------------- So I'm a much bigger fan of this alternative than any magic like keying off the version of presence of .git. FWIW, my latest build system (https://live.gnome.org/OSTree/GnomeOSTree) ONLY builds from git. Now I should specify the configure options explicitly in the manifest anyways, but still. Less environmental magic in the makefile rules is good. This looks good to commit to me; feel free to keep --enable-developer or not as you wish. ::: configure.ac @@ +123,5 @@ > > +# this must come first: other options use this to set their defaults > +AC_ARG_ENABLE([developer], > + [AS_HELP_STRING([--enable-developer], > + [set defaults to be appropriate for a D-Bus developer instead of a distribution/end-user]), Does this really win us much over just having developers use --enable-asserts --enable-verbose-mode? I'm not opposed though.
(In reply to comment #9) > Does this really win us much over just having developers use --enable-asserts > --enable-verbose-mode? It's already more than that: it's (almost) equivalent to "--enable-asserts --enable-verbose-mode --enable-Werror --enable-embedded-tests" which is rather more to remember. I might make wider-scoped later - "insist on all the tools to build documentation", "enable Stats", and "insist on having GLib so we get more coverage" are all possibilities that spring to mind.
Comment on attachment 56858 [details] [review] [alternative 1/2] Set configure defaults from --enable-developer, not Automake maintainer mode Review of attachment 56858 [details] [review]: ----------------------------------------------------------------- Ok, sounds good. Count this as r+ from me on patch 1/2 (--enable-developer alternative).
(In reply to comment #11) > Ok, sounds good. Count this as r+ from me on patch 1/2 (--enable-developer > alternative). Oh, I thought your earlier comments were already a positive review ("This looks good to commit to me"), and merged both --enable-developer and "Enable Automake maintainer mode by default"...
Fixed in 1.6.2, with a bug that meant developer mode was on-by-default. Really fixed for 1.6.4.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.