diff mbox series

[bug#72106,v2,2/5] gnu: klee: Enable the test suite.

Message ID b65bd1448a381db7821c51380d549643a0942601.1720981528.git.soeren@soeren-tempel.net
State New
Headers show
Series [bug#72106,v2,1/5] gnu: klee: Wrap klee-stats for Python dependencies. | expand

Commit Message

Sören Tempel July 14, 2024, 6:25 p.m. UTC
From: Sören Tempel <soeren@soeren-tempel.net>

* gnu/packages/check.scm (klee): Enable all tests.
[arguments]: Add phase to patch lit configuration, set #:test-target.
<#:configure-flags?>: Enable system and unit tests, configure gtest.
[inputs]: Add googletest and python-lit.
---
 gnu/packages/check.scm | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

Comments

Liliana Marie Prikler July 14, 2024, 6:53 p.m. UTC | #1
Am Sonntag, dem 14.07.2024 um 20:25 +0200 schrieb
soeren@soeren-tempel.net:
> From: Sören Tempel <soeren@soeren-tempel.net>
> 
> * gnu/packages/check.scm (klee): Enable all tests.
> [arguments]: Add phase to patch lit configuration, set #:test-target.
> <#:configure-flags?>: Enable system and unit tests, configure gtest.
> [inputs]: Add googletest and python-lit.
> ---
>  gnu/packages/check.scm | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
> index 6b0ea0aaa8..552cb39de5 100644
> --- a/gnu/packages/check.scm
> +++ b/gnu/packages/check.scm
> @@ -1063,13 +1063,23 @@ (define-public klee
>        (base32
> "1nma6dqi8chjb97llsa8mzyskgsg4dx56lm8j514j5wmr8vkafz6"))))
>     (arguments
>      (list
> +     #:test-target "check"
>       #:phases
>       #~(modify-phases %standard-phases
> -                      (add-after 'unpack 'patch
> +                      (add-after 'unpack 'patch-cmake
>                          (lambda _
>                            (substitute* "CMakeLists.txt"
>                             
> (("\\$\\{KLEE_UCLIBC_PATH\\}/lib/libc\\.a")
>                               "${KLEE_UCLIBC_PATH}"))))
> +                      (add-after 'unpack 'patch-lit-config
> +                        (lambda _
> +                          ;; Make sure that we retain the value of
> the GUIX_PYTHONPATH
> +                          ;; environment variable in the test
> environmented created by
> +                          ;; python-lit. Otherwise, the test scripts
> won't be able to
> +                          ;; find the python-tabulate dependency,
> causing test failures.
> +                          (substitute* "test/lit.cfg"
> +                            (("addEnv\\('PWD'\\)" env)
> +                             (string-append env "\n"
> "addEnv('GUIX_PYTHONPATH')")))))
If this is a test, then we should use native-inputs below.
>                        (add-after 'install 'wrap-klee-stats
>                          (lambda* (#:key outputs #:allow-other-keys)
>                            (let* ((out (assoc-ref outputs "out"))
> @@ -1088,7 +1098,13 @@ (define-public klee
>                                `("KLEE_RUNTIME_LIBRARY_PATH" =
>                                  (,(string-append lib
> "/klee/runtime/"))))))))
>       #:configure-flags
> -     #~(list (string-append "-DLLVMCC="
> +     #~(list "-DENABLE_UNIT_TESTS=ON"
> +             "-DENABLE_SYSTEM_TESTS=ON"
> +             (string-append "-DGTEST_SRC_DIR="
> +                            (assoc-ref %build-inputs "googletest"))
> +             (string-append "-DGTEST_INCLUDE_DIR="
> +                            (assoc-ref %build-inputs "googletest")
> "/googletest/include")
> +             (string-append "-DLLVMCC="
>                              (search-input-file %build-inputs
> "/bin/clang"))
>               (string-append "-DLLVMCXX="
>                              (search-input-file %build-inputs
> "/bin/clang++"))
> @@ -1096,7 +1112,15 @@ (define-public klee
>                              (search-input-file %build-inputs
> "/lib/klee/libc.a"))
>               "-DENABLE_POSIX_RUNTIME=ON")))
>     (native-inputs (list clang-13 llvm-13 python-lit))
> -   (inputs (list bash-minimal klee-uclibc gperftools sqlite z3
> python python-tabulate))
> +   (inputs
> +     `(("bash-minimal" ,bash-minimal)
> +       ("klee-uclibc" ,klee-uclibc)
> +       ("gperftools" ,gperftools)
> +       ("sqlite" ,sqlite)
> +       ("z3" ,z3)
> +       ("python", python)
> +       ("python-tabulate" ,python-tabulate)
> +       ("googletest" ,(package-source googletest))))
Why the package source and not the compiled package?  Can we make it so
that we can use a prebuilt compiled one?

Cheers
Sören Tempel July 14, 2024, 8:04 p.m. UTC | #2
Hello Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> wrote:
> Why the package source and not the compiled package?  Can we make it so
> that we can use a prebuilt compiled one?

Unfortunately, this does not seem to be possible. You can only point the
KLEE build system to a googletest source [1]. This seems to be related
to some peculiarity of googletest as other Guix package do the same
thing [2] [3] [4] (there are more grep for "package-source googletest").

> If this is a test, then we should use native-inputs below.

What exact dependency are you referring to? python-lit is already
declared as a dependency through native-inputs.

> Should be klee-uclibc.

Good catch! I can send a revision fixing the commit message.

Is there anything else I should adjust in a v3 revision?

Cheers,
Sören

[1]: https://github.com/klee/klee/blob/master/README-CMake.md?plain=1#L60-L62
[2]: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/crypto.scm?id=eb508e32d2d359c94d2cabebfe90dc32ca5dcf4f#n336
[3]: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/benchmark.scm?id=eb508e32d2d359c94d2cabebfe90dc32ca5dcf4f#n241
[4]: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/chemistry.scm?id=eb508e32d2d359c94d2cabebfe90dc32ca5dcf4f#n407
Ludovic Courtès July 20, 2024, 9:56 a.m. UTC | #3
Hello Sören,

soeren@soeren-tempel.net skribis:

>                                  (,(string-append lib "/klee/runtime/"))))))))
>       #:configure-flags
> -     #~(list (string-append "-DLLVMCC="
> +     #~(list "-DENABLE_UNIT_TESTS=ON"
> +             "-DENABLE_SYSTEM_TESTS=ON"
> +             (string-append "-DGTEST_SRC_DIR="
> +                            (assoc-ref %build-inputs "googletest"))
> +             (string-append "-DGTEST_INCLUDE_DIR="
> +                            (assoc-ref %build-inputs "googletest") "/googletest/include")

I would replace (assoc-ref …) by:

  #+(package-source googletest)

> -   (inputs (list bash-minimal klee-uclibc gperftools sqlite z3 python python-tabulate))
> +   (inputs
> +     `(("bash-minimal" ,bash-minimal)
> +       ("klee-uclibc" ,klee-uclibc)
> +       ("gperftools" ,gperftools)
> +       ("sqlite" ,sqlite)
> +       ("z3" ,z3)
> +       ("python", python)
> +       ("python-tabulate" ,python-tabulate)
> +       ("googletest" ,(package-source googletest))))

… and leave ‘googletest’ out of ‘inputs’ entirely, keeping the concise
input list without labels.  (I think we shouldn’t reintroduce input
labels; the goal has always been to remove them.)

WDYT?

Besides, to answer Liliana, Googletest is often used as a “source
library” like you’re doing here; there are quite a few other packages
that do that in Guix already, as you explained.  That’s OK.

Thanks,
Ludo’.
Sören Tempel July 25, 2024, 8:43 p.m. UTC | #4
Ludovic Courtès <ludo@gnu.org> wrote:
> Hello Sören,

Hi Ludo,

> … and leave ‘googletest’ out of ‘inputs’ entirely, keeping the concise
> input list without labels.  (I think we shouldn’t reintroduce input
> labels; the goal has always been to remove them.)
> 
> WDYT?

Thanks for this suggestion, this sounds very good to me!

I just send a v3 which implements this and also fixes the commit message
typo that Liliana pointed out. Let me know if there is anything else
that needs to be done :)

Best,
Sören
diff mbox series

Patch

diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 6b0ea0aaa8..552cb39de5 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -1063,13 +1063,23 @@  (define-public klee
       (base32 "1nma6dqi8chjb97llsa8mzyskgsg4dx56lm8j514j5wmr8vkafz6"))))
    (arguments
     (list
+     #:test-target "check"
      #:phases
      #~(modify-phases %standard-phases
-                      (add-after 'unpack 'patch
+                      (add-after 'unpack 'patch-cmake
                         (lambda _
                           (substitute* "CMakeLists.txt"
                             (("\\$\\{KLEE_UCLIBC_PATH\\}/lib/libc\\.a")
                              "${KLEE_UCLIBC_PATH}"))))
+                      (add-after 'unpack 'patch-lit-config
+                        (lambda _
+                          ;; Make sure that we retain the value of the GUIX_PYTHONPATH
+                          ;; environment variable in the test environmented created by
+                          ;; python-lit. Otherwise, the test scripts won't be able to
+                          ;; find the python-tabulate dependency, causing test failures.
+                          (substitute* "test/lit.cfg"
+                            (("addEnv\\('PWD'\\)" env)
+                             (string-append env "\n" "addEnv('GUIX_PYTHONPATH')")))))
                       (add-after 'install 'wrap-klee-stats
                         (lambda* (#:key outputs #:allow-other-keys)
                           (let* ((out (assoc-ref outputs "out"))
@@ -1088,7 +1098,13 @@  (define-public klee
                               `("KLEE_RUNTIME_LIBRARY_PATH" =
                                 (,(string-append lib "/klee/runtime/"))))))))
      #:configure-flags
-     #~(list (string-append "-DLLVMCC="
+     #~(list "-DENABLE_UNIT_TESTS=ON"
+             "-DENABLE_SYSTEM_TESTS=ON"
+             (string-append "-DGTEST_SRC_DIR="
+                            (assoc-ref %build-inputs "googletest"))
+             (string-append "-DGTEST_INCLUDE_DIR="
+                            (assoc-ref %build-inputs "googletest") "/googletest/include")
+             (string-append "-DLLVMCC="
                             (search-input-file %build-inputs "/bin/clang"))
              (string-append "-DLLVMCXX="
                             (search-input-file %build-inputs "/bin/clang++"))
@@ -1096,7 +1112,15 @@  (define-public klee
                             (search-input-file %build-inputs "/lib/klee/libc.a"))
              "-DENABLE_POSIX_RUNTIME=ON")))
    (native-inputs (list clang-13 llvm-13 python-lit))
-   (inputs (list bash-minimal klee-uclibc gperftools sqlite z3 python python-tabulate))
+   (inputs
+     `(("bash-minimal" ,bash-minimal)
+       ("klee-uclibc" ,klee-uclibc)
+       ("gperftools" ,gperftools)
+       ("sqlite" ,sqlite)
+       ("z3" ,z3)
+       ("python", python)
+       ("python-tabulate" ,python-tabulate)
+       ("googletest" ,(package-source googletest))))
    (build-system cmake-build-system)
    (home-page "https://klee-se.org/")
    (synopsis "Symbolic execution engine")