Patchwork Add googletest

login
register
mail settings
Submitter Lukas Gradl
Date May 31, 2016, 1:53 p.m.
Message ID <87lh2q2v1u.fsf@openmailbox.org>
Download mbox | patch
Permalink /patch/12645/
State New
Headers show

Comments

Lukas Gradl - May 31, 2016, 1:53 p.m.
Hello,

Sorry, I accidently attached the wrong patch.  Please disregard the
patch in my previous email, attached is an updated one.

Sorry about that!
Best,
Lukas
Leo Famulari - May 31, 2016, 9:44 p.m.
On Tue, May 31, 2016 at 08:53:17AM -0500, Lukas Gradl wrote:
> * gnu/packages/check.scm (googletest): New variable.

Awesome, thanks for this patch!

> +    (build-system cmake-build-system)

I noticed in the README.md that upstream suggests use of GNU Make unless
building from a Git checkout. Did you try that?

> +    (native-inputs
> +     `(("python-2" ,python-2)))

The README also suggests that this is only necessary for building from
Git, although it's worth finding out what they mean by "re-generating
certain source files from templates". We prefer to re-build "generated"
source files since they are not really "source files" in many cases.

> +                  (replace 'install
> +                    (lambda _
> +                      (let ((out (assoc-ref %outputs "out")))
> +                        (and
> +                         (mkdir-p (string-append out "/lib"))
> +                         (mkdir-p (string-append out "/include"))
> +                         (zero?
> +                          (system* "cp" "-r"
> +                                   "../googletest-release-1.7.0/include"
> +                                   out))
> +                         (zero? (system* "cp" "libgtest.a" "libgtest_main.a"
> +                                         (string-append out "/lib"))))))))))

I think these uses of (system*) could be replaced by (copy-recursively)
and (install-file), respectively.
Efraim Flashner - June 1, 2016, 4:48 a.m.
On Tue, May 31, 2016 at 05:44:00PM -0400, Leo Famulari wrote:
> On Tue, May 31, 2016 at 08:53:17AM -0500, Lukas Gradl wrote:
> > * gnu/packages/check.scm (googletest): New variable.
> 
> Awesome, thanks for this patch!
> 
> > +    (build-system cmake-build-system)
> 
> I noticed in the README.md that upstream suggests use of GNU Make unless
> building from a Git checkout. Did you try that?
> 
> > +    (native-inputs
> > +     `(("python-2" ,python-2)))
> 
> The README also suggests that this is only necessary for building from
> Git, although it's worth finding out what they mean by "re-generating
> certain source files from templates". We prefer to re-build "generated"
> source files since they are not really "source files" in many cases.
> 
> > +                  (replace 'install
> > +                    (lambda _
> > +                      (let ((out (assoc-ref %outputs "out")))
> > +                        (and
> > +                         (mkdir-p (string-append out "/lib"))
> > +                         (mkdir-p (string-append out "/include"))
> > +                         (zero?
> > +                          (system* "cp" "-r"
> > +                                   "../googletest-release-1.7.0/include"
> > +                                   out))
> > +                         (zero? (system* "cp" "libgtest.a" "libgtest_main.a"
> > +                                         (string-append out "/lib"))))))))))
> 
> I think these uses of (system*) could be replaced by (copy-recursively)
> and (install-file), respectively.
> 

I haven't tried building this package yet

(and -> you can probably switch this for a (begin

googletest-release-1.7.0 ->
(string-append "../google-release-" version "/include")

  (zero? (system* "cp" "libgtest.a" "libgtest_main.a"
won't this just copy libgtest.a to libgtest_main.a ?
Ludovic Courtès - June 1, 2016, 7:47 a.m.
Leo Famulari <leo@famulari.name> skribis:

> On Tue, May 31, 2016 at 08:53:17AM -0500, Lukas Gradl wrote:

[...]

>> +                  (replace 'install
>> +                    (lambda _
>> +                      (let ((out (assoc-ref %outputs "out")))
>> +                        (and
>> +                         (mkdir-p (string-append out "/lib"))
>> +                         (mkdir-p (string-append out "/include"))
>> +                         (zero?
>> +                          (system* "cp" "-r"
>> +                                   "../googletest-release-1.7.0/include"
>> +                                   out))
>> +                         (zero? (system* "cp" "libgtest.a" "libgtest_main.a"
>> +                                         (string-append out "/lib"))))))))))
>
> I think these uses of (system*) could be replaced by (copy-recursively)
> and (install-file), respectively.

s/could/should/ even  ;-)

Ludo’.
Lukas Gradl - June 1, 2016, 2:57 p.m.
Thank you for your review!

Leo Famulari <leo@famulari.name> writes:

> On Tue, May 31, 2016 at 08:53:17AM -0500, Lukas Gradl wrote:
>> * gnu/packages/check.scm (googletest): New variable.
>
> Awesome, thanks for this patch!
>
>> +    (build-system cmake-build-system)
>
> I noticed in the README.md that upstream suggests use of GNU Make unless
> building from a Git checkout. Did you try that?

I have not tried that.  Their README.md left me under the impression
that tests are not supported useing GNU Make, but I just looked at their
Makefile and there are test targets, so I can try using the
gnu-build-system, if that is preferred?

>
>> +    (native-inputs
>> +     `(("python-2" ,python-2)))
>
> The README also suggests that this is only necessary for building from
> Git, although it's worth finding out what they mean by "re-generating
> certain source files from templates". We prefer to re-build "generated"
> source files since they are not really "source files" in many cases.
>

It seems they generate c++ header files using pump.py.  I will look into
that more.  Python is also needed for tests.

>> +                  (replace 'install
>> +                    (lambda _
>> +                      (let ((out (assoc-ref %outputs "out")))
>> +                        (and
>> +                         (mkdir-p (string-append out "/lib"))
>> +                         (mkdir-p (string-append out "/include"))
>> +                         (zero?
>> +                          (system* "cp" "-r"
>> +                                   "../googletest-release-1.7.0/include"
>> +                                   out))
>> +                         (zero? (system* "cp" "libgtest.a" "libgtest_main.a"
>> +                                         (string-append out "/lib"))))))))))
>
> I think these uses of (system*) could be replaced by (copy-recursively)
> and (install-file), respectively.

These indeed sound very useful.  I will send an updated patch shortly.


Thank you!

Best,
Lukas
Leo Famulari - June 2, 2016, 1:49 a.m.
On Wed, Jun 01, 2016 at 09:57:53AM -0500, Lukas Gradl wrote:
> Leo Famulari <leo@famulari.name> writes:
> > I noticed in the README.md that upstream suggests use of GNU Make unless
> > building from a Git checkout. Did you try that?
> 
> I have not tried that.  Their README.md left me under the impression
> that tests are not supported useing GNU Make, but I just looked at their
> Makefile and there are test targets, so I can try using the
> gnu-build-system, if that is preferred?

All else being equal, my personal preference is to use GNU tools. But
otherwise, we should use the build system that upstream supports most
fully. Especially, we should use the one that runs the tests.

> It seems they generate c++ header files using pump.py.  I will look into
> that more.  Python is also needed for tests.

Thanks for looking into that. If the header files are generated by other
code, then we prefer to build them ourselves rather than use the
"pre-compiled" headers.

Patch

From f5f229eafcf2ebf24703e6becc291b8378dff2e0 Mon Sep 17 00:00:00 2001
From: Lukas Gradl <lgradl@openmailbox.org>
Date: Tue, 31 May 2016 08:48:29 -0500
Subject: [PATCH] gnu: check: Add googletest.

* gnu/packages/check.scm (googletest): New variable.
---
 gnu/packages/check.scm | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 9eef7a9..fca2e98 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -24,6 +24,7 @@ 
 (define-module (gnu packages check)
   #:use-module (gnu packages)
   #:use-module (gnu packages autotools)
+  #:use-module (gnu packages python)
   #:use-module (guix licenses)
   #:use-module (guix packages)
   #:use-module (guix download)
@@ -193,3 +194,44 @@  in the code.  Cppcheck primarily detects the types of bugs that the compilers
 normally do not detect.  The goal is to detect only real errors in the code
 (i.e. have zero false positives).")
     (license gpl3+)))
+
+
+(define-public googletest
+  (package
+    (name "googletest")
+    (version "1.7.0")
+    (source
+     (origin
+       (method url-fetch)
+       (uri
+        (string-append
+         "https://github.com/google/googletest/archive/release-"
+         version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "1k0nf1l9cb3prdmsvaajl5i31bx86c1mw0d5jgzykz7rzm36afpp"))))
+    (build-system cmake-build-system)
+    (native-inputs
+     `(("python-2" ,python-2)))
+    (arguments
+     `(#:configure-flags '("-Dgtest_build_tests=ON")
+       #:phases (modify-phases %standard-phases
+                  (replace 'install
+                    (lambda _
+                      (let ((out (assoc-ref %outputs "out")))
+                        (and
+                         (mkdir-p (string-append out "/lib"))
+                         (mkdir-p (string-append out "/include"))
+                         (zero?
+                          (system* "cp" "-r"
+                                   "../googletest-release-1.7.0/include"
+                                   out))
+                         (zero? (system* "cp" "libgtest.a" "libgtest_main.a"
+                                         (string-append out "/lib"))))))))))
+    (home-page "https://github.com/google/googletest/")
+    (synopsis "Test discovery and XUnit test framework")
+    (description "Google Test (GTest) features an XUnit test framework,
+automated test discovery, death tests, assertions, parameterized tests and XML
+test report generation.")
+    (license bsd-3)))
-- 
2.7.4