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

Done

Details

3 participants

Greg Hogan
Hartmut Goebel
Ludovic Courtès

Owner: unassigned

Submitted by: Hartmut Goebel

Severity: normal

Debbugs page

Hartmut Goebel wrote on 4 Mar 13:48 -0800

Recipients:(address . guix-patches@gnu.org)

Message-ID:b1d275cf773efa2cc9898f4439e4e5a7aac0af90.1709588880.git.h.goebel@crazy-compilers.com

* 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(-)

Toggle diff (50 lines)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

Hartmut Goebel wrote on 4 Mar 14:58 -0800

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

Recipients:(address . 69554@debbugs.gnu.org)

Message-ID:7676fd973fa640750306df216feb95c335b345de.1709593063.git.h.goebel@crazy-compilers.com

* 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(-)

Toggle diff (50 lines)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

Ludovic Courtès wrote on 15 Jul 02:40 -0700

Recipients:(name . Hartmut Goebel)(address . h.goebel@crazy-compilers.com)(address . 69554@debbugs.gnu.org)

Message-ID:87ed7v0z8m.fsf@gnu.org

Hi Hartmut,

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

Toggle quote (27 lines)> * 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’.

Hartmut Goebel wrote on 16 Jul 08:36 -0700

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 69554@debbugs.gnu.org)

Message-ID:079bf725-1855-498f-8863-d7ac7a6c6065@crazy-compilers.com

Am 15.07.24 um 11:40 schrieb Ludovic Courtès:

Toggle quote (5 lines)

> 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: file

Greg Hogan wrote on 7 Oct 08:45 -0700

Recipients:(name . Hartmut Goebel)(address . h.goebel@crazy-compilers.com)

Message-ID:CA+3U0ZmUVXkcDD9gDeBgQgaQMamFtZJSAni7AZ78_1aw6NbLRg@mail.gmail.com

On Tue, Jul 16, 2024 at 11:37 AM Hartmut Goebel

<h.goebel@crazy-compilers.com> wrote:

Toggle quote (24 lines)>
> 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

Hartmut Goebel wrote on 8 Oct 02:06 -0700

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:59738aa6-a912-4b21-b559-1a952d7d6bb9@crazy-compilers.com

Am 07.10.24 um 17:45 schrieb Greg Hogan:

Toggle quote (3 lines)

> 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: file

Greg Hogan wrote on 8 Oct 10:19 -0700

Recipients:(name . Hartmut Goebel)(address . h.goebel@crazy-compilers.com)

Message-ID:CA+3U0ZnBt3KiUPTA=_5wi2tis=TZR6-jFs0MdtF5pQ9z0cKUow@mail.gmail.com

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

Hartmut Goebel wrote on 9 Oct 00:32 -0700

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:7a1b7263-3526-43f9-910b-3148fb9b56ba@crazy-compilers.com

Am 08.10.24 um 19:19 schrieb Greg Hogan:

Toggle quote (7 lines)

> 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: file

Greg Hogan wrote on 9 Oct 08:04 -0700

Recipients:(name . Hartmut Goebel)(address . h.goebel@crazy-compilers.com)

Message-ID:CA+3U0Zkqbc1-a4vSCo1kwgM+F_CO8S0EVi-eowCwWLzb25u9TA@mail.gmail.com

On Wed, Oct 9, 2024 at 3:32 AM Hartmut Goebel

<h.goebel@crazy-compilers.com> wrote:

Toggle quote (14 lines)>
> 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.

Toggle snippet (6 lines)

CMake Warning:

Manually-specified variables were not used by the project:

CMAKE_INSTALL_LIBDIR

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

Hartmut Goebel wrote 3 days ago

Recipients:(name . Greg Hogan)(address . code@greghogan.com)

Message-ID:8ddae1f4-da16-42b2-9281-6f71c9b3fed9@crazy-compilers.com

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 |

Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 69554@patchwise.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 69554

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date