Commit b01505be authored by Michael Pyne's avatar Michael Pyne

Fix cxxflags being set to a space when globally empty.

When cxxflags is globally set to an empty value, the getOption magic
that appends module values to global values for cxxflags causes it the
result to equal ' ' (i.e. one space). This is because the space is a
separator between two empty values.

This causes code testing against cxxflags to think it's actually been
set to a value and to add it in the cmake calls.

I fix this and add a test case, but also add some insurance by trimming
leading/trailing white space so that the existing check for empty
cxxflags would have had a chance to catch this.

Differential Revision: https://phabricator.kde.org/D18165
parent 845e9da4
...@@ -810,11 +810,9 @@ sub _splitOptionAndValue ...@@ -810,11 +810,9 @@ sub _splitOptionAndValue
# So, skip spaces and pick up the rest of the line. # So, skip spaces and pick up the rest of the line.
(?:\s+(.*))?$/x); (?:\s+(.*))?$/x);
$value //= ''; $value = trimmed($value // '');
# Simplify whitespace. # Simplify whitespace.
$value =~ s/\s+$//;
$value =~ s/^\s+//;
$value =~ s/\s+/ /g; $value =~ s/\s+/ /g;
# Check for false keyword and convert it to Perl false. # Check for false keyword and convert it to Perl false.
...@@ -1048,9 +1046,9 @@ sub _readConfigurationOptions ...@@ -1048,9 +1046,9 @@ sub _readConfigurationOptions
# Read in global settings # Read in global settings
while ($_ = $fileReader->readLine()) while ($_ = $fileReader->readLine())
{ {
s/#.*$//; # Remove comments s/#.*$//; # Remove comments
s/^\s*//; # Remove leading whitespace s/^\s+//; # Remove leading whitespace
next if (/^\s*$/); # Skip blank lines next unless $_; # Skip blank lines
# First command in .kdesrc-buildrc should be a global # First command in .kdesrc-buildrc should be a global
# options declaration, even if none are defined. # options declaration, even if none are defined.
......
...@@ -844,18 +844,20 @@ sub getOption ...@@ -844,18 +844,20 @@ sub getOption
# If module-only, check that first. # If module-only, check that first.
return $self->{options}{$key} if $levelLimit eq 'module'; return $self->{options}{$key} if $levelLimit eq 'module';
my $ctxValue = $ctx->getOption($key); # we'll use this a lot from here
# Some global options always override module options. # Some global options always override module options.
return $ctx->getOption($key) if $ctx->hasStickyOption($key); return $ctxValue if $ctx->hasStickyOption($key);
# Some options append to the global (e.g. conf flags) # Some options append to the global (e.g. conf flags)
my @confFlags = qw(cmake-options configure-flags cxxflags); my @confFlags = qw(cmake-options configure-flags cxxflags);
if (list_has(\@confFlags, $key) && $ctx->hasOption($key)) { if (list_has(\@confFlags, $key) && $ctxValue) {
return $ctx->getOption($key) . " " . ($self->{options}{$key} || ''); return trimmed("$ctxValue " . ($self->{options}{$key} || ''));
} }
# Everything else overrides the global option, unless it's simply not # Everything else overrides the global option, unless it's simply not
# set at all. # set at all.
return $self->{options}{$key} // $ctx->getOption($key); return $self->{options}{$key} // $ctxValue;
} }
# Gets persistent options set for this module. First parameter is the name # Gets persistent options set for this module. First parameter is the name
......
...@@ -21,6 +21,7 @@ use Exporter qw(import); # Use Exporter's import method ...@@ -21,6 +21,7 @@ use Exporter qw(import); # Use Exporter's import method
our @EXPORT = qw(list_has assert_isa assert_in any unique_items our @EXPORT = qw(list_has assert_isa assert_in any unique_items
absPathToExecutable absPathToExecutable
fileDigestMD5 log_command disable_locale_message_translation fileDigestMD5 log_command disable_locale_message_translation
trimmed
split_quoted_on_whitespace safe_unlink safe_system p_chdir split_quoted_on_whitespace safe_unlink safe_system p_chdir
pretend_open safe_rmtree get_list_digest is_dir_empty pretend_open safe_rmtree get_list_digest is_dir_empty
super_mkdir filter_program_output prettify_seconds); super_mkdir filter_program_output prettify_seconds);
...@@ -504,6 +505,15 @@ EOF ...@@ -504,6 +505,15 @@ EOF
} }
} }
# removes leading/trailing whitespace
sub trimmed
{
my $str = shift;
$str =~ s/^\s+//;
$str =~ s/\s+$//;
return $str;
}
# This subroutine acts like split(' ', $_) except that double-quoted strings # This subroutine acts like split(' ', $_) except that double-quoted strings
# are not split in the process. # are not split in the process.
# #
...@@ -513,11 +523,7 @@ EOF ...@@ -513,11 +523,7 @@ EOF
sub split_quoted_on_whitespace sub split_quoted_on_whitespace
{ {
use Text::ParseWords qw(parse_line); use Text::ParseWords qw(parse_line);
my $line = shift; my $line = trimmed(shift);
# Remove leading/trailing whitespace
$line =~ s/^\s+//;
$line =~ s/\s+$//;
# 0 means not to keep delimiters or quotes # 0 means not to keep delimiters or quotes
return parse_line('\s+', 0, $line); return parse_line('\s+', 0, $line);
......
...@@ -4,6 +4,8 @@ global ...@@ -4,6 +4,8 @@ global
source-dir /tmp source-dir /tmp
make-options -j4 make-options -j4
git-repository-base fake git://localhost/git-set/ git-repository-base fake git://localhost/git-set/
cmake-options "-DCMAKE_BUILD_TYPE=a b" bar=c baz
cxxflags # empty
end global end global
module-set set1 module-set set1
......
...@@ -7,6 +7,7 @@ use warnings; ...@@ -7,6 +7,7 @@ use warnings;
use Test::More; use Test::More;
use ksb::Application; use ksb::Application;
use ksb::Util qw(trimmed);
my $app = ksb::Application->new(qw(--pretend --rc-file t/data/sample-rc/kdesrc-buildrc)); my $app = ksb::Application->new(qw(--pretend --rc-file t/data/sample-rc/kdesrc-buildrc));
my @moduleList = @{$app->{modules}}; my @moduleList = @{$app->{modules}};
...@@ -28,4 +29,7 @@ is($moduleList[1]->name(), 'setmod2', 'Right module name from module-set'); ...@@ -28,4 +29,7 @@ is($moduleList[1]->name(), 'setmod2', 'Right module name from module-set');
is($branch, 'refs/tags/tag-setmod2', 'Right tag name (options block)'); is($branch, 'refs/tags/tag-setmod2', 'Right tag name (options block)');
is($type, 'tag', 'options block came back as tag'); is($type, 'tag', 'options block came back as tag');
# See https://phabricator.kde.org/D18165
is($moduleList[1]->getOption('cxxflags'), '', 'empty cxxflags renders with no whitespace in module');
done_testing(); done_testing();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment