Subject: Re: [bug#29733] Add virtaal (translation app)

From: Oleg Pykhalov <[email-masked]>
Date: Sat, 16 Dec 2017 14:49:30 +0300

Hello Mark,

This is an amazing program!  Thank you for packaging this!

But I see some issues which need to be solved before merging if you
could do it, please.  Ask questions if you need here or on on any Guix
provided resources <https://www.gnu.org/software/guix/help/>.

Mark Meyer <[email-masked]> writes:

> I just wanted to quickly have someone look over the following changes:
>
>   - Add python2-pycurl adds pycurl (it's Py2 only)
>   - Update python2-gobject-2 to 2.28.7 (the most recent, patch still
>   seems to apply
>
>     This update pulls in a lot of changes that were present in
>     distrubutions for a long time
>
>   - Add virtaal
>
>     This just adds virtaal with proper dependencies.
>
> From 6b2b028d4500a535faa3442daa0c228f9c98b93f Mon Sep 17 00:00:00 2001
> From: Mark Meyer <[email-masked]>
> Date: Fri, 15 Dec 2017 10:09:28 +0100
> Subject: [PATCH] gnu: Add python2-pycurl.
>
> * gnu/packages/python-web.scm (python2-pycurl): New public variable.
> ---
>  gnu/packages/python-web.scm | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/gnu/packages/python-web.scm b/gnu/packages/python-web.scm
> index 12e45b2ef..502834bdc 100644
> --- a/gnu/packages/python-web.scm
> +++ b/gnu/packages/python-web.scm
> @@ -22,6 +22,7 @@
>  ;;; Copyright  2016 David Craven <[email-masked]>
>  ;;; Copyright  2017 Oleg Pykhalov <[email-masked]>
>  ;;; Copyright  2015, 2016 David Thompson <[email-masked]>
> +;;; Copyright  2017 Mark Meyer <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -45,11 +46,13 @@
>    #:use-module (gnu packages)
>    #:use-module (gnu packages check)
>    #:use-module (gnu packages compression)
> +  #:use-module (gnu packages curl)
>    #:use-module (gnu packages databases)
>    #:use-module (gnu packages django)
>    #:use-module (gnu packages pkg-config)
>    #:use-module (gnu packages python)
>    #:use-module (gnu packages python-crypto)
> +  #:use-module (gnu packages tls)
>    #:use-module (gnu packages time)
>    #:use-module (gnu packages xml)
>    #:use-module ((guix licenses) #:prefix license:)
> @@ -238,6 +241,30 @@ C, yielding parse times that can be a thirtieth of the html5lib parse times.")
>  (define-public python2-html5-parser
>    (package-with-python2 python-html5-parser))
>  
> +(define-public python2-pycurl
> +  (package
> +    (name "python2-pycurl")
> +    (version "7.43.0.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://dl.bintray.com/pycurl/pycurl/pycurl-" version ".tar.gz"))

Please, use guix lint before sending patches.

./pre-inst-env guix lint python2-pycurl

/home/natsu/src/guix/gnu/packages/python-web.scm:245:2: [email-masked]: line 251 is way too long (93 characters)

There is etc/indent-code.el to make life easier if you not Emacs user,
but I didn't check it myself.

https://www.gnu.org/software/guix/manual/html_node/Formatting-Code.html

> +       (sha256
> +        (base32 "1ali1gjs9iliwjra7w0y5hwg79a2fd0f4ydvv6k27sgxpbr1n8s3"))))
> +    (build-system python-build-system)
> +    (arguments `(#:python ,python-2)) ; no Python 3 support

Does it actually not support?  Requirements in documentation
<http://pycurl.io/docs/latest/> include Python 3.

> +    (native-inputs
> +     `(("python-nose" ,python-nose)
> +       ("python-bottle" ,python-bottle)))
> +    (inputs
> +     `(("curl" ,curl)
> +       ("gnutls" ,gnutls)))
> +    (home-page "http://pycurl.io/")
> +    (synopsis "Lightweight Python wrapper around libcurl")
> +    (description "Pycurl is a lightweight wrapper around libcurl. It provides
> +high speed transfers via libcurl and frequently outperforms alternatives.")

Another linter warning.

gnu/packages/python-web.scm:264:17: [email-masked]: sentences in description should be followed by two spaces; possible infraction at 46

> +    (license license:lgpl2.0)))

tar xf $(./pre-inst-env guix build -S python2-pycurl)

There are two files in licenses in source tarball COPYING-LGPL
COPYING-MIT.  The second (expat) license probably should be included in
the package recipe.

> +
>  (define-public python-webencodings
>    (package
>      (name "python-webencodings")
> -- 
> 2.15.1

Also there is a tests directory in source tarball.  Could you look on
them?  .travis.yml file in pycurl git repository could help
<https://github.com/pycurl/pycurl/blob/master/.travis.yml>

> From 8a71cee704727f80e4146ee87a06c462bf59a9c5 Mon Sep 17 00:00:00 2001
> From: Mark Meyer <[email-masked]>
> Date: Sat, 16 Dec 2017 10:57:21 +0100
> Subject: [PATCH] gnu: python2-gobject-2: Update to 2.28.7
>
> * gnu/packages/glib.scm (python2-gobject-2): Update to 2.28.7.
> ---
>  gnu/packages/glib.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
> index de7427859..40d23e104 100644
> --- a/gnu/packages/glib.scm
> +++ b/gnu/packages/glib.scm
> @@ -543,7 +543,7 @@ useful for C++.")
>      (name "python2-pygobject")
>      ;; This was the last version to declare the 2.0 platform number, i.e. its
>      ;; pkg-config files were named pygobject-2.0.pc
> -    (version "2.28.6")
> +    (version "2.28.7")
>      (source
>       (origin
>         (method url-fetch)
> @@ -552,7 +552,7 @@ useful for C++.")
>                             "/pygobject-" version ".tar.xz"))
>         (sha256
>          (base32
> -         "1f5dfxjnil2glfwxnqr14d2cjfbkghsbsn8n04js2c2icr7iv2pv"))
> +         "0nkam61rsn7y3wik3vw46wk5q2cjfh2iph57hl9m39rc8jijb7dv"))
>         (patches (search-patches
>                   "python2-pygobject-2-gi-info-type-error-domain.patch"))))
>      (build-system gnu-build-system)
> -- 
> 2.15.1

LGTM

>
> From c36fde0cfda0d5f9830876abbff804504004451d Mon Sep 17 00:00:00 2001
> From: Mark Meyer <[email-masked]>
> Date: Fri, 15 Dec 2017 12:35:38 +0100
> Subject: [PATCH] gnu: Add virtaal.
>
> * gnu/packages/text-editors.scm (virtaal): New public variable.
> ---
>  gnu/packages/text-editors.scm | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/gnu/packages/text-editors.scm b/gnu/packages/text-editors.scm
> index 44b42ce9b..773000ff4 100644
> --- a/gnu/packages/text-editors.scm
> +++ b/gnu/packages/text-editors.scm
> @@ -6,6 +6,7 @@
>  ;;; Copyright  2017 ng0 <[email-masked]>
>  ;;; Copyright  2014 Taylan Ulrich Bayrl/Kammer <[email-masked]>
>  ;;; Copyright  2017 Tobias Geerinckx-Rice <[email-masked]>
> +;;; Copyright  2017 Mark Meyer <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -29,6 +30,7 @@
>    #:use-module (guix utils)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system glib-or-gtk)
> +  #:use-module (guix build-system python)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (gnu packages)
>    #:use-module (gnu packages assembly)
> @@ -41,11 +43,42 @@
>    #:use-module (gnu packages lua)
>    #:use-module (gnu packages ncurses)
>    #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages python)
> +  #:use-module (gnu packages python-web)
>    #:use-module (gnu packages regex)
>    #:use-module (gnu packages ruby)
>    #:use-module (gnu packages terminals)
>    #:use-module (gnu packages xml))
>  
> +(define-public virtaal
> +  (package
> +    (name "virtaal")
> +    (version "0.7.1")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "mirror://sourceforge/translate/Virtaal/"
> +                                  version "/virtaal-" version ".tar.bz2"))
> +              (sha256
> +               (base32
> +                "0cyimjp3191qlmw6n0ipqdr9xr0cq4f6dqvz4rl9q31h6l3kywf9"))))
> +    (build-system python-build-system)
> +    (arguments
> +     `(#:python ,python-2
> +       #:use-setuptools? #f
> +       #:tests? #f))

Could you check tests in virtaal-0.7.1/virtaal/test/ please?

> +    (propagated-inputs
> +     `(("python2-pycurl" ,python2-pycurl)
> +       ("python2-pygtk" ,python2-pygtk)
> +       ("python2-lxml" ,python2-lxml)
> +       ("python2-translate-toolkit" ,python2-translate-toolkit)
> +       ("python2-simplejson" ,python2-simplejson)))
> +    (synopsis "An editor for translation files")

gnu/packages/text-editors.scm:75:14: [email-masked]: no article allowed at the beginning of the synopsis

> +    (description
> +     "Virtaal, a feature rich translation tool that allows you to focus on
> +translation, without the tool getting in the way.")
> +    (home-page "http://virtaal.translatehouse.org/")
> +    (license license:gpl2)))
> +
>  (define-public vis
>    (package
>      (name "vis")

Could't be it builded With Python 3?  I see in
<https://github.com/translate/virtaal/blob/master/.travis.yml> that they
use only python 2.7 and 2.7, but still?

Thanks,
Oleg.