From e9970474f5591e2c3c39531359ee4f3eac5a2721 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 12:22:02 +0100 Subject: [PATCH 01/10] [libcalamares] Allow Python to log an Error as well --- src/libcalamares/PythonJobApi.cpp | 15 +++++++++++++-- src/libcalamares/PythonJobApi.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/PythonJobApi.cpp b/src/libcalamares/PythonJobApi.cpp index 1713569a4..b1ff536be 100644 --- a/src/libcalamares/PythonJobApi.cpp +++ b/src/libcalamares/PythonJobApi.cpp @@ -139,17 +139,28 @@ check_target_env_output( const bp::list& args, const std::string& stdin, int tim } static const char output_prefix[] = "[PYTHON JOB]:"; +static inline void +log_action( unsigned int level, const std::string& s ) +{ + Logger::CDebug( level ) << output_prefix << QString::fromStdString( s ); +} void debug( const std::string& s ) { - Logger::CDebug( Logger::LOGDEBUG ) << output_prefix << QString::fromStdString( s ); + log_action( Logger::LOGDEBUG, s ); } void warning( const std::string& s ) { - Logger::CDebug( Logger::LOGWARNING ) << output_prefix << QString::fromStdString( s ); + log_action( Logger::LOGWARNING, s ); +} + +void +error( const std::string& s ) +{ + log_action( Logger::LOGERROR, s ); } PythonJobInterface::PythonJobInterface( Calamares::PythonJob* parent ) diff --git a/src/libcalamares/PythonJobApi.h b/src/libcalamares/PythonJobApi.h index 48bd4f87c..0ecc43abd 100644 --- a/src/libcalamares/PythonJobApi.h +++ b/src/libcalamares/PythonJobApi.h @@ -60,6 +60,7 @@ boost::python::list gettext_languages(); void debug( const std::string& s ); void warning( const std::string& s ); +void error( const std::string& s ); class PythonJobInterface { From 7f643010b259017bc7af90186b3f25d7f6fade93 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 12:26:07 +0100 Subject: [PATCH 02/10] [libcalamares] Expose error() and warn() to Python --- src/libcalamares/PythonJob.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index afeebbe07..6176f0312 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -85,6 +85,12 @@ BOOST_PYTHON_MODULE( libcalamares ) &CalamaresPython::warning, bp::args( "s" ), "Writes the given string to the Calamares warning stream." ); + bp::def( "warn", + &CalamaresPython::warning, + bp::args( "s" ), + "Writes the given string to the Calamares warning stream." ); + bp::def( + "error", &CalamaresPython::warning, bp::args( "s" ), "Writes the given string to the Calamares error stream." ); bp::def( "mount", &CalamaresPython::mount, From fcbe8d3a3e6dae3de9c08eaf292b5c9093aac395 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 12:46:12 +0100 Subject: [PATCH 03/10] [libcalamares] API for YAML-loading from Python --- src/libcalamares/PythonJob.cpp | 11 ++++++++++- src/libcalamares/PythonJobApi.cpp | 8 ++++++++ src/libcalamares/PythonJobApi.h | 5 +++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index 6176f0312..291adbc54 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -79,6 +79,7 @@ BOOST_PYTHON_MODULE( libcalamares ) bp::scope utilsScope = utilsModule; Q_UNUSED( utilsScope ) + // .. Logging functions bp::def( "debug", &CalamaresPython::debug, bp::args( "s" ), "Writes the given string to the Calamares debug stream." ); bp::def( "warning", @@ -92,6 +93,11 @@ BOOST_PYTHON_MODULE( libcalamares ) bp::def( "error", &CalamaresPython::warning, bp::args( "s" ), "Writes the given string to the Calamares error stream." ); + + // .. YAML functions + bp::def( "load_yaml", &CalamaresPython::load_yaml, bp::args( "path" ), "Loads YAML from a file." ); + + // .. Filesystem functions bp::def( "mount", &CalamaresPython::mount, mount_overloads( bp::args( "device_path", "mount_point", "filesystem_name", "options" ), @@ -100,6 +106,8 @@ BOOST_PYTHON_MODULE( libcalamares ) "-1 = QProcess crash\n" "-2 = QProcess cannot start\n" "-3 = bad arguments" ) ); + + // .. Process functions bp::def( "target_env_call", static_cast< int ( * )( const std::string&, const std::string&, int ) >( &CalamaresPython::target_env_call ), @@ -158,6 +166,7 @@ BOOST_PYTHON_MODULE( libcalamares ) host_env_process_output_overloads( bp::args( "command", "callback", "stdin", "timeout" ), "Runs the specified command in the host system." ) ); + // .. String functions bp::def( "obscure", &CalamaresPython::obscure, bp::args( "s" ), @@ -166,7 +175,7 @@ BOOST_PYTHON_MODULE( libcalamares ) "Applying the function to a string obscured by this function will result " "in the original string." ); - + // .. Translation functions bp::def( "gettext_languages", &CalamaresPython::gettext_languages, "Returns list of languages (most to least-specific) for gettext." ); diff --git a/src/libcalamares/PythonJobApi.cpp b/src/libcalamares/PythonJobApi.cpp index b1ff536be..869bda81e 100644 --- a/src/libcalamares/PythonJobApi.cpp +++ b/src/libcalamares/PythonJobApi.cpp @@ -19,6 +19,7 @@ #include "utils/RAII.h" #include "utils/Runner.h" #include "utils/String.h" +#include "utils/Yaml.h" #include #include @@ -163,6 +164,13 @@ error( const std::string& s ) log_action( Logger::LOGERROR, s ); } +boost::python::dict +load_yaml( const std::string& path ) +{ + return variantMapToPyDict( CalamaresUtils::loadYaml( QString::fromStdString( path ) ) ); +} + + PythonJobInterface::PythonJobInterface( Calamares::PythonJob* parent ) : m_parent( parent ) { diff --git a/src/libcalamares/PythonJobApi.h b/src/libcalamares/PythonJobApi.h index 0ecc43abd..62346ceda 100644 --- a/src/libcalamares/PythonJobApi.h +++ b/src/libcalamares/PythonJobApi.h @@ -62,6 +62,11 @@ void debug( const std::string& s ); void warning( const std::string& s ); void error( const std::string& s ); +/** @brief Loads YAML and returns (nested) dicts representing it + * + */ +boost::python::dict load_yaml( const std::string& path ); + class PythonJobInterface { public: From 3e0c9ba0569eef55af35437cae9712a9030f4006 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 13:04:44 +0100 Subject: [PATCH 04/10] [packages] Expand tests with PM-specific bits --- src/modules/packages/tests/1.global | 3 +++ .../packages/{test.yaml => tests/2.job} | 0 src/modules/packages/tests/CMakeTests.txt | 13 +++++++++++ src/modules/packages/tests/test-pm-pacman.py | 22 +++++++++++++++++++ 4 files changed, 38 insertions(+) create mode 100644 src/modules/packages/tests/1.global rename src/modules/packages/{test.yaml => tests/2.job} (100%) create mode 100644 src/modules/packages/tests/CMakeTests.txt create mode 100644 src/modules/packages/tests/test-pm-pacman.py diff --git a/src/modules/packages/tests/1.global b/src/modules/packages/tests/1.global new file mode 100644 index 000000000..ee06ccfe1 --- /dev/null +++ b/src/modules/packages/tests/1.global @@ -0,0 +1,3 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +rootMountPoint: /tmp diff --git a/src/modules/packages/test.yaml b/src/modules/packages/tests/2.job similarity index 100% rename from src/modules/packages/test.yaml rename to src/modules/packages/tests/2.job diff --git a/src/modules/packages/tests/CMakeTests.txt b/src/modules/packages/tests/CMakeTests.txt new file mode 100644 index 000000000..6e343533c --- /dev/null +++ b/src/modules/packages/tests/CMakeTests.txt @@ -0,0 +1,13 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +# +# We have tests to load (some) of the package-managers specifically, to +# test their configuration code and implementation. Those tests conventionally +# live in Python files here in the tests/ directory. Add them. +foreach(_pm pacman) + add_test( + NAME configure-packages-${_pm} + COMMAND env PYTHONPATH=.: python3 ${CMAKE_CURRENT_LIST_DIR}/test-pm-${_pm}.py + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} + ) +endforeach() diff --git a/src/modules/packages/tests/test-pm-pacman.py b/src/modules/packages/tests/test-pm-pacman.py new file mode 100644 index 000000000..aa0df2f7e --- /dev/null +++ b/src/modules/packages/tests/test-pm-pacman.py @@ -0,0 +1,22 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +# +# Calamares Boilerplate +import libcalamares +libcalamares.globalstorage = libcalamares.GlobalStorage(None) +libcalamares.globalstorage.insert("testing", True) + +# Module prep-work +from src.modules.packages import main + +# .. we don't have a job in this test, so fake one +class Job(object): + def __init__(self): + self.configuration = libcalamares.utils.load_yaml("pm-pacman.yaml") +libcalamares.job = Job() + +# Specific PM test +p = main.PMPacman() +assert p.pacman_num_retries == 0 +assert p.pacman_disable_timeout == False +assert p.pacman_needed_only == False From 1260d3fcb9e67c6d59707fc21d8ebb01fb4bc820 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 13:21:50 +0100 Subject: [PATCH 05/10] [packages] Expand tests for PM-specifics more --- src/modules/packages/tests/CMakeTests.txt | 25 +++++++++++++------ src/modules/packages/tests/pm-pacman-1.yaml | 11 +++++++++ src/modules/packages/tests/pm-pacman-2.yaml | 10 ++++++++ src/modules/packages/tests/test-pm-pacman.py | 26 +++++++++++++++----- 4 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 src/modules/packages/tests/pm-pacman-1.yaml create mode 100644 src/modules/packages/tests/pm-pacman-2.yaml diff --git a/src/modules/packages/tests/CMakeTests.txt b/src/modules/packages/tests/CMakeTests.txt index 6e343533c..9ee9c6682 100644 --- a/src/modules/packages/tests/CMakeTests.txt +++ b/src/modules/packages/tests/CMakeTests.txt @@ -4,10 +4,21 @@ # We have tests to load (some) of the package-managers specifically, to # test their configuration code and implementation. Those tests conventionally # live in Python files here in the tests/ directory. Add them. -foreach(_pm pacman) - add_test( - NAME configure-packages-${_pm} - COMMAND env PYTHONPATH=.: python3 ${CMAKE_CURRENT_LIST_DIR}/test-pm-${_pm}.py - WORKING_DIRECTORY ${CMAKE_BINARY_DIR} - ) -endforeach() + +# Pacman (Arch) tests +set(_pm pacman) +add_test( + NAME configure-packages-${_pm} + COMMAND env PYTHONPATH=.: python3 ${CMAKE_CURRENT_LIST_DIR}/test-pm-${_pm}.py + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} +) +add_test( + NAME configure-packages-${_pm}-ops-1 + COMMAND env PYTHONPATH=.: python3 ${CMAKE_CURRENT_LIST_DIR}/test-pm-${_pm}.py ${CMAKE_CURRENT_LIST_DIR}/pm-pacman-1.yaml 4 1 1 + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} +) +add_test( + NAME configure-packages-${_pm}-ops-2 + COMMAND env PYTHONPATH=.: python3 ${CMAKE_CURRENT_LIST_DIR}/test-pm-${_pm}.py ${CMAKE_CURRENT_LIST_DIR}/pm-pacman-2.yaml 3 0 0 + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} +) diff --git a/src/modules/packages/tests/pm-pacman-1.yaml b/src/modules/packages/tests/pm-pacman-1.yaml new file mode 100644 index 000000000..1ad048b61 --- /dev/null +++ b/src/modules/packages/tests/pm-pacman-1.yaml @@ -0,0 +1,11 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +backend: pacman +rootMountPoint: /tmp/mount +operations: [] + +pacman: + num_retries: 4 + disable_timeout: yes + needed_only: true + diff --git a/src/modules/packages/tests/pm-pacman-2.yaml b/src/modules/packages/tests/pm-pacman-2.yaml new file mode 100644 index 000000000..2bbf70758 --- /dev/null +++ b/src/modules/packages/tests/pm-pacman-2.yaml @@ -0,0 +1,10 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +backend: pacman +rootMountPoint: /tmp/mount +operations: [] + +# Leave some things unspecified +pacman: + num_retries: 3 + diff --git a/src/modules/packages/tests/test-pm-pacman.py b/src/modules/packages/tests/test-pm-pacman.py index aa0df2f7e..f57e2a761 100644 --- a/src/modules/packages/tests/test-pm-pacman.py +++ b/src/modules/packages/tests/test-pm-pacman.py @@ -11,12 +11,26 @@ from src.modules.packages import main # .. we don't have a job in this test, so fake one class Job(object): - def __init__(self): - self.configuration = libcalamares.utils.load_yaml("pm-pacman.yaml") -libcalamares.job = Job() + def __init__(self, filename): + self.configuration = libcalamares.utils.load_yaml(filename) if filename is not None else dict() + +import sys +if len(sys.argv) > 4: + filename = sys.argv[1] + retry = int(sys.argv[2]) + timeout = bool(int(sys.argv[3])) + needed = bool(int(sys.argv[4])) +else: + filename = None + retry = 0 + timeout = False + needed = False + +libcalamares.utils.warning("Expecting {!s} retry={!s} timeout={!s} needed={!s}".format(filename, retry, timeout, needed)) # Specific PM test +libcalamares.job = Job(filename) p = main.PMPacman() -assert p.pacman_num_retries == 0 -assert p.pacman_disable_timeout == False -assert p.pacman_needed_only == False +assert p.pacman_num_retries == retry, "{!r} vs {!r}".format(p.pacman_num_retries, retry) +assert p.pacman_disable_timeout == timeout +assert p.pacman_needed_only == needed From 65488ca174b3cddeb6cba1a6eaca19e871bacb50 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 14:19:06 +0100 Subject: [PATCH 06/10] [libcalamares] More verbose when loading YAML for Python --- src/libcalamares/PythonJobApi.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/PythonJobApi.cpp b/src/libcalamares/PythonJobApi.cpp index 869bda81e..bb2b8749e 100644 --- a/src/libcalamares/PythonJobApi.cpp +++ b/src/libcalamares/PythonJobApi.cpp @@ -167,7 +167,14 @@ error( const std::string& s ) boost::python::dict load_yaml( const std::string& path ) { - return variantMapToPyDict( CalamaresUtils::loadYaml( QString::fromStdString( path ) ) ); + const QString filePath = QString::fromStdString( path ); + bool ok = false; + auto map = CalamaresUtils::loadYaml( filePath, &ok ); + if ( !ok ) + { + cWarning() << "Loading YAML from" << filePath << "failed."; + } + return variantMapToPyDict( map ); } From 474aaf7603b2a3322735c6b96b71501be4cafb8e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 14:23:15 +0100 Subject: [PATCH 07/10] [packages] Fix loading of the subkeys for pacman --- src/modules/packages/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/packages/main.py b/src/modules/packages/main.py index 518ff4907..10371777e 100644 --- a/src/modules/packages/main.py +++ b/src/modules/packages/main.py @@ -403,9 +403,9 @@ class PMPacman(PackageManager): if type(pacman) is not dict: libcalamares.utils.warning("Job configuration *pacman* will be ignored.") pacman = dict() - self.pacman_num_retries = pacman.get("pacman_num_retries", 0) - self.pacman_disable_timeout = pacman.get("pacman_disable_download_timeout", False) - self.pacman_needed_only = pacman.get("pacman_needed_only", False) + self.pacman_num_retries = pacman.get("num_retries", 0) + self.pacman_disable_timeout = pacman.get("disable_download_timeout", False) + self.pacman_needed_only = pacman.get("needed_only", False) def reset_progress(self): self.in_package_changes = False From 61e0d538e9f0b83f66b11f3c63d55b8495eb8ac2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 14:44:12 +0100 Subject: [PATCH 08/10] [packages] Be more explicit in test failures, fix test data --- src/modules/packages/tests/pm-pacman-1.yaml | 2 +- src/modules/packages/tests/test-pm-pacman.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/packages/tests/pm-pacman-1.yaml b/src/modules/packages/tests/pm-pacman-1.yaml index 1ad048b61..3fa75d481 100644 --- a/src/modules/packages/tests/pm-pacman-1.yaml +++ b/src/modules/packages/tests/pm-pacman-1.yaml @@ -6,6 +6,6 @@ operations: [] pacman: num_retries: 4 - disable_timeout: yes + disable_download_timeout: yes needed_only: true diff --git a/src/modules/packages/tests/test-pm-pacman.py b/src/modules/packages/tests/test-pm-pacman.py index f57e2a761..ee814b620 100644 --- a/src/modules/packages/tests/test-pm-pacman.py +++ b/src/modules/packages/tests/test-pm-pacman.py @@ -32,5 +32,5 @@ libcalamares.utils.warning("Expecting {!s} retry={!s} timeout={!s} needed={!s}". libcalamares.job = Job(filename) p = main.PMPacman() assert p.pacman_num_retries == retry, "{!r} vs {!r}".format(p.pacman_num_retries, retry) -assert p.pacman_disable_timeout == timeout -assert p.pacman_needed_only == needed +assert p.pacman_disable_timeout == timeout, "{!r} vs {!r}".format(p.pacman_disable_timeout, timeout) +assert p.pacman_needed_only == needed, "{!r} vs {!r}".format(p.pacman_needed_only, needed) From 28bd7370620ecf1d8ad6869df3149af63f3fa486 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 14:44:49 +0100 Subject: [PATCH 09/10] [packages] Validate test-configs, too - The config-files has a typo, so didn't validate, so the loaded data was wrong, leading to test-failures. See 61e0d538e9f0b83f66b11f3c63d55b8495eb8ac2 . --- src/modules/packages/tests/CMakeTests.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/modules/packages/tests/CMakeTests.txt b/src/modules/packages/tests/CMakeTests.txt index 9ee9c6682..4f7d6185f 100644 --- a/src/modules/packages/tests/CMakeTests.txt +++ b/src/modules/packages/tests/CMakeTests.txt @@ -22,3 +22,21 @@ add_test( COMMAND env PYTHONPATH=.: python3 ${CMAKE_CURRENT_LIST_DIR}/test-pm-${_pm}.py ${CMAKE_CURRENT_LIST_DIR}/pm-pacman-2.yaml 3 0 0 WORKING_DIRECTORY ${CMAKE_BINARY_DIR} ) + +if ( BUILD_TESTING AND BUILD_SCHEMA_TESTING AND PYTHONINTERP_FOUND AND PYTHON_EXECUTABLE ) + set( _module packages ) + set( _schema_file "${CMAKE_CURRENT_SOURCE_DIR}/${_module}/${_module}.schema.yaml" ) + message(STATUS "Schema ${_schema_file}") + foreach( _cf pm-pacman-1.yaml pm-pacman-2.yaml ) + set( _conf_file "${CMAKE_CURRENT_SOURCE_DIR}/${_module}/tests/${_cf}" ) + if ( EXISTS "${_schema_file}" AND EXISTS "${_conf_file}" ) + add_test( + NAME validate-packages-${_cf} + COMMAND ${PYTHON_EXECUTABLE} "${CMAKE_SOURCE_DIR}/ci/configvalidator.py" "${_schema_file}" "${_conf_file}" + ) + else() + message(FATAL_ERROR "Missing ${_conf_file}") + endif() + endforeach() +endif() + From dfd13b4948596b11fd5c77827b3fb5b3a5be1072 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 29 Nov 2021 15:02:05 +0100 Subject: [PATCH 10/10] [packages] Remove bad config-lines - rootMountPoint is a global thing, not a job-configuration item --- src/modules/packages/tests/2.job | 1 - src/modules/packages/tests/pm-pacman-1.yaml | 1 - src/modules/packages/tests/pm-pacman-2.yaml | 1 - 3 files changed, 3 deletions(-) diff --git a/src/modules/packages/tests/2.job b/src/modules/packages/tests/2.job index 130214dfd..ba205ed44 100644 --- a/src/modules/packages/tests/2.job +++ b/src/modules/packages/tests/2.job @@ -1,7 +1,6 @@ # SPDX-FileCopyrightText: no # SPDX-License-Identifier: CC0-1.0 backend: dummy -rootMountPoint: /tmp/mount operations: - install: - pre-script: touch /tmp/foo diff --git a/src/modules/packages/tests/pm-pacman-1.yaml b/src/modules/packages/tests/pm-pacman-1.yaml index 3fa75d481..e876bd0fe 100644 --- a/src/modules/packages/tests/pm-pacman-1.yaml +++ b/src/modules/packages/tests/pm-pacman-1.yaml @@ -1,7 +1,6 @@ # SPDX-FileCopyrightText: no # SPDX-License-Identifier: CC0-1.0 backend: pacman -rootMountPoint: /tmp/mount operations: [] pacman: diff --git a/src/modules/packages/tests/pm-pacman-2.yaml b/src/modules/packages/tests/pm-pacman-2.yaml index 2bbf70758..8b0bda397 100644 --- a/src/modules/packages/tests/pm-pacman-2.yaml +++ b/src/modules/packages/tests/pm-pacman-2.yaml @@ -1,7 +1,6 @@ # SPDX-FileCopyrightText: no # SPDX-License-Identifier: CC0-1.0 backend: pacman -rootMountPoint: /tmp/mount operations: [] # Leave some things unspecified