Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support multiple PHP_INI_SCAN_DIR's #2941

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Nov 11, 2024

Description

In case a user sets the PHP_INI_SCAN_DIR environment variable to a special value with a leading or trailing : or has multiple directories separated by the default path separator, we need to handle this accordingly.

In case they use the trailing or leading : to just add or prepend to the default directory, there is no way for us to find that original value anymore, so we "just" always use the first explicitly defined path.

This is documented upstream at https://www.php.net/manual/en/configuration.file.php#configuration.file.scan

Thanks @rp-thomas for reporting

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@realFlowControl realFlowControl requested a review from a team as a code owner November 11, 2024 10:52
@realFlowControl realFlowControl force-pushed the florian/support-multi-php-ini-scan-dir branch from 599d8d6 to 4ab0132 Compare November 11, 2024 10:54
@realFlowControl realFlowControl self-assigned this Nov 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.97%. Comparing base (7b487bd) to head (dcf645a).
Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (7b487bd) and HEAD (dcf645a). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (7b487bd) HEAD (dcf645a)
tracer-php 12 11
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2941      +/-   ##
============================================
- Coverage     82.10%   73.97%   -8.13%     
  Complexity     2527     2527              
============================================
  Files           108      108              
  Lines         10360    10360              
============================================
- Hits           8506     7664     -842     
- Misses         1854     2696     +842     
Flag Coverage Δ
tracer-php 73.97% <ø> (-8.13%) ⬇️

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

see 8 files 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 7b487bd...dcf645a. Read the comment docs.

@realFlowControl realFlowControl force-pushed the florian/support-multi-php-ini-scan-dir branch from 4ab0132 to f922472 Compare November 11, 2024 11:01
@realFlowControl realFlowControl added cat:installation Issues while installing the tracer profiling Relates to the Continuous Profiler tracing and removed profiling Relates to the Continuous Profiler tracing labels Nov 11, 2024
@realFlowControl realFlowControl force-pushed the florian/support-multi-php-ini-scan-dir branch from f922472 to 6479b61 Compare November 11, 2024 11:25
datadog-setup.php Outdated Show resolved Hide resolved
Co-authored-by: Bob Weinand <[email protected]>
Copy link
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

@realFlowControl realFlowControl merged commit 1c8e506 into master Nov 11, 2024
411 of 475 checks passed
@realFlowControl realFlowControl deleted the florian/support-multi-php-ini-scan-dir branch November 11, 2024 13:36
@github-actions github-actions bot added this to the 1.5.0 milestone Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:installation Issues while installing the tracer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants