Skip to content

Fix PHP 8.4 builds on Windows #131

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

Merged
merged 7 commits into from
Dec 26, 2024
Merged

Conversation

SRWieZ
Copy link
Member

@SRWieZ SRWieZ commented Dec 17, 2024

Fix the following:

  • Fix: "7za | tar" pipe not working on the bash shell
  • Fix: php-bin directory not existing when you fork the project without a previous build cache
  • Fix: used the cli generator to update essential dependencies

Unfortunately, this does not resolve the PHP 8.4 Windows build issue, but it does fix and improve the current build action.

The last remaining error is related to the following patch that @mpociot commented on: static-php/phpmicro#3 (comment)

If you're curious about my research that led to those commits, see https://github.com/SRWieZ/fork-nativephp-php-bin/pull/1.

@SRWieZ SRWieZ changed the title Fix: windows builds & PHP 8.4 Fix and improve build action Dec 17, 2024
@SRWieZ SRWieZ marked this pull request as ready for review December 17, 2024 14:12
@SRWieZ
Copy link
Member Author

SRWieZ commented Dec 17, 2024

Research is complete and ready to be merged.

During the build, there is a linking issue with php_win32_init_gettimeofday, which cannot be found. This is due to PHP 8.4 using an alternative function from the Windows system, which has made this Windows polyfill outdated.

A patch from php-micro is applied when building the CLI version, but it is outdated with the recent changes in PHP 8.4. Once this is resolved, everything should work smoothly.

I suggest merging my PR to address the current unrelated problems and waiting for this fix to be implemented.

Have a good day!

@SRWieZ
Copy link
Member Author

SRWieZ commented Dec 25, 2024

I did something silly.

While taking a shower, I had an idea that I didn't think would actually work, but it did. I had to learn about the .patch syntax—never thought I would! 😄

In the end, Windows 8.4 builds work! 🎉 See the build and the PR.

Is it stupid if it works?


Afterward, I did some research, and the original patch for the Windows CLI is based on this file:

I can safely assume that this line is no longer needed and that we don't require new code here either.

I will submit a pull request to the appropriate repository later.

@SRWieZ SRWieZ changed the title Fix and improve build action Fix PHP 8.4 builds on Windows Dec 25, 2024
Copy link
Member

@simonhamp simonhamp left a comment

Choose a reason for hiding this comment

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

While I'd definitely prefer that these patches happen upstream, I think for now this is fine.

It just feels like we'd have a lot more to manage long-term if we try to take care of this here and it could make our GH Actions kinda flakey... so I'd avoid adding further patch logic here

@simonhamp simonhamp merged commit 5a7dd0d into NativePHP:main Dec 26, 2024
@SRWieZ
Copy link
Member Author

SRWieZ commented Dec 26, 2024

Of course!

I have already created a PR upstream: static-php/phpmicro#5.

The minute it's merged, I will propose another PR to clean this up.

@simonhamp
Copy link
Member

Ouchy, looks like this broke some builds:

https://github.com/NativePHP/php-bin/actions/runs/12531270993

@SRWieZ
Copy link
Member Author

SRWieZ commented Dec 29, 2024

Yeah I see why, sed doesn't work the same on unix and windows. PR incoming.

@SRWieZ
Copy link
Member Author

SRWieZ commented Dec 29, 2024

Here: #133

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