Skip to content

Conversation

henderkes
Copy link
Collaborator

What does this PR do?

@dunglas I hope this is the idiomatic way to achieve this?

Checklist before merging

@crazywhalecc adding frankenphp as a library is a bit of a dirty hack to depend on the source to be downloaded. better ideas?

@henderkes henderkes force-pushed the frankenphp/mbed branch 2 times, most recently from 064c57a to c56646f Compare September 5, 2025 14:47
@crazywhalecc
Copy link
Owner

crazywhalecc commented Sep 7, 2025

No better idea too, unless we implement fetch-on-build. If the frankenphp pull barely affects your normal non-frankenphp builds, then I think that's acceptable.

However, if you want to implement fetch-on-build for frankenphp alone, you could just add it as a source and then manually call Downloader before processFrankenphpApp.

@henderkes
Copy link
Collaborator Author

I thought about fetch-on-build, but then remembered that you wanted to keep all network calls and downloads in the download stage (which makes sense, github release fetching is surprisingly prone to failure).

Frankenphp source is fairly small, I think it's acceptable too.

Copy link
Owner

@crazywhalecc crazywhalecc left a comment

Choose a reason for hiding this comment

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

Will we still use dunglas ?

* Process the --with-frankenphp-app option
* Creates app.tar and app.checksum in source/frankenphp directory
*/
protected function processFrankenphpApp(): void
Copy link
Owner

Choose a reason for hiding this comment

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

How about processing frankenphp app before building everything? Although I don't have any build requirements for frankenphp embed app, but if something goes wrong here, it would be annoying to wait for dozens of minutes to build and then report an error.

Copy link
Owner

@crazywhalecc crazywhalecc left a comment

Choose a reason for hiding this comment

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

Will we still use dunglas ?

@henderkes
Copy link
Collaborator Author

Will we still use dunglas ?

@dunglas not sure about plans to switch the go package name. The frankenphp.dev PR has been active for months.

@dunglas
Copy link
Contributor

dunglas commented Sep 9, 2025

Currently yes, we still use dunglas. It may be possible to migrate thanks to Go 1.25, which fixed some issues blocking the migration, but I didn't have the time to take another look and it's low priority TBH.

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