You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2015/06/04 16:44:26 UTC

Re: Review Request 33849: mesos: use standard macros for compiler and vendor detection


> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > <https://reviews.apache.org/r/33849/diff/1/?file=950367#file950367line575>
> >
> >     Can you check ax_cxx_compiler_version >= Clang/LLVM 3.5 here? Or does that not work on both OS X and Linux?
> 
> James Peach wrote:
>     Hmm. I don't have a Linux system with clang 3.5 and I'm not quite sure what you are asking here. I do have a later version of OS X clang and this change works (and automatically disables unused local typedef warnings).
>     
>     I tried to leave the compiler logic alone in this patch since I don't have systems to test all the possible combinations. I agree that it would be desirable to separate the typedef warnings from the compiler versions (though I was confused that GCC and clang seem to have subtly different names for the same warning). I'm happy to knock up a separate patch for that if you like.
> 
> James Peach wrote:
>     If you're asking whether we can drop the ``-Wno-unused-local-typedef`` flags on later clangs, I'm not confident that I have the right set of build dependencies to answer that. I tested dropping that warning on my version of clang on OS X (later that 3.5) dropping; both boost and generated protobuf code still spew that warning.
> 
> Cody Maloney wrote:
>     Apple Clang gives a different version string when you give it `clang --version` than Linux clang
>     
>     Apple:
>     ```
>     Apple LLVM version 6.1.0 (clang-602.0.49) (based on LLVM 3.6.0svn)
>     Target: x86_64-apple-darwin14.3.0
>     Thread model: posix
>     ```
>     
>     Linux:
>     ```
>     clang version 3.6.0 (tags/RELEASE_360/final)
>     Target: x86_64-unknown-linux-gnu
>     Thread model: posix
>     ```
>     
>     
>     For the `-Wno-unused-local-typedef` - that is definitely a needed flag for some of the Clang + GCC versions we support. It's much more of "Lets try compiling some code which checks if we'll generate a warning, then if the compiler is clang add `-Wno-unused-local-typedef`, if it is gcc add `-Wno-unused-local-typedefs` (Note the s).
> 
> James Peach wrote:
>     That sounds like you want to just unconditionally turn ``-Wunused-local-typedef`` off. Either if you test whether it works, then turn it off if it does, then the net effect is to always turn it off :)
> 
> Cody Maloney wrote:
>     The warning didn't exist in Clang 3.5 though I think, so one of our platforms doesn't need it. Other than that one compiler+version combo though it is always on
> 
> James Peach wrote:
>     So it should be safe (and desirable) to do ``-Wno-unused-local-typedef`` ``-Wno-unknown-warning-option`` then?
> 
> Cody Maloney wrote:
>     I'd much rather keep around the "unknwon option" warnings if at all possible. There is a reasonable amount of work I do where I test-enable misc. random warning options to try to cleanup the codebase some / see if they'd be feasible long-term, and typos do happen.

I dropped this issue for now since this change doesn't remove any functionality only cleans up how we check for clang versus gcc. James, it would be great if you could follow this patch up with one that replaces what Cody had to do with something that just checks for clang 3.5 directly and then acts accordingly. Does that make sense?


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33849/#review82524
-----------------------------------------------------------


On May 5, 2015, 5:52 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33849/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2666
>     https://issues.apache.org/jira/browse/MESOS-2666
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Import the AX_COMPILER_VERSION and AX_COMPILER_VENDOR macros from
> the autoconf archive and use them to detect and configure the
> supported copmilers.
> 
> 
> Diffs
> -----
> 
>   configure.ac 14658f6f6920549f1948b8bd8890c67bc067e3dd 
>   m4/ax_compiler_vendor.m4 PRE-CREATION 
>   m4/ax_compiler_version.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33849/diff/
> 
> 
> Testing
> -------
> 
> Make check on CentOS 7 and Mac OS X 10.10.3. Verified that the default CentOS 6 GCC toolchain is still rejected as too old.
> 
> 
> Thanks,
> 
> James Peach
> 
>