List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:January 22 2011 2:51pm
Subject:RE: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969
View as plain text  
Hi Joerg, 
now that I tried you patch,  some more comments. I also built a patch based on yours, 
with couple of things changed/fixed.

So, the comments:

1) You execute VERSION_BIN twice, at cmake time and at build time . At cmake time you
execute it via inclusion of
INCLUDE(version_files.cmake), at build time via cmake -P version_files.cmake
Unfortunately, at cmake time this leads to VERSION_bin in source directory. This is best
avoided, out-of-source builds treat source
directory as read-only (at least they should try to be clean. I also think that build time
is not  the correct time to run those
scripts. I think the install/package time is the best time, but more on this below.

2) on Windows,
EXECUTE_PROCESS(COMMAND "date" "/T"  OUTPUT_VARIABLE TMP_DATE)
 Will not always lead to a good result. You want to run the cmd.exe bultin, however if
there is a date.exe in the PATH (like in my
case , from C:\Gnuwin32\bin), date.exe will be taken
You can force cmd.exe builtin adding  cmd /c to the command, there is no need for quotes.

 EXECUTE_PROCESS(COMMAND cmd /c date /T  OUTPUT_VARIABLE TMP_DATE)

3) Executing script at *package/install* time. I believe this is what is appropriate in
this case, not cmake time neither build time
(there is no benefit from doing it during cmake or make, the goal is having those files in
package or after "make install") . 

CMake can do  that run scripts at install time with INSTALL(CODE  <your code here>)
The code snippet in question lands in ${CMAKE_CURRENT_BINARY_DIR}/cmake_install.cmake, and
you can inspect it for correctness (the
cmake_install.cmake is something that runs on "make install" or also during the packaging
phase). 

4) CMAKE_HOST_SYSTEM  variable you mention seem to be crosscompiling related.
 CMAKE_SYSTEM_NAME, CMAKE_SYSTEM_VERSION and CMAKE_SYSTEM_PROCESSOR is what is used
everywhere else (e.g in the same top-level
CMakeLists.txt)
Also , CMake 2.6.0 is really old now, the online documentation reflects the latest patch
version (for 2.6, it would be 2.6.4 ( May
2009), and not  2.6.0 (May 2008))


I allowed myself  to modify your patch (see attachment), the changes are related to
a)  avoid writing to CMAKE_SOURCE_DIR. I made VERSION_SRC to always write to
${CMAKE_BINARY_DIR}/VERSION_src
b) avoid doing extra stuff during cmake or build stage. Moved generation of VERSION_xxx to
packaging/install/make_dist stage
c) fixed Windows date mentioned above

I did not fix the CMAKE_HOST_SYSTEM though

Have a look, perhaps you would find it useful.

Vlad

> -----Original Message-----
> From: Joerg Bruehe [mailto:joerg.bruehe@stripped]
> Sent: Samstag, 22. Januar 2011 12:08
> To: Vladislav Vaintroub
> Cc: commits@stripped
> Subject: Re: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969
> 
> Hi Vlad,
> 
> 
> thank you very much for looking at my patch and commenting.
> 
> Vladislav Vaintroub wrote:
> > Hi Joerg,
> >
> >> -----Original Message-----
> >> From: Joerg Bruehe [mailto:joerg@stripped]
> >> Sent: Freitag, 21. Januar 2011 21:40
> >> To: commits@stripped
> >> Subject: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969
> >
> >
> >> +ADD_CUSTOM_TARGET(VERSION_BIN ALL
> >> +  COMMAND ${CMAKE_COMMAND} -P ${CMAKE_BINARY_DIR}/version_files.cmake
> >> +  WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
> >> +)
> >
> > A problem with ADD_CUSTOM_TARGET when it is used like this is that it is always
> rebuilt (there are no inputs and not outputs ,
so it
> > is always considered out-of-date). However there is a way to make it work more
> like classic make utility with input and output,
> > where rebuild is done only if input changes.
> 
> Yes, I am aware of it being rebuilt every time. While that is considered
> waste of cycles in a typical compilation, I in fact favor it here:
> 
> - Creating that file should be little effort, so it should not increase
>   the build time in any noticeable way.
> 
> - If we extend the rule for "VERSION_bin" by looking at further files,
>   we would need to extend the dependencies in your approach. As written
>   above, this has proved to be difficult.
> 
> - The brute force approach "do it always" ensures that even manual
>   interventions which change any of the prerequisite files (like
>   "flags.cmake") will be reflected in "VERSION_bin".
> 
> - "VERSION_bin" also contains a timestamp, IMO that should be that of
>   the (last) "make" run.
> 
> >
> > How about something like:
> >
> > ADD_CUSTOM_COMMAND(
> >    OUTPUT ${CMAKE_BINARY_DIR}/VERSION_bin
> >    COMMAND ${CMAKE_COMMAND} -P ${CMAKE_BINARY_DIR}/version_files.cmake
> >    DEPENDS ${CMAKE_BINARY_DIR}/CMakeCache.txt
> >   )
> >
> > ADD_CUSTOM_TARGET(VERSION_BIN  ALL
> >   DEPENDS ${CMAKE_BINARY_DIR}/VERSION_bin)
> >
> > Disclaimer: I have not tried that with your patch :)   just took the technique
> from the Cmake FAQ (
> > http://www.cmake.org/Wiki/CMake_FAQ , section "How do I generate an executable,
> then use the executable to generate a file? "
> ) and
> > applied to your example (your example is simpler, because you do not have to
> generate an executable).   I assumed you generate
> the
> > VERSION_bin from CMakeCache.txt and there are no other dependencies , at least
> it seems so to me.
> 
> Well, the other dependency is "sql/CMakeFiles/sql.dir/flags.make", and I
> wouldn't be surprised if we in the future add information which must be
> collected from other files.
> 
> Sadly, I have found the cmake documentation to disagree from the acctual
> behavior:
> While "http://www.cmake.org/cmake/help/cmake2.6docs.html" mentions
> several interesting "CMAKE_{HOST_}SYSTEM*" variables, they were not
> provided in my tests (cmake 2.6.0, Debian "stable").
> 
> Also, I had trouble with rules that used files (like "flags.make" and
> "CMakeCache.txt") which were created only at the end of the cmake phase.
> That is also the reason why several of my new actions are guarded by an
> "IF(EXISTS ...)".
> Maybe this can be solved by explicitly adding information these files
> will be generated, but all in all cmake seems to do some existence and
> dependency checks which I had expected in the make phase only.
> 
> Because of this, I'm reluctant to do more complicated cmake constructs,
> and rather favor a "keep it simple" approach. I know it isn't elegant.
> 
> 
> However, I very much appreciate your comments and thoughts!
> 
> Regards,
> Jörg
> 
> --
> Joerg Bruehe,  MySQL Build Team,  joerg.bruehe@stripped
>                (+49 30) 417 01 487
> ORACLE Deutschland B.V. & Co. KG,   Komturstrasse 18a,   D-12099 Berlin
> Geschaeftsfuehrer: Juergen Kunz, Marcel v.d. Molen, Alexander v.d. Ven
> Amtsgericht Muenchen: HRA 95603


=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2010-12-15 10:30:09 +0000
+++ CMakeLists.txt	2011-01-22 14:25:10 +0000
@@ -1,4 +1,4 @@
-# Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2006, 2011, 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
@@ -317,6 +317,19 @@
                ${CMAKE_BINARY_DIR}/include/mysql_version.h )
 CONFIGURE_FILE(${CMAKE_SOURCE_DIR}/sql/sql_builtin.cc.in
     ${CMAKE_BINARY_DIR}/sql/sql_builtin.cc)
+CONFIGURE_FILE(
+    ${CMAKE_SOURCE_DIR}/cmake/version_files.cmake.in
${CMAKE_BINARY_DIR}/version_files.cmake @ONLY)
+INSTALL(CODE
+  "
+  EXECUTE_PROCESS( 
+  COMMAND \"${CMAKE_COMMAND}\" -P \"${CMAKE_BINARY_DIR}/version_files.cmake\" 
+  WORKING_DIRECTORY \"${CMAKE_BINARY_DIR}\")
+  FILE(INSTALL DESTINATION \"\${CMAKE_INSTALL_PREFIX}/.\" TYPE FILE FILES
+    \"${CMAKE_BINARY_DIR}/VERSION_src\"
+    \"${CMAKE_BINARY_DIR}/VERSION_bin\"
+    )
+  "
+)
 
 # Packaging
 IF(WIN32)
@@ -331,6 +344,7 @@
 SET(CPACK_MONOLITHIC_INSTALL 1 CACHE INTERNAL "")
 
 INCLUDE(CPack)
+
 IF(UNIX)
   INSTALL(FILES Docs/mysql.info DESTINATION ${INSTALL_INFODIR} OPTIONAL COMPONENT Info)
 ENDIF()

=== modified file 'cmake/make_dist.cmake.in'
--- cmake/make_dist.cmake.in	2010-11-20 14:47:50 +0000
+++ cmake/make_dist.cmake.in	2011-01-22 14:19:37 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2009 Sun Microsystems, Inc
+# Copyright (c) 2009, 2011, 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
@@ -106,6 +106,12 @@
   EXECUTE_PROCESS(COMMAND ${CMAKE_COMMAND} -E copy_directory "${MYSQL_DOCS_LOCATION}"
"${PACKAGE_DIR}")
 ENDIF()
 
+# Ensure there is a "VERSION_src" file.
+INCLUDE(${CMAKE_BINARY_DIR}/version_files.cmake)
+  CONFIGURE_FILE(${CMAKE_BINARY_DIR}/VERSION_src
+       ${PACKAGE_DIR}/VERSION_src COPYONLY)
+ENDIF()
+
 # In case we used CPack, it could have copied some
 # extra files that are not usable on different machines.
 FILE(REMOVE ${PACKAGE_DIR}/CMakeCache.txt)

=== added file 'cmake/version_files.cmake.in'
--- cmake/version_files.cmake.in	1970-01-01 00:00:00 +0000
+++ cmake/version_files.cmake.in	2011-01-22 14:05:40 +0000
@@ -0,0 +1,113 @@
+# Copyright (c) 2010, 2011, 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 
+
+
+# Handle/create the "VERSION_*" files describing a MySQL (server) binary.
+# This is part of the fix for bug#42969.
+
+SET(VERSION "@VERSION@")
+SET(CMAKE_SOURCE_DIR "@CMAKE_SOURCE_DIR@")
+SET(CMAKE_BINARY_DIR "@CMAKE_BINARY_DIR@")
+SET(CPACK_SOURCE_PACKAGE_FILE_NAME "@CPACK_SOURCE_PACKAGE_FILE_NAME@")
+SET(BZR_EXECUTABLE "@BZR_EXECUTABLE@")
+SET(PACKAGE_DIR  ${CMAKE_BINARY_DIR}/${CPACK_SOURCE_PACKAGE_FILE_NAME})
+ 
+ 
+# This may be used during "make dist".
+# So this is the moment to start a "VERSION_src" file with information about the source
(only).
+# We use the "VERSION" contents and try to add "bzr version-info".
+#
+# It will also be used during build, if that is done without a preceding "make dist"
+# (typically, by developers directly in their tree).
+  
+MACRO(CREATE_VERSION_SRC)
+  IF(EXISTS ${CMAKE_SOURCE_DIR}/.bzr)
+    # We have a working "bzr" and sources are in a BZR repository: Always update.
+    EXECUTE_PROCESS(
+      COMMAND ${BZR_EXECUTABLE} version-info ${CMAKE_SOURCE_DIR}
+      WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
+      OUTPUT_FILE "VERSION_src"
+      RESULT_VARIABLE RESULT
+    )
+    # For better readability ...
+    FILE(APPEND "${CMAKE_BINARY_DIR}/VERSION_src" "\nMySQL source ${VERSION}\n")
+  ELSEIF(NOT EXISTS ${CMAKE_BINARY_DIR}/VERSION_src)
+    # The following is a fall-back if there should be no BZR available.
+    FILE(WRITE "${CMAKE_BINARY_DIR}/VERSION_src" "\nMySQL source ${VERSION}\n")
+  ENDIF()
+ENDMACRO(CREATE_VERSION_SRC)
+
+
+# This is for the "real" build, must be run again with each cmake run
+# to make sure we report the current flags (not those of some previous run).
+
+MACRO(CREATE_VERSION_BIN)
+  SET(VERSION_BIN "VERSION_bin")
+
+  FILE(WRITE ${VERSION_BIN} "===== Information about the build process: =====\n")
+  IF (WIN32)
+    EXECUTE_PROCESS(COMMAND cmd /c date /T OUTPUT_VARIABLE TMP_DATE)
+  ELSEIF(UNIX)
+    EXECUTE_PROCESS(COMMAND "date" "+%Y-%m-%d %H:%M:%S" OUTPUT_VARIABLE TMP_DATE
OUTPUT_STRIP_TRAILING_WHITESPACE)
+  ELSE()
+    SET(TMP_DATE "No date command known")
+  ENDIF()
+  SITE_NAME(HOSTNAME)
+  FILE(APPEND ${VERSION_BIN} "Build was run at: ${TMP_DATE} on host '${HOSTNAME}'\n\n")
+
+  # According to the cmake docs, this variable should always be set.
+  # However, it is empty in my local tests, cmake 2.6.0 on Linux (Debian stable, Jan
2011).
+  # Include this code, so we will profit if a build environment does provide that info.
+  IF(${CMAKE_HOST_SYSTEM})
+    IF(${CMAKE_CROSSCOMPILING})
+      FILE(APPEND ${VERSION_BIN} "Build was done for ${CMAKE_SYSTEM} using
${CMAKE_SYSTEM_PROCESSOR}\n")
+    ENDIF()
+    FILE(APPEND ${VERSION_BIN} "Build was done on  ${CMAKE_HOST_SYSTEM} using
${CMAKE_HOST_SYSTEM_PROCESSOR}\n\n")
+  ENDIF()
+
+  FILE(APPEND ${VERSION_BIN} "===== Compiler flags used (from the 'sql/' subdirectory):
=====\n")
+  IF(EXISTS sql/CMakeFiles/sql.dir/flags.make)
+    # current dir == ${CMAKE_BINARY_DIR}, see the call in top "CMakeLists.txt"
+    FILE(STRINGS sql/CMakeFiles/sql.dir/flags.make COMPILE_FLAGS REGEX "^#
compile|^C_|^CXX_")
+    STRING(REPLACE "# compile" "\n# compile" COMPILE_FLAGS_1 ${COMPILE_FLAGS})
+    STRING(REPLACE "C_"        "\nC_"        COMPILE_FLAGS_2 ${COMPILE_FLAGS_1})
+    STRING(REPLACE "CXX_"      "\nCXX_"      COMPILE_FLAGS_LINES ${COMPILE_FLAGS_2})
+    # The below line did not work in tests using cmake 2.6.0, the matter should be
checked again with newer versions.
+    # STRING(REGEX REPLACE "(^# compile|^C_|^CXX_)" "\n\\1" COMPILE_FLAGS_LINES
${COMPILE_FLAGS})
+    FILE(APPEND ${VERSION_BIN} ${COMPILE_FLAGS_LINES} "\n\n")
+  ELSE()
+    FILE(APPEND ${VERSION_BIN} "File 'sql/CMakeFiles/sql.dir/flags.make' is not yet
found.\n\n")
+  ENDIF()
+
+  FILE(APPEND ${VERSION_BIN} "===== Feature flags used: =====\n")
+  IF(EXISTS ${CMAKE_BINARY_DIR}/CMakeCache.txt)
+    # The intention is to do the equivalent of "grep '^WITH' CMakeCache.txt".
+    # Note that "FILE(STRINGS ...)" will simply ignore carriage return (CR) characters,
+    # so the variable will have all those lines joined to one.
+    # To get readable output, "STRING(REPLACE ...)" is used, making it multi-line again.
+    FILE(STRINGS ${CMAKE_BINARY_DIR}/CMakeCache.txt FEATURE_FLAGS REGEX "^WITH")
+    STRING(REPLACE "WITH" "\nWITH" FEATURE_FLAGS_LINES ${FEATURE_FLAGS})
+    FILE(APPEND ${VERSION_BIN} ${FEATURE_FLAGS_LINES} "\n\n")
+  ELSE()
+    FILE(APPEND ${VERSION_BIN} "File 'CMakeCache.txt' is not yet found.\n\n")
+  ENDIF()
+
+  FILE(APPEND ${VERSION_BIN} "===== EOF =====\n")
+
+ENDMACRO(CREATE_VERSION_BIN)
+
+CREATE_VERSION_BIN()
+CREATE_VERSION_SRC()
+

=== modified file 'support-files/mysql.spec.sh'
--- support-files/mysql.spec.sh	2010-12-29 00:26:31 +0000
+++ support-files/mysql.spec.sh	2011-01-22 12:41:02 +0000
@@ -1,4 +1,4 @@
-# Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2000, 2011, 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
@@ -907,6 +907,8 @@
 %if %{defined license_files_server}
 %doc %{license_files_server}
 %endif
+%doc %{src_dir}/VERSION_src
+%doc release/VERSION_bin
 %doc %{src_dir}/Docs/ChangeLog
 %doc release/support-files/my-*.cnf
 
@@ -1085,6 +1087,10 @@
 # merging BK trees)
 ##############################################################################
 %changelog
+* Fri Jan 21 2011 Joerg Bruehe <joerg.bruehe@stripped>
+
+- Add the new "manifest" files: "VERSION_src" and "VERSION_bin".
+
 * Tue Nov 23 2010 Jonathan Perkin <jonathan.perkin@stripped>
 
 - EXCEPTIONS-CLIENT has been deleted, remove it from here too


Thread
bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Joerg Bruehe21 Jan
  • RE: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Vladislav Vaintroub21 Jan
    • Re: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Joerg Bruehe22 Jan
      • RE: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Vladislav Vaintroub22 Jan
        • RE: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Vladislav Vaintroub22 Jan
          • Re: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Joerg Bruehe31 Jan
            • RE: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Vladislav Vaintroub31 Jan
              • Re: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Joerg Bruehe31 Jan
        • Re: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Joerg Bruehe25 Jan
          • Re: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Joerg Bruehe26 Jan
          • RE: bzr commit into mysql-5.5-bugteam branch (joerg:3209) Bug#42969Vladislav Vaintroub26 Jan