Skip to content

Conversation

lavarou
Copy link
Member

@lavarou lavarou commented Jan 27, 2025

With the use of containerized build platforms, there's no need to vendor agent's build dependencies.

@lavarou lavarou requested a review from mfulb January 27, 2025 20:47
@lavarou lavarou self-assigned this Jan 27, 2025
@newrelic-php-agent-bot
Copy link

newrelic-php-agent-bot commented Jan 27, 2025

Test Suite Status Result
Multiverse 8/8 passing
SOAK 79/79 passing

@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.05%. Comparing base (9409457) to head (e28f504).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1012      +/-   ##
==========================================
- Coverage   77.40%   77.05%   -0.36%     
==========================================
  Files         198      198              
  Lines       27715    27717       +2     
==========================================
- Hits        21453    21357      -96     
- Misses       6262     6360      +98     
Flag Coverage Δ
agent-for-php-7.2 77.17% <ø> (-0.37%) ⬇️
agent-for-php-7.3 77.18% <ø> (-0.37%) ⬇️
agent-for-php-7.4 76.90% <ø> (-0.37%) ⬇️
agent-for-php-8.0 76.29% <ø> (-0.37%) ⬇️
agent-for-php-8.1 76.80% <ø> (-0.37%) ⬇️
agent-for-php-8.2 76.41% <ø> (-0.37%) ⬇️
agent-for-php-8.3 76.41% <ø> (-0.37%) ⬇️
agent-for-php-8.4 76.42% <ø> (-0.37%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@zsistla zsistla changed the title chore: remove venodor subdir chore: remove vendor subdir Feb 5, 2025
@lavarou lavarou force-pushed the chore/remove-venodor-subdir branch from d90a2f7 to d516758 Compare February 13, 2025 15:32
Abort build if protobuf-c cannot be found in the system without falling
back to building and using protobuf-c from vendor subdir. axiom build
needs protoc-c (provided by protobuf-c-compiler package) and agent needs
to link to protobuf-c static library (provided by protobuf-c-devel).
Static library is required to avoid shared objects runtime dependencies
on customers' systems.

Don't attempt to build protobuf compiler from vendor subdir - assume it
is installed and let the build fail if it is not.  protobuf compiler
alone is not enough for the rule that used it anyway - protoc-gen-go is
needed too, and it has never been vendored but rather was assumed to be
installed on the build system. The rule that uses protobuf compiler and
protoc-gen-go is not part of the build anyway - its output is checked in
to the repository. The rule exists as documentation how to regenerate
infinite tracing code for the daemon.
The agent uses protobuf-c library to provide support infinite tracing - infinite
tracing endpoint speaks protobuf.
@lavarou lavarou force-pushed the chore/remove-venodor-subdir branch from d516758 to e28f504 Compare February 18, 2025 18:28
export GIT

.PHONY: vendor vendor-clean
.PHONY: protobuf-c
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the export GIT line removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

export GIT was removed because it is not used anywhere. It used to be used in vendor/Makefile here and here and vendor/Makefile is removed in this PR. However, even vendor/Makefile used to set the default value for GIT if it was not set - see here.

@lavarou lavarou merged commit 8a993c3 into dev Feb 21, 2025
66 checks passed
@lavarou lavarou deleted the chore/remove-venodor-subdir branch February 21, 2025 16:07
lavarou added a commit that referenced this pull request Feb 25, 2025
Make the `HAVE_PROTOBUF_C != 0` check part of recipe where its needed
rather than making the check in a phony target. Using phony target results
in axiom rebuilding every time any target is build.

Fixup of #1012.
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.

5 participants