Re: [PATCH 2/2] gnu: Add python-lzo.

[ng0]

On 17-02-05 01:31:18, Maxim Cournoyer wrote:
> Hi ng0,
> 
> contact@cryptolab.net writes:
> 
> > From: ng0 
> >
> > * gnu/packages/compression.scm (python-lzo): New variable.
> >
> > Co-authored-by: Danny Milosavljevic 
> > ---
> >  gnu/packages/compression.scm | 36 
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
> > index 64518fb6b..2545c4517 100644
> > --- a/gnu/packages/compression.scm
> > +++ b/gnu/packages/compression.scm
> > @@ -365,6 +365,42 @@ LZO is written in ANSI C.  Both the source code and 
> > the compressed data
> >  format are designed to be portable across platforms.")
> >  (license license:gpl2+)))
> >  
> > +(define-public python-lzo
> > +  (package
> > +(name "python-lzo")
> > +(version "1.11")
> > +(source
> > + (origin
> > +   (method url-fetch)
> > +   (uri (pypi-uri "python-lzo" version))
> > +   (sha256
> > +(base32
> > + "11p3ifg14p086byhhin6azx5svlkg8dzw2b5abixik97xd6fm81q"
> > +(build-system python-build-system)
> > +(arguments
> > + `(#:test-target "check"
> > +   #:phases
> > +   (modify-phases %standard-phases
> > + (add-after 'unpack 'patch-setuppy
> > +   (lambda _
> > + (substitute* "setup.py"
> > +   (("include_dirs.append.*")
> > +(string-append "include_dirs.append(\""
> > +   (assoc-ref %build-inputs "lzo") 
> > "/include/lzo" "\")
> > +"
> 
> A bit hard to read/long line here. Maybe the various string components
> being appended could be aligned below the string-append call? Also, you
> could use single quotes for the Python code which might help to
> differentiate what is what.

It is already applied. I think "it's hard to read" is more than just a
cosmetic fix, so if you have an idea how to make this more readable,
feel free to send a patch.

> Is the guix linter happy?
> 
> > +(inputs
> > + `(("lzo" ,lzo)))
> > +(home-page "https://github.com/jd-boyd/python-lzo;)
> > +(synopsis "Python bindings for the LZO data compression library")
> > +(description
> > + "Python-LZO provides Python bindings for LZO, i.e. you can access
> > +the LZO library from your Python scripts thereby compressing ordinary
> > +Python strings.")
> > +(license license:gpl2+)))
> > +
> > +(define-public python2-lzo
> > +  (package-with-python2 python-lzo))
> > +
> >  (define-public lzop
> >(package
> >  (name "lzop")
> 
> Otherwise these 2 patches LGTM!
> 
> Maxim



-- 
ng0 -- https://www.inventati.org/patternsinthechaos/

Re: [PATCH 2/2] gnu: Add python-lzo.

[Maxim Cournoyer]

Hi ng0,

contact@cryptolab.net writes:

> From: ng0 
>
> * gnu/packages/compression.scm (python-lzo): New variable.
>
> Co-authored-by: Danny Milosavljevic 
> ---
>  gnu/packages/compression.scm | 36 
>  1 file changed, 36 insertions(+)
>
> diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
> index 64518fb6b..2545c4517 100644
> --- a/gnu/packages/compression.scm
> +++ b/gnu/packages/compression.scm
> @@ -365,6 +365,42 @@ LZO is written in ANSI C.  Both the source code and the 
> compressed data
>  format are designed to be portable across platforms.")
>  (license license:gpl2+)))
>  
> +(define-public python-lzo
> +  (package
> +(name "python-lzo")
> +(version "1.11")
> +(source
> + (origin
> +   (method url-fetch)
> +   (uri (pypi-uri "python-lzo" version))
> +   (sha256
> +(base32
> + "11p3ifg14p086byhhin6azx5svlkg8dzw2b5abixik97xd6fm81q"
> +(build-system python-build-system)
> +(arguments
> + `(#:test-target "check"
> +   #:phases
> +   (modify-phases %standard-phases
> + (add-after 'unpack 'patch-setuppy
> +   (lambda _
> + (substitute* "setup.py"
> +   (("include_dirs.append.*")
> +(string-append "include_dirs.append(\""
> +   (assoc-ref %build-inputs "lzo") 
> "/include/lzo" "\")
> +"

A bit hard to read/long line here. Maybe the various string components
being appended could be aligned below the string-append call? Also, you
could use single quotes for the Python code which might help to
differentiate what is what.

Is the guix linter happy?

> +(inputs
> + `(("lzo" ,lzo)))
> +(home-page "https://github.com/jd-boyd/python-lzo;)
> +(synopsis "Python bindings for the LZO data compression library")
> +(description
> + "Python-LZO provides Python bindings for LZO, i.e. you can access
> +the LZO library from your Python scripts thereby compressing ordinary
> +Python strings.")
> +(license license:gpl2+)))
> +
> +(define-public python2-lzo
> +  (package-with-python2 python-lzo))
> +
>  (define-public lzop
>(package
>  (name "lzop")

Otherwise these 2 patches LGTM!

Maxim


signature.asc
Description: PGP signature

Re: [PATCH 2/2] gnu: Add python-lzo.

[ng0]

Hartmut Goebel writes:

> Am 03.02.2017 um 14:13 schrieb ng0:
>> ERROR: In procedure scm_lreadr: gnu/packages/compression.scm:387:44: illegal 
>> character in escape sequence: #\)
>
> IC, yes, this needs to be written as \\) (two backslashes)

I see. Okay, do you know which guix or guile module I do have to
rtf to understand once and for all how the (substitute*) behaves?
I'm doing this for too long to continue to run into problems with
this.

Just the (substitute*)? Or is there some complimentary literature
I should consider (something in guile, grep, sed, …)?
-- 
ng0 . https://www.inventati.org/patternsinthechaos/

Re: [PATCH 2/2] gnu: Add python-lzo.

[Hartmut Goebel]

Am 03.02.2017 um 14:13 schrieb ng0:
> ERROR: In procedure scm_lreadr: gnu/packages/compression.scm:387:44: illegal 
> character in escape sequence: #\)

IC, yes, this needs to be written as \\) (two backslashes)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel  | h.goe...@crazy-compilers.com   |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: [PATCH 2/2] gnu: Add python-lzo.

[ng0]

Hartmut Goebel writes:

> Am 02.02.2017 um 16:16 schrieb contact@cryptolab.net:
>> +(string-append "include_dirs.append(\""
>> +   (assoc-ref %build-inputs "lzo") 
>> "/include/lzo" "\")
>
> You could use single quotes for teh python string to make it more readable:

I think there was a reason for the way it was written. Your
suggestion below makes the substitute fail with:

ERROR: In procedure primitive-load-path:
ERROR: In procedure scm_lreadr: gnu/packages/compression.scm:387:44: illegal 
character in escape sequence: #\)

>
> string-append "include_dirs.append('" + (assoc-ref %build-inputs "lzo")
> "/include/lzo" "')
>
>
>> +"
>
> Also this line-break is confusing. I suggest either using "\n" or to
> change the substitute into something like:
>
>
> + (substitute* "setup.py"
> +   (("include_dirs.append\(.*\)")  ;; match up to the trailing 
> parent only, not the new-line
> +(string-append "include_dirs.append('"
> +   (assoc-ref %build-inputs "lzo") "/include/lzo"
> +   "')"   ;; put these last few 
> characters on the next line
>
> Otherwise LGTM.


-- 
ng0 . https://www.inventati.org/patternsinthechaos/

Re: [PATCH 2/2] gnu: Add python-lzo.

[Hartmut Goebel]

Am 02.02.2017 um 16:16 schrieb contact@cryptolab.net:
> +(string-append "include_dirs.append(\""
> +   (assoc-ref %build-inputs "lzo") 
> "/include/lzo" "\")

You could use single quotes for teh python string to make it more readable:


string-append "include_dirs.append('" + (assoc-ref %build-inputs "lzo")
"/include/lzo" "')


> +"

Also this line-break is confusing. I suggest either using "\n" or to
change the substitute into something like:


+ (substitute* "setup.py"
+   (("include_dirs.append\(.*\)")  ;; match up to the trailing 
parent only, not the new-line
+(string-append "include_dirs.append('"
+   (assoc-ref %build-inputs "lzo") "/include/lzo"
+   "')"   ;; put these last few characters 
on the next line

Otherwise LGTM.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel  | h.goe...@crazy-compilers.com   |
| www.crazy-compilers.com | compilers which you thought are impossible |



0xBF773B65.asc
Description: application/pgp-keys