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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69554 in the body.
You can then email your comments to 69554 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] build-system: cmake: Build tests depending on `#:tests?`.
Date: Mon,  4 Mar 2024 22:48:51 +0100

* 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

Message #8 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
To: 69554 <at> debbugs.gnu.org
Subject: [PATCH v2] build-system: cmake: Build tests depending on `#:tests?`.
Date: Mon,  4 Mar 2024 23:58:32 +0100

* 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

Message #11 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
Cc: 69554 <at> debbugs.gnu.org
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests
 depending on `#:tests?`.
Date: Mon, 15 Jul 2024 11:40:41 +0200

Hi Hartmut,

Hartmut Goebel <h.goebel <at> 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’.

Message #14 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 69554 <at> debbugs.gnu.org
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Tue, 16 Jul 2024 17:36:49 +0200

[Message part 1 (text/plain, inline)]

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 <at> crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

[Message part 2 (text/html, inline)]

Message #17 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Greg Hogan <code <at> greghogan.com>
To: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
Cc: 69554 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Mon, 7 Oct 2024 11:45:18 -0400

On Tue, Jul 16, 2024 at 11:37 AM Hartmut Goebel
<h.goebel <at> 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

Message #20 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
To: Greg Hogan <code <at> greghogan.com>
Cc: 69554 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Tue, 8 Oct 2024 11:06:34 +0200

[Message part 1 (text/plain, inline)]

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 <at> crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

[Message part 2 (text/html, inline)]

Message #23 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Greg Hogan <code <at> greghogan.com>
To: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
Cc: 69554 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Tue, 8 Oct 2024 13:19:57 -0400

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

Message #26 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
To: Greg Hogan <code <at> greghogan.com>
Cc: 69554 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Wed, 9 Oct 2024 09:32:17 +0200

[Message part 1 (text/plain, inline)]

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 <at> crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

[Message part 2 (text/html, inline)]

Message #29 received at 69554 <at> debbugs.gnu.org (full text, mbox):

From: Greg Hogan <code <at> greghogan.com>
To: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
Cc: 69554 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Wed, 9 Oct 2024 11:04:58 -0400

On Wed, Oct 9, 2024 at 3:32 AM Hartmut Goebel
<h.goebel <at> 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

Message #34 received at 69554-close <at> debbugs.gnu.org (full text, mbox):

From: Hartmut Goebel <h.goebel <at> crazy-compilers.com>
To: Greg Hogan <code <at> greghogan.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 69554-close <at> debbugs.gnu.org
Subject: Re: [bug#69554] [PATCH v2] build-system: cmake: Build tests depending
 on `#:tests?`.
Date: Tue, 15 Oct 2024 11:17:00 +0200

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 <at> crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.