Merge lp:~abychko/percona-server/bug1103328 into lp:percona-server/5.5

Proposed by Alexey Bychko
Status: Merged
Approved by: Alexey Bychko
Approved revision: no longer in the source branch.
Merged at revision: 423
Proposed branch: lp:~abychko/percona-server/bug1103328
Merge into: lp:percona-server/5.5
Diff against target: 67 lines (+14/-8)
2 files modified
Percona-Server/configure.cmake (+4/-3)
build/build-binary.sh (+10/-5)
To merge this branch: bzr merge lp:~abychko/percona-server/bug1103328
Reviewer Review Type Date Requested Status
Stewart Smith (community) Approve
Alexey Kopytov (community) Approve
Laurynas Biveinis (community) Needs Fixing
Review via email: mp+144449@code.launchpad.net

Description of the change

[+] added --valgrind option to build-binary.sh
[+] added fatal error to cmake if WITH_VALGRIND=ON, but dev files not installed
[+] added -valgrind suffix to tarball name

To post a comment you must log in.
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Mostly OK, but whitespace issue at diff lines 41--42.

I am also nervous about diverging from the upstream behaviour by making WITH_VALGRIND fatal in the absence of Valgrind headers. Should be OK, but please get a second opinion.

review: Needs Fixing
Revision history for this message
Alexey Bychko (abychko) wrote :

whitespace issue at diff lines 41-42 fixed. the reason for fatal error is:
WITH_VALGRIND option default is OFF. so you can have it ON only if you have it manually specified. in that case valgrind support must be built. but if you have valgrind dev fies not installed, cmake will skip building valgrind support, disregarding WITH_VALGRIND option

Revision history for this message
Alexey Kopytov (akopytov) :
review: Approve
Revision history for this message
Stewart Smith (stewart) wrote :

00:00:18.133 + bzr merge lp:~abychko/percona-server/bug1103328
00:00:24.508 M Percona-Server/configure.cmake
00:00:24.508 M build/build-binary.sh
00:00:24.509 Text conflict in build/build-binary.sh
00:00:24.513 1 conflicts encountered.
00:00:26.590 Build step 'Execute shell' marked build as failure
00:00:26.607 Finished: FAILURE

review: Needs Fixing
410. By Alexey Bychko

fixed bug 1103328, added --valgrind option for build and cmake error if valgrind required, but devel files not installed. changed tarball name appropriately

Revision history for this message
Alexey Bychko (abychko) wrote :

fixed TAB vs SPACE issue

Revision history for this message
Stewart Smith (stewart) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Percona-Server/configure.cmake'
--- Percona-Server/configure.cmake 2012-04-23 14:18:55 +0000
+++ Percona-Server/configure.cmake 2013-02-06 07:04:20 +0000
@@ -1018,11 +1018,12 @@
1018MARK_AS_ADVANCED(WITH_ATOMIC_LOCKS MY_ATOMIC_MODE_RWLOCK MY_ATOMIC_MODE_DUMMY)1018MARK_AS_ADVANCED(WITH_ATOMIC_LOCKS MY_ATOMIC_MODE_RWLOCK MY_ATOMIC_MODE_DUMMY)
10191019
1020IF(WITH_VALGRIND)1020IF(WITH_VALGRIND)
1021 CHECK_INCLUDE_FILES("valgrind/memcheck.h;valgrind/valgrind.h" 1021 CHECK_INCLUDE_FILES("valgrind/memcheck.h;valgrind/valgrind.h" HAVE_VALGRIND_HEADERS)
1022 HAVE_VALGRIND_HEADERS)
1023 IF(HAVE_VALGRIND_HEADERS)1022 IF(HAVE_VALGRIND_HEADERS)
1024 SET(HAVE_VALGRIND 1)1023 SET(HAVE_VALGRIND 1)
1025 ENDIF()1024 ELSE()
1025 MESSAGE(FATAL_ERROR "No Valgrind headers found! Please install Valgrind development files!")
1026 ENDIF(HAVE_VALGRIND_HEADERS)
1026ENDIF()1027ENDIF()
10271028
1028#--------------------------------------------------------------------1029#--------------------------------------------------------------------
10291030
=== modified file 'build/build-binary.sh'
--- build/build-binary.sh 2013-01-16 13:23:11 +0000
+++ build/build-binary.sh 2013-02-06 07:04:20 +0000
@@ -25,7 +25,7 @@
25# Check if we have a functional getopt(1)25# Check if we have a functional getopt(1)
26if ! getopt --test26if ! getopt --test
27then27then
28 go_out="$(getopt --options="iqd" --longoptions=i686,quiet,debug \28 go_out="$(getopt --options="iqdv" --longoptions=i686,quiet,debug,valgrind \
29 --name="$(basename "$0")" -- "$@")"29 --name="$(basename "$0")" -- "$@")"
30 test $? -eq 0 || exit 130 test $? -eq 0 || exit 1
31 eval set -- $go_out31 eval set -- $go_out
@@ -43,7 +43,12 @@
43 -d | --debug )43 -d | --debug )
44 shift44 shift
45 CMAKE_BUILD_TYPE='Debug'45 CMAKE_BUILD_TYPE='Debug'
46 DEBUG_COMMENT='-debug'46 BUILD_COMMENT="${BUILD_COMMENT:-}-debug"
47 ;;
48 -v | --valgrind )
49 shift
50 CMAKE_OPTS="${CMAKE_OPTS:-} -DWITH_VALGRIND=ON"
51 BUILD_COMMENT="${BUILD_COMMENT:-}-valgrind"
47 ;;52 ;;
48 -q | --quiet )53 -q | --quiet )
49 shift54 shift
@@ -97,9 +102,9 @@
97# Build information102# Build information
98REVISION="$(cd "$SOURCEDIR"; bzr log -r-1 | grep ^revno: | cut -d ' ' -f 2)"103REVISION="$(cd "$SOURCEDIR"; bzr log -r-1 | grep ^revno: | cut -d ' ' -f 2)"
99PRODUCT_FULL="Percona-Server-$MYSQL_VERSION-$PERCONA_SERVER_VERSION"104PRODUCT_FULL="Percona-Server-$MYSQL_VERSION-$PERCONA_SERVER_VERSION"
100PRODUCT_FULL="$PRODUCT_FULL-$REVISION$DEBUG_COMMENT.$(uname -s).$TARGET"105PRODUCT_FULL="$PRODUCT_FULL-$REVISION${BUILD_COMMENT:-}.$(uname -s).$TARGET"
101COMMENT="Percona Server with XtraDB (GPL), Release $PERCONA_SERVER_VERSION"106COMMENT="Percona Server with XtraDB (GPL), Release $PERCONA_SERVER_VERSION"
102COMMENT="$COMMENT, Revision $REVISION$DEBUG_COMMENT"107COMMENT="$COMMENT, Revision $REVISION${BUILD_COMMENT:-}"
103108
104# Compilation flags109# Compilation flags
105export CC=${CC:-gcc}110export CC=${CC:-gcc}
@@ -120,7 +125,7 @@
120 make clean all125 make clean all
121126
122 cd "$PRODUCT"127 cd "$PRODUCT"
123 cmake . -DBUILD_CONFIG=mysql_release \128 cmake . ${CMAKE_OPTS:-} -DBUILD_CONFIG=mysql_release \
124 -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" \129 -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" \
125 -DWITH_EMBEDDED_SERVER=OFF \130 -DWITH_EMBEDDED_SERVER=OFF \
126 -DFEATURE_SET=community \131 -DFEATURE_SET=community \

Subscribers

People subscribed via source and target branches