[Nix] Provide c-ares cmake config
ClosedPublic
Actions

Authored by • jon on Sep 13 2022, 4:26 PM.

Details

Reviewers

atul
varun
• abosh

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rCOMM264a7f504ba1: [Nix] Provide c-ares cmake config

Summary

Nixpkgs provides just a file, however, this doesn't get picked up
automatically by cmake during the configuration step (e.g. cmake ..).
Installing the file at <store path>/lib/cmake/c-ares/c-ares-config.cmake makes
it readily available.

This fixes the issue where cmake will then try other locations to find the file
which causes it to pick up on other installed packages such as c-ares installed
install through homebrew. Similar to D5106.

Example failures: https://buildkite.com/comm/tunnelbroker-unittests/builds/1#01833852-6772-424a-9e9b-332fe70fce85
Example success with this fix: https://buildkite.com/comm/tunnelbroker-unittests/builds/2#_

Test Plan

Build succeeds: https://buildkite.com/comm/tunnelbroker-unittests/builds/2#_

Or, test locally:

nix develop
cd services/tunnelbroker
rm -rf build && mkdir build && cd build && cmake ..
# should see "-- Found c-ares: /nix/store/.../c-ares-config.cmake (found version "1.18.1")"

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jon created this revision.Sep 13 2022, 4:26 PM

Owners added a reviewer: Restricted Owners Package.Sep 13 2022, 4:27 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 13 2022, 4:27 PM

• jon added a child revision: D5129: [CI] Add tunnelbroker unittests.Sep 13 2022, 4:30 PM

Harbormaster completed remote builds in B12124: Diff 16642.Sep 13 2022, 4:37 PM

• jon requested review of this revision.Sep 13 2022, 4:37 PM

Actually, just use cmake to build c-ares

Harbormaster completed remote builds in B12126: Diff 16644.Sep 13 2022, 5:46 PM

Got the following as expected:

-- Found c-ares: /nix/store/1axpb3xdsicc3fcchak26mkblcbsigvg-c-ares-1.18.1/lib/cmake/c-ares/c-ares-config.cmake (found version "1.18.1")

However, no idea what nativeBuildInputs = (o.nativeBuildInputs or []) ++ [ prev.cmake ]; means (I just don't know enough nix), so will defer to the O1: Nix blocking reviewers on that.

varun accepted this revision.Sep 13 2022, 11:04 PM

This revision is now accepted and ready to land.Sep 13 2022, 11:04 PM

However, no idea what nativeBuildInputs = (o.nativeBuildInputs or []) ++ [ prev.cmake ];

Pretty much, "take the previous nativeBuildInputs, if there are any, and add cmake to it"

Closed by commit rCOMM264a7f504ba1: [Nix] Provide c-ares cmake config (authored by • jon). · Explain WhySep 13 2022, 11:47 PM

This revision was automatically updated to reflect the committed changes.

• jon added a commit: rCOMM264a7f504ba1: [Nix] Provide c-ares cmake config.

Just wondering, what do we need c-ares for?

Just wondering, what do we need c-ares for?

We don't directly consume it. But when you do find_package(gRPC), it eventually will look for c-ares as well. Essentially it's a transitive dependency.

This was an attempt in having the cmake "fallback" logic picking up something from homebrew.

michal mentioned this in D5219: [native] Unify thread settings accessibility.Sep 23 2022, 5:49 AM