You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2015/05/05 17:53:47 UTC
Review Request 33849: mesos: use standard macros for compiler and
vendor detection
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33849/
-----------------------------------------------------------
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
Posted by Cody Maloney <co...@mesosphere.io>.
> 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 :)
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
Posted by James Peach <jp...@apache.org>.
> 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).
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
Posted by James Peach <jp...@apache.org>.
> 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
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
Posted by Cody Maloney <co...@mesosphere.io>.
> 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.
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
Posted by James Peach <jp...@apache.org>.
> 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.
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
Posted by Cody Maloney <co...@mesosphere.io>.
> 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?
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
Posted by James Peach <jp...@apache.org>.
> 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?
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
Posted by Benjamin Hindman <be...@berkeley.edu>.
> 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
>
>
Re: Review Request 33849: mesos: use standard macros for compiler and
vendor detection
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/33849/#comment133264>
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
<https://reviews.apache.org/r/33849/#comment133263>
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
<https://reviews.apache.org/r/33849/#comment133265>
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
Posted by Benjamin Hindman <be...@berkeley.edu>.
> 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
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
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
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
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
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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
>
>