Skip to content

Conversation

@xmkg
Copy link
Member

@xmkg xmkg commented Jan 20, 2026

Description

  • What does this PR do?
    • Removes the vcpkg gRPC portfile overlay in favor of per package customizations
  • Why is this change needed?
    • Maintaining the existing portfile overlay and the custom patch makes us lag behind the upstream

MULTI-2334

index dcdd5a0..8d43c3f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -497,7 +497,7 @@ if (NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/third_party/xds AND gRPC_DOWNLOAD_ARC
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the target to eliminate. Porting this patch to newer releases is a real headache since the patch touches a rather large list of moving parts.

@codecov
Copy link

codecov bot commented Jan 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.64%. Comparing base (27d32d7) to head (efdd90b).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4621   +/-   ##
=======================================
  Coverage   87.64%   87.64%           
=======================================
  Files         250      250           
  Lines       14276    14276           
=======================================
  Hits        12511    12511           
  Misses       1765     1765           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
@xmkg xmkg force-pushed the enhancement/update-grpc-to-1-71.0-v2 branch from 44d5f01 to 8b13425 Compare January 21, 2026 07:55
Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
@xmkg xmkg changed the base branch from enhancement/migrate-libssh-to-vcpkg to main January 21, 2026 08:30
Copy link
Contributor

@tobe2098 tobe2098 left a comment

Choose a reason for hiding this comment

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

LGTM Mustafa, great job! Good to see those red numbers.

@sharder996
Copy link
Collaborator

I notice a significant increase in compilation time:

On main:

ninja  310.05s user 76.58s system 139% cpu 4:37.45 total

and on efdd90b:

ninja  1640.84s user 321.99s system 558% cpu 5:51.18 total

Just from watching the builds, I notice test_daemon_* are taking quite a while to compile. Is there maybe a difference in linking against grpc?

@xmkg
Copy link
Member Author

xmkg commented Jan 22, 2026

I notice a significant increase in compilation time:

On main:

ninja  310.05s user 76.58s system 139% cpu 4:37.45 total

and on efdd90b:

ninja  1640.84s user 321.99s system 558% cpu 5:51.18 total

Just from watching the builds, I notice test_daemon_* are taking quite a while to compile. Is there maybe a difference in linking against grpc?

Yeah... I was expecting no impact on link times, but it looks like that's not the case. Which os/toolchain/linker is this? The toolchain default?

@sharder996
Copy link
Collaborator

I notice a significant increase in compilation time:
On main:

ninja  310.05s user 76.58s system 139% cpu 4:37.45 total

and on efdd90b:

ninja  1640.84s user 321.99s system 558% cpu 5:51.18 total

Just from watching the builds, I notice test_daemon_* are taking quite a while to compile. Is there maybe a difference in linking against grpc?

Yeah... I was expecting no impact on link times, but it looks like that's not the case. Which os/toolchain/linker is this? The toolchain default?

This was a standard setup on macOS: Apple clang/default linker

% clang --version
Apple clang version 17.0.0 (clang-1700.3.19.1)
Target: arm64-apple-darwin25.2.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

I do also have ccache installed, but this wasn't the first time I did a build on this branch. So, ccache won't be totally optimized, but I don't think it should account for the difference in build times that I see here.

@tobe2098
Copy link
Contributor

tobe2098 commented Jan 23, 2026

My tests (Ubuntu VM -j4):
main:

real	13m9.156s
user	46m42.496s
sys	2m43.183s

This branch:

real	12m48.602s
user	45m36.284s
sys	2m38.582s

No caches

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.

4 participants