pinentry-mac: 1.1.0.3 (with support for Apple Silicon) #68265

LinusU · 2021-01-04T17:16:14Z

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

This pull request adds to more patches (GPGTools/pinentry-mac#8 and GPGTools/pinentry-mac#9) in order to build a binary that is can run on Apple Silicon.

As it currently is, the bottle for Big Sur ARM64 contains a binary that cannot run on Apple Silicon, which looks like this:

/opt/homebrew/bin/pinentry-mac: line 2: /opt/homebrew/Cellar/pinentry-mac/0.9.4/pinentry-mac.app/Contents/MacOS/pinentry-mac: Bad CPU type in executable
/opt/homebrew/bin/pinentry-mac: line 2: /opt/homebrew/Cellar/pinentry-mac/0.9.4/pinentry-mac.app/Contents/MacOS/pinentry-mac: Undefined error: 0

Since the current bottle is broken, I also bumped the revision to 1.

In order to get brew audit --strict pinentry-mac to pass, I also had to update the licence field which was done in a separate commit. The commit message includes a link to where I found the license.

Formula/pinentry-mac.rb

fxcoudert · 2021-01-04T20:35:58Z

The two commits are not from upstream:

This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository

It seems to me like the software is not actually maintained.

carlocab · 2021-01-04T20:40:29Z

This is part of https://gpgtools.org, which is maintained. They ship pinentry-mac as part of GPG Suite. The main repo has recent commits: https://github.com/GPGTools/MacGPG2

fxcoudert · 2021-01-05T00:15:55Z

They ship pinentry-mac as part of GPG Suite

Then we want to build it from the source that is maintained (https://releases.gpgtools.org/GPG_Suite-2020.2.txz), instead of the repo, I suppose.

What I'm saying is simply: we don't want to carry patches that have been opened at a github repo that hasn't seen any activity for years, and where our previous patches for 10.14 support (!) still haven't been merged or commented upon.

fxcoudert · 2021-01-05T14:00:09Z

What I'm wondering is: this is a formula with 2,458 monthly installs, on user request (not as dependency). This must be because there is a guide/tutorial/doc somewhere that recommends installing this. We need to tell them we are deprecating this, unless it is maintained, and seek suggestions on what users should use instead.

fxcoudert · 2021-01-05T14:02:25Z

I've asked GPGTools at https://twitter.com/fxcoudert/status/1346456781204168704
But I think since they're selling the binaries, they are not actively maintaining the open source version

SMillerDev · 2021-01-05T14:03:47Z

I think it's the only way to sign git commits on macOS. Or at least the recommended way to manage the certificates for it.

LinusU · 2021-01-05T14:04:32Z

Then we want to build it from the source that is maintained (https://releases.gpgtools.org/GPG_Suite-2020.2.txz), instead of the repo, I suppose.

Hmm, I tried downloading that file and building pinentry from it, but didn't have much luck. It seems like maybe some files are missing, but I'm not too familiar with the build tools to tell...

I also tried to investigate why the build was failing on Intel Macs, and I'm at a point where I don't really understand it at all. If I run brew install --build-from-source --debug pinentry-mac on an Intel Mac, the make step fails. I can then open a shell there to investigate.

Running make makes the build fail with the clang compiler does not support '-march=nehalem'. But running the only command that make runs (xcodebuild -project pinentry-mac.xcodeproj -target pinentry-mac -configuration Release build) succeeds.

The only reference to nehalem I can find is in HOMEBREW_OPTFLAGS, so my only conclusion is that that variable is somehow used when invoking via make.

I also realised that the middle patch isn't actually required, so I've updated the commit to only apply that patch, and only apply it for arm builds.

This means that the only thing that is changed for arm builds is to add arm64 to the VALID_ARCHS list.

What I'm saying is simply: we don't want to carry patches that have been opened at a github repo that hasn't seen any activity for years, and where our previous patches for 10.14 support (!) still haven't been merged or commented upon.

I understand this sentiment, since it's not Homebrews place to maintain the software. How do you feel about this when it's just patching VALID_ARCHS? We could also do it with something like:

inreplace "pinentry-mac.xcodeproj/project.pbxproj", /VALID_ARCHS = "(.*)";/, "VALID_ARCHS = \"\\1 arm64\";"

or write out an xcconfig file like is done in the mas formula:

homebrew-core/Formula/mas.rb

Lines 24 to 32 in 9adfd41

    
               # - Ensure dependencies build for the current CPU; otherwise Commandant will 
        
               #   build for x86_64 when running arm64 
        
               xcconfig = buildpath/"Overrides.xcconfig" 
        
               xcconfig.write <<~EOS 
        
                 GCC_TREAT_WARNINGS_AS_ERRORS = NO 
        
                 OTHER_LDFLAGS = -headerpad_max_install_names 
        
                 VALID_ARCHS = #{Hardware::CPU.arch} 
        
               EOS 
        
               ENV["XCODE_XCCONFIG_FILE"] = xcconfig

LinusU · 2021-01-05T14:08:37Z

What I'm wondering is: this is a formula with 2,458 monthly installs, on user request (not as dependency). This must be because there is a guide/tutorial/doc somewhere that recommends installing this. We need to tell them we are deprecating this, unless it is maintained, and seek suggestions on what users should use instead.

I installed it since I wanted to sign git commits. On my previous (Intel) Mac I have used their commercial offering (because I thought it was hard to setup the open source version), but since that didn't have native M1 support (without installing Rosetta) I went through the extra trouble.

Now that I know how easy it was to get commit signing to work without the commercial package I will definitely prefer this way in the future.

I've asked GPGTools at https://twitter.com/fxcoudert/status/1346456781204168704

Neat 👍

carlocab · 2021-01-05T14:26:52Z

I think it's the only way to sign git commits on macOS. Or at least the recommended way to manage the certificates for it.

It's not the only way, but it certainly makes signing commits a lot easier, as it allows you to use Keychain with gpg.

But I think since they're selling the binaries, they are not actively maintaining the open source version

My understanding is that their binaries are the same as the open-source version. What they sell are support plans to go with the binaries.

fxcoudert · 2021-01-05T14:56:48Z

their binaries are the same as the open-source version

So their binaries don't build on 10.14 and later? Seems unlikely… GPGTools/pinentry-mac#7

carlocab · 2021-01-05T15:00:06Z

Well, they say their software is still open source:

GPG Suite was released under an Open Source license in the past. Is that stil the case?
GPG Suite including GPG Mail is still going to be released under an Open Source license.

https://gpgtools.org/faq

carlocab · 2021-01-06T01:16:37Z

They also might be a bit more responsive on their forums: https://gpgtools.tenderapp.com/discussions/problems

SMillerDev · 2021-01-08T10:03:13Z

Added https://gpgtools.tenderapp.com/discussions/problems/109039-maintenance-status-pinentry-mac

LinusU · 2021-01-09T11:26:20Z

@SMillerDev it seems like they made the thread private, did they give you an answer? can you share it?

SMillerDev · 2021-01-09T11:28:29Z

thanks for bringing this up. We will take a closer look at the open PRs when we find more time. We are currently looking into the outstanding M1 support for MacGPG and fixing the most important bugs in the 2020.2 release.

It sounds like they still mean to support it but just forgot it existed?

zeha · 2021-01-11T00:15:52Z

IIRC upstream builds their binaries from this repo/branch: https://github.com/GPGTools/pinentry/tree/dev

LinusU · 2021-01-12T15:02:28Z

Nice find @zeha!

It seems like the "pinentry" folder in GPG_Suite-2020.2.txz is the "macosx" folder in https://github.com/GPGTools/pinentry/tree/dev.

It's still not straight forward how to actually build it though, but I managed to get out a binary with theses steps:

autoreconf --install
autoconf
./configure --disable-ncurses
make

It does however fail after building macosx and moving on to doc, due to a missing file doc/version.texi. I haven't been able to pass a flag to skip building doc, and I haven't figured out what should be in that file.

edit: the binary built with these instructions reports itself as "pinentry-mac (pinentry) 1.1.0" as opposed to the previous "pinentry-mac (pinentry) 0.9.4", so this seems to be a newer version ☺️

lukele · 2021-01-14T17:27:33Z

Hi! We are sorry for not chiming in here earlier. It appears that the brew formula is based on a repository which has not been in use by us for quite some time. The pinentry-mac repository contains an old version which was disconnected from the upstream version of pinentry. We have since integrated pinentry-mac into a fork of the upstream pinentry, as the idea was to have it included in the upstream version (which has yet to happen however).

So the always up to date version of pinentry can be found at https://github.com/GPGTools/pinentry (use the dev branch).

@LinusU: You have to add the configure option --enable-maintainer-mode in order to have it build the doc target.

@SMillerDev I remember that in the past (quite some time ago) when a formula had a dependency on gnupg and MacGPG was installed, brew was happy to accept our version instead. This seems to no longer be the case, which unfortunately leads to compatibility issues if our users have installed both versions.

In order to support all features of pinentry-mac it would also make sense to add the following patch to gpg-agent:
https://github.com/GPGTools/MacGPG2/blob/dev/patches/gnupg/agent-cache-bug-workaround.patch

carlocab · 2021-01-14T17:31:18Z

Thanks for the response, @lukele. There are no releases tagged in https://github.com/GPGTools/pinentry. Is it possible for you to tag the stable release there?

lukele · 2021-01-14T18:10:32Z

@carlocab The stable release is now tagged: v1.1.0.3

carlocab · 2021-01-14T18:12:30Z

Thanks, @lukele.

@LinusU, can you update this PR to use the tarball at https://github.com/GPGTools/pinentry/archive/v1.1.0.3.tar.gz ?

samuelsson · 2021-01-15T20:49:20Z

So should this be replacing all installations of pinentry-mac, not just for when it's Hardware::CPU.arm? So it won't be using an old (from 2015) version. Or will we need another fix for that if we also want agent-cache-bug-workaround.patch included as @lukele mentioned above.

carlocab · 2021-01-15T20:52:20Z

So should this be replacing all installations of pinentry-mac, not just for when it's Hardware::CPU.arm?

Yes, it should. That will be handled by the fact that the switch in source will be a version bump.

Or will we need another fix for that if we also want agent-cache-bug-workaround.patch included

Just include the patch in the new version of the formula, the version bump takes care of the rest.

In some parts the license is defined as GPL 3 or later: https://github.com/GPGTools/pinentry-mac/blob/8d811e25d58e88d804f61e2bcba3b0b2ddef402f/Source/main.m#L13-L16 At other places it's GPL 2 or later: https://github.com/GPGTools/pinentry-mac/blob/8d811e25d58e88d804f61e2bcba3b0b2ddef402f/Source/pinentry-0.9.4/pinentry/password-cache.h#L6-L9

carlocab · 2021-01-18T11:29:57Z

Thanks, @LinusU!

BrewTestBot · 2021-01-18T11:48:13Z

🤖 A scheduled task has triggered a merge.

LinusU · 2021-01-20T15:00:34Z

Submitted #69405 as a follow up with the agent-cache-bug-workaround.patch

carlocab reviewed Jan 4, 2021

View reviewed changes

Formula/pinentry-mac.rb Outdated Show resolved Hide resolved

LinusU force-pushed the pinentry-mac-m1 branch from cfd0602 to fda64df Compare January 4, 2021 17:27

carlocab reviewed Jan 4, 2021

View reviewed changes

Formula/pinentry-mac.rb Outdated Show resolved Hide resolved

carlocab requested a review from fxcoudert January 4, 2021 19:40

LinusU force-pushed the pinentry-mac-m1 branch from fda64df to d44053b Compare January 5, 2021 13:52

LinusU force-pushed the pinentry-mac-m1 branch from d44053b to 35926dd Compare January 18, 2021 10:53

LinusU changed the title ~~pinentry-mac: add support for Apple Silicon~~ pinentry-mac: 1.1.0.3 (with support for Apple Silicon) Jan 18, 2021

pinentry-mac: 1.1.0.3

23efec9

LinusU force-pushed the pinentry-mac-m1 branch from 35926dd to 23efec9 Compare January 18, 2021 11:02

fxcoudert approved these changes Jan 18, 2021

View reviewed changes

BrewTestBot closed this in 9e9672f Jan 18, 2021

LinusU mentioned this pull request Jan 20, 2021

gnupg: add patch to work with pinentry-mac #69405

Closed

5 tasks

BrewTestBot added the outdated PR was locked due to age label Feb 20, 2021

Homebrew locked as resolved and limited conversation to collaborators Feb 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pinentry-mac: 1.1.0.3 (with support for Apple Silicon) #68265

pinentry-mac: 1.1.0.3 (with support for Apple Silicon) #68265

LinusU commented Jan 4, 2021

fxcoudert commented Jan 4, 2021

carlocab commented Jan 4, 2021

fxcoudert commented Jan 5, 2021 •

edited

Loading

fxcoudert commented Jan 5, 2021

fxcoudert commented Jan 5, 2021

SMillerDev commented Jan 5, 2021

LinusU commented Jan 5, 2021

LinusU commented Jan 5, 2021

carlocab commented Jan 5, 2021

fxcoudert commented Jan 5, 2021

carlocab commented Jan 5, 2021

carlocab commented Jan 6, 2021

SMillerDev commented Jan 8, 2021

LinusU commented Jan 9, 2021

SMillerDev commented Jan 9, 2021

zeha commented Jan 11, 2021

LinusU commented Jan 12, 2021 •

edited

Loading

lukele commented Jan 14, 2021 •

edited

Loading

carlocab commented Jan 14, 2021

lukele commented Jan 14, 2021

carlocab commented Jan 14, 2021

samuelsson commented Jan 15, 2021

carlocab commented Jan 15, 2021

carlocab commented Jan 18, 2021

BrewTestBot commented Jan 18, 2021

LinusU commented Jan 20, 2021

pinentry-mac: 1.1.0.3 (with support for Apple Silicon) #68265

pinentry-mac: 1.1.0.3 (with support for Apple Silicon) #68265

Conversation

LinusU commented Jan 4, 2021

fxcoudert commented Jan 4, 2021

carlocab commented Jan 4, 2021

fxcoudert commented Jan 5, 2021 • edited Loading

fxcoudert commented Jan 5, 2021

fxcoudert commented Jan 5, 2021

SMillerDev commented Jan 5, 2021

LinusU commented Jan 5, 2021

LinusU commented Jan 5, 2021

carlocab commented Jan 5, 2021

fxcoudert commented Jan 5, 2021

carlocab commented Jan 5, 2021

carlocab commented Jan 6, 2021

SMillerDev commented Jan 8, 2021

LinusU commented Jan 9, 2021

SMillerDev commented Jan 9, 2021

zeha commented Jan 11, 2021

LinusU commented Jan 12, 2021 • edited Loading

lukele commented Jan 14, 2021 • edited Loading

carlocab commented Jan 14, 2021

lukele commented Jan 14, 2021

carlocab commented Jan 14, 2021

samuelsson commented Jan 15, 2021

carlocab commented Jan 15, 2021

carlocab commented Jan 18, 2021

BrewTestBot commented Jan 18, 2021

LinusU commented Jan 20, 2021

fxcoudert commented Jan 5, 2021 •

edited

Loading

LinusU commented Jan 12, 2021 •

edited

Loading

lukele commented Jan 14, 2021 •

edited

Loading