From 7f92da7b307c784deec804f65137b10733e6a88e Mon Sep 17 00:00:00 2001
From: Michael Pyne <mpyne@kde.org>
Date: Sun, 1 Jul 2018 19:15:04 -0400
Subject: [PATCH] Don't double-set CMAKE_PREFIX_PATH if set by user.

This fixes bug 395627, where the error is actually that we set
CMAKE_PREFIX_PATH twice if qtdir is set to a non-system path and the
user is also setting CMAKE_PREFIX_PATH.

Unfortunately the second value overrides the first (the one the user
set).

Also added a test for this, which fails before the fix and passes
afterwards. The full test suite (all 5...) pass.

BUG:395627
FIXED-IN:18.08
---
 modules/ksb/BuildSystem/KDE4.pm  |  7 ++-
 t/bug-395627-keep-cmake-prefix.t | 78 ++++++++++++++++++++++++++++++++
 t/data/bug-395627/kdesrc-buildrc | 26 +++++++++++
 3 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 t/bug-395627-keep-cmake-prefix.t
 create mode 100644 t/data/bug-395627/kdesrc-buildrc

diff --git a/modules/ksb/BuildSystem/KDE4.pm b/modules/ksb/BuildSystem/KDE4.pm
index 6f54a0e..181dd2d 100644
--- a/modules/ksb/BuildSystem/KDE4.pm
+++ b/modules/ksb/BuildSystem/KDE4.pm
@@ -179,9 +179,12 @@ sub _safe_run_cmake
 
     push @commands, "-DCMAKE_INSTALL_PREFIX=$prefix";
 
-    # Add custom Qt to the prefix
+    # Add custom Qt to the prefix (but don't overwrite a user-set prefix)
     my $qtdir = $module->getOption('qtdir');
-    if ($qtdir && $qtdir ne $prefix) {
+    if ($qtdir && $qtdir ne $prefix &&
+        !grep { /^\s*-DCMAKE_PREFIX_PATH/ } (@commands)
+       )
+    {
         push @commands, "-DCMAKE_PREFIX_PATH=$qtdir";
     }
 
diff --git a/t/bug-395627-keep-cmake-prefix.t b/t/bug-395627-keep-cmake-prefix.t
new file mode 100644
index 0000000..4e3a1b3
--- /dev/null
+++ b/t/bug-395627-keep-cmake-prefix.t
@@ -0,0 +1,78 @@
+use 5.014;
+use strict;
+use warnings;
+
+# Verify that a user-set CMAKE_PREFIX_PATH is not removed, even if we supply
+# "magic" of our own
+# See bug 395627 -- https://bugs.kde.org/show_bug.cgi?id=395627
+
+my @savedCommand;
+my $log_called = 0;
+
+# Redefine log_command to capture whether it was properly called. This is all
+# very order-dependent, we need to load ksb::Util before kdesrc-build itself
+# does to install the new subroutine before it's copied over into the other
+# package symbol tables.
+BEGIN {
+    use ksb::Util;
+
+    no strict 'refs';
+    no warnings 'redefine';
+
+    *ksb::Util::log_command = sub {
+        $log_called = 1;
+
+        my ($module, $filename, $argRef, $optionsRef) = @_;
+        my @command = @{$argRef};
+        @savedCommand = @command;
+        return 0; # success
+    };
+}
+
+use Test::More;
+
+use ksb::Application;
+use ksb::Module;
+
+my @args = qw(--pretend --rc-file t/data/bug-395627/kdesrc-buildrc);
+
+{
+    my $app = ksb::Application->new(@args);
+    my @moduleList = @{$app->{modules}};
+
+    is (scalar @moduleList, 6, 'Right number of modules');
+    isa_ok ($moduleList[0]->buildSystem(), 'ksb::BuildSystem::KDE4');
+
+    my $result;
+    my @prefixes;
+    my $prefix;
+
+    # This requires log_command to be overridden above
+    $result = $moduleList[0]->setupBuildSystem();
+    is ($log_called, 1, 'Overridden log_command was called');
+    ok ($result, 'Setup build system for auto-set prefix path');
+
+    # We should expect an auto-set -DCMAKE_PREFIX_PATH passed to cmake somewhere
+    ($prefix) = grep { /-DCMAKE_PREFIX_PATH/ } @savedCommand;
+    is ($prefix, '-DCMAKE_PREFIX_PATH=/tmp/qt5', 'Prefix path set to custom Qt prefix');
+
+    $result = $moduleList[2]->setupBuildSystem();
+    ok ($result, 'Setup build system for manual-set prefix path');
+
+    (@prefixes) = grep { /-DCMAKE_PREFIX_PATH/ } @savedCommand;
+    is (scalar @prefixes, 1, 'Only one set prefix path in manual mode');
+    if (@prefixes) {
+        is ($prefixes[0], '-DCMAKE_PREFIX_PATH=FOO', 'Manual-set prefix path is as set by user');
+    }
+
+    $result = $moduleList[4]->setupBuildSystem();
+    ok ($result, 'Setup build system for manual-set prefix path');
+
+    (@prefixes) = grep { /-DCMAKE_PREFIX_PATH/ } @savedCommand;
+    is (scalar @prefixes, 1, 'Only one set prefix path in manual mode');
+    if (@prefixes) {
+        is ($prefixes[0], '-DCMAKE_PREFIX_PATH:PATH=BAR', 'Manual-set prefix path is as set by user');
+    }
+}
+
+done_testing();
diff --git a/t/data/bug-395627/kdesrc-buildrc b/t/data/bug-395627/kdesrc-buildrc
new file mode 100644
index 0000000..ad9891c
--- /dev/null
+++ b/t/data/bug-395627/kdesrc-buildrc
@@ -0,0 +1,26 @@
+global
+    source-dir /tmp
+    qtdir /tmp/qt5
+    git-repository-base fake git://localhost/git-set/
+    override-build-system KDE # Use CMake everywhere w/out source probing
+end global
+
+module-set test
+    repository fake
+    use-modules sample1 sample2
+    # Should have auto-set CMAKE_PREFIX_PATH
+end module-set
+
+module-set test2-set
+    repository fake
+    use-modules sample3 sample4
+    cmake-options -DCMAKE_PREFIX_PATH=FOO
+    # Should not auto-set CMAKE_PREFIX_PATH since it's already set
+end module-set
+
+module-set test3-set
+    repository fake
+    use-modules sample5 sample6
+    cmake-options -DCMAKE_PREFIX_PATH:PATH=BAR
+    # Uses a slightly different syntax, should still be retained
+end module-set