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

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


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
> 
>

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

[Benjamin Hindman]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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
> 
>

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

[Benjamin Hindman]

> On June 4, 2015, 2:44 p.m., Benjamin Hindman wrote:
> > Ship It!

Like another patch you submitted you went from +2 to +4 indentation here, 
please be on the look out for that in the future. Thanks!


- Benjamin


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


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
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [33849]

All tests passed.

- Mesos ReviewBot


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
> 
>

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

[Cody Maloney]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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?

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.


- Cody


---
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
> 
>

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

[James Peach]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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

So it should be safe (and desirable) to do ``-Wno-unused-local-typedef`` 
``-Wno-unknown-warning-option`` then?


- James


---
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
> 
>

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

[Cody Maloney]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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 :)

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


- Cody


---
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
> 
>

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

[James Peach]

---
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.


Changes
---

Separated GCC version check from setting GCC-specific warning flags.


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 (updated)
-

  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

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

[James Peach]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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).

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 :)


- James


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


On May 5, 2015, 3:53 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, 3:53 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
> 
>

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

[Cody Maloney]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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.

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).


- Cody


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


On May 5, 2015, 3:53 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, 3:53 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
> 
>

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

[James Peach]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > 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.

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.


- James


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


On May 5, 2015, 3:53 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, 3:53 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
> 
>

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

[James Peach]

> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > 
> >
> > Can you check ax_cxx_compiler_version >= Clang/LLVM 3.5 here? Or does 
> > that not work on both OS X and Linux?

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


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


On May 5, 2015, 3:53 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, 3:53 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
> 
>

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

[Cody Maloney]

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


A couple nits, but structurally looks good to me overall. Mainly just some 
things I'd like to see cleaned up "while we're here"


configure.ac


Can you check ax_cxx_compiler_version >= Clang/LLVM 3.5 here? Or does that 
not work on both OS X and Linux?



configure.ac


Not something you need to do here, but would be nice if we used more or 
less the same check for 'Wno-unused-local-typedef' for both clang and gcc since 
you can easily tell which compiler it is now and update the added flag.



configure.ac


I know this is a mess I left, but it's not overly clean here that we bundle 
the gcc 4.8 version warning with the "unused local typedef" flag adding, could 
you pull out the "is <= gcc 4.8" check to be its own line, then unconditionally 
adding '-Wno-unused-local-typedefs', or using the clang check mentioned above.


- Cody Maloney


On May 5, 2015, 3:53 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, 3:53 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
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [33849]

All tests passed.

- Mesos ReviewBot


On May 5, 2015, 3:53 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, 3:53 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
> 
>