List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:December 14 2010 9:13am
Subject:Re: bzr commit into mysql-5.5-bugteam branch (davi:3188) Bug#58871
View as plain text  
Hi Davi

Inline comments below.

-- didrik

On Mon, Dec 13, 2010 at 6:36 PM, Davi Arnaut <davi.arnaut@stripped> wrote:

> # At a local mysql-5.5-bugteam repository of davi
>
>  3188 Davi Arnaut       2010-12-13
>      Bug#58871: Reorganize maintainer mode compiler flags to allow
>                 option for specific compilers
>
>      Reorganize the maintainer mode cmake code to allow options
>      for specific compilers. For now, enable -Wall for ICC, but
>      do not turn warnings into errors.
>     @ CMakeLists.txt
>        Move the code that sets options to cmake/maintainer.cmake
>     @ cmake/maintainer.cmake
>        Add macros for each specific compiler.
>
>    added:
>      cmake/maintainer.cmake
>    modified:
>      CMakeLists.txt
> === modified file 'CMakeLists.txt'
> --- a/CMakeLists.txt    2010-11-24 10:23:44 +0000
> +++ b/CMakeLists.txt    2010-12-13 17:36:41 +0000
> @@ -115,30 +115,22 @@ ENDIF()
>  # Control aspects of the development environment which are
>  # specific to MySQL maintainers and developers.
>  #
> -INCLUDE (CheckCCompilerFlag)
> +INCLUDE(maintainer)
>  OPTION(MYSQL_MAINTAINER_MODE "MySQL maintainer-specific development
> environment" OFF)
>

please fix this long line


> -# Whether the maintainer mode should be enabled.
> +
> +# Whether the maintainer mode compiler options should be enabled.
>  IF(MYSQL_MAINTAINER_MODE)
> -  IF(CMAKE_COMPILER_IS_GNUCC)
> -    CHECK_C_COMPILER_FLAG("-Wdeclaration-after-statement"
> HAVE_DECLARATION_AFTER_STATEMENT)
> -    IF(HAVE_DECLARATION_AFTER_STATEMENT)
> -      SET(MY_MAINTAINER_DECLARATION_AFTER_STATEMENT
> "-Wdeclaration-after-statement")
> -    ENDIF()
> -    SET(MY_MAINTAINER_C_WARNINGS
> -        "-Wall -Wextra -Wunused -Wwrite-strings -Wno-strict-aliasing
> -Werror")
> +  IF("${CMAKE_C_COMPILER_ID}" MATCHES "GNU")
>

remove the variable substitution and quoting, here and below:

IF(CMAKE_C_COMPILER_ID MATCHES "GNU")

CMAKE_C_COMPILER_ID seems to be an internal cmake variable,
but we are already using it elsewhere, so I guess it is ok.

There's no regex on the right-hand-side, so you might as well write

IF(CMAKE_C_COMPILER_ID STREQUAL "GNU")



> +    SET_MYSQL_MAINTAINER_GNU_C_OPTIONS()
>   ENDIF()
> -  IF(CMAKE_COMPILER_IS_GNUCXX)
> -    SET(MY_MAINTAINER_CXX_WARNINGS "${MY_MAINTAINER_C_WARNINGS}
> -Wno-unused-parameter"
> -        CACHE STRING "C++ warning options used in maintainer builds.")
> +  IF("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
> +    SET_MYSQL_MAINTAINER_GNU_CXX_OPTIONS()
>   ENDIF()
> -  IF(CMAKE_COMPILER_IS_GNUCC)
> -    SET(MY_MAINTAINER_C_WARNINGS
> -        "${MY_MAINTAINER_C_WARNINGS}
> ${MY_MAINTAINER_DECLARATION_AFTER_STATEMENT}"
> -        CACHE STRING "C warning options used in maintainer builds.")
> +  IF("${CMAKE_C_COMPILER_ID}" MATCHES "Intel")
> +    SET_MYSQL_MAINTAINER_INTEL_C_OPTIONS()
>   ENDIF()
> -  # Do not make warnings in checks into errors.
> -  IF(CMAKE_COMPILER_IS_GNUCC AND CMAKE_COMPILER_IS_GNUCXX)
> -    SET(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Wno-error")
> +  IF("${CMAKE_CXX_COMPILER_ID}" MATCHES "Intel")
> +    SET_MYSQL_MAINTAINER_INTEL_CXX_OPTIONS()
>   ENDIF()
>  ENDIF()
>
>

Please fix the long lines in the file below.


>
> === added file 'cmake/maintainer.cmake'
> --- a/cmake/maintainer.cmake    1970-01-01 00:00:00 +0000
> +++ b/cmake/maintainer.cmake    2010-12-13 17:36:41 +0000
> @@ -0,0 +1,51 @@
> +# Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
>  USA
> +
> +INCLUDE(CheckCCompilerFlag)
> +
> +# Setup GCC (GNU C compiler) warning options.
> +MACRO(SET_MYSQL_MAINTAINER_GNU_C_OPTIONS)
> +  SET(MY_MAINTAINER_WARNINGS
> +      "-Wall -Wextra -Wunused -Wwrite-strings -Wno-strict-aliasing
> -Werror")
> +  CHECK_C_COMPILER_FLAG("-Wdeclaration-after-statement"
> HAVE_DECLARATION_AFTER_STATEMENT)
> +  IF(HAVE_DECLARATION_AFTER_STATEMENT)
> +    SET(MY_MAINTAINER_DECLARATION_AFTER_STATEMENT
> "-Wdeclaration-after-statement")
> +  ENDIF()
> +  SET(MY_MAINTAINER_C_WARNINGS
> +      "${MY_MAINTAINER_WARNINGS}
> ${MY_MAINTAINER_DECLARATION_AFTER_STATEMENT}"
> +      CACHE STRING "C warning options used in maintainer builds.")
> +  # Do not make warnings in checks into errors.
> +  SET(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Wno-error")
> +ENDMACRO()
> +
> +# Setup G++ (GNU C++ compiler) warning options.
> +MACRO(SET_MYSQL_MAINTAINER_GNU_CXX_OPTIONS)
> +  SET(MY_MAINTAINER_CXX_WARNINGS "${MY_MAINTAINER_WARNINGS}
> -Wno-unused-parameter"
> +      CACHE STRING "C++ warning options used in maintainer builds.")
> +ENDMACRO()
> +
> +# Setup ICC (Intel C Compiler) warning options.
> +MACRO(SET_MYSQL_MAINTAINER_INTEL_C_OPTIONS)
> +  SET(MY_MAINTAINER_WARNINGS "-Wall")
> +  SET(MY_MAINTAINER_C_WARNINGS "${MY_MAINTAINER_WARNINGS}"
> +      CACHE STRING "C warning options used in maintainer builds.")
> +ENDMACRO()
> +
> +# Setup ICPC (Intel C++ Compiler) warning options.
> +MACRO(SET_MYSQL_MAINTAINER_INTEL_CXX_OPTIONS)
> +  SET(MY_MAINTAINER_CXX_WARNINGS "${MY_MAINTAINER_WARNINGS}"
> +      CACHE STRING "C++ warning options used in maintainer builds.")
> +ENDMACRO()
> +
>
>
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>

Thread
bzr commit into mysql-5.5-bugteam branch (davi:3188) Bug#58871Davi Arnaut13 Dec
Re: bzr commit into mysql-5.5-bugteam branch (davi:3188) Bug#58871Tor Didriksen14 Dec
  • Re: bzr commit into mysql-5.5-bugteam branch (davi:3188) Bug#58871Davi Arnaut14 Dec
    • Re: bzr commit into mysql-5.5-bugteam branch (davi:3188) Bug#58871Tor Didriksen14 Dec
      • Re: bzr commit into mysql-5.5-bugteam branch (davi:3188) Bug#58871Davi Arnaut14 Dec