libfido2 1.3.0 (new formula)

[xanderlent]

(This is a revival of PR #46072, which is now used by OpenSSH.)

Co-authored-by: Alexander Lent [email protected]

• 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 --new-formula <formula> (after doing brew install <formula>)?

---

There are multiple open issues with this PR, I'll take another look at it later this week:

• libcbor is not yet upstream (see libcbor 0.5.0 (new formula) #50305 for why)
• Manual pages aren't installed in the correct location
• There are probably additional ways to test this library.
• Yubico's official releases page gives different URLs than the GitHub URL we're using, but the artifacts are identical.
• libudev is a Linux-only dependency
• Upstream prefers LibreSSL to OpenSSL. (Possibly true of OpenSSH-portable as well.) We're using OpenSSL for OpenSSH portable, probably best to link dependencies against the same library.

[SMillerDev]

Test suggestion

[xanderlent]

Questions for reviewers:

• Do you thing the second test is useful? It tests different functions with empty parameters, but I'm not sure it actually hits crypto codepaths, since the parameters are empty.
• Does the homebrew DSL support Linux-only dependencies, for ex depends_on "libuv" => :linux?
• Yubico prefers releases on their servers, though the tarballs are identical. Should we do the same?

I'm re-examining upstream's support for installing manual pages since they don't seem to be correctly installed in the first place. For now, I've got a partially working copy operation. Fixed.

[jonchang]

Do you thing the second test is useful? It tests different functions with empty parameters, but I'm not sure it actually hits crypto codepaths, since the parameters are empty.

If the Makefile provides a make check step we can use it in the install step to exercise the right code paths since this is a crypto library.

Does the homebrew DSL support Linux-only dependencies, for ex depends_on "libuv" => :linux?

uses_from_macos "libuv"

Yubico prefers releases on their servers, though the tarballs are identical. Should we do the same?

If they're identical we might as well use the Github releases since this means brew livecheck will "just work" with them.

[jonchang]

-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) 
-- Checking for one of the modules 'libcbor'
-- Checking for one of the modules 'libcrypto'
CMake Error at /usr/local/Cellar/cmake/3.16.4/share/cmake/Modules/FindPkgConfig.cmake:707 (message):
  None of the required 'libcrypto' found
Call Stack (most recent call first):
  CMakeLists.txt:82 (pkg_search_module)

This probably needs depends_on "pkg-config" => :build?

[gzm55]

-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) 
-- Checking for one of the modules 'libcbor'
-- Checking for one of the modules 'libcrypto'
CMake Error at /usr/local/Cellar/cmake/3.16.4/share/cmake/Modules/FindPkgConfig.cmake:707 (message):
  None of the required 'libcrypto' found
Call Stack (most recent call first):
  CMakeLists.txt:82 (pkg_search_module)

This probably needs depends_on "pkg-config" => :build?

append /usr/local/opt/[email protected]/lib/pkgconfig to PKG_CONFIG_PATH

[xanderlent]

Do you thing the second test is useful? It tests different functions with empty parameters, but I'm not sure it actually hits crypto codepaths, since the parameters are empty.

If the Makefile provides a make check step we can use it in the install step to exercise the right code paths since this is a crypto library.

After double-checking, it appears the regression tests are automatically run as part of the build, but only on debug builds. I assume that it would be unusual and unnecessarily complicated to build the formula in debug mode first solely to run tests?

Yubico prefers releases on their servers, though the tarballs are identical. Should we do the same?

If they're identical we might as well use the Github releases since this means brew livecheck will "just work" with them.

Thanks, I'll leave it as is.

[xanderlent]

Does the homebrew DSL support Linux-only dependencies, for ex depends_on "libuv" => :linux?

uses_from_macos "libuv"

Apologies, the dependency is libudev, which I don't think we package in homebrew-core.

Formula in Homebrew/linuxbrew-core have depends_on "systemd" if OS.linux? # for libudev but that seems to belong in linuxbrew for now, if I understand Homebrew/brew#7028 correctly.

[issyl0]

Formula in Homebrew/linuxbrew-core have depends_on "systemd" if OS.linux? # for libudev but that seems to belong in linuxbrew for now, if I understand Homebrew/brew#7028 correctly.

Correct. This repo is merged into Homebrew/linuxbrew-core a couple of times a day, then fixes for Linux dependencies are made in PRs there.

[xanderlent]

Great! I'll open a PR there with the necessary changes once this gets merged.

As a note to myself, those are:
depends_on "systemd" if OS.linux? # libfido2 uses libudev on Linux but IOKit on macOS
mv prefix/"man", share/"man" if OS.mac? # This path bug on macOS might be upstream.

[zbeekman]

LGTM, provided we can't or don't want to enable compile-time tests.

[zbeekman]

LGTM

There are no tests besides some regression tests that appear to get run at compile time, and fuzzing that only really happens on Linux. I added a suggestion to double check that the regression tests are being run, but they should be ™️ because it looks like the regress target was added to the special ALL CMake target.

[xanderlent]

@zbeekman If that was it, then we're good to merge.

Thanks to everyone for contributing to the review of this PR! I really appreciate the feedback and look forward to contributing in the future.

[jonchang]

Thanks again @xanderlent for contributing to Homebrew! This formula has been merged.