Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@pmatilai approved this pull request. Ok, I think we're good to go now. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/938#pullrequestreview-436342767___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
Merged #938 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/938#event-3475894319___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
Rebased and fixed the `%m` thing. Apologies for the long "round-trip delay time" on this PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/938#issuecomment-648253508___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 2 commits. 5b3831f7d5d76bfefcd2a28a5aabddc39866da19 GPG: Switch back to pipe(7) for signing e91e2ad55e18131aea621f3c2e6772bc891d1aa4 GPG: refactor: clean up exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/5e81da9a19f576ad35d6358ed3d29787f8a708cc..e91e2ad55e18131aea621f3c2e6772bc891d1aa4 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks commented on this pull request.
>
if (gpg_path && *gpg_path != '\0')
(void) setenv("GNUPGHOME", gpg_path, 1);
+ if (tty)
+ setenv("GPG_TTY", tty, 0);
+ else if (!getenv("GPG_TTY"))
+ rpmlog(RPMLOG_WARNING, _("Could not set GPG_TTY to stdin: %m\n"));
Yeah, this was a bit sloppy, I agree. The `getenv()` test was supposed to
handle the case where the automatic setup of `GPG_TTY` failed and the variable
wasn't explicitly set by the user either. Let me try to express that better in
a followup amend :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/938#discussion_r444301782___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 2 commits. 7080e2456d513d54538129a4d414848591b49508 GPG: Switch back to pipe(7) for signing 5e81da9a19f576ad35d6358ed3d29787f8a708cc GPG: refactor: clean up exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/fe877767eb2350c1537f163e5f07d9ab950f5ab6..5e81da9a19f576ad35d6358ed3d29787f8a708cc ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
...but other than the nit about the setenv/getenv thing, looks fine. Special thanks for the thorough explanation of the issues in the commit message! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/938#issuecomment-642446435___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@pmatilai commented on this pull request.
>
if (gpg_path && *gpg_path != '\0')
(void) setenv("GNUPGHOME", gpg_path, 1);
+ if (tty)
+ setenv("GPG_TTY", tty, 0);
+ else if (!getenv("GPG_TTY"))
+ rpmlog(RPMLOG_WARNING, _("Could not set GPG_TTY to stdin: %m\n"));
This seems a bit fishy: technically, setenv() can fail too, but this getenv()
check would not catch that even if the message basically says to do so. Instead
that %m might be related to ttyname() or setenv() for GNUPGHOME failing, or
even earlier, getenv() does not set errno itself at all. I'd suggest moving
this section before the GNUPGHOME section and doing the GPG_TTY environment
check always.
Seems to me this should be moved section should be moved before the
gpg_path/GNUPGHOME thing to eliminate that ambiguity, and the getenv("GPG_TTY")
check always performed (ie remove the 'else').
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/938#pullrequestreview-427995405___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 2 commits. 9d638d25afc3f211671ce93192cf99af8a679948 GPG: Switch back to pipe(7) for signing c471ad104992c950e42afd12079c67c43642841e GPG: refactor: clean up exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/14593f9924be7fbe6a4ba9849b0fd833b8fc83a5..c471ad104992c950e42afd12079c67c43642841e ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@pmatilai I've taken a different approach (by re-introducing the pipe), details in the commit message. Please review when you get a chance. Thanks! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/938#issuecomment-637398084___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 2 commits. 88d650776bfc93bf4fd863a7081aeb889bd04bf1 GPG: Switch back to pipe(7) for signing a58d462040774da53f91d8388b3bdd0b86916c25 GPG: refactor: exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/4540e1a699758cef5f33c49d37e1a6561265d9f5..a58d462040774da53f91d8388b3bdd0b86916c25 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 2 commits. 838c9c6dbdc974563e4f8d7301638991ed247950 GPG: Switch back to pipe(7) for signing 927790f8808488626993f33c88d97ca1755d4c3d GPG: refactor: exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/940b93a130654f4d7bb0a94cacda395c26d7c7e2..927790f8808488626993f33c88d97ca1755d4c3d ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 2 commits. ab87b1ef439f7bc74302cf4e36720711d2ae93f5 GPG: Switch back to pipe(7) for signing 940b93a130654f4d7bb0a94cacda395c26d7c7e2 GPG: refactor: exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/2d0b1f6c07dce3885d7d2761c6e1c98aa22b83b0..940b93a130654f4d7bb0a94cacda395c26d7c7e2 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] GPG: Switch back to pipe(7) for signing (#938)
@dmnks pushed 1 commit. 2d0b1f6c07dce3885d7d2761c6e1c98aa22b83b0 GPG: refactor: exit label -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/938/files/285d1823ca30f4a19bf7058b248d2dfba428a11b..2d0b1f6c07dce3885d7d2761c6e1c98aa22b83b0 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
