On Sun, Jul 19, 2015 at 1:19 PM, Mathieu Lirzin <[email protected]> wrote:
> David Thompson <[email protected]> writes:
>
>> Here's a package recipe for a Markdown library written in C.  I want to
>> try to make Guile bindings for it with the FFI.
>
> Nice idea!
>
>> +(define-module (gnu packages markdown)
>> +  #:use-module (guix licenses)
>> +  #:use-module (guix packages)
>> +  #:use-module (guix download)
>> +  #:use-module (guix packages)
>> +  #:use-module (guix build-system gnu)
>> +  #:use-module (gnu packages python)
>> +  #:use-module (gnu packages web))
>
> guix packages is added two times.

Good catch.  Thanks.

> Maybe "prefix:" for licences to be safe ?

We can prefix identifiers if/when the need arises.

>> +                                (substitute* '("Makefile")
>> +                                  (("/usr/local")
>> +                                   (assoc-ref outputs "out")))
>
> No need to use a list of files here. I noticed some warnings in the
> compilation process which can be fixed like this.
>
> --8<---------------cut here---------------start------------->8---
>   (substitute* "Makefile"
>     (("-ansi") "-std=c99")
>     (("/usr/local") (assoc-ref outputs "out")))
> --8<---------------cut here---------------end--------------->8---

'substitute*' operates on a single string *or* a list of strings.
Here's the relevant pattern matching code in the implementation:

    (match file
             ((files (... ...))
              (for-each substitute-one-file files))
             ((? string? f)
              (substitute-one-file f)))

I'm inclined to use a list, as I think that this macro should ought to
demand a list and only a list.
>> +    (native-inputs
>> +     `(("python" ,python-2)
>> +       ("tidy" ,tidy)))
>
> perl is needed in native-inputs for "test/MarkdownTest_1.0.3/MarkdownTest.pl"

There's a Python and a Perl version of the same test suite.  I just
use Python and don't bother with Perl.

> Otherwise LGTM! (if i'm entitled to ;))
>
> I will send a patch for moving the "markdown" package to this new file after
> that.

Cool, I'm doing to fix the double import and push this.

Thanks

- Dave

Reply via email to