| Summary: | [PATCH] autotools cleanup series | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Rhys Kidd <rhyskidd> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: |
0001-config-remove-unrequired-AC_HEADER_STDC.patch
0002-config-Set-automake-options-consistenly-in-one-locat.patch 0003-Makefile.am-Consistent-indent-and-align.patch 0004-config-Conditionalize-finline-functions-on-compiler-.patch 0005-macOS-Don-t-duplicate-fno-stack-protector.patch 0006-Add-missing-documentation-file-from-EXTRA_DIST.patch |
||
|
Description
Rhys Kidd
2019-02-28 03:51:32 UTC
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. |