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
1=== modified file 'Percona-Server/configure.cmake'
2--- Percona-Server/configure.cmake 2012-04-23 14:18:55 +0000
3+++ Percona-Server/configure.cmake 2013-02-06 07:04:20 +0000
4@@ -1018,11 +1018,12 @@
5 MARK_AS_ADVANCED(WITH_ATOMIC_LOCKS MY_ATOMIC_MODE_RWLOCK MY_ATOMIC_MODE_DUMMY)
6
7 IF(WITH_VALGRIND)
8- CHECK_INCLUDE_FILES("valgrind/memcheck.h;valgrind/valgrind.h"
9- HAVE_VALGRIND_HEADERS)
10+ CHECK_INCLUDE_FILES("valgrind/memcheck.h;valgrind/valgrind.h" HAVE_VALGRIND_HEADERS)
11 IF(HAVE_VALGRIND_HEADERS)
12 SET(HAVE_VALGRIND 1)
13- ENDIF()
14+ ELSE()
15+ MESSAGE(FATAL_ERROR "No Valgrind headers found! Please install Valgrind development files!")
16+ ENDIF(HAVE_VALGRIND_HEADERS)
17 ENDIF()
18
19 #--------------------------------------------------------------------
20
21=== modified file 'build/build-binary.sh'
22--- build/build-binary.sh 2013-01-16 13:23:11 +0000
23+++ build/build-binary.sh 2013-02-06 07:04:20 +0000
24@@ -25,7 +25,7 @@
25 # Check if we have a functional getopt(1)
26 if ! getopt --test
27 then
28- go_out="$(getopt --options="iqd" --longoptions=i686,quiet,debug \
29+ go_out="$(getopt --options="iqdv" --longoptions=i686,quiet,debug,valgrind \
30 --name="$(basename "$0")" -- "$@")"
31 test $? -eq 0 || exit 1
32 eval set -- $go_out
33@@ -43,7 +43,12 @@
34 -d | --debug )
35 shift
36 CMAKE_BUILD_TYPE='Debug'
37- DEBUG_COMMENT='-debug'
38+ BUILD_COMMENT="${BUILD_COMMENT:-}-debug"
39+ ;;
40+ -v | --valgrind )
41+ shift
42+ CMAKE_OPTS="${CMAKE_OPTS:-} -DWITH_VALGRIND=ON"
43+ BUILD_COMMENT="${BUILD_COMMENT:-}-valgrind"
44 ;;
45 -q | --quiet )
46 shift
47@@ -97,9 +102,9 @@
48 # Build information
49 REVISION="$(cd "$SOURCEDIR"; bzr log -r-1 | grep ^revno: | cut -d ' ' -f 2)"
50 PRODUCT_FULL="Percona-Server-$MYSQL_VERSION-$PERCONA_SERVER_VERSION"
51-PRODUCT_FULL="$PRODUCT_FULL-$REVISION$DEBUG_COMMENT.$(uname -s).$TARGET"
52+PRODUCT_FULL="$PRODUCT_FULL-$REVISION${BUILD_COMMENT:-}.$(uname -s).$TARGET"
53 COMMENT="Percona Server with XtraDB (GPL), Release $PERCONA_SERVER_VERSION"
54-COMMENT="$COMMENT, Revision $REVISION$DEBUG_COMMENT"
55+COMMENT="$COMMENT, Revision $REVISION${BUILD_COMMENT:-}"
56
57 # Compilation flags
58 export CC=${CC:-gcc}
59@@ -120,7 +125,7 @@
60 make clean all
61
62 cd "$PRODUCT"
63- cmake . -DBUILD_CONFIG=mysql_release \
64+ cmake . ${CMAKE_OPTS:-} -DBUILD_CONFIG=mysql_release \
65 -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" \
66 -DWITH_EMBEDDED_SERVER=OFF \
67 -DFEATURE_SET=community \

Subscribers

People subscribed via source and target branches