coin: Use configure and qt-configure-module in instructions

We want to use configure and qt-configure-module when building in Coin, rather than pure cmake and qt-cmake. There are a few benefits: - CI tests the scripts, making sure we don't introduce regressions - CI uses the same scripts that we mention in our documentation for developers to use - The platform configurations become a bit less verbose and easier to read due to less shouty-case CMake options To ensure a more gradual porting, Coin will only use the new instructions if the 'UseConfigure' feature is set on a platform configuration in qt5.git. This allows going back to the old instructions in case if something isn't working properly. Due the opt-in, we need to support both old and new instructions in the implementation. The change strives to remove as much duplicate code as possible, by moving it into common includes. The README.md is updated to mention the overview of how the different environment variables are used. There are a few important things to point out. 1) Because during the porting we have to allow mixing of the old style and new style, platform configs have to separate CMake-style options from configure-style options in different environment variables. Otherwise the instructions wouldn't be able to create a valid configure call, where all CMake-style options have to go at the end after a double dash --. After all platform configs are ported to the new style, it should be possible to combine all the options in a single environment variable if that is desired, but it will require another round of porting to remove all the '-D' prefixes in CMake-style options, and just use regular variable assignment which configure supports. e.g. -DQT_BUILD_EXAMPLES=ON becomes QT_BUILD_EXAMPLES=ON, which can be mixed in-between configure-style args. 2) Configure is more strict in that it doesn't allow passing unknown options. Due to that, we can't pass non-qtbase configure options via NON_QTBASE_CONFIGURE_ARGS. qt-configure-module would error out in the repos where the configure option is unknown. Because we don't have a Coin configure variable for each repo, we circumvent the issue by continuing to pass CMake-style options via NON_QTBASE_CMAKE_ARGS instead, which does not do validation checks. In the future, we could introduce a configure flag that disables the validation checks. Pick-to: 6.2 6.3 Task-number: QTQAINFRA-4357 Task-number: QTQAINFRA-4815 Change-Id: I72d8ba0b3a543b42982e22ae8d6566c0e885c446 Reviewed-by: Jörg Bornemann <[email protected]> Reviewed-by: Toni Saario <[email protected]>
Alegriabaile · Mar 17, 2022 · 4404380 · 4404380
1 parent b3ec373
commit 4404380
Show file tree

Hide file tree

Showing 19 changed files with 327 additions and 122 deletions.
diff --git a/coin/instructions/README.md b/coin/instructions/README.md
@@ -18,3 +18,61 @@
   the ability to build and run tests for ``qemu`` cross-compiling configurations.
 * ``coin_module_test_template_v3`` changed the run test instructions to not ignore the exit code
   and thus enforce that tests pass in the CI.
+
+# Environment variable description and usage
+
+The following environment variables are used in Coin instructions when building Qt, tests, etc:
+
+`CONFIGURE_ARGS`               - contains platform-specific ``configure-style`` arguments
+                                 (e.g. `-shared`), that will be passed to a qtbase configure call
+`CMAKE_ARGS`                   - contains platform-specific ``CMake-style`` arguments
+                                 (e.g. `-DOPENSSL_ROOT_DIR=Foo`) that will be passed to a qtbase
+                                 configure call
+`NON_QTBASE_CONFIGURE_ARGS`    - contains platform-specific ``configure-style`` arguments
+                                 that will be passed to a non-qtbase qt-configure-module call
+`NON_QTBASE_CMAKE_ARGS`        - contains platform-specific ``CMake-style`` arguments
+                                 that will be passed to a non-qtbase qt-configure-module call
+`COMMON_CMAKE_ARGS`            - platform-independent ``CMake-style`` args set in
+                                 `prepare_building_env.yaml` that apply to qtbase configurations
+                                  only.
+`COMMON_NON_QTBASE_CMAKE_ARGS` - platform-independent ``CMake-style`` args set in
+                                 `prepare_building_env.yaml` that apply to
+                                 configuration of repos other than qtbase
+`COMMON_TEST_CMAKE_ARGS`       - platform-independent ``CMake-style`` args set in
+                                 `prepare_building_env.yaml` that apply to configuration of
+                                 all standalone tests
+
+All of the above apply to host builds only.
+
+There is a a set of environment variables that apply to target builds when cross-compiling which
+mirror the ones above. They are:
+
+`TARGET_CONFIGURE_ARGS`
+`TARGET_CMAKE_ARGS`
+`NON_QTBASE_TARGET_CONFIGURE_ARGS`
+`NON_QTBASE_TARGET_CMAKE_ARGS`
+
+`COMMON_TARGET_CMAKE_ARGS`
+`COMMON_NON_QTBASE_TARGET_CMAKE_ARGS`
+`COMMON_TARGET_TEST_CMAKE_ARGS`
+
+Currently, there are no common ``configure-style`` variables for configuring
+repos or tests, only ``CMake-style` ones.
+
+
+`COIN_CMAKE_ARGS` contains the final set of cmake args that is passed to
+`configure` / `qt-configure-module`, it is built up from the variables above + any additional values added
+by custom instructions, like specification of `CMAKE_INSTALL_PREFIX` etc.
+
+`INSTALL_DIR_SUFFIX` is used to append either `/host` or `/target` suffixes to install paths in
+instructions when cross-building.
+
+`CONFIGURE_EXECUTABLE` contains a platform-specific path to `configure` / `qt-configure-module`
+or `cmake`/ `qt-cmake` depending on whether `UseConfigure` feature is enabled.
+
+`CONFIGURE_ENV_PREFIX` contains the value of either `ENV_PREFIX` or `TARGET_ENV_PREFIX` depending on
+whether it's a cross-build configure call. The values are used when configuring and building, to ensure
+that things like compilers are found correctly.
+
+We use `unixPathSeparators` to pass an install prefix with forward slashes even on Windows,
+to avoid escaping issues when using configure.
diff --git a/coin/instructions/call_cmake.yaml → coin/instructions/call_configure_module.yaml b/coin/instructions/call_cmake.yaml → coin/instructions/call_configure_module.yaml
@@ -1,24 +1,25 @@
 type: Group
 instructions:
+  - !include "{{qt/qtbase}}/prepare_configure_module_executable.yaml"
   - type: ExecuteCommand
-    command: "{{.Env.ENV_PREFIX}} {{.InstallDir}}\\bin\\qt-cmake-private {{.Env.COIN_CMAKE_ARGS}}"
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} {{.Env.CONFIGURE_EXECUTABLE}} {{.SourceDir}} {{.Env.COIN_CONFIGURE_ARGS}} -- {{.Env.COIN_CMAKE_ARGS}}"
     executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
     maxTimeInSeconds: 6000
     maxTimeBetweenOutput: 1200
     userMessageOnFailure: >
       Failed to call cmake.
     enable_if:
       condition: property
-      property: host.os
-      equals_value: Windows
+      property: features
+      contains_value: UseConfigure
   - type: ExecuteCommand
-    command: "{{.InstallDir}}/bin/qt-cmake-private {{.Env.COIN_CMAKE_ARGS}}"
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} {{.Env.CONFIGURE_EXECUTABLE}} {{.SourceDir}} {{.Env.COIN_CONFIGURE_ARGS}} {{.Env.COIN_CMAKE_ARGS}}"
     executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
     maxTimeInSeconds: 6000
     maxTimeBetweenOutput: 1200
     userMessageOnFailure: >
       Failed to call cmake.
     disable_if:
       condition: property
-      property: host.os
-      equals_value: Windows
+      property: features
+      contains_value: UseConfigure
diff --git a/coin/instructions/call_configure_qtbase.yaml b/coin/instructions/call_configure_qtbase.yaml
@@ -0,0 +1,27 @@
+type: Group
+instructions:
+  - !include "{{qt/qtbase}}/prepare_configure_executable.yaml"
+  - type: ExecuteCommand
+    # There is no SourceDir on purpose, because configure is called directly from qtbase,
+    # so the script knows it's own source dir.
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} {{.Env.CONFIGURE_EXECUTABLE}} {{.Env.COIN_CONFIGURE_ARGS}} -- {{.Env.COIN_CMAKE_ARGS}}"
+    executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
+    maxTimeInSeconds: 6000
+    maxTimeBetweenOutput: 1200
+    userMessageOnFailure: >
+      Failed to call cmake.
+    enable_if:
+      condition: property
+      property: features
+      contains_value: UseConfigure
+  - type: ExecuteCommand
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} {{.Env.CONFIGURE_EXECUTABLE}} {{.SourceDir}} {{.Env.COIN_CONFIGURE_ARGS}} {{.Env.COIN_CMAKE_ARGS}}"
+    executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
+    maxTimeInSeconds: 6000
+    maxTimeBetweenOutput: 1200
+    userMessageOnFailure: >
+      Failed to call cmake.
+    disable_if:
+      condition: property
+      property: features
+      contains_value: UseConfigure
diff --git a/coin/instructions/call_host_cmake.yaml b/coin/instructions/call_host_cmake.yaml
diff --git a/coin/instructions/call_target_cmake.yaml b/coin/instructions/call_target_cmake.yaml
diff --git a/coin/instructions/cmake_build_and_upload_test_artifacts_host.yaml b/coin/instructions/cmake_build_and_upload_test_artifacts_host.yaml
@@ -10,7 +10,7 @@ instructions:
         variableValue: "{{.InstallDir}}"
       - type: EnvironmentVariable
         variableName: COIN_CMAKE_ARGS
-        variableValue: "{{.Env.TEST_CONFIGURE_ARGS}}"
+        variableValue: "{{.Env.COMMON_TEST_CMAKE_ARGS}}"
       - !include "{{qt/qtbase}}/cmake_build_and_upload_test_artifacts.yaml"
     disable_if:
       condition: property

diff --git a/coin/instructions/cmake_build_and_upload_test_artifacts_target.yaml b/coin/instructions/cmake_build_and_upload_test_artifacts_target.yaml
@@ -21,7 +21,7 @@ instructions:
           equals_value: Windows
       - type: EnvironmentVariable
         variableName: COIN_CMAKE_ARGS
-        variableValue: "{{.Env.TARGET_TEST_CONFIGURE_ARGS}}"
+        variableValue: "{{.Env.COMMON_TARGET_TEST_CMAKE_ARGS}}"
       - !include "{{qt/qtbase}}/cmake_build_and_upload_test_artifacts.yaml"
     disable_if:
       condition: property

diff --git a/coin/instructions/cmake_cross_compilation_module_build_instructions.yaml b/coin/instructions/cmake_cross_compilation_module_build_instructions.yaml
@@ -30,13 +30,21 @@ instructions:
         directory: "{{.SourceDir}}/build/host"
       - type: ChangeDirectory
         directory: "{{.BuildDir}}"
+
+      - type: EnvironmentVariable
+        variableName: COIN_CONFIGURE_ARGS
+        variableValue: "{{.Env.NON_QTBASE_CONFIGURE_ARGS}}"
       - type: EnvironmentVariable
         variableName: COIN_CMAKE_ARGS
-        # The lack of space between the non qtbase configure args and the rest of the args is important!
-        variableValue: "{{.Env.NON_QTBASE_CONFIGURE_ARGS}} {{.SourceDir}}"
-      - !include "{{qt/qtbase}}/call_host_cmake.yaml"
+        variableValue: "{{.Env.NON_QTBASE_CMAKE_ARGS}} {{.Env.COMMON_NON_QTBASE_CMAKE_ARGS}}"
+      - type: EnvironmentVariable
+        variableName: CONFIGURE_ENV_PREFIX
+        variableValue: "{{.Env.ENV_PREFIX}}"
+      - !include "{{qt/qtbase}}/prepare_install_dir_suffix_host.yaml"
+      - !include "{{qt/qtbase}}/call_configure_module.yaml"
+
       - type: ExecuteCommand
-        command: "{{.Env.ENV_PREFIX}} cmake --build . --parallel -v"
+        command: "{{.Env.CONFIGURE_ENV_PREFIX}} cmake --build . --parallel -v"
         maxTimeInSeconds: "{{.Env.CMAKE_BUILD_TIMEOUT}}"
         maxTimeBetweenOutput: "{{.Env.CMAKE_BUILD_OUTPUT_TIMEOUT}}"
         userMessageOnFailure: >
@@ -60,13 +68,21 @@ instructions:
     directory: "{{.SourceDir}}/build/target"
   - type: ChangeDirectory
     directory: "{{.BuildDir}}"
+
+  - type: EnvironmentVariable
+    variableName: COIN_CONFIGURE_ARGS
+    variableValue: "{{.Env.NON_QTBASE_TARGET_CONFIGURE_ARGS}}"
   - type: EnvironmentVariable
     variableName: COIN_CMAKE_ARGS
-    # The lack of space between the non qtbase configure args and the rest of the args is important!
-    variableValue: "{{.Env.NON_QTBASE_TARGET_CONFIGURE_ARGS}} {{.SourceDir}}"
-  - !include "{{qt/qtbase}}/call_target_cmake.yaml"
+    variableValue: "{{.Env.NON_QTBASE_TARGET_CMAKE_ARGS}} {{.Env.COMMON_NON_QTBASE_TARGET_CMAKE_ARGS}}"
+  - type: EnvironmentVariable
+    variableName: CONFIGURE_ENV_PREFIX
+    variableValue: "{{.Env.TARGET_ENV_PREFIX}}"
+  - !include "{{qt/qtbase}}/prepare_install_dir_suffix_target.yaml"
+  - !include "{{qt/qtbase}}/call_configure_module.yaml"
+
   - type: ExecuteCommand
-    command: "{{.Env.TARGET_ENV_PREFIX}} cmake --build . --parallel -v"
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} cmake --build . --parallel -v"
     maxTimeInSeconds: "{{.Env.CMAKE_BUILD_TIMEOUT}}"
     maxTimeBetweenOutput: "{{.Env.CMAKE_BUILD_OUTPUT_TIMEOUT}}"
     userMessageOnFailure: >

diff --git a/coin/instructions/cmake_cross_compilation_qtbase_build_instructions.yaml b/coin/instructions/cmake_cross_compilation_qtbase_build_instructions.yaml
@@ -16,15 +16,21 @@ instructions:
         directory: "{{.SourceDir}}/build/host"
       - type: ChangeDirectory
         directory: "{{.BuildDir}}"
+
+      - type: EnvironmentVariable
+        variableName: COIN_CONFIGURE_ARGS
+        variableValue: "{{.Env.CONFIGURE_ARGS}}"
+      - type: EnvironmentVariable
+        variableName: COIN_CMAKE_ARGS
+        # Use unix separators even on Windows, to avoid escaping issues the in configure script.
+        variableValue: "{{.Env.CMAKE_ARGS}} {{.Env.COMMON_CMAKE_ARGS}} -DCMAKE_INSTALL_PREFIX:PATH={{unixPathSeparators .InstallDir}}/host"
+      - type: EnvironmentVariable
+        variableName: CONFIGURE_ENV_PREFIX
+        variableValue: "{{.Env.ENV_PREFIX}}"
+      - !include "{{qt/qtbase}}/call_configure_qtbase.yaml"
+
       - type: ExecuteCommand
-        command: "{{.Env.ENV_PREFIX}} cmake {{.Env.CONFIGURE_ARGS}} -DCMAKE_INSTALL_PREFIX:PATH={{.InstallDir}}/host {{.SourceDir}}"
-        executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
-        maxTimeInSeconds: 6000
-        maxTimeBetweenOutput: 1200
-        userMessageOnFailure: >
-          Failed to call cmake.
-      - type: ExecuteCommand
-        command: "{{.Env.ENV_PREFIX}} cmake --build . --parallel -v"
+        command: "{{.Env.CONFIGURE_ENV_PREFIX}} cmake --build . --parallel -v"
         maxTimeInSeconds: 6000
         maxTimeBetweenOutput: 4800
         userMessageOnFailure: >
@@ -48,29 +54,36 @@ instructions:
     directory: "{{.SourceDir}}/build/target"
   - type: ChangeDirectory
     directory: "{{.BuildDir}}"
+
   - type: AppendToEnvironmentVariable
-    variableName: TARGET_CONFIGURE_ARGS
+    variableName: TARGET_CMAKE_ARGS
     variableValue: " -DQT_HOST_PATH={{.AgentWorkingDir}}/install"
     disable_if:
       condition: property
       property: platformDependency
       equals_value: null
   - type: AppendToEnvironmentVariable
-    variableName: TARGET_CONFIGURE_ARGS
+    variableName: TARGET_CMAKE_ARGS
     variableValue: " -DQT_HOST_PATH={{.InstallDir}}/host"
     enable_if:
       condition: property
       property: platformDependency
       equals_value: null
+
+  - type: EnvironmentVariable
+    variableName: COIN_CONFIGURE_ARGS
+    variableValue: "{{.Env.TARGET_CONFIGURE_ARGS}}"
+  - type: EnvironmentVariable
+    variableName: COIN_CMAKE_ARGS
+    # Use unix separators even on Windows, to avoid escaping issues in the configure script.
+    variableValue: "{{.Env.TARGET_CMAKE_ARGS}} {{.Env.COMMON_TARGET_CMAKE_ARGS}} -DCMAKE_INSTALL_PREFIX:PATH={{unixPathSeparators .InstallDir}}/target"
+  - type: EnvironmentVariable
+    variableName: CONFIGURE_ENV_PREFIX
+    variableValue: "{{.Env.TARGET_ENV_PREFIX}}"
+  - !include "{{qt/qtbase}}/call_configure_qtbase.yaml"
+
   - type: ExecuteCommand
-    command: "{{.Env.TARGET_ENV_PREFIX}} cmake {{.Env.TARGET_CONFIGURE_ARGS}} -DCMAKE_STAGING_PREFIX:PATH={{.InstallDir}}/target {{.SourceDir}}"
-    executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
-    maxTimeInSeconds: 6000
-    maxTimeBetweenOutput: 1200
-    userMessageOnFailure: >
-      Failed to call cmake.
-  - type: ExecuteCommand
-    command: "{{.Env.TARGET_ENV_PREFIX}} cmake --build . --parallel -v"
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} cmake --build . --parallel -v"
     maxTimeInSeconds: 6000
     maxTimeBetweenOutput: 4800
     userMessageOnFailure: >

diff --git a/coin/instructions/cmake_module_build_instructions.yaml b/coin/instructions/cmake_module_build_instructions.yaml
@@ -6,11 +6,18 @@ instructions:
     directory: "{{.SourceDir}}"
   - type: ChangeDirectory
     directory: "{{.BuildDir}}"
+
+  - type: EnvironmentVariable
+    variableName: COIN_CONFIGURE_ARGS
+    variableValue: "{{.Env.NON_QTBASE_CONFIGURE_ARGS}}"
   - type: EnvironmentVariable
     variableName: COIN_CMAKE_ARGS
-    # The lack of space between the non qtbase configure args and the rest of the args is important!
-    variableValue: "{{.Env.NON_QTBASE_CONFIGURE_ARGS}} {{.SourceDir}}"
-  - !include "{{qt/qtbase}}/call_cmake.yaml"
+    variableValue: "{{.Env.NON_QTBASE_CMAKE_ARGS}} {{.Env.COMMON_NON_QTBASE_CMAKE_ARGS}}"
+  - type: EnvironmentVariable
+    variableName: CONFIGURE_ENV_PREFIX
+    variableValue: "{{.Env.ENV_PREFIX}}"
+  - !include "{{qt/qtbase}}/call_configure_module.yaml"
+
   - type: EnvironmentVariable
     variableName: CMAKE_BUILD_TIMEOUT
     variableValue: "6000"
@@ -26,7 +33,7 @@ instructions:
       env_var: CMAKE_BUILD_OUTPUT_TIMEOUT
       equals_value: null
   - type: ExecuteCommand
-    command: "{{.Env.ENV_PREFIX}} cmake --build . --parallel -v"
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} cmake --build . --parallel -v"
     maxTimeInSeconds: "{{.Env.CMAKE_BUILD_TIMEOUT}}"
     maxTimeBetweenOutput: "{{.Env.CMAKE_BUILD_OUTPUT_TIMEOUT}}"
     userMessageOnFailure: >

diff --git a/coin/instructions/cmake_qtbase_build_instructions.yaml b/coin/instructions/cmake_qtbase_build_instructions.yaml
@@ -6,15 +6,21 @@ instructions:
     directory: "{{.SourceDir}}"
   - type: ChangeDirectory
     directory: "{{.BuildDir}}"
+
+  - type: EnvironmentVariable
+    variableName: COIN_CONFIGURE_ARGS
+    variableValue: "{{.Env.CONFIGURE_ARGS}}"
+  - type: EnvironmentVariable
+    variableName: COIN_CMAKE_ARGS
+    # Use unix separators even on Windows, to avoid escaping issues in the configure script.
+    variableValue: "{{.Env.CMAKE_ARGS}} {{.Env.COMMON_CMAKE_ARGS}} -DCMAKE_INSTALL_PREFIX:PATH={{unixPathSeparators .InstallDir}}"
+  - type: EnvironmentVariable
+    variableName: CONFIGURE_ENV_PREFIX
+    variableValue: "{{.Env.ENV_PREFIX}}"
+  - !include "{{qt/qtbase}}/call_configure_qtbase.yaml"
+
   - type: ExecuteCommand
-    command: "{{.Env.ENV_PREFIX}} cmake {{.Env.CONFIGURE_ARGS}} -DCMAKE_INSTALL_PREFIX:PATH={{.InstallDir}} {{.SourceDir}}"
-    executeCommandArgumentSplitingBehavior: SplitAfterVariableSubstitution
-    maxTimeInSeconds: 6000
-    maxTimeBetweenOutput: 1200
-    userMessageOnFailure: >
-      Failed to call cmake.
-  - type: ExecuteCommand
-    command: "{{.Env.ENV_PREFIX}} cmake --build . --parallel -v"
+    command: "{{.Env.CONFIGURE_ENV_PREFIX}} cmake --build . --parallel -v"
     maxTimeInSeconds: 6000
     maxTimeBetweenOutput: 1200
     userMessageOnFailure: >

diff --git a/coin/instructions/coin_module_test_docs.yaml b/coin/instructions/coin_module_test_docs.yaml
@@ -14,10 +14,15 @@ instructions:
     command: "cp -rfs /opt/qt-doctools/. {{.InstallDir}}"
     userMessageOnFailure: >
       Failed to create links to provisioned binaries.
+
   - type: EnvironmentVariable
     variableName: COIN_CMAKE_ARGS
-    variableValue: "-DQT_BUILD_TESTS=OFF {{.SourceDir}}"
-  - !include "{{qt/qtbase}}/call_cmake.yaml"
+    variableValue: "-DQT_BUILD_TESTS=OFF"
+  - type: EnvironmentVariable
+    variableName: CONFIGURE_ENV_PREFIX
+    variableValue: "{{.Env.ENV_PREFIX}}"
+  - !include "{{qt/qtbase}}/call_configure_module.yaml"
+
   - type: ExecuteCommand
     command: "{{.Env.ENV_PREFIX}} cmake --build . --target generate_docs -v"
     ignoreExitCode: false