-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: gmime3: init at 3.0.1 #28796
WIP: gmime3: init at 3.0.1 #28796
Conversation
@@ -8105,6 +8105,8 @@ with pkgs; | |||
|
|||
gmime = callPackage ../development/libraries/gmime { }; | |||
|
|||
gmime3 = callPackage ../development/libraries/gmime/3.0.nix { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it incompatible to gmime2? Or does it break applications using gmime2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be breakage, yes. They have a porting document for migrating code from v2 API to v3 API:
https://github.com/jstedfast/gmime/blob/master/PORTING
E.g.: the notmuch project just went through that, so it has to be checked on a per-project basis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add an explanation to the commit message.
{ stdenv, fetchurl, pkgconfig, glib, gpgme, zlib, libgpgerror, libidn, gobjectIntrospection }: | ||
|
||
stdenv.mkDerivation rec { | ||
# TODO: libidn support, PGP/MIME support, S/MIME support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you want to address the TODO items before the merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, probably should have said so in the description. From a birdview it seems to me that it should be working, all the dependencies are there at least. Do you see anything missing, or that could be taken away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to configure this is already enabled:
PGP/MIME support: yes (via GpgME)
S/MIME support: yes (via GpgME)
libidn support: yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw that here too.
meta = { | ||
homepage = https://github.com/jstedfast/gmime/; | ||
description = "A C/C++ library for creating, editing and parsing MIME messages and structures"; | ||
maintainers = [ stdenv.lib.maintainers.chaoflow ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add yourself as maintainer too, if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do that then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I'd like to have some coordination with @chaoflow first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not seen anything from chaoflow, since I am maintainer. You can actually only add yourself as maintainer.
outputs = [ "out" "dev" ]; | ||
|
||
buildInputs = [ | ||
pkgconfig glib gpgme zlib libgpgerror libidn gobjectIntrospection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkgconfig is already in nativeBuildInputs
.
The reason for having the |
Can you provide an example? This is pretty unusual. |
This needs to be added as a separate package because the API changes in 3.0, see: https://mail.gnome.org/archives/gmime-devel-list/2017-April/msg00002.html https://developer.gnome.org/gmime/3.0/gmime-changes-3-0.html https://github.com/jstedfast/gmime/blob/3fd646c95503acb0d11bf67afde1c7bdd4fcc341/PORTING
I am trying to build one with: nix-shell -E 'with import <nixpkgs> { }; runCommand "dummy" { buildInputs = [ (notmuch.override { gmime = gmime3 ; }) ]; }' -I "nixpkgs=$PWD" but keep getting the error
So I added to notmuchGMime3 = callPackage ../applications/networking/mailreaders/notmuch { gmime = gmime3; }; And then ran I just so happens that it needs GLib anyways, and if I add it to the build inputs there GMime 3 does get detected, but I suspect that packages that don't depend directly on GLib will need |
notmuch explicitly requested glib:
So I added it. |
I looked at the definition of GMime 2.6 and it has the The option was introduced in f4c84b3, but nothing in the commit explains why it was introduced. At that moment notmuch still had a glib as a dependency, but it was lost in the transition from 0.20 to 0.22 in commit 58771c0, with no explanation. The correct solution seems to be getting rid of What do you think? |
That is the problem of |
Seems we did some duplicate work here (I came here from notmuch ML
discussion).
Notable differences I see from my expression
- I dropped `libgpgerror` from the `buildInputs` as there were no
references to it in the source.
- I left `gobjectIntrospection` in `nativeBuildInputs` in my expression.
I'm not sure about which is the correct way.
Also, I think changes to `notmuch` should be in a separate PR. My
changes there are much bigger (all the tests pass!) but I'm not
satisfied with them yet (see discussion in notmuch ML).
|
Yes, I recognize your username :)
I hadn't looked into this, will now.
That's the right way.
+1
Can you create a MR or point to the branch where you have these changes, please? |
Servilio Afre Puentes <notifications@github.com> writes:
> Seems we did some duplicate work here (I came here from notmuch ML discussion).
Yes, I recognize your username :)
> Notable differences I see from my expression
>
> - I dropped `libgpgerror` from the `buildInputs` as there were no references to it in the source.
I hadn't looked into this, will now.
> - I left `gobjectIntrospection` in `nativeBuildInputs` in my expression. I'm not sure about which is the correct way.
That's the right way.
> Also, I think changes to `notmuch` should be in a separate PR.
+1
> My changes there are much bigger (all the tests pass!) but I'm not satisfied with them yet (see discussion in notmuch ML).
Can you create a MR or point to the branch where you have these changes, please?
master...oxij:pkg/notmuch-with-new-gmime
The patch assumes `gmime-3` as the default gmime (not included in the
branch).
In principle, the expression should work (all the tests pass), but I
wanted to create a PR after I understood whenever all that patching in
`postPatch` is really necessary with `gmime-3`. It doesn't look like it
references any of those `bin/gpg` in the output which looks strange to
me.
|
Has been merged elsewhere |
Motivation for this change
Personal motivation is for using it with notmuch, but as it is the latest and stable release of the library, seems to be good idea to have it in the repository.
I have used the same maintainer as there is for 2.6, but I would be glad to be the maintainer.
cc @chaoflow
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)