Skip to content

Link test binaries statically when run coverage#39354

Merged
phlax merged 1 commit intoenvoyproxy:mainfrom
krinkinmu:static-coverage
May 6, 2025
Merged

Link test binaries statically when run coverage#39354
phlax merged 1 commit intoenvoyproxy:mainfrom
krinkinmu:static-coverage

Conversation

@krinkinmu
Copy link
Copy Markdown
Contributor

@krinkinmu krinkinmu commented May 6, 2025

Commit Message:

Static linking helps to work around the issue with LLVM/Clang source-based coverage (see
llvm/llvm-project#32849). On the flip side, the coverage build and test run will take a bit longer (e.g., around 30m) with this change.

This PR switches to static linking for just test coverage and does not do the same for fuzz coverage. That's because Envoy CI is hitting some resource constraints when building fuzz targets with coverage instrumentation.

We will fix that by striping the fuzz targets of some unncessary dependencies and switching to EngFlow backend for coverage builds and tests, but that will require addressing a couple of hard to understand issues, so, for now, I'm just switching the coverage tests to static linking and will follow up with the fuzz tests later once we addressed all the blockers and switched to EngFlow backend for coverage.

Additional Description:

#39030 provides some context for EngFlow migration and #39248 is a tracking bug.

Risk Level: low
Testing: regular Envoy CI
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

+cc @phlax @yanavlasov

Static linking helps to work around the issue with LLVM/Clang
source-based coverage (see
llvm/llvm-project#32849).

This PR switches to static linking for just test coverage and does not
do the same for fuzz coverage. That's because Envoy CI is hitting some
resource constraints when building fuzz targets with coverage
instrumentation.

We will fix that by striping the fuzz targets of some unncessary
dependencies and switching to EngFlow backend for coverage builds and
tests, but that will require addressing a couple of hard to understand
issues, so, for now, I'm just switching the coverage tests to static
linking and will follow up with the fuzz tests later once we addressed
all the blockers and switched to EngFlow backend for coverage.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #39354 was opened by krinkinmu.

see: more, trace.

@krinkinmu krinkinmu marked this pull request as ready for review May 6, 2025 12:12
Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @krinkinmu

@phlax phlax merged commit 2c5d7c3 into envoyproxy:main May 6, 2025
24 checks passed
phlax pushed a commit to phlax/envoy that referenced this pull request Jun 11, 2025
Static linking helps to work around the issue with LLVM/Clang
source-based coverage (see
llvm/llvm-project#32849). On the flip side,
the coverage build and test run will take a bit longer (e.g., around
30m) with this change.

This PR switches to static linking for just test coverage and does not
do the same for fuzz coverage. That's because Envoy CI is hitting some
resource constraints when building fuzz targets with coverage
instrumentation.

We will fix that by striping the fuzz targets of some unncessary
dependencies and switching to EngFlow backend for coverage builds and
tests, but that will require addressing a couple of hard to understand
issues, so, for now, I'm just switching the coverage tests to static
linking and will follow up with the fuzz tests later once we addressed
all the blockers and switched to EngFlow backend for coverage.

Additional Description:

envoyproxy#39030 provides some context for
EngFlow migration and envoyproxy#39248
is a tracking bug.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
phlax pushed a commit to phlax/envoy that referenced this pull request Jun 11, 2025
Static linking helps to work around the issue with LLVM/Clang
source-based coverage (see
llvm/llvm-project#32849). On the flip side,
the coverage build and test run will take a bit longer (e.g., around
30m) with this change.

This PR switches to static linking for just test coverage and does not
do the same for fuzz coverage. That's because Envoy CI is hitting some
resource constraints when building fuzz targets with coverage
instrumentation.

We will fix that by striping the fuzz targets of some unncessary
dependencies and switching to EngFlow backend for coverage builds and
tests, but that will require addressing a couple of hard to understand
issues, so, for now, I'm just switching the coverage tests to static
linking and will follow up with the fuzz tests later once we addressed
all the blockers and switched to EngFlow backend for coverage.

Additional Description:

envoyproxy#39030 provides some context for
EngFlow migration and envoyproxy#39248
is a tracking bug.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax pushed a commit that referenced this pull request Jun 11, 2025
Static linking helps to work around the issue with LLVM/Clang
source-based coverage (see
llvm/llvm-project#32849). On the flip side,
the coverage build and test run will take a bit longer (e.g., around
30m) with this change.

This PR switches to static linking for just test coverage and does not
do the same for fuzz coverage. That's because Envoy CI is hitting some
resource constraints when building fuzz targets with coverage
instrumentation.

We will fix that by striping the fuzz targets of some unncessary
dependencies and switching to EngFlow backend for coverage builds and
tests, but that will require addressing a couple of hard to understand
issues, so, for now, I'm just switching the coverage tests to static
linking and will follow up with the fuzz tests later once we addressed
all the blockers and switched to EngFlow backend for coverage.

Additional Description:

#39030 provides some context for
EngFlow migration and #39248
is a tracking bug.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax pushed a commit to phlax/envoy that referenced this pull request Jun 11, 2025
Static linking helps to work around the issue with LLVM/Clang
source-based coverage (see
llvm/llvm-project#32849). On the flip side,
the coverage build and test run will take a bit longer (e.g., around
30m) with this change.

This PR switches to static linking for just test coverage and does not
do the same for fuzz coverage. That's because Envoy CI is hitting some
resource constraints when building fuzz targets with coverage
instrumentation.

We will fix that by striping the fuzz targets of some unncessary
dependencies and switching to EngFlow backend for coverage builds and
tests, but that will require addressing a couple of hard to understand
issues, so, for now, I'm just switching the coverage tests to static
linking and will follow up with the fuzz tests later once we addressed
all the blockers and switched to EngFlow backend for coverage.

Additional Description:

envoyproxy#39030 provides some context for
EngFlow migration and envoyproxy#39248
is a tracking bug.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
phlax pushed a commit to phlax/envoy that referenced this pull request Jun 11, 2025
Static linking helps to work around the issue with LLVM/Clang
source-based coverage (see
llvm/llvm-project#32849). On the flip side,
the coverage build and test run will take a bit longer (e.g., around
30m) with this change.

This PR switches to static linking for just test coverage and does not
do the same for fuzz coverage. That's because Envoy CI is hitting some
resource constraints when building fuzz targets with coverage
instrumentation.

We will fix that by striping the fuzz targets of some unncessary
dependencies and switching to EngFlow backend for coverage builds and
tests, but that will require addressing a couple of hard to understand
issues, so, for now, I'm just switching the coverage tests to static
linking and will follow up with the fuzz tests later once we addressed
all the blockers and switched to EngFlow backend for coverage.

Additional Description:

envoyproxy#39030 provides some context for
EngFlow migration and envoyproxy#39248
is a tracking bug.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants