[bug#69554] [PATCH] build-system: cmake: Build tests depending on `#:tests?`.

[bug#69554] [PATCH] build-system: cmake: Build tests depending on `#:tests?`.

[Hartmut Goebel]

* guix/build/cmake-build-system.scm (configure): New paremeter `#:tests?`.
  Add cmake option "-DBUILD_TESTING=" with value "ON" or "OFF" depending
  on build-system argument `#:tests?`.
* * doc/guix.texi (Inspecting Services)[cmake-build-system]: Document it.
---
 doc/guix.texi                     | 10 ++++++++++
 guix/build/cmake-build-system.scm |  7 ++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 87fe9f803c..409d076d12 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9617,6 +9617,16 @@ parameter specifies in abstract terms the flags passed to the compiler;
 it defaults to @code{"RelWithDebInfo"} (short for ``release mode with
 debugging information''), which roughly means that code is compiled with
 @code{-O2 -g}, as is the case for Autoconf-based packages by default.
+
+Depending on the @code{#:tests?} parameter, the configure-flag
+@code{BUILD_TESTING} is set to @code{ON} resp. @code{OFF}.
+@code{BUILD_TESTING} is a
+@url{https://cmake.org/cmake/help/v3.28/module/CTest.html, standard
+defined by CMake} to enable or disable building tests.  This aims to
+save build time if tests are not run anyway, while trying to ensure
+tests are build if they should be run.  Anyhow, the CMakeLists.txt needs
+to implement handling this flag.
+
 @end defvar
 
 @defvar composer-build-system
diff --git a/guix/build/cmake-build-system.scm b/guix/build/cmake-build-system.scm
index d1ff5071be..71e8ca8a83 100644
--- a/guix/build/cmake-build-system.scm
+++ b/guix/build/cmake-build-system.scm
@@ -33,7 +33,7 @@ (define-module (guix build cmake-build-system)
 ;; Code:
 
 (define* (configure #:key outputs (configure-flags '()) (out-of-source? #t)
-                    build-type target
+                    (tests? #t) build-type target
                     #:allow-other-keys)
   "Configure the given package."
   (let* ((out        (assoc-ref outputs "out"))
@@ -62,6 +62,11 @@ (define* (configure #:key outputs (configure-flags '()) (out-of-source? #t)
                   ,(string-append "-DCMAKE_INSTALL_RPATH=" out "/lib")
                   ;; enable verbose output from builds
                   "-DCMAKE_VERBOSE_MAKEFILE=ON"
+                  ;; ask for (not) building tests depending on #:tests?
+                  ;; (CMakeLists.txt may or may not implement this check)
+                  ,@(if tests?
+                        '("-DBUILD_TESTING=OFF") ; not run anyway
+                        '("-DBUILD_TESTING=ON")) ; overwrite any default option
 
                   ;;  Cross-build
                   ,@(if target

base-commit: 3da49b1472919a62df1fe399638f23a246aa325d
-- 
2.41.0

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Hartmut Goebel]

* guix/build/cmake-build-system.scm (configure): New paremeter `#:tests?`.
  Add cmake option "-DBUILD_TESTING=" with value "ON" or "OFF" depending
  on build-system argument `#:tests?`.
* * doc/guix.texi (Inspecting Services)[cmake-build-system]: Document it.
---
 doc/guix.texi                     | 10 ++++++++++
 guix/build/cmake-build-system.scm |  7 ++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 87fe9f803c..409d076d12 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9617,6 +9617,16 @@ parameter specifies in abstract terms the flags passed to the compiler;
 it defaults to @code{"RelWithDebInfo"} (short for ``release mode with
 debugging information''), which roughly means that code is compiled with
 @code{-O2 -g}, as is the case for Autoconf-based packages by default.
+
+Depending on the @code{#:tests?} parameter, the configure-flag
+@code{BUILD_TESTING} is set to @code{ON} resp. @code{OFF}.
+@code{BUILD_TESTING} is a
+@url{https://cmake.org/cmake/help/v3.28/module/CTest.html, standard
+defined by CMake} to enable or disable building tests.  This aims to
+save build time if tests are not run anyway, while trying to ensure
+tests are build if they should be run.  Anyhow, the CMakeLists.txt needs
+to implement handling this flag.
+
 @end defvar
 
 @defvar composer-build-system
diff --git a/guix/build/cmake-build-system.scm b/guix/build/cmake-build-system.scm
index d1ff5071be..3f5449c438 100644
--- a/guix/build/cmake-build-system.scm
+++ b/guix/build/cmake-build-system.scm
@@ -33,7 +33,7 @@ (define-module (guix build cmake-build-system)
 ;; Code:
 
 (define* (configure #:key outputs (configure-flags '()) (out-of-source? #t)
-                    build-type target
+                    (tests? #t) build-type target
                     #:allow-other-keys)
   "Configure the given package."
   (let* ((out        (assoc-ref outputs "out"))
@@ -62,6 +62,11 @@ (define* (configure #:key outputs (configure-flags '()) (out-of-source? #t)
                   ,(string-append "-DCMAKE_INSTALL_RPATH=" out "/lib")
                   ;; enable verbose output from builds
                   "-DCMAKE_VERBOSE_MAKEFILE=ON"
+                  ;; ask for (not) building tests depending on #:tests?
+                  ;; (CMakeLists.txt may or may not implement this check)
+                  ,@(if tests?
+                        '("-DBUILD_TESTING=ON") ; overwrite any default option
+                        '("-DBUILD_TESTING=OFF")) ; not run anyway
 
                   ;;  Cross-build
                   ,@(if target

base-commit: 3da49b1472919a62df1fe399638f23a246aa325d
-- 
2.41.0

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Ludovic Courtès]

Hi Hartmut,

Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> * guix/build/cmake-build-system.scm (configure): New paremeter `#:tests?`.
>   Add cmake option "-DBUILD_TESTING=" with value "ON" or "OFF" depending
>   on build-system argument `#:tests?`.
> * * doc/guix.texi (Inspecting Services)[cmake-build-system]: Document it.
> ---
>  doc/guix.texi                     | 10 ++++++++++
>  guix/build/cmake-build-system.scm |  7 ++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 87fe9f803c..409d076d12 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -9617,6 +9617,16 @@ parameter specifies in abstract terms the flags passed to the compiler;
>  it defaults to @code{"RelWithDebInfo"} (short for ``release mode with
>  debugging information''), which roughly means that code is compiled with
>  @code{-O2 -g}, as is the case for Autoconf-based packages by default.
> +
> +Depending on the @code{#:tests?} parameter, the configure-flag
> +@code{BUILD_TESTING} is set to @code{ON} resp. @code{OFF}.
> +@code{BUILD_TESTING} is a
> +@url{https://cmake.org/cmake/help/v3.28/module/CTest.html, standard
> +defined by CMake} to enable or disable building tests.  This aims to
> +save build time if tests are not run anyway, while trying to ensure
> +tests are build if they should be run.  Anyhow, the CMakeLists.txt needs
> +to implement handling this flag.

My understanding is that ‘BUILD_TESTING’ is not standard, as the last
sentence above suggests.  Thus I’m reluctant to passing this flag
unconditionally, as I guess it would fail for ‘CMakeLists.txt’ that do
not implement it, right?

Thanks,
Ludo’.

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Hartmut Goebel]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

Am 15.07.24 um 11:40 schrieb Ludovic Courtès:
> My understanding is that ‘BUILD_TESTING’ is not standard, as the last
> sentence above suggests.  Thus I’m reluctant to passing this flag
> unconditionally, as I guess it would fail for ‘CMakeLists.txt’ that do
> not implement it, right?

I just tested it, and all you get is a warning:

$ cat CMakeLists.txt
cmake_minimum_required(VERSION 3.0)
project(doo)
$ cmake -DBUILD_TESTING=OFF .
…
-- Generating done (0.0s)
CMake Warning:
  Manually-specified variables were not used by the project:

    BUILD_TESTING


-- Build files have been written to: /tmp/xxx


-- 

Regards
Hartmut Goebel

| Hartmut Goebel          |h.goebel@crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

[-- Attachment #2: Type: text/html, Size: 2262 bytes --]

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Greg Hogan]

On Tue, Jul 16, 2024 at 11:37 AM Hartmut Goebel
<h.goebel@crazy-compilers.com> wrote:
>
> Am 15.07.24 um 11:40 schrieb Ludovic Courtès:
>
> My understanding is that ‘BUILD_TESTING’ is not standard, as the last
> sentence above suggests.  Thus I’m reluctant to passing this flag
> unconditionally, as I guess it would fail for ‘CMakeLists.txt’ that do
> not implement it, right?
>
> I just tested it, and all you get is a warning:
>
> $ cat CMakeLists.txt
> cmake_minimum_required(VERSION 3.0)
> project(doo)
> $ cmake -DBUILD_TESTING=OFF .
> …
> -- Generating done (0.0s)
> CMake Warning:
>  Manually-specified variables were not used by the project:
>
>    BUILD_TESTING
>
>
> -- Build files have been written to: /tmp/xxx

Hi Hartmut,

Could we detect with `cmake -L` [1] that the package has a
BUILD_TESTING option and only then pass this flag?

[1] https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-L-A-H

Greg

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Hartmut Goebel]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

Am 07.10.24 um 17:45 schrieb Greg Hogan:
> Could we detect with `cmake -L` [1] that the package has a
> BUILD_TESTING option and only then pass this flag?

Running `cmake -L` would run the configure phase. Thus we would need to 
configure twice: first to learn whether BUILD_TESTING is defined and 
then a second time to change it. We might be able to optimize this: if 
BUILD_TESTING is already set to the value we want, we can skip the 
second run. While we can hope that the second run will be cheap, if 
might not if things change depending on the values.

IMHO this is too much effort just to suppress a warning.

Anyhow, there is a much simpler solution, according to 
<https://stackoverflow.com/questions/36451368/> we could just add a line 
to the CMakeList.txt file.

$ cat CMakeLists.txt
cmake_minimum_required(VERSION 3.0)
project(lalala)
set(__guix_suppress_BUILD_TESTING_warning__ "${BUILD_TESTING}")
$ cmake -DBUILD_TESTING=OFF .
…
-- Generating done (0.0s)
-- Build files have been written to: /tmp/xxx

Voila, no warning.

WDYT?

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          |h.goebel@crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

[-- Attachment #2: Type: text/html, Size: 2277 bytes --]

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Greg Hogan]

I think we can do this without warnings in the output logs or
modifications to the project CMakeLists.txt. We can preload
BUILD_TESTING into the cache:

$ cat cache.txt
set(BUILD_TESTING OFF CACHE BOOL "Build the testing tree.")
$ cmake --build build -C cache.txt .

It's not a "manually-specified variable" so no warning and can be
overwritten by a configure flag ("-DBUILD_TESTING=...") or from
CMakeLists.txt (as patched by the rosegarden package).

I have added a commit titled "build-system/cmake: Optionally build
tests." to my branch at
https://github.com/greghogan/guix/commits/master-cmake/ as part of a
v2 for #70031.

After resolving this feature we should look to create a branch on the
build service as with #73135.

[1] https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-C
[2] https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry
[3] https://cmake.org/cmake/help/book/mastering-cmake/chapter/CMake%20Cache.html

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Hartmut Goebel]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

Am 08.10.24 um 19:19 schrieb Greg Hogan:
> I think we can do this without warnings in the output logs or
> modifications to the project CMakeLists.txt. We can preload
> BUILD_TESTING into the cache:
>
> $ cat cache.txt
> set(BUILD_TESTING OFF CACHE BOOL "Build the testing tree.")

This is a neat trick!

Did you check whether this also overwrites any default in CMakeList.txt? 
Otherwise we might need to pass `-DBUILD_TESTING=…` on the command line, 
too.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          |h.goebel@crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

[-- Attachment #2: Type: text/html, Size: 1382 bytes --]

[bug#69554] [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Greg Hogan]

On Wed, Oct 9, 2024 at 3:32 AM Hartmut Goebel
<h.goebel@crazy-compilers.com> wrote:
>
> Am 08.10.24 um 19:19 schrieb Greg Hogan:
>
> I think we can do this without warnings in the output logs or
> modifications to the project CMakeLists.txt. We can preload
> BUILD_TESTING into the cache:
>
> $ cat cache.txt
> set(BUILD_TESTING OFF CACHE BOOL "Build the testing tree.")
>
> This is a neat trick!
>
> Did you check whether this also overwrites any default in CMakeList.txt? Otherwise we might need to pass `-DBUILD_TESTING=…` on the command line, too.

As I understand it, setting a variable in a "-C" cache file and with a
"-D" command-line argument is equivalent. The only difference is the
requisite precedence, and since we are not specifying "FORCE" for the
"-C" cache file entries any "-D" configure-flags from a Guix package
take precedence.

I also noticed the following in my simple test project, so I am
looking to also move these "-D" arguments into the "-C" cache file in
cmake-build's configure.

--8<---------------cut here---------------start------------->8---
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_INSTALL_LIBDIR
--8<---------------cut here---------------end--------------->8---

This persistent cache is the lowest-level scope [1] so is overwritten
by a "set" command in any CMakeLists.txt. An "option" (for example
BUILD_TESTING in the CTest module) does not affect a normal or cache
variable [2].

[1] https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#cmake-language-variables
[2] https://cmake.org/cmake/help/latest/command/option.html

bug#69554: [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.

[Hartmut Goebel]

Hi Greg,

I did some testing with different combinations and with defining the 
option in CMakeList.txt. Everything works as I would expect. So please 
go for it.

I'm closing this patch in favor of your #70031.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |