Tidying - #83
Merged
Merged
Conversation
* removed the duplicated **GitHub Actions** checklist from *Functional improvements*, it being already recorded under *Packaging improvements*; * corrected a doubled terminating semicolon, and code-formatted the `u` integer-literal suffix, in the `TEST_INT_EQ` bullet; * collapsed a stray double space in the **CMake** / **STLSoft** `find_package()` bullet;
* renamed the CI badge alt-text from the retired **cmake-multi-platform.yml** workflow title, "CMake on multiple platforms", to "CI", agreeing with the `name:` of **.github/workflows/ci.yml** and with the label used by **BDUT**, **CLASP**, **Pantheios**, **STLSoft**, **woad**, and the rest of the modernised libraries;
* added **.github/actions/install-sis-deps/action.yml**, a composite action
that clones, builds, and installs a whitespace-separated list of
synesissoftware dependencies - given in dependency order - into a single
CMake prefix, exporting it as the `SIS_DEPS` environment variable and as
the `prefix` output;
* replaced the duplicated STLSoft-plus-shwild installation block in
**.github/workflows/ci-cell.yml**, and the near-identical STLSoft block
in the `install-smoke` job of **.github/workflows/ci.yml**, with calls to
that action, removing 114 lines of copy-pasted shell;
* the action follows the **woad** rule that only the generator and the
build-type/compiler options may differ between the `cl`, `mingw`, and
default paths, so that no branch can silently ignore a `-D` option;
* replaced the hardcoded `${RUNNER_TEMP}/sis-deps` prefix literals in the
**ci-cell.yml** configure step with `${{ env.SIS_DEPS }}`, so that the
dependency prefix is named in exactly one place;
…ying
* added **.sis/project_name.txt**, holding the canonical short project name
`xTests`, alongside the existing **.sis/script_info_lines.txt**;
* added the standard `ProjectName=$(tr -d '[:space:]' < "$ProjectNameFile")`
load to **build_cmake.sh**, **clean_cmake.sh**, **ctest_cmake.sh**,
**prepare_cmake.sh**, **remove_cmake_artefacts.sh**,
**run_all_examples.sh**, **run_all_scratch_tests.sh**, and
**run_all_unit_tests.sh**, positioned as in **woad**;
* qualified the status messages of the CMake-driving scripts with the
project name, per **woad** - "Executing build of xTests", "Cleaning build
of xTests", "Executing CMake for xTests", "Removing all xTests cmake
artefacts", "Running xTests CMake tests";
* qualified the status messages of the `run_all_*` scripts with the project
name, per **sistools-common-c** - "Running all xTests example programs",
and likewise for scratch and automated test programs;
* added the equivalent project-name load, with a read-failure guard, and a
status line to **run_all_unit_tests.cmd**, retaining that script's
library-style `test.*unit.*` discovery rather than the utility-repo
`${ProjectName}.*test.*` form;
* added the `--build-shared-libs` flag to **prepare_cmake.sh**, per the
**woad** pilot, `BUILD_SHARED_LIBS` having been the one configure-time
option that obliged the user to abandon the helper scripts and invoke
CMake directly; the resulting `-DBUILD_SHARED_LIBS:BOOL` is passed on
both the MinGW and the default CMake paths, so that the flag cannot be
silently ignored by whichever branch is taken;
* documented the `(no arguments)` and `<target> ...` forms in
**build_cmake.sh**'s `--help`;
* corrected **run_all_examples.sh**'s `--no-make` help text, which
described running tests via CMake and make rather than running examples
via make;
* added `idiomatic` to the CI push-branch set, and a Created/Updated header
comment, in **.github/workflows/ci.yml**;
* added **generate_doxygen.sh**, completing the helper-script corpus for this project, with the same option surface as the **sistools-common-c** exemplar - `-q` / `--quiet`, `--help`, and `SIS_CMAKE_BUILD_DIR` - and writing to `<build-dir>/doxygen/html/`; * the script runs Doxygen **in the project directory**, Doxygen resolving relative `INPUT` and `EXAMPLE_PATH` entries against the current directory; the exemplar instead changes to the build directory before reading its Doxyfile, so that the read fails, Doxygen falls back to its defaults, and an empty document set is produced while success is reported; * the script derives `PROJECT_NUMBER` from `_XTESTS_VER_MAJOR`, `_XTESTS_VER_MINOR`, and `_XTESTS_VER_PATCH` in **include/xtests/xtests.h**, as **CMakeLists.txt** does, so that the documented version cannot fall out of step with the header; * the script verifies that `doxygen` is on the `PATH` and that the Doxyfile exists, and propagates Doxygen's exit status rather than losing it through the pipeline; * **Doxyfile**: set `MARKDOWN_ID_STYLE` to `GITHUB`, so that the GitHub-style table-of-contents anchors in **README.md** and **INSTALL.md** resolve, clearing thirteen unresolved-`\ref` warnings; * **Doxyfile**: set `GENERATE_LATEX` to `NO`, matching **BDUT** and **sistools-common-c**, and eliminating the `epstopdf` failure emitted on every run; * **Doxyfile**: removed the `DOT_MULTI_TARGETS` tag, obsolete as of Doxygen 1.18; * **examples/descriptors.cpp**: corrected the sixth `\example` path, which named **example.cpp.temp_file/main.cpp** twice where the second described, and should have named, **example.cpp.udt/main.cpp**; that example had therefore been undocumented; * **include/xtests/xtests.h**: documented `_XTESTS_VER_ALPHABETA` and `_XTESTS_VER_REVISION`, alongside the neighbouring version macros; * **include/xtests/xtests.h**: corrected two `failure..` typos, the doubled full stop being read by Doxygen as an end-of-list marker; * **include/xtests/xtests.h**: removed the `#` explicit-link prefixes from `<code>#len</code>` and `<code>#ptr</code>`, those being literal member names rather than documented entities; * the one remaining warning, the `\ref` to **LICENSE** from **README.md**, is accepted: an extension-less file cannot become a Doxygen page, and both `EXTENSION_MAPPING = no_extension=md` and adding **LICENSE** to `INPUT` were tried and rejected, the former re-breaking the Markdown anchors;
BuildType.cmake used `set(CACHE CMAKE_BUILD_TYPE …)`, which sets a normal variable literally named `CACHE` and leaves `CMAKE_BUILD_TYPE` unset, so the Debug/Release default was never applied; corrected to `set(CMAKE_BUILD_TYPE … CACHE STRING … FORCE)`. Renamed the local to `DEFAULT_BUILD_TYPE` and added the standard file banner. prepare_cmake.sh carried two divergent `cmake` invocations, and the MinGW one omitted `-DCMAKE_VERBOSE_MAKEFILE` and `-DMSVC_USE_MT`, so `--mingw -v` and `--mingw --msvc-mt` were silently ignored. Collapsed to a single invocation in which the generator is the only thing the MinGW path contributes, matching woad; also quoted the `-S`/`-B` paths.
The four artifact-download jobs re-hydrated the build tree via upload-artifact/download-artifact, which does not preserve the executable bit, so the helper scripts' `find … -exec test -x` discovery matched nothing and the test jobs passed without running anything. Folding examples, unit, component, and scratch runs into the build job removes the round-trip rather than patching around it. The added ctest step uses --no-tests=error, so an empty test set now fails the job instead of reporting success. * C1: removed the build-tree artifact round-trip; scratch tests keep tolerated-failure status via step-level continue-on-error; * C2: added a Test (CTest) step with --output-on-failure, --timeout 60, and --no-tests=error;
The installed xtests-config.cmake was only @PACKAGE_INIT@ plus the targets include, yet the exported xTests::core target links STLSoft::STLSoft. A downstream find_package(xTests) therefore failed with an unresolved imported target unless the consumer had already found STLSoft for itself. The find_dependency call is conditional, because the export references STLSoft::STLSoft only when xTests was built against STLSoft as a CMake package; a build using the STLSOFT directory variable has no package for a consumer to re-find. * added a conditional find_dependency(STLSoft) to cmake/xtests-config.cmake.in, guarded by the new STLSOFT_IS_PACKAGE_; * hoisted STLSoft_REQUIRED_VERSION_ so it is always defined; * dropped find_package(STLSoft) from the ci.yml install-smoke consumer, which now serves as the regression test;
CMakeLists.txt offers three ways to locate STLSoft, but every existing job took the same one: install-sis-deps installs the package and each Configure step passes only CMAKE_PREFIX_PATH. The two STLSOFT-directory routes short-circuit find_package() entirely, so nothing covered them - which is why K7's absolute-path leak into the installed export went unnoticed, and why the STLSOFT_IS_PACKAGE_ guard added under K1 was only ever exercised in its TRUE state. Each cell asserts, from the configure log, that the route it names was actually taken, so that a regression which silently reverted to find_package() cannot leave the cell green while testing nothing new. NOTE: the final assertion of this job fails on purpose, demonstrating K7. CI is red until K7 is fixed. * added the `stlsoft-routes` job to **ci.yml**, with `environment` and `variable` cells on `ubuntu-latest`; * each cell clones STLSoft as a source tree and configures with no `CMAKE_PREFIX_PATH`, so neither can fall back to the package; * each cell then builds, runs `ctest`, installs, and asserts that the installed package configuration does not require STLSoft; * added a deliberately failing assertion that the installed export does not name the build machine's STLSoft path (K7);
On the STLSOFT-directory routes the STLSoft include directory was added to core's PUBLIC includes with no BUILD_INTERFACE guard, so the builder's own absolute path was written into the installed export - as the stlsoft-routes cells added under C7 demonstrated, naming the runner's own /home/runner/work/_temp/stlsoft-src. The STREQUAL generator expression put that path into INTERFACE_LINK_LIBRARIES as well, even where it evaluated false, so it leaked on the package route too. Guarding the include directory alone would leave those installs relocatable but unusable, since xTests' public headers include STLSoft's and nothing would supply them. The package configuration therefore recovers STLSoft from the package when it can, and says so plainly when it cannot. * replaced the STREQUAL generator expression in src/CMakeLists.txt with a branch on STLSoft_IS_PACKAGE_: the package route links STLSoft::STLSoft, the directory route adds its include path under BUILD_INTERFACE; * added an else() branch to cmake/xtests-config.cmake.in which finds the STLSoft package and appends it to xTests::core, or warns that the consumer must supply STLSoft; * made the install-smoke job assert `if(TRUE)` in the installed configuration, since an undefined substitution variable expands to an empty - and silently false - `if()`;
Three packaging defects in the same install path, none of which a local
build would reveal.
K2 is latent rather than active: PACKAGE_PREFIX_DIR computes the same
../../../ either way, because both orderings are three levels deep. It is
fixed so that the two statements agree, not because it currently misbehaves.
The same transposition is present in 26 of the 28 C/C++ projects in the
corpus, so it warrants a corpus-wide pass separately.
K3 keeps the exported target spelled xTests::core, via EXPORT_NAME, so no
consumer of an installed xTests changes. An in-tree consumer that linked the
bare `core` through add_subdirectory must now say xTests::core;
cmake/TargetMacros.cmake was the only such place.
* K2: aligned INSTALL_DESTINATION with where install(EXPORT) and
install(FILES) actually write, ${CMAKE_INSTALL_LIBDIR}/cmake/${EXPORT_NAME};
* K3: renamed the target to xtests.core, added an xTests::core ALIAS and
EXPORT_NAME "core", and linked the alias from cmake/TargetMacros.cmake, so
nothing as generic as `core` enters a parent project;
* K4: replaced the literal include/ in both install(DIRECTORY) calls and in
the target's INSTALL_INTERFACE with ${CMAKE_INSTALL_INCLUDEDIR};
`mingw` was carried in the `c_compiler` matrix field, which is a toolchain rather than a compiler. That forced eleven sentinel comparisons against a compiler name across ci.yml, ci-cell.yml, and the install-sis-deps action, and it made the compiler options unspellable on that path: because `c-compiler` held `mingw`, the action's MinGW branch could not pass CMAKE_C_COMPILER at all, and so relied on the generator's default compiler search while every other branch was explicit. The install-smoke consumer had the same omission. A `toolchain` input now carries the generator and the PATH, `c_compiler`/`cpp_compiler` carry gcc and g++, and both branches pass the compiler options like the others. Neither item is verifiable on macOS, as neither the MinGW nor the MSVC path can be configured there. Both were checked structurally: the files parse, all `run:` blocks pass `bash -n` after expression substitution, and the caller's `with:` keys match the declared inputs of the reusable workflow and of the action, with nothing unknown and no required input unsupplied. * C4: added `permissions: contents: read` at workflow level to ci.yml and ci-cell.yml, so that a job added later inherits it; * C6: added a `toolchain` input to ci-cell.yml and to the install-sis-deps action, and moved all eleven `mingw` comparisons onto it; * C6: the action's and the consumer's MinGW branches now pass CMAKE_C_COMPILER and CMAKE_CXX_COMPILER, and the two MinGW configure calls gained the `-S .` they were doing without; * corrected the stale "KNOWN FAILURE until K7 is fixed" comment on the relocatability assertion, which now passes and reads as K7's regression test;
mwsis
approved these changes
Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.