Skip to content

fix(build): Link gmock into the vector index aggregate test - #19942

Merged
meta-codesync[bot] merged 2 commits into
facebookincubator:mainfrom
galipremsagar:fix-vector-index-test-gmock
Oct 30, 2026
Merged

meta-codesync[bot] merged 2 commits into
facebookincubator:mainfrom
galipremsagar:fix-vector-index-test-gmock

Conversation

@galipremsagar

@galipremsagar galipremsagar commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

VectorIndexBuildTest.cpp (added in #19886) includes gmock/gmock.h, but velox_aggregates_VectorIndexTest links only the gtest targets, so its compile line carries no googlemock include directory. On macOS, where googletest is no longer installed from Homebrew and only the bundled copy exists, every macOS Build run on main since #19886 landed fails with:

velox/functions/prestosql/aggregates/tests/VectorIndexBuildTest.cpp:17:10: fatal error: 'gmock/gmock.h' file not found

For example run 40951976246 (at 4eb621c) and run 41026575083 (at 0bb7029). Linux builds pass because the system gmock headers sit on the default include path.

This links GTest::gmock into the target, as the other gmock-using tests do.

Note: macOS CI currently fails even earlier, while building Folly against Homebrew's OpenSSL 4 on the new macos-15 runner image (#19936 addresses that). This change fixes the failure that sits behind it, so the macOS job here can only turn green once #19936 has landed and main is merged into this branch.

@netlify

netlify Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit d2b0d8a
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/6ae5081f3320d888822bf1ca

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 30, 2026
VectorIndexBuildTest.cpp, added in facebookincubator#19886, includes gmock/gmock.h, but
velox_aggregates_VectorIndexTest links only the gtest targets, so its
compile line carries no googlemock include directory. On macOS, where
googletest is no longer installed from Homebrew and only the bundled
copy exists, every macOS Build run on main since facebookincubator#19886 landed fails
with:

    velox/functions/prestosql/aggregates/tests/VectorIndexBuildTest.cpp:17:10:
    fatal error: 'gmock/gmock.h' file not found

Linux builds pass because the system gmock headers sit on the default
include path. Link GTest::gmock into the target, as the other
gmock-using tests do.
@galipremsagar
galipremsagar force-pushed the fix-vector-index-test-gmock branch from 720c829 to 7b30513 Compare October 30, 2026 17:57
@github-actions

Copy link
Copy Markdown

CI Failure Analysis

Auto-generated by the CI Failure Analysis workflow. This comment is updated in place each time CI fails on a new commit, so it always reflects the latest run — re-pushing or re-running CI will refresh the analysis below. Last updated 2026-10-30 18:25:23 UTC from workflow run 41102758241.

❌ Build — INFRA Failure View logs

Build compilation succeeded (1970/1970 targets linked). The failure occurred in the post-build signature check step, not during compilation.

Root cause: Azure Blob Storage egress limit (HTTP 503)

The CI workflow tried to download cached function signatures from a previous main-branch build, but the Azure artifact storage returned an HTTP 503 error:

error downloading function-signatures-10d88f9129d3cd53c3b1d45c8c7be8db73ebdb3d-main:
HTTP 503: 503 Egress is over the account limit.

This caused the file /tmp/signatures/presto_signatures_main.json to never be downloaded. Later, the signature bias check script tried to open this missing file and crashed:

Traceback (most recent call last):
  File "/__w/velox/velox/velox/scripts/ci/signature.py", line 292, in gh_bias_check
    bias_status = bias(bias_args)
  File "/__w/velox/velox/velox/scripts/ci/signature.py", line 177, in bias
    with open(args.base) as f:
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/signatures/presto_signatures_main.json'

The ccache artifact download also failed with the same HTTP 503 egress error.


Correlation with PR changes:

  • ✅ Not related to PR changes. This PR only adds GTest::gmock as a link dependency in velox/functions/prestosql/aggregates/tests/CMakeLists.txt. The failure is an infrastructure issue with Azure Blob Storage hitting its egress bandwidth limit, which prevented the signature cache artifact from being downloaded. The build itself compiled successfully.

Known issues:

  • No open issues found tracking this specific failure.
  • The last 5 runs on the main branch all passed successfully, confirming this is a transient infrastructure issue, not a pre-existing failure.

Recommended fix:

  • Re-run the CI workflow. This is a transient Azure storage egress limit error. The signature download is wrapped in error handling (# Catch errors in the download with || to avoid the whole workflow failing), but the downstream signature.py script does not gracefully handle the missing file. Re-running CI should resolve it once the Azure egress limit resets.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Selective Build Plan

Linux release with adapters is running a full build (PR has a standing approval). See the CI workflows README for what this means.


Selective build plan

@pedroerp

Copy link
Copy Markdown
Contributor

@galipremsagar already has a fixed out: #19945

Closing this one so we can get theirs merged.

@pedroerp pedroerp closed this Oct 30, 2026
@pedroerp

Copy link
Copy Markdown
Contributor

Apologies for the mixup, I meant to make the comment above in my other PR: #19936

This one is also good. Will get them merged.

@pedroerp pedroerp reopened this Oct 30, 2026
@pedroerp pedroerp added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Oct 30, 2026
@meta-codesync

meta-codesync Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

@pedroerp has imported this pull request. If you are a Meta employee, you can view this in D128692602.

meta-codesync Bot pushed a commit that referenced this pull request Oct 30, 2026
Homebrew's unversioned `openssl` formula has pointed at OpenSSL 4 since 2026-09-27 (Homebrew/homebrew-core 9a12b94a88), and that formula is no longer keg-only, so OpenSSL 4 headers and libraries are linked under `$(brew --prefix)`. The macos-15 runner image 20261028.0489.1 ships it, and since it rolled out every `macOS Build` run fails while installing dependencies, because Folly at `FB_OS_VERSION` (v2026.09.07.00) reads `ASN1_STRING` fields directly and assigns `X509_get_subject_name()` to a non-const pointer, both of which OpenSSL 4 removed:

    folly/io/async/ssl/OpenSSLUtils.cpp:126:62: error: member access into incomplete type 'ASN1_OCTET_STRING' (aka 'asn1_string_st')

Moving `FB_OS_VERSION` forward is not enough on its own: Folly supports OpenSSL 4 from v2026.10.12.00 (wangle from v2026.09.21.00), but the gRPC 1.48.1 that Velox bundles for its tests fails the same way (`src/core/tsi/ssl_transport_security.cc`; fixed upstream only in gRPC 1.85.0), as the FBOS bump in #19909 shows (run 41084854386).

- `scripts/setup-macos.sh` installs `openssl@3` and exports `OPENSSL_ROOT_DIR` for it, so every dependency it builds uses OpenSSL 3. A caller that already set `OPENSSL_ROOT_DIR` keeps its choice.
- `cmake_install` also passes `OPENSSL_ROOT_DIR` as a cache variable. fbthrift ships its own `FindOpenSSL.cmake` that ignores the environment and otherwise runs `brew --prefix openssl`, which now resolves to OpenSSL 4.
- The macOS and pyvelox workflows carry `OPENSSL_ROOT_DIR` into their later steps, which run in a new shell.
- `CMakeLists.txt` defaults a macOS configure to Homebrew's OpenSSL 3 when no OpenSSL root was chosen (and warns when `openssl@3` is missing), so a plain `make` in a fresh shell keeps matching the dependencies built by the script.
- On macOS the OpenSSL include directory is added first: `Folly::folly_deps` exports `$(brew --prefix)/include` ahead of it, where OpenSSL 4's headers now live, so Velox sources that include `<openssl/*.h>` would otherwise compile against OpenSSL 4 headers while linking OpenSSL 3.

This can be reverted once the pinned Folly and gRPC releases both build against OpenSSL 4.

Related: #19936 and #19257 pin `openssl@3` the same way. This PR additionally covers fbthrift, whose own `FindOpenSSL.cmake` ignores `OPENSSL_ROOT_DIR` from the environment (the macOS run of #19936, 41096009930, configures fbthrift against `/opt/homebrew/opt/openssl@4` while folly, fizz, wangle and mvfst use `openssl@3`), and the include order on Velox's own compile lines.

Note: with dependencies building again, the macOS Build step still fails on `main` with `VectorIndexBuildTest.cpp:17:10: fatal error: 'gmock/gmock.h' file not found` (from #19886); #19942 fixes that, so the macOS job here can only turn green once it has landed and `main` is merged into this branch.

Differential Revision: D128692789

Pull Request resolved: #19945
@meta-codesync
meta-codesync Bot merged commit 9e25a47 into facebookincubator:main Oct 30, 2026
55 of 56 checks passed
tinadu1018 added a commit to tinadu1018/velox that referenced this pull request Oct 30, 2026
## Summary
Merge upstream main at 8f6c946 into the
FBOS upgrade branch. Align the signature-comparison baseline with upstream
changes already present in CI's merged checkout.

Keep upstream's OpenSSL 3 selection and targeted VectorIndex GoogleMock
linkage. Remove our superseded OpenSSL selection block, duplicate setup
export, and broad aggregates-test GoogleMock dependency. The remaining
PR diff contains only the dependency upgrade and its FBThrift/xxhash
compatibility fixes.

## Prompting Intent
Fix the reported Fuzzer Jobs failures reasonably without pipeline,
test-selection, coverage, or unnecessary component changes. The engineer
explicitly approved a normal merge of upstream main, without history
rewriting or force-pushing, and requested inspection of macOS 15 Debug.

## Linked Sources
- Upgrade PR: facebookincubator#19909
- Failed Fuzzer Jobs: https://github.com/facebookincubator/velox/actions/runs/41132095439
- Baseline and contender provenance: https://github.com/facebookincubator/velox/actions/runs/41132095439/job/124325288109
- Upstream timezone signature change: facebookincubator#19632
- Upstream collect_set companion: facebookincubator#19894
- Upstream OpenSSL fix: facebookincubator#19945
- Upstream targeted GoogleMock fix: facebookincubator#19942
- Current macOS Debug job: https://github.com/facebookincubator/velox/actions/runs/41132095473/job/124331293031
- Presto mismatch: https://github.com/facebookincubator/velox/actions/runs/41132095439/job/124333376983

## Rationale
CI compared signatures saved for merge base 5df6f9b with a contender
containing upstream commits c853cce and b8e1609. Those upstream commits
intentionally removed at_timezone_convert and added collect_set_merge_extract.
Do not restore removed functions, filter signatures, modify skip lists, or
replace the failing jobs. Updating the branch baseline avoids attributing
inherited upstream changes to this dependency upgrade.

Upstream now supplies the macOS build fixes, so preserve its implementation
instead of keeping duplicate or broader local workarounds. Retain the
version/checksum updates and the proven compiler-export and xxhash-target
compatibility fixes. No independent fix for the saved Presto value/null
mismatch is claimed by this merge.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fdd7945e-1c8f-4e79-ac56-0fbeeedaffbd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants