guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Add googletest


From: Lukas Gradl
Subject: Re: [PATCH] Add googletest
Date: Wed, 01 Jun 2016 09:57:53 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Thank you for your review!

Leo Famulari <address@hidden> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]