pt-config-diff doesn't handle end-of-line comments

[Kaiwang CHEN]

option file supports end-of-line comments started with semi-comma(;) or sharp (#), but MySQLConfig treats those comments part of value.

[kc@dns1 percona-toolkit]$ cat mycnf-kc-001.txt
[mysqld]
user=mysql ; comment
[kc@dns1 percona-toolkit]$ cat mycnf-kc-002.txt
[mysqld]
user=mysql
[kc@dns1 percona-toolkit]$ perl bin/pt-config-diff mycnf-kc-001.txt mycnf-kc-002.txt
1 config difference
Variable mycnf-kc-001.txt mycnf-kc-002.txt
========================= ================ ================
user mysql ; comment mysql

I think the problem is in _parse_varvals()
=== modified file 'lib/MySQLConfig.pm'
--- lib/MySQLConfig.pm 2012-01-19 19:46:56 +0000
+++ lib/MySQLConfig.pm 2012-06-02 20:07:03 +0000
@@ -339,6 +339,7 @@
       if ( $item ) {
          # Strip leading and trailing whitespace.
          $item =~ s/^\s+//;
+ $item =~ s/[#;].*$//; # trailing comment
          $item =~ s/\s+$//;
       }

The enclosed patch(bin/pt-config-diff not updated) should solve the problem, covered by this test
not ok 26 - end of line comment in option file
# Failed test 'end of line comment in option file'
# in t/lib/MySQLConfig.t at line 805.
# got: 'mysql ; comment'
# expected: 'mysql'
# Looks like you failed 1 test of 30.

Thanks,
kc

[Brian Fraser (fraserbn)]

Hey,

Thank you for the patch! It's an unexpected but welcome surprise to get one with a test case. I have just created a branch with it, and will be running tests shortly, but a bit of a heads up: It might be a bit too late to get this into the 2.1.2 release, so it'll probably have to wait until 2.1.3.

[Brian Fraser (fraserbn)]

Heh, we've both been corrected; turns out that MySQL only supports # as the start-of-comment character. I've updated the branch.

[Kaiwang CHEN (kaiwangchen)]

Yes, I neglected the manual does not define ";" as a comment starter in the middle of a line. However, it really could start a comment at the beginning of line with whitespace trimmed, as cited:
    Comment lines start with “#” or “;”. A “#” comment can start in the middle of a line as well.

I verified it by

[kc@dns1 msb_5_1_46]$ grep datadir my.sandbox.cnf;./my _print_defaults mysqld | grep datadir
;datadir = /home/kc/sandboxes/msb_5_1_46/data
[kc@dns1 msb_5_1_46]$ grep datadir my.sandbox.cnf;./my _print_defaults mysqld | grep datadir
#datadir = /home/kc/sandboxes/msb_5_1_46/data

[kc@dns1 msb_5_1_46]$ grep datadir my.sandbox.cnf;./my _print_defaults mysqld | grep datadir
datadir = ;/home/kc/sandboxes/msb_5_1_46/data
--datadir=;/home/kc/sandboxes/msb_5_1_46/data
[kc@dns1 msb_5_1_46]$ grep datadir my.sandbox.cnf;./my _print_defaults mysqld | grep datadir
datadir = #/home/kc/sandboxes/msb_5_1_46/data
--datadir=

Additionally I found my original patch is not against the right place. Enclosed is fixed version and with more tests.

[Kaiwang CHEN (kaiwangchen)]

Forgive me, my patch minutes ago is problematic, it does not deal with quotes and escapes... Good job, Brian. Fantastic regexp work.

I guess you will take semi-comma comment line into consideration, it is defined, as explained previously.

[Brian Fraser (fraserbn)]

Yep, right you are! I've went ahead an added skips for lines starting with ; (with optional whitespace in front). Thanks for the correction!

[Shahriyar Rzayev (rzayev-sehriyar)]

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PT-780