diff mbox series

[bug#54396,v2,WIP] gnu: python-notmuch2: Fix build.

Message ID 164751023805.11899.12185326929295576108@localhost
State Accepted
Headers show
Series [bug#54396,v2,WIP] gnu: python-notmuch2: Fix build. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Tanguy LE CARROUR March 17, 2022, 9:43 a.m. UTC 
Hi Liliana,


Quoting Liliana Marie Prikler (2022-03-17 07:59:48)
> Am Mittwoch, dem 16.03.2022 um 17:48 +0100 schrieb Tanguy LE CARROUR:
> > > That is actually a mistake on my part, search-input-file should
> > > search inputs for "include/notmuch.h" and likewise
> > > "lib/libnotmuch.so". 
> > > The dirname is important here, since both constants want
> > > directories. 
> > > Alternatively, forgoing those constants might be a good idea, but
> > > is not worth investing too much time in.
> > 
> > So, I'm leaving those as they are for now.
> I think you misunderstood me.  It is absolutely necessary that you
> instantiate these variables correctly.  It is not necessary to
> substitute* their uses.

You right, I **totally** misunderstood you! Sorry!
I didn't pay attention to the fact that I left the actual file names in,
instead of using the folder names.
Hope I've gotten it right this time and that you are not getting tired
of reviewing my patch! :-)


> > > > If I set `NOTMUCH_VERSION_FILE=''`, I get a "file does not exist"
> > > > error at build time. So I decided to set it to `/dev/null`
> > > > instead.
> > > > Or would it be better if I patched `notmuch` to create the actual
> > > > version file?!
> > > I think you ought to debug the build and short-circuit the logic by
> > > inferring the version number directly where it is needed.  Without
> > > actual build output that's as much as I can propose.
> > 
> > Mmm… I've added a new phase to patch the `setup.py`, but couldn't get
> > the version variable to work. Sorry for my… noobiness!? ^_^'
> > 
> > ```
> > ;; version.txt is not included in notmuch, so we patch in the version
> > number
> > (add-after 'create-notmuch-config 'patch-setup.py
> >   (lambda _
> >     (substitute* "setup.py"
> >       (("with open\\(NOTMUCH_VERSION_FILE\\) as fp:") "")
> >       (("    VERSION = fp.read\\(\\).strip\\(\\)") "")
> >       (("version=VERSION,") (string-append "version='" "0.0.0"
> > "',")))))
> > ```
> > 
> > I've tried to replace `0.0.0` with `version` or `,version` but that
> > doesn't seem to be the proper way to do it.
> substitute* is not nice when dealing with multi-line expressions.  I
> think you could replace NOTMUCH_VERSION_FILE with '/dev/null' and
> "VERSION = .*" with (string-append "VERSION = " #$version).

Oh, looks like gexp magic to me, right? I'm not (yet) comfortable with it!
So I came up with another version. Seems to work as I've built,
installed and used it!

I'm attaching v5. Finger crossed! :-)

Thanks again for you time!

Comments

Liliana Marie Prikler March 17, 2022, 9:51 a.m. UTC | #1 
Am Donnerstag, dem 17.03.2022 um 10:43 +0100 schrieb Tanguy LE CARROUR:
> Hi Liliana,
> 
> Quoting Liliana Marie Prikler (2022-03-17 07:59:48)
> > Am Mittwoch, dem 16.03.2022 um 17:48 +0100 schrieb Tanguy LE
> > CARROUR:
> > > > That is actually a mistake on my part, search-input-file should
> > > > search inputs for "include/notmuch.h" and likewise
> > > > "lib/libnotmuch.so". 
> > > > The dirname is important here, since both constants want
> > > > directories. 
> > > > Alternatively, forgoing those constants might be a good idea,
> > > > but is not worth investing too much time in.
> > > 
> > > So, I'm leaving those as they are for now.
> > I think you misunderstood me.  It is absolutely necessary that you
> > instantiate these variables correctly.  It is not necessary to
> > substitute* their uses.
> 
> You right, I **totally** misunderstood you! Sorry!
> I didn't pay attention to the fact that I left the actual file names
> in, instead of using the folder names.
> Hope I've gotten it right this time and that you are not getting
> tired of reviewing my patch! :-)
You'd do well using search-input-file as I've suggested for a while now
;)
> > > 
> > > > 

> > substitute* is not nice when dealing with multi-line expressions. 
> > I think you could replace NOTMUCH_VERSION_FILE with '/dev/null' and
> > "VERSION = .*" with (string-append "VERSION = " #$version).
> 
> Oh, looks like gexp magic to me, right? I'm not (yet) comfortable
> with it!  So I came up with another version. Seems to work as I've
> built, installed and used it!
I think we need to convert this package to the newstyle anyway, so it's
not something you have to do, but it'd be appreciated if you first
translated the existing package to use gexps and then applied your
modifications on top.  That being said, using this-package is IIRC fine
for such purposes.

Regarding NOTMUCH_VERSION_FILE, I don't think we should put an
"implemented" value into a file that (as far as I can see) ends up
being installed.  So here you have to substitute* the use in setup.py
instead.

Cheers
Tanguy LE CARROUR March 17, 2022, 5:32 p.m. UTC | #2 
Hi Liliana,


Quoting Liliana Marie Prikler (2022-03-17 10:51:41)
> Am Donnerstag, dem 17.03.2022 um 10:43 +0100 schrieb Tanguy LE CARROUR:
> > Quoting Liliana Marie Prikler (2022-03-17 07:59:48)
> > > Am Mittwoch, dem 16.03.2022 um 17:48 +0100 schrieb Tanguy LE
> > > CARROUR:
> > > > > That is actually a mistake on my part, search-input-file should
> > > > > search inputs for "include/notmuch.h" and likewise
> > > > > "lib/libnotmuch.so". 
> > > > > The dirname is important here, since both constants want
> > > > > directories. 
> > > > > Alternatively, forgoing those constants might be a good idea,
> > > > > but is not worth investing too much time in.
> > > > 
> > > > So, I'm leaving those as they are for now.
> > > I think you misunderstood me.  It is absolutely necessary that you
> > > instantiate these variables correctly.  It is not necessary to
> > > substitute* their uses.
> > 
> > You right, I **totally** misunderstood you! Sorry!
> > I didn't pay attention to the fact that I left the actual file names
> > in, instead of using the folder names.
> > Hope I've gotten it right this time and that you are not getting
> > tired of reviewing my patch! :-)
> You'd do well using search-input-file as I've suggested for a while now
> ;)

You're right! So, now, it looks like:

```
(add-after 'enter-python-dir 'create-notmuch-config
  (lambda* (#:key inputs #:allow-other-keys)
    (with-output-to-file "_notmuch_config.py"
      (lambda _
        (display
          (string-append
           "NOTMUCH_INCLUDE_DIR="
           "'" (dirname (search-input-file inputs "include/notmuch.h")) "'\n"
           "NOTMUCH_LIB_DIR="
           "'" (dirname (search-input-file inputs "lib/libnotmuch.so")) "'"))))))
```


> > > substitute* is not nice when dealing with multi-line expressions. 
> > > I think you could replace NOTMUCH_VERSION_FILE with '/dev/null' and
> > > "VERSION = .*" with (string-append "VERSION = " #$version).
> > 
> > Oh, looks like gexp magic to me, right? I'm not (yet) comfortable
> > with it!  So I came up with another version. Seems to work as I've
> > built, installed and used it!
> I think we need to convert this package to the newstyle anyway,
> not something you have to do, but it'd be appreciated if you first
> translated the existing package to use gexps and then applied your
> modifications on top.  That being said, using this-package is IIRC fine
> for such purposes.

OK, I'll try to do that. But, in order not to spam you with stupid code and
questions, I'll think about it for few days and probably do it at the
week end.


> Regarding NOTMUCH_VERSION_FILE, I don't think we should put an
> "implemented" value into a file that (as far as I can see) ends up
> being installed.  So here you have to substitute* the use in setup.py
> instead.

It now looks like this:

```
 (add-after 'create-notmuch-config 'patch-setup.py
   (lambda _
     (substitute* "setup.py"
       (("NOTMUCH_VERSION_FILE")
        "'/dev/null'")
       (("version=VERSION,")
        (string-append "version='" ,(package-version this-package) "',")))))
```

I'll submit a new patch later this week with the gexp version!… if I
manage to write it! ^_^'

Thanks again for your valuable advice and time!
diff mbox series

Patch

From ecdb6dab3901e661544f4cd8aaf892f939b5e4ee Mon Sep 17 00:00:00 2001
From: Tanguy Le Carrour <tanguy@bioneland.org>
Date: Tue, 15 Mar 2022 10:38:14 +0100
Subject: [PATCH v5] gnu: python-notmuch2: Fix build.

* gnu/packages/mail.scm (python-notmuch2):
(%standard-phases): Add 'create-notmuch-config' and 'patch-setup.py'
after 'enter-python-dir'.
---
 gnu/packages/mail.scm | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
index d253ca7011..2b44f84446 100644
--- a/gnu/packages/mail.scm
+++ b/gnu/packages/mail.scm
@@ -28,7 +28,7 @@ 
 ;;; Copyright © 2018 Alex Vong <alexvong1995@gmail.com>
 ;;; Copyright © 2018 Gábor Boskovits <boskovits@gmail.com>
 ;;; Copyright © 2018, 2019, 2020, 2021, 2022 Ricardo Wurmus <rekado@elephly.net>
-;;; Copyright © 2019, 2020, 2021 Tanguy Le Carrour <tanguy@bioneland.org>
+;;; Copyright © 2019, 2020-2022 Tanguy Le Carrour <tanguy@bioneland.org>
 ;;; Copyright © 2020 Vincent Legoll <vincent.legoll@gmail.com>
 ;;; Copyright © 2020 Justus Winter <justus@sequoia-pgp.org>
 ;;; Copyright © 2020 Eric Brown <ecbrown@ericcbrown.com>
@@ -1493,7 +1493,26 @@  (define-public python-notmuch2
          ;; This python package lives in a subdirectory of the notmuch source
          ;; tree, so chdir into it before building.
          (add-after 'unpack 'enter-python-dir
-           (lambda _ (chdir "bindings/python-cffi"))))))
+           (lambda _ (chdir "bindings/python-cffi")))
+         ;; python-build-system does not invoke the configure script
+         ;; so _notmuch_config.py is missing
+         (add-after 'enter-python-dir 'create-notmuch-config
+           (lambda* (#:key inputs #:allow-other-keys)
+             (with-output-to-file "_notmuch_config.py"
+               (lambda _
+                 (display
+                  (string-append
+                   "NOTMUCH_VERSION_FILE='/dev/null'\n"
+                   "NOTMUCH_INCLUDE_DIR="
+                   "'" (assoc-ref inputs "notmuch") "/include/" "'\n"
+                   "NOTMUCH_LIB_DIR="
+                   "'" (assoc-ref inputs "notmuch") "/lib/" "'"))))))
+         ;; version.txt is not included in notmuch, so we patch in the version number
+         (add-after 'create-notmuch-config 'patch-setup.py
+           (lambda _
+             (substitute* "setup.py"
+               (("version=VERSION,")
+                (string-append "version='" ,(package-version this-package) "',"))))))))
     (synopsis "Pythonic bindings for the notmuch mail database using CFFI")
     (license license:gpl3+)))
 
-- 
2.34.0