A short series of autotools-related cleanup patches. I'd appreciate if I could get a review from at least one of the other valgrind developers before I actually commit these patches to master. It started out as some small things noticed whilst looking at a larger feature and here we are six patches later. The series itself should be fairly straightforward. Has been tested on macOS 10.11, 10.12 and 10.13 and Ubuntu 16.04. The series itself can also be found on my GitHub repo at: https://github.com/Echelon9/valgrind/tree/series/autotools-series An interested reviewer could add the repo, check out a local branch with the patches and re-build valgrind as follows: # Add repo and checkout a local branch with the patches git remote add rhyskidd-gh https://github.com/Echelon9/valgrind.git git checkout -b series/autotools-series rhyskidd-gh/series/autotools-series # Clean build environment, re-run autotools and build make distclean ./autogen.sh && ./configure && make
Created attachment 118414 [details] 0001-config-remove-unrequired-AC_HEADER_STDC.patch
Created attachment 118415 [details] 0002-config-Set-automake-options-consistenly-in-one-locat.patch
Created attachment 118416 [details] 0003-Makefile.am-Consistent-indent-and-align.patch
Created attachment 118417 [details] 0004-config-Conditionalize-finline-functions-on-compiler-.patch
Created attachment 118418 [details] 0005-macOS-Don-t-duplicate-fno-stack-protector.patch
Created attachment 118419 [details] 0006-Add-missing-documentation-file-from-EXTRA_DIST.patch
I also wonder whether valgrind should be explicit about a minimum supported version of autoconf (via AC_PREREQ() macro)? This would allow some of the existing backwards compatible hacks to be dropped. The only current location that a minimum version is mentioned is at http://valgrind.org/downloads/repository.html, where autoconf >=2.68 is stated as a dependency upon automake >= 1.10. A minimum automake version dependency is currently specified in the code. Thoughts?
Rhys, thank you for the cleanup patches. I know next to nothing about auto*, but these look OK to land, at least to my inexperienced eye. One minor question: 0004-config-Conditionalize-finline-functions-on-compiler-.patch +safe_CFLAGS=$CFLAGS +CFLAGS="-finline-functions -Werror" Why -Werror for the test compiler invokation? Is that standard?
(In reply to Rhys Kidd from comment #7) > I also wonder whether valgrind should be explicit about a minimum supported > version of autoconf (via AC_PREREQ() macro)? I would be in favour of that. My only comment is that it should nevertheless be a pretty old minimum version, because we'll want to support building V on pretty old systems -- think ancient RHEL setups, etc. Is it realistic to hope for a minimum version that is, simultaneously, actually useful to enforce, and yet allows us to build on systems that are 5 years old?
(In reply to Julian Seward from comment #8) > One minor question: > > 0004-config-Conditionalize-finline-functions-on-compiler-.patch > > +safe_CFLAGS=$CFLAGS > +CFLAGS="-finline-functions -Werror" > > Why -Werror for the test compiler invokation? Is that standard? Yes, this is an approach used many times within Valgrind's configure.ac. Look for any of the examples of: safe_CFLAGS=$CFLAGS CFLAGS="$CFLAGS <something> -Werror" ... CFLAGS=$safe_CFLAGS Between the first and last line, the CFLAGS is temporarily overwritten with the compiler option we are testing for the presence of. The "-Werror" works to cause a hard error if the compiler option isn't present, which is then handled. At the end of the block the "-Werror" is reverted, and autotools continues to the next block.
(In reply to Julian Seward from comment #9) > (In reply to Rhys Kidd from comment #7) > > I also wonder whether valgrind should be explicit about a minimum supported > > version of autoconf (via AC_PREREQ() macro)? > > I would be in favour of that. My only comment is that it should nevertheless > be a pretty old minimum version, because we'll want to support building V on > pretty old systems -- think ancient RHEL setups, etc. Is it realistic to > hope > for a minimum version that is, simultaneously, actually useful to enforce, > and > yet allows us to build on systems that are 5 years old? I was considering relying upon one of one of the following minimum versions, which provide a better trade off between quality-of-life features and support for older distributions: autoconf-2.60 released 2006-06-26 autoconf-2.63 released 2008-09-09 Even autoconf-2.68 is nearly a decade old having been released on 2010-09-22. I'd be surprised if RHEL 6+ didn't support at least autoconf-2.60.
(In reply to Rhys Kidd from comment #10) > At the end of the block the "-Werror" is reverted, and autotools continues > to the next block. Ah, yes. I was aware of the flag save/restore game, but I didn't know that -Werror was an important part of it. This all sounds fine. Please land.
(In reply to Rhys Kidd from comment #11) > autoconf-2.60 released 2006-06-26 > autoconf-2.63 released 2008-09-09 > > Even autoconf-2.68 is nearly a decade old having been released on > 2010-09-22. > > I'd be surprised if RHEL 6+ didn't support at least autoconf-2.60. Any of those three versions sounds fine to me, even 2.68.
OK, I'll land this autotools cleanup series sometime this week. Will spin up a separate series that sets the minimum autoconf version, and drops the need for some hacks for functionality that valgrind can now rely upon being in autotools.
Patches in master Valgrind git.