8000 Made all patches part of the PHPBrew source tree by morozov · Pull Request #1093 · phpbrew/phpbrew · GitHub
[go: up one dir, main page]
Skip to content

Made all patches part of the PHPBrew source tree#1093

Merged
morozov merged 1 commit intophpbrew:masterfrom
morozov:local-patches
Dec 13, 2019
Merged

Made all patches part of the PHPBrew source tree#1093
morozov merged 1 commit intophpbrew:masterfrom
morozov:local-patches

Conversation

@morozov
Copy link
Contributor
@morozov morozov commented Dec 12, 2019 •
edited
Loading

As it was pointed out in #1044 (comment), the remote patch sources aren't guaranteed to be always available in the dev environment, and it just happed to the freetype.patch:

$ phpbrew install 7.3.12 +neutral +gd
...
---> Fetching patch from https://git.archlinux.org/svntogit/packages.git/plain/trunk/freetype.patch?h=packages/php ...

Warning: file_get_contents(https://git.archlinux.org/svntogit/packages.git/plain/trunk/freetype.patch?h=packages/php): failed to open stream: HTTP request failed! HTTP/1.1 404 Not found
 in phar:///usr/local/bin/phpbrew/src/PhpBrew/PatchKit/DiffPatchRule.php on line 53

As of now, all patches should be included in the source tree to guarantee integrity. The checksum functionality is removed as no longer needed.

The patch sources are stored alongside the classes they are used by for better cohesion. Additionally, storing them as separate files would require creating a copy before applying anyways because the patch command wouldn't be able to use a file from within a PHAR. This issue is avoided by passing the patch to the CLI command via STDIN.

@morozov morozov added Improvement Patching PHP Resolving the issue requires patching PHP sources labels Dec 12, 2019
@morozov morozov self-assigned this Dec 12, 2019
@morozov morozov added this to the Release 1.25.0 milestone Dec 12, 2019
@morozov
Made all patches part of the PHPBrew source tree
cf4b2a0 
As of now, all patches should be included in the source tree to guarantee integrity. The checksum functionality is removed as no longer needed.

The patch sources are stored alongside the classes they are used by for better cohesion. Additionally, storing them as separate files would require creating a copy before applying anyways because the `patch` command wouldn't be able to use a file from within a PHAR. This issue is avoided by passing the patch to the CLI command via STDIN.
@morozov morozov merged commit 2e73ac6 into phpbrew:master Dec 13, 2019
@morozov morozov deleted the local-patches branch December 13, 2019 01:36
@morozov morozov added the Bug label Dec 14, 2019
@morozov morozov mentioned this pull request Dec 14, 2019
Merged
@morozov morozov added Apache2 GD OpenSSL Issues related to compiling PHP with OpenSSL labels Dec 17, 2019
@markwu
Copy link
Contributor
markwu commented Dec 24, 2019 •
edited
Loading

I get the patch file copy and paste again, and diff the file you committed.

It seems you remove all ^M carriage-return character from the patch, that's why it shows errors. Should we add them back?

image

@markwu markwu mentioned this pull request Dec 24, 2019
Merged
@morozov
Copy link
Contributor Author
morozov commented Dec 24, 2019

Thanks @markwu. I've filed #1123.

@morozov
Copy link
Contributor Author
morozov commented Dec 24, 2019

It seems you remove all ^M carriage-return character from the patch, thats' it shows errors. Should we add them back?

I copied the patch to the code via an IDE and it automatically converted the line endings.

@markwu
Copy link
Contributor
markwu commented Dec 24, 2019

Okay, tested. Everything works.

@morozov morozov mentioned this pull request Oct 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@morozov morozov

Labels

Apache2 Bug GD Improvement OpenSSL Issues related to compiling PHP with OpenSSL Patching PHP Resolving the issue requires patching PHP sources

Projects

None yet

Milestone

Release 1.25.0

Development

Successfully merging this pull request may close these issues.

2 participants

@morozov @markwu

Comments
0