Skip to content
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

gpgme: 1.23.2 -> 1.24.0 #354635

Merged
merged 2 commits into from
Nov 23, 2024
Merged

gpgme: 1.23.2 -> 1.24.0 #354635

merged 2 commits into from
Nov 23, 2024

Conversation

dotlambda
Copy link
Member

fixes #354166
cc @lucc

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

pkgs/development/python-modules/gpgme/default.nix Outdated Show resolved Hide resolved
meta = {
description = "Python bindings to the GPGME API of the GnuPG cryptography library";
homepage = "https://dev.gnupg.org/source/gpgme/browse/master/lang/python/";
inherit (gpgme.meta) changelog license maintainers;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should merge all of meta to not forget anything, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that creates problems with meta.position but it must be some other problem.

Copy link
Member Author

@dotlambda dotlambda Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Atemu @eclairevoyant Can you explain why I get

nix-repl> python3.pkgs.gpgme.meta.position
"nixpkgs/pkgs/development/python-modules/gpgme/default.nix:45"

here despite using meta = gpgme.meta // ...? C.f. #324127 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because, not despite.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But removing gpgme.meta // yields the same meta.position.

Copy link
Member

@emilazy emilazy Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think meta.position is unconditionally overwritten based on pos (when present), or something.

Copy link
Contributor

@lucc lucc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some code nitpicks. Thanks for splitting it into a proper python derivation with the import check.

Do I understand the code correctly, that the python derivation depends on the binary library derivation and also executes the configure script in the sources again but does not build the binary library again?

pkgs/development/python-modules/gpgme/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/gpgme/default.nix Outdated Show resolved Hide resolved
@dotlambda
Copy link
Member Author

Do I understand the code correctly, that the python derivation depends on the binary library derivation and also executes the configure script in the sources again but does not build the binary library again?

Yes. The configure script at least sets the version, i.e. replaces @version@, while turning setup.py.in into setup.py. I could do that manually instead but I don't think there's a lot of harm in running the script.

@dotlambda
Copy link
Member Author

dotlambda commented Nov 9, 2024

The question is: Is this acceptable for staging-next? Otherwise it won't land in 24.11.
I tried only applying the second commit but ran into the same problem as #354166, even after adding distutils to build-system.
cc @NixOS/nixos-release-managers

@RossComputerGuy
Copy link
Member

RossComputerGuy commented Nov 9, 2024

The question is: Is this acceptable for staging-next?

I'm not sure, lemme ask the staging folks. I would say yes.

Otherwise it won't land in 24.11.

Yeah, we should fix this before 24.11's release.

@dotlambda dotlambda changed the base branch from staging to staging-next November 9, 2024 16:38
@RossComputerGuy
Copy link
Member

Well it looks like this won't make it to 24.11, should go back to staging and it can land in there once approved enough.

@dotlambda dotlambda changed the base branch from staging-next to staging November 9, 2024 20:58
@dotlambda dotlambda added the backport staging-24.11 Backport PR automatically label Nov 9, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 11, 2024
@dotlambda
Copy link
Member Author

I don't understand why python313.pkgs.gpgme fails to build with

checking for swig... /nix/store/8w5lfkjysan93sv31rmjxbpw1g1iv115-swig-4.2.1/bin/swig
checking for a Python interpreter with version >= 2.7... none
checking for a Python interpreter with version >= 3.6... none
checking for a Python interpreter with version >= 3.8... none
checking for a Python interpreter with version >= 3.9... none
checking for a Python interpreter with version >= 3.10... none
checking for a Python interpreter with version >= 3.11... none
checking for a Python interpreter with version >= 3.12... none
checking for a Python interpreter with version >= all... none
configure: error: 
***
*** Please install the python development packages.
***

@mweinelt Any idea?

@lucc
Copy link
Contributor

lucc commented Nov 20, 2024

@dotlambda I think that is the configure script of gpgme which tries to find a suitable python version and they have hardcoded a list :/

Maybe there is an option to tell configure to just use one specific python executable and not search for it, but I did not find that option at first glance.

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 20, 2024
@dotlambda
Copy link
Member Author

@lucc Did you even look at this PR's diff? There's a patch that changes said list.

@dotlambda
Copy link
Member Author

Should we merge this anyway and fix Python 3.13 later?

@dotlambda
Copy link
Member Author

@ofborg build gpgme.passthru.tests python313.pkgs.gpgme

@dotlambda dotlambda merged commit b161824 into NixOS:staging Nov 23, 2024
17 of 21 checks passed
@dotlambda dotlambda deleted the gpgme branch November 23, 2024 20:46
Copy link
Contributor

Backport failed for staging-24.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin staging-24.11
git worktree add -d .worktree/backport-354635-to-staging-24.11 origin/staging-24.11
cd .worktree/backport-354635-to-staging-24.11
git switch --create backport-354635-to-staging-24.11
git cherry-pick -x b3797b6d48b59ca5d8e1c991278d33ad48bde9bd 29c88a0ce64fed33d93d02605c8420ff42d570ee

@FliegendeWurst
Copy link
Member

Fix for python313Packages.gpgme: #371188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants