[RDY] init gpg python bindings + Alot: 0.5.1 -> 0.7.0 #30429
Conversation
| with pythonPackages; | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
This change uncovers that the library still fails to build on python3. But I have not investigated the reason for that.
|
2nd failure seems to be unrelated to PR (network failure). The one on macos I am not sure why it fails on why it installs this peculiar set of applications . How can I run that test locally ? |
|
You would have to run the following command: However it fails on macOS. If you cannot test it there, I would suggest to restrict |
| homepage = https://www.gnupg.org; | ||
| description = "GPG bindings for python"; | ||
| license = licenses.lgpl2; | ||
| platforms = platforms.linux; |
There was a problem hiding this comment.
platform is already restricted to linux isn't it ?
There was a problem hiding this comment.
right, then gpgme is broken on darwin for unrelated reasons and we can ignore this build on travis.
| with pythonPackages; | ||
|
|
||
| buildPythonApplication rec { | ||
| rev = "0.6"; |
| with pythonPackages; | ||
|
|
||
| buildPythonApplication rec { | ||
| rev = "0.6"; |
| @@ -0,0 +1,60 @@ | |||
| { stdenv, pkgs, pythonPackages | |||
| ]; | ||
|
|
||
| doCheck = false; | ||
| postBuild= stdenv.lib.optionalString withManpage "make -C docs man"; |
|
|
||
| sed "s,/usr/bin,$out/bin,g" extra/alot.desktop > $out/share/applications/alot.desktop | ||
| wrapProgram $out/bin/alot \ | ||
| --prefix LD_LIBRARY_PATH : '${pkgs.lib.makeLibraryPath [ pkgs.notmuch pkgs.file pkgs.gpgme ]}' |
There was a problem hiding this comment.
Can you not patch the sources to hardcode the paths instead?
There was a problem hiding this comment.
why ? this is a part I haven't touched so I would rather not change it as it works
| version = "1.9.0"; | ||
|
|
||
| passthru = { | ||
| pythonSourceRoot = "lang/python"; |
There was a problem hiding this comment.
is this worth it, considering you only use it for one derivation?
There was a problem hiding this comment.
If gpgme next updates changes the path, it makes more sense to change the path in gpgme derviation then in the related ones imo. No strong opinion though.
There was a problem hiding this comment.
While looking at clang package for an unrelated problem, I noticed it had a "python" output . Maybe I should do the same here instead ?
outputs = [ "out" "python" ]
++ stdenv.lib.optional enableManpages "man";
There was a problem hiding this comment.
Can the Python bindings be build without building gpgme? If so, then a separate derivation. If not, then an output is typically preferred to reduce building time. It is not required however. I'd say leave it as it is.
Anyway, please remove pythonSourceRoot.
There was a problem hiding this comment.
I removed pythonSourceRoot and rebased
|
I've addressed your comments locally . Should I squash the history into to 2 commits ("gpg init" then "alot update") ? |
|
I've tried to adress the review & squashed the commits. I tested (signature mode) the gpg support of alot and it worked fine. |
|
Meson seems to keep failing but it's not linked to this PR |
|
should I disable the mac platform ? I don't see where is the wrong rm call and I can't test it on mac. |
|
for my future self or anyone willing to take on; |
|
I made some progress on the gpgme python packaging but it still fails in the postinstallation phase with I dont know what invokes the egg_info, I grepped for it in nixpkgs to no avail. @FRidh any idea ? I use for my tests a local src if that helps |
| # src = fetchurl { | ||
| # url = "mirror://gnupg/gpgme/${name}.tar.bz2"; | ||
| # sha256 = "14q619lxbk64vz7lih5gjb928qm28jrnn1h3yhsrrff3jw8yv3qs"; | ||
| # }; |
teto
changed the title
| @@ -0,0 +1,62 @@ | |||
| { stdenv, lib | |||
| , pythonPackages, fetchFromGitHub, notmuch, file, gpgme | |||
| , withManpage ? true }: | |||
There was a problem hiding this comment.
This can be removed. It hardly takes any time to produce the manpages so we should.
| ]; | ||
|
|
||
| # 3 tests fail, some unicode | ||
| doCheck = false; |
| nativeBuildInputs = [ pkgconfig gnupg autoreconfHook git texinfo5 ]; | ||
|
|
||
| preConfigure = '' | ||
| ./autogen.sh |
|
I can't manage to get a wheel. Even if I could I am not sure how to write the package are the C parts 1 python part are intertwined. Any help would be greatly appreciated. My PR is a bit of a mess but basically
I've tried to add a target |
|
I think it's LGTM to disable the tests, it seems like they rely on twisted functions which need the network, hence the failures (twisted tests were disabled for that reason). |
|
Success on x86_64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
teto
changed the title
| buildPythonApplication rec { | ||
| pname = "alot"; | ||
| version = "0.7"; | ||
| outputs = [ "out" "man" ]; |
|
|
||
| postInstall = '' | ||
| mkdir -p $out/share/applications $out/man | ||
| cp -r docs/build/man $out/man |
| ++ lib.optionals withPython [ python swig2 which ]; | ||
|
|
||
| preConfigure = '' | ||
| ./autogen.sh |
There was a problem hiding this comment.
is this needed with the autoreconfHook?
|
|
||
| sed "s,/usr/bin,$out/bin,g" extra/alot.desktop > $out/share/applications/alot.desktop | ||
| wrapProgram $out/bin/alot \ | ||
| --prefix LD_LIBRARY_PATH : '${lib.makeLibraryPath [ notmuch file gpgme ]}' |
There was a problem hiding this comment.
these should be patched in the code
There was a problem hiding this comment.
leftover from the current derivation code. it seems to work fine without it so I removed it
|
Success on x86_64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
- added gnupg to checkInputs - generate manpage (optional) - move alot.desktop file to $out/share/applications - disabled tests as they need the network (dependency on twisted) Thanks to Sarah Brofeldt, Ben Mcginnes for their help (and other) and to FRidh for the repeated careful reviews.
Useful for MUAs mostly, like alot
|
I hope the last patch fixed everything ? |
|
@GrahamcOfBorg build alot gpgme |
|
Success on x86_64-darwin (full log) Attempted: gpgme The following builds were skipped because they don't evaluate on x86_64-darwin: alot Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Seems to pass :D |
Let's be honest with what we expose.
pkgs/top-level/python-packages.nix
Outdated
| @@ -8173,7 +8173,7 @@ in { | |||
|
|
|||
| google_gax = callPackage ../development/python-modules/google_gax { }; | |||
|
|
|||
| gpg = toPythonModule (pkgs.gpgme.override { withPython=true; }); | |||
| gpgme = toPythonModule (pkgs.gpgme.override { withPython=true; }); | |||
There was a problem hiding this comment.
@teto do you agree with this change?
There was a problem hiding this comment.
I hesitated between the 2 and ended up with gpg because it's import gpg and it's like the official ports for gpg (even if gpgme expose a simpler interface). So I have no problem with changing the name :)
|
@GrahamcOfBorg build alot gpgme |
|
Success on x86_64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Success on x86_64-darwin (full log) Attempted: gpgme The following builds were skipped because they don't evaluate on x86_64-darwin: alot Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: alot, gpgme Partial log (click to expand)
|
|
thanks for the keeping up with the good reviews despite my numerous hesitations in the implementation. Really happy to see it merged ! |
|
The show must go on. |
Motivation for this change
Updating the CLI notmuch-based mailreader alot
Things done
Nevertheless some things are wrong:
man alotdoesn't work after installation via nix-env. I believe it's a known nixos problem. I've tried using multiple outputs but not sure how to make it better.I've not tested yet the gpg support but other than that it seems to work
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)