diff mbox

Add googletest

Message ID 87lh2q2v1u.fsf@openmailbox.org
State New
Headers show

Commit Message

Lukas Gradl May 31, 2016, 1:53 p.m. UTC
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

Comments

Leo Famulari May 31, 2016, 9:44 p.m. UTC | #1
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. UTC | #2
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. UTC | #3
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. UTC | #4
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. UTC | #5
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.
diff mbox

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