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

pinentry: 1.1.0 -> 2020-07-17 #93196

Closed
wants to merge 1 commit into from

Conversation

bagnaram
Copy link
Contributor

@bagnaram bagnaram commented Jul 15, 2020

(Support for pinentry-efl. Pinentry 1.1.0 relase from 2017, without a
recent tagged release)

Motivation for this change

Pinentry hasn't seen an update to its release since 2017, however there has been quite a bit of development, including an addition of pinentry-efl for Elementary toolkit DEs like enlightenment. This change rebuilds pinentry from MASTER and includes pinentry-efl

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@bagnaram
Copy link
Contributor Author

bagnaram commented Jul 15, 2020

I would like to include some logic in this package so that when ${version} = "master" then it builds from source, otherwise use the pre-defined packages.

@bagnaram bagnaram changed the title WIP: pinentry: 1.1.0 -> MASTER pinentry: 1.1.0 -> ae58404 Jul 15, 2020
pkgs/tools/security/pinentry/default.nix Outdated Show resolved Hide resolved
pkgs/tools/security/pinentry/default.nix Outdated Show resolved Hide resolved
pkgs/tools/security/pinentry/default.nix Outdated Show resolved Hide resolved
pkgs/tools/security/pinentry/default.nix Show resolved Hide resolved
pkgs/tools/security/pinentry/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/aliases.nix Outdated Show resolved Hide resolved
@bagnaram bagnaram force-pushed the feature/pinentry-efl branch 2 times, most recently from 28cc6ce to 1a68e9f Compare July 16, 2020 13:51
@bagnaram
Copy link
Contributor Author

Fixes addressed. Many thanks for the peer review

@ofborg ofborg bot requested review from ttuegel and fpletz July 16, 2020 13:56
@worldofpeace worldofpeace changed the title pinentry: 1.1.0 -> ae58404 pinentry: 1.1.0 -> 2020-07-17 Jul 16, 2020
@bagnaram
Copy link
Contributor Author

bagnaram commented Sep 9, 2020

Any further action needed for this PR?

Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Changes look okay, do not see anything problematic in the upstream commits since 1.1.0: https://dev.gnupg.org/source/pinentry/history/master/

@ajs124
Copy link
Member

ajs124 commented Oct 8, 2020

This should be rebased, instead of having master merged into it.

@bagnaram bagnaram force-pushed the feature/pinentry-efl branch 2 times, most recently from b56849a to c37f243 Compare November 11, 2020 02:47
@bagnaram bagnaram closed this Nov 11, 2020
@bagnaram bagnaram reopened this Nov 11, 2020
@bagnaram
Copy link
Contributor Author

Squashed and rebased to recent master. Please advise further action

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Can you please update the commit title to pinentry: 1.1.0 -> 2020-07-17?

pkgs/tools/security/pinentry/default.nix Outdated Show resolved Hide resolved
pkgs/tools/security/pinentry/default.nix Outdated Show resolved Hide resolved
@bagnaram
Copy link
Contributor Author

bagnaram commented Dec 2, 2020

Thank you for the comments. Updated repo with suggestions

@jtojnar
Copy link
Contributor

jtojnar commented Dec 2, 2020

And the branch commit history still looks messy/not squashed.

@jtojnar jtojnar self-requested a review December 2, 2020 01:47
@@ -57,7 +60,6 @@ pinentryMkDerivation rec {
dontWrapQtApps = true;

patches = [
./autoconf-ar.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, without this patch, cross-compilation (pkgsCross.aarch64-multiplatform.pinentry) fails:

/nix/store/pmrhk324fkidrm5ffd5jckb21s9zys6r-bash-4.4-p23/bin/bash: ar: command not found

cc @nspin who added it in #68889

@bagnaram bagnaram force-pushed the feature/pinentry-efl branch 3 times, most recently from acd0858 to fc72c90 Compare December 2, 2020 02:42
@stale
Copy link

stale bot commented Jul 21, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021
@fpletz
Copy link
Member

fpletz commented Jul 30, 2022

Superseeded by #143120.

@fpletz fpletz closed this Jul 30, 2022
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.

None yet

5 participants