-
-
Notifications
You must be signed in to change notification settings - Fork 349
Fix/micro patches #811
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
Fix/micro patches #811
Conversation
Co-authored-by: Copilot <[email protected]>
|
Seems it broke windows builds. I'll figure it out tomorrow. |
|
Seems it has no patch file checking before applying patches. The directory separator caused the patch does not exist and failed to patch, but no exception. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors how micro patches are applied by centralizing patch logic into the existing patchFile method and updating default patch lists in configuration.
- Removed legacy
FileSystem::addSourceExtractHook('micro', …)and simplifiedpatchMicrosignature. - Switched manual file concatenation in
patchMicroto reusepatchFile. - Updated
BuilderBaseto invokepatchMicroafter source extraction and trimmedSPC_MICRO_PATCHESdefaults inenv.ini.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/SPC/store/SourcePatcher.php | Simplified patchMicro, enhanced patchFile to handle absolute paths |
| src/SPC/builder/BuilderBase.php | Added invocation of SourcePatcher::patchMicro() after ext extraction |
| config/env.ini | Reduced default SPC_MICRO_PATCHES entries for Win32 and macOS contexts |
Comments suppressed due to low confidence (3)
src/SPC/store/SourcePatcher.php:107
- Update the docblock above
patchMicroto reflect its new signature (removing$nameand$target, and documenting the$itemsparameter correctly).
public static function patchMicro(?array $items = null): bool
src/SPC/store/SourcePatcher.php:176
- Inconsistent handling of missing patch files: sometimes returns
falseand later throws an exception. Consider standardizing on one approach for missing patches.
return false;
src/SPC/store/SourcePatcher.php:107
- [nitpick] The parameter name
$itemsis ambiguous; consider renaming it to something more descriptive like$patchesto clarify its purpose.
public static function patchMicro(?array $items = null): bool
| } else { | ||
| $patch_file = $patch_name; | ||
| } | ||
| if (!file_exists($patch_file)) { |
Copilot
AI
Jun 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The initial file_exists($patch_file) check makes the subsequent file_exists($patch_str) check redundant; consider removing one or consolidating the logic.
| if (\Phar::running() !== '') { | ||
| file_put_contents(SOURCE_PATH . '/' . $patch_name, file_get_contents($patch_file)); | ||
| $patch_str = str_replace('/', DIRECTORY_SEPARATOR, SOURCE_PATH . '/' . $patch_name); | ||
| if (str_starts_with($patch_str, 'phar://')) { |
Copilot
AI
Jun 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking str_starts_with($patch_str, 'phar://') may never detect the PHAR context correctly since $patch_str is a filesystem path. Consider using \Phar::running() to detect runtime in a PHAR archive as before.
| if (str_starts_with($patch_str, 'phar://')) { | |
| if (\Phar::running() !== '') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
What does this PR do?
Fix #803 .
This PR will put micro patches after init souces and reuse
patchFilefunction to automatically skip the patched patches.Checklist before merging
*.phpor*.json, run them locally to ensure your changes are valid:PHP_CS_FIXER_IGNORE_ENV=1 composer cs-fixcomposer analysecomposer testbin/spc dev:sort-configsrc/globals/test-extensions.php.extension testortest extensionsto trigger full test suite.