Merge lp:~percona-toolkit-dev/percona-toolkit/pt-stalk-fails-to-start-if-HOME-environment-variable-is-not-set-1266869 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.13

Proposed by Frank Cizmich
Status: Merged
Approved by: Daniel Nichter
Approved revision: 613
Merged at revision: 616
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/pt-stalk-fails-to-start-if-HOME-environment-variable-is-not-set-1266869
Merge into: lp:~percona-toolkit-dev/percona-toolkit/release-2.2.13
Diff against target: 159 lines (+47/-9)
9 files modified
bin/pt-ioprofile (+4/-1)
bin/pt-mext (+4/-1)
bin/pt-mysql-summary (+4/-1)
bin/pt-pmp (+4/-1)
bin/pt-sift (+4/-1)
bin/pt-stalk (+4/-1)
bin/pt-summary (+4/-1)
lib/bash/parse_options.sh (+5/-1)
t/lib/bash/parse_options.sh (+14/-1)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/pt-stalk-fails-to-start-if-HOME-environment-variable-is-not-set-1266869
Reviewer Review Type Date Requested Status
Daniel Nichter Pending
Review via email: mp+246351@code.launchpad.net

This proposal supersedes a proposal from 2014-12-19.

Description of the change

Problem:
If $HOME is not set shell tools fail when trying to find config files.
This is a problem for users who want to start tools from init (e.g. pt-stalk)

Solution:
Allowed undefined variables for that line of code in parse_options.sh
Synced with rest of tools.

To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) wrote : Posted in a previous version of this proposal

This approach is too blunt. set -u was a specific design decision for this libs to force good code and avoid weird problems with undefined vars. If $HOME is the only consideration, can you conditionalize the call? I.e. if HOME is set, call it one way; if not set, call it another way?

review: Needs Fixing
613. By Frank Cizmich

added test. removed space

Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

Let's use

   if [ "${HOME:-}" ]; then

for style consistency. Else good to go.

614. By Frank Cizmich

simplified conditional for testing if HOME is set

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/pt-ioprofile'
2--- bin/pt-ioprofile 2014-11-11 13:28:27 +0000
3+++ bin/pt-ioprofile 2015-01-19 16:59:37 +0000
4@@ -201,7 +201,10 @@
5 _parse_config_files "$user_config_file"
6 done
7 else
8- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
9+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
10+ if [ "${HOME:-}" ]; then
11+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
12+ fi
13 fi
14
15 _parse_command_line "${@:-""}"
16
17=== modified file 'bin/pt-mext'
18--- bin/pt-mext 2014-11-11 13:28:27 +0000
19+++ bin/pt-mext 2015-01-19 16:59:37 +0000
20@@ -242,7 +242,10 @@
21 _parse_config_files "$user_config_file"
22 done
23 else
24- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
25+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
26+ if [ "${HOME:-}" ]; then
27+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
28+ fi
29 fi
30
31 _parse_command_line "${@:-""}"
32
33=== modified file 'bin/pt-mysql-summary'
34--- bin/pt-mysql-summary 2014-11-11 13:28:27 +0000
35+++ bin/pt-mysql-summary 2015-01-19 16:59:37 +0000
36@@ -203,7 +203,10 @@
37 _parse_config_files "$user_config_file"
38 done
39 else
40- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
41+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
42+ if [ "${HOME:-}" ]; then
43+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
44+ fi
45 fi
46
47 _parse_command_line "${@:-""}"
48
49=== modified file 'bin/pt-pmp'
50--- bin/pt-pmp 2014-11-11 13:28:27 +0000
51+++ bin/pt-pmp 2015-01-19 16:59:37 +0000
52@@ -244,7 +244,10 @@
53 _parse_config_files "$user_config_file"
54 done
55 else
56- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
57+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
58+ if [ "${HOME:-}" ]; then
59+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
60+ fi
61 fi
62
63 _parse_command_line "${@:-""}"
64
65=== modified file 'bin/pt-sift'
66--- bin/pt-sift 2014-11-11 13:28:27 +0000
67+++ bin/pt-sift 2015-01-19 16:59:37 +0000
68@@ -242,7 +242,10 @@
69 _parse_config_files "$user_config_file"
70 done
71 else
72- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
73+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
74+ if [ "${HOME:-}" ]; then
75+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
76+ fi
77 fi
78
79 _parse_command_line "${@:-""}"
80
81=== modified file 'bin/pt-stalk'
82--- bin/pt-stalk 2014-11-11 13:28:27 +0000
83+++ bin/pt-stalk 2015-01-19 16:59:37 +0000
84@@ -255,7 +255,10 @@
85 _parse_config_files "$user_config_file"
86 done
87 else
88- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
89+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
90+ if [ "${HOME:-}" ]; then
91+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
92+ fi
93 fi
94
95 _parse_command_line "${@:-""}"
96
97=== modified file 'bin/pt-summary'
98--- bin/pt-summary 2014-11-11 13:28:27 +0000
99+++ bin/pt-summary 2015-01-19 16:59:37 +0000
100@@ -210,7 +210,10 @@
101 _parse_config_files "$user_config_file"
102 done
103 else
104- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
105+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
106+ if [ "${HOME:-}" ]; then
107+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
108+ fi
109 fi
110
111 _parse_command_line "${@:-""}"
112
113=== modified file 'lib/bash/parse_options.sh'
114--- lib/bash/parse_options.sh 2014-10-06 21:04:49 +0000
115+++ lib/bash/parse_options.sh 2015-01-19 16:59:37 +0000
116@@ -213,7 +213,11 @@
117 _parse_config_files "$user_config_file"
118 done
119 else
120- _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
121+ _parse_config_files "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf"
122+ # conditional in case $HOME isn't set; e.g. tool launched from init
123+ if [ "${HOME:-}" ]; then
124+ _parse_config_files "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
125+ fi
126 fi
127
128 # Finally, parse the command line.
129
130=== modified file 't/lib/bash/parse_options.sh'
131--- t/lib/bash/parse_options.sh 2012-08-20 22:22:31 +0000
132+++ t/lib/bash/parse_options.sh 2015-01-19 16:59:37 +0000
133@@ -1,6 +1,6 @@
134 #!/usr/bin/env bash
135
136-plan 83
137+plan 84
138
139 TMPFILE="$TEST_PT_TMPDIR/parse-opts-output"
140 TOOL="pt-stalk"
141@@ -259,5 +259,18 @@
142 is "$OPT_NOTIFY_BY_EMAIL" "foo@bar.com" "Bug 1038995: ...but gets set without errors if specified"
143
144 # ############################################################################
145+# Bug 1266869: fails when $HOME unset
146+# https://bugs.launchpad.net/percona-toolkit/+bug/1266869
147+# ############################################################################
148+
149+TMP_HOME="$HOME"
150+unset HOME
151+OUTPUT=`parse_options $T_LIB_DIR/samples/bash/po001.sh 2>&1`
152+echo "$OUTPUT" > "$TMPFILE"
153+cmd_ok "grep -q -v unbound $TMPFILE" "No error when \$HOME is not set"
154+HOME="$TMP_HOME" # just in case further tests below need it
155+
156+
157+# ############################################################################
158 # Done
159 # ############################################################################

Subscribers

People subscribed via source and target branches

to all changes: