Skip to content

Fix build on macos (probs with case insensitivity)#3549

Closed
cataphract wants to merge 1 commit intomasterfrom
glopes/rename-version
Closed

Fix build on macos (probs with case insensitivity)#3549
cataphract wants to merge 1 commit intomasterfrom
glopes/rename-version

Conversation

@cataphract
Copy link
Contributor

Description

Having a file named VERSION in the root makes it difficult to use the root as an include path (and from there have more meaningful include paths, like #include <components-rs/telemetry.h> instead of possibly conflicting #include <telemetry.h>.

This is because on mac os, #include <version>, which sometimes comes from other libraries/stdlib, will try to include this file.

Rather than trying to mess with the exact order of the components of the include path (I think this is even complicated because -I has even priority over -isystem iirc), rename VERSION.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@cataphract cataphract requested review from a team as code owners December 24, 2025 13:44
@cataphract cataphract force-pushed the glopes/rename-version branch from 864dd92 to f12de2b Compare December 24, 2025 13:46
@pr-commenter
Copy link

pr-commenter bot commented Dec 24, 2025

Benchmarks [ profiler ]

Benchmark execution time: 2025-12-24 14:51:51

Comparing candidate commit efef243 in PR branch glopes/rename-version with baseline commit 575faf4 in branch master.

Found 0 performance improvements and 6 performance regressions! Performance is the same for 24 metrics, 6 unstable metrics.

scenario:php-profiler-timeline-memory-control

  • 🟥 cpu_user_time [+33.112ms; +42.328ms] or [+5.469%; +6.992%]
  • 🟥 execution_time [+35.682ms; +39.849ms] or [+5.639%; +6.298%]

scenario:php-profiler-timeline-memory-with-profiler

  • 🟥 cpu_system_time [+15.260ms; +34.774ms] or [+5.446%; +12.409%]
  • 🟥 execution_time [+22.204ms; +42.277ms] or [+2.391%; +4.553%]

scenario:php-profiler-timeline-memory-with-profiler-and-timeline

  • 🟥 cpu_user_time [+37.275ms; +77.284ms] or [+2.399%; +4.973%]
  • 🟥 execution_time [+24.935ms; +50.784ms] or [+2.049%; +4.173%]

@cataphract cataphract force-pushed the glopes/rename-version branch from f12de2b to cbce4e9 Compare December 24, 2025 14:00
@cataphract cataphract force-pushed the glopes/rename-version branch from cbce4e9 to efef243 Compare December 24, 2025 14:39
@cataphract cataphract requested a review from a team as a code owner December 24, 2025 14:39
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.65%. Comparing base (575faf4) to head (efef243).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3549      +/-   ##
==========================================
+ Coverage   61.64%   61.65%   +0.01%     
==========================================
  Files         139      139              
  Lines       13051    13051              
  Branches     1712     1712              
==========================================
+ Hits         8045     8047       +2     
+ Misses       4241     4238       -3     
- Partials      765      766       +1     

see 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 575faf4...efef243. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link

pr-commenter bot commented Dec 24, 2025

Benchmarks [ tracer ]

Benchmark execution time: 2025-12-24 15:47:50

Comparing candidate commit efef243 in PR branch glopes/rename-version with baseline commit 575faf4 in branch master.

Found 2 performance improvements and 7 performance regressions! Performance is the same for 184 metrics, 1 unstable metrics.

scenario:ComposerTelemetryBench/benchTelemetryParsing

  • 🟩 execution_time [-1357.325ns; -442.675ns] or [-11.803%; -3.849%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟥 execution_time [+9.486µs; +10.134µs] or [+9.629%; +10.288%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟥 execution_time [+11.778µs; +12.962µs] or [+11.473%; +12.626%]

scenario:SamplingRuleMatchingBench/benchRegexMatching1

  • 🟥 execution_time [+79.963ns; +151.237ns] or [+6.747%; +12.762%]

scenario:SamplingRuleMatchingBench/benchRegexMatching2

  • 🟥 execution_time [+63.085ns; +150.715ns] or [+5.301%; +12.664%]

scenario:SamplingRuleMatchingBench/benchRegexMatching3

  • 🟥 execution_time [+48.540ns; +125.860ns] or [+3.987%; +10.338%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4

  • 🟥 execution_time [+65.186ns; +143.014ns] or [+5.479%; +12.020%]

scenario:SpanBench/benchOpenTelemetryAPI

  • 🟥 mem_peak [+1.437MB; +1.463MB] or [+3.457%; +3.518%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-37.087µs; -21.813µs] or [-8.433%; -4.960%]

@cataphract
Copy link
Contributor Author

I'm going on a less intrusive route: creating a directory with symlinks to the relevant subdirectories and using that as the include path

@cataphract cataphract closed this Dec 24, 2025
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