[bug#33185,v2,1/2] gnu: Add patchwork.

Message ID 20190122220928.17927-1-mail@cbaines.net
State Accepted
Headers show
Series [bug#33185,v2,1/2] gnu: Add patchwork. | expand

Checks

Context Check Description
cbaines/applying patch success Successfully applied
cbaines/applying patch success Successfully applied

Commit Message

Christopher Baines Jan. 22, 2019, 10:09 p.m. UTC
* gnu/packages/patchutils.scm (patchwork): New variable.
---
 gnu/packages/patchutils.scm | 154 ++++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

Comments

Ricardo Wurmus Jan. 23, 2019, 9:28 a.m. UTC | #1
Hi Chris,

thanks for the patch!

> * gnu/packages/patchutils.scm (patchwork): New variable.
[…]
> +         (replace 'check
> +           (lambda* (#:key tests? #:allow-other-keys)
> +             (or (not tests?)
> +                 (begin
> +                   (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
> +                   (invoke
> +                    "python" "-Wonce" "./manage.py" "test" "--noinput")
> +                   #t))))

Maybe write this as

         (replace 'check
           (lambda* (#:key tests? #:allow-other-keys)
             (when tests?
               (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
               (invoke
                "python" "-Wonce" "./manage.py" "test" "--noinput"))
             #t))

> +         (replace 'install
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out"))) […]

This phase might be less verbose if you let-bound the result of
(site-packages inputs outputs) at the beginning.  It would also be good
if there were more comments about what’s going on.  It’s not all obvious
(e.g. why “lib” is copied to “docs”).

> +               (simple-format #t "replacing template pwclient symlink")

Use “display” instead of “simple-format #t”?

> +         (add-after 'install 'install-hasher
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out")))
> +               (chmod (string-append (site-packages inputs outputs)
> +                                     "/patchwork/hasher.py")
> +                      #o555)
> +               (symlink (string-append (site-packages inputs outputs)
> +                                       "/patchwork/hasher.py")
> +                        (string-append out "/bin/hasher")))
> +             #t))

Here also consider simplifying with let.

> +         ;; Create a patchwork specific version of Django's command line admin
> +         ;; utility.
> +         (add-after 'install 'install-patchwork-admin
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out")))
> +               (mkdir-p (string-append out "/bin"))
> +               (call-with-output-file (string-append out "/bin/patchwork-admin")
> +                 (lambda (port)
> +                   (display "#!/usr/bin/env python3

Should this really say “#!/usr/bin/env python3”?

--
Ricardo
Christopher Baines Jan. 25, 2019, 9:04 p.m. UTC | #2
Ricardo Wurmus <rekado@elephly.net> writes:

> Hi Chris,
>
> thanks for the patch!

And thanks for your feedback :) I've sent some updated patches.

>> * gnu/packages/patchutils.scm (patchwork): New variable.
> […]
>> +         (replace 'check
>> +           (lambda* (#:key tests? #:allow-other-keys)
>> +             (or (not tests?)
>> +                 (begin
>> +                   (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
>> +                   (invoke
>> +                    "python" "-Wonce" "./manage.py" "test" "--noinput")
>> +                   #t))))
>
> Maybe write this as
>
>          (replace 'check
>            (lambda* (#:key tests? #:allow-other-keys)
>              (when tests?
>                (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
>                (invoke
>                 "python" "-Wonce" "./manage.py" "test" "--noinput"))
>              #t))

Yep, I've updated it to use when now.

>> +         (replace 'install
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let ((out (assoc-ref outputs "out"))) […]
>
> This phase might be less verbose if you let-bound the result of
> (site-packages inputs outputs) at the beginning.  It would also be good
> if there were more comments about what’s going on.  It’s not all obvious
> (e.g. why “lib” is copied to “docs”).

I've used let more now, and tried to be a bit more descriptive in the
comments. I'll reconfigure the patchwork instances I host at some point
and hopefully add more detail then.

>> +               (simple-format #t "replacing template pwclient symlink")
>
> Use “display” instead of “simple-format #t”?

That was just a debugging thing, I've removed it.

>> +         (add-after 'install 'install-hasher
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let* ((out (assoc-ref outputs "out")))
>> +               (chmod (string-append (site-packages inputs outputs)
>> +                                     "/patchwork/hasher.py")
>> +                      #o555)
>> +               (symlink (string-append (site-packages inputs outputs)
>> +                                       "/patchwork/hasher.py")
>> +                        (string-append out "/bin/hasher")))
>> +             #t))
>
> Here also consider simplifying with let.

Yep, I've used let here more now.

>> +         ;; Create a patchwork specific version of Django's command line admin
>> +         ;; utility.
>> +         (add-after 'install 'install-patchwork-admin
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let* ((out (assoc-ref outputs "out")))
>> +               (mkdir-p (string-append out "/bin"))
>> +               (call-with-output-file (string-append out "/bin/patchwork-admin")
>> +                 (lambda (port)
>> +                   (display "#!/usr/bin/env python3
>
> Should this really say “#!/usr/bin/env python3”?

So this was fine, as it was replaced in a future stage. But yes, it's
probably better to use a more appropriate value that isn't changed later
on. I've updated this now.

So yes, I've sent some updated patches, still a few niggles, but they're
getting better.

Patch

diff --git a/gnu/packages/patchutils.scm b/gnu/packages/patchutils.scm
index 09f5afbb28..cba2076175 100644
--- a/gnu/packages/patchutils.scm
+++ b/gnu/packages/patchutils.scm
@@ -31,6 +31,8 @@ 
   #:use-module (gnu packages base)
   #:use-module (gnu packages bash)
   #:use-module (gnu packages check)
+  #:use-module (gnu packages databases)
+  #:use-module (gnu packages django)
   #:use-module (gnu packages file)
   #:use-module (gnu packages gawk)
   #:use-module (gnu packages gettext)
@@ -305,3 +307,155 @@  directories, and has support for many popular version control systems.
 Meld helps you review code changes and understand patches.  It might even help
 you to figure out what is going on in that merge you keep avoiding.")
     (license gpl2)))
+
+(define-public patchwork
+  (package
+    (name "patchwork")
+    (version "2.1.1")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/getpatchwork/patchwork.git")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1wpcgkji9cb50lyv12ifgk08sjn7dkqkzis9qjwhx6y855dfdfn1"))))
+    (build-system python-build-system)
+    (arguments
+     `(;; TODO: Tests require a running database
+       #:tests? #f
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (delete 'build)
+         (add-after 'unpack 'replace-wsgi.py
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (delete-file "patchwork/wsgi.py")
+             (call-with-output-file "patchwork/wsgi.py"
+               (lambda (port)
+                 ;; Embed the PYTHONPATH containing the dependencies, as well
+                 ;; as the python modules in this package in the wsgi.py file,
+                 ;; as this will ensure they are available at runtime.
+                 (define pythonpath
+                   (string-append (getenv "PYTHONPATH")
+                                  ":"
+                                  (site-packages inputs outputs)))
+                 (display
+                  (string-append "
+import os, sys
+
+sys.path.extend('" pythonpath "'.split(':'))
+
+from django.core.wsgi import get_wsgi_application
+
+# By default, assume that patchwork is running as a Guix service, which
+# provides the settings as the 'guix.patchwork.settings' Python module.
+#
+# When using httpd, it's hard to set environment variables, so rely on the
+# default set here.
+os.environ['DJANGO_SETTINGS_MODULE'] = os.getenv(
+  'DJANGO_SETTINGS_MODULE',
+  'guix.patchwork.settings' # default
+)
+
+application = get_wsgi_application()\n") port)))))
+         (replace 'check
+           (lambda* (#:key tests? #:allow-other-keys)
+             (or (not tests?)
+                 (begin
+                   (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.dev")
+                   (invoke
+                    "python" "-Wonce" "./manage.py" "test" "--noinput")
+                   #t))))
+         (replace 'install
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((out (assoc-ref outputs "out")))
+               (for-each (lambda (directory)
+                           (copy-recursively
+                            directory
+                            (string-append (site-packages inputs outputs)
+                                           "/" directory)))
+                         '("patchwork"
+                           "templates"))
+               (delete-file-recursively (string-append
+                                    (site-packages inputs outputs)
+                                    "patchwork/tests"))
+               ;; pwclient
+               (for-each (lambda (file)
+                           (install-file file (string-append out "/bin")))
+                         (list
+                          (string-append (site-packages inputs outputs)
+                                         "/patchwork/bin/pwclient")
+                          (string-append (site-packages inputs outputs)
+                                         "/patchwork/bin/parsemail.sh")
+                          (string-append (site-packages inputs outputs)
+                                         "patchwork/bin/parsemail-batch.sh")))
+
+               (simple-format #t "replacing template pwclient symlink")
+               (let ((template-pwclient (string-append
+                                         (site-packages inputs outputs)
+                                         "/patchwork/templates/patchwork/pwclient")))
+                 (delete-file template-pwclient)
+                 (copy-file (string-append (site-packages inputs outputs)
+                                           "/patchwork/bin/pwclient")
+                            template-pwclient))
+
+               (let ((static-root
+                      (string-append out "/share/patchwork/htdocs")))
+                 (mkdir-p static-root)
+                 (copy-file "patchwork/settings/production.example.py"
+                            "patchwork/settings/assets.py")
+                 (setenv "DJANGO_SECRET_KEY" "dummyvalue")
+                 (setenv "DJANGO_SETTINGS_MODULE" "patchwork.settings.assets")
+                 (setenv "STATIC_ROOT" static-root)
+                 (invoke "./manage.py" "collectstatic" "--no-input"))
+
+               (copy-recursively "lib"
+                                 (string-append
+                                  out "/share/doc/" ,name "-" ,version)))
+             #t))
+         (add-after 'install 'install-hasher
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out")))
+               (chmod (string-append (site-packages inputs outputs)
+                                     "/patchwork/hasher.py")
+                      #o555)
+               (symlink (string-append (site-packages inputs outputs)
+                                       "/patchwork/hasher.py")
+                        (string-append out "/bin/hasher")))
+             #t))
+         ;; Create a patchwork specific version of Django's command line admin
+         ;; utility.
+         (add-after 'install 'install-patchwork-admin
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out")))
+               (mkdir-p (string-append out "/bin"))
+               (call-with-output-file (string-append out "/bin/patchwork-admin")
+                 (lambda (port)
+                   (display "#!/usr/bin/env python3
+import os, sys
+
+if __name__ == \"__main__\":
+    from django.core.management import execute_from_command_line
+
+    execute_from_command_line(sys.argv)" port)))
+               (chmod (string-append out "/bin/patchwork-admin") #o555))
+             #t)))))
+    (inputs
+     `(("python-wrapper" ,python-wrapper)))
+    (propagated-inputs
+     `(("python-django" ,python-django)
+       ;; TODO: Make this configurable
+       ("python-psycopg2" ,python-psycopg2)
+       ("python-mysqlclient" ,python-mysqlclient)
+       ("python-django-filter" ,python-django-filter)
+       ("python-djangorestframework" ,python-djangorestframework)
+       ("python-django-debug-toolbar" ,python-django-debug-toolbar)))
+    (synopsis "Web based patch tracking system")
+    (description
+     "Patchwork is a patch tracking system.  It takes in emails containing
+patches, and displays the patches along with comments and state information.
+Users can login allowing them to change the state of patches.")
+    (home-page "http://jk.ozlabs.org/projects/patchwork/")
+    (license gpl2+)))