Skip to content

Conversation

JMoodyFWD
Copy link
Contributor

@JMoodyFWD JMoodyFWD commented May 6, 2025


name: 🐞 Bug Fix - Change TOOLS_DIR to SETUP_PHP_TOOLS_DIR to prevent regression
about: Rename "TOOLS_DIR" to "SETUP_PHP_TOOLS_DIR" to prevent regression issue.
labels: enhancement


Related discussion:
#943 - Original Issue
#937 - Original PR by @Sn0wCrack
#937 (comment) - Proposed fix by @shivammathur

Description

As outlined in issue #943 the "TOOLS_DIR" env variable encounters an issue. @shivammathur mentioned changing "TOOLS_DIR" to "SETUP_PHP_TOOLS_DIR" to prevent this regression issue. This PR makes that change.

  • I have checked the edited scripts for syntax.
  • I have tested the changes in an integration test (If yes, provide workflow YAML and link).

Workflow Action Test: https://github.com/JMoodyFWD/cphalcon/actions/runs/14865714594
Workflow Action YAML: https://github.com/JMoodyFWD/cphalcon/actions/runs/14865714594/workflow

Copy link

codecov bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (316da6e) to head (dc7e73e).
Report is 6 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop      #945   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          811       811           
  Branches       293       293           
=========================================
  Hits           811       811           

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shivammathur shivammathur merged commit d000f49 into shivammathur:develop May 6, 2025
66 checks passed
@mvorisek
Copy link

Is is possible that this fixes also #956? If yes, why?

@JMoodyFWD
Copy link
Contributor Author

Is is possible that this fixes also #956? If yes, why?

This PR fixed #943 and has fixed any other similar situations locally on my system. I have not done a deep dive to find out why specifically the issue occurred in the first place, but I have been able to verify on multiple unique use-cases that this type of error no longer occurs with this change.

My best guess is that "TOOLS_DIR" was too ambiguous of an env name and is referenced in other libraries that setup-php is used in conjunction with, which causes a conflict. By giving a more granular env name of "SETUP_PHP_TOOLS_DIR", it prevents a conflict of the same env name being used.

I would have to assume that since #956 is referencing the same issues relating to composer that I've seen on my local tests that this PR would also fix this issue, but as there hasn't been an official release since before this PR, you would need to specify using the "develop" branch of setup-php in your workflow to utilize this change or await an official release.

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.

3 participants