Feat/register phpinfo entries by henderkes · Pull Request #2578 · php/frankenphp

henderkes · 2026-07-29T09:56:04Z

Edit: the entries are sorted now.

@dunglas I know you've worked on the watcher-go library before, perhaps you could expose a proper version number instead?

…ercure versions

henderkes · 2026-07-29T11:55:20Z

@dunglas I think we should revert the last commit and make not passing the nobrotli tag a build error when libbrotli isn't available. It would shift the runtime error to a compile time error.

dunglas · 2026-07-29T21:15:22Z

For watcher, this is because of e-dant/watcher#102

henderkes · 2026-08-04T16:30:23Z

@dunglas I'm not sure if we really should rename AddPhpinfoEntry to AddPHPInfoEntry. Phpinfo refers to the phpinfo() function to print this.

dunglas · 2026-08-04T17:47:03Z

I'm not sure either but this sounds more idiomatic to me. People who will call this function will do it in Go, it in PHP.

henderkes · 2026-08-04T18:32:43Z

Alright, I'm okay with it like this either way.

…ised

henderkes · 2026-08-13T07:55:05Z

Reminder to self: cli registration logic is now split between <= 8.5 and >= 8.6. Needs a rebase.

…_entries # Conflicts: # cli.go # cli_test.go # frankenphp.c

henderkes · 2026-08-21T13:10:07Z

Cli registration now works in both paths (<=8.5 and >=8,.6). Last commit is technically unrelated to the branch, I just got tired of the tests failing locally due to my ini settings.

All points resolved

Copilot

🟡 Changes recommended

The primary Caddy-version behavior lacks direct integration coverage, and minor documentation issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds FrankenPHP, Caddy, Go toolchain, and linked-module metadata to phpinfo() across server and CLI modes.

Changes:

Registers and renders sorted PHP info entries.
Reports versions for Caddy, Mercure, watcher, Brotli, and Go modules.
Extends CLI lifecycle and metadata tests.

File summaries

File	Description
`worker_test.go`	Enables displayed HTML errors.
`watcher.go`	Registers watcher metadata.
`types_test.go`	Tests module-entry generation.
`testdata/command-detached.php`	Adds detached-process fixture.
`server_test.go`	Enables displayed errors.
`mercure.go`	Registers Mercure metadata.
`go.mod`	Promotes `x/sys` to direct dependency.
`frankenphp.h`	Declares PHP info arrays.
`frankenphp.go`	Builds and manages metadata entries.
`frankenphp.c`	Renders metadata and registers CLI module.
`frankenphp_test.go`	Tests rendered metadata.
`emulate_php_cli.c`	Enables text-mode CLI phpinfo.
`cli.go`	Initializes metadata for CLI execution.
`cli_test.go`	Expands CLI behavior coverage.
`cli_linux_test.go`	Tests detached Linux children.
`caddy/caddy.go`	Registers Caddy version.
`caddy/br.go`	Registers Brotli metadata.

Review details

Files reviewed: 17/17 changed files
Comments generated: 3
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 	caddy.RegisterModule(&FrankenPHPAdmin{})

+	// Report Caddy version in phpinfo()
+	simpleVersion, fullVersion := caddy.Version()


…_entries # Conflicts: # frankenphp.go

Copilot

🟡 Changes recommended

The requested Caddy-version path still lacks a Caddy-package integration assertion, as noted in the unresolved review thread.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 19/19 changed files
Comments generated: 0 new
Review effort level: Balanced

AlliBalliBaba · 2026-09-09T09:20:44Z

+func init() {
+	buildInfo, ok := debug.ReadBuildInfo()
+	if !ok {
+		return
+	}
+
+	AddPHPInfoEntry("go", buildInfo.GoVersion)
+
+	goModuleEntries = buildGoModuleEntries(buildInfo)
+}


Would it make sense to call this only when phpinfo() is called and not on init()?

We could do it, but it's more code, locking for the first phpinfo call, and we'd need to hook phpinfo. I don't think it's worth it given it's just a one-time cost on Init.

I don't think we need to hook into phpinfo, we just need to fetch the info from go in the PHP_MINFO_FUNCTION, unless I'm missing something.

Would only need pinning, no allocations.

PHP_MINFO_FUNCTION(frankenphp) { char **frankenphp_phpinfo_entries = go_get_frankenphp_info_entries() ....

Maybe even simpler would be to call C.php_info_print_table_row(...) directly from go, which does not even require pinning (unless this is a macro)

Ah that's what you meant. I still don't really think it's advantageous over what we have though,

I could still go for pinning instead of C allocations though. May help reduce code a little.

In that case I'd prefer having it all in one place, but no hard block from me

Do you mean the module version reads? The reason they're scattered is because I want to only show versions of modules the build actually has enabled. If we just registered everything at once in frankenphp.go, it would always list e.g. mercure even if-tags=nomercure was used.

The phpinfo logic stuff we can move out to its own go file.

Hmm is the module still present if it is built with -tags=nomercure? In that case I guess this is the only way, otherwise I'd prefer just looping once over debug.ReadBuildInfo() when the PHP_MINFO_FUNCTION is called instead of having it spread out with many init()

Yeah, the module is still present in build info. We could read the build info once and then toggle the main entries from the module, but honestly it doesn't really matter.

I should try moving all the logic into its own file where possible though. It really clutters frankenphp.go a bit.

henderkes added 5 commits July 29, 2026 15:24

feat: register phpinfo info entries under frankenphp extension

922de2d

report e-dant/watcher, dunglas/caddy-cbrotli, libbrotli and dunglas/m…

ce102e5

…ercure versions

keep go array, only convert to c array in init function

032a742

clang-format

98aadc4

why is this missing in CI? @dunglas

87a0874

dunglas previously requested changes Aug 4, 2026

View reviewed changes

Comment thread frankenphp.go Outdated

Comment thread frankenphp.go Outdated

Comment thread mercure.go

henderkes force-pushed the feat/register_phpinfo_entries branch 2 times, most recently from 33a4a54 to f1f8ca5 Compare August 4, 2026 16:25

rename method

f06d146

henderkes force-pushed the feat/register_phpinfo_entries branch from f1f8ca5 to f06d146 Compare August 4, 2026 16:29

henderkes added 3 commits August 11, 2026 18:32

test for phpinfo as plaintext

fdbf473

suggestion by @dunglas - also include all go modules and go version

ef483ee

don't capitalise Go version in PHPInfo entry, nothing else is capital…

b95c966

…ised

henderkes requested a review from dunglas August 11, 2026 19:18

henderkes added 4 commits August 21, 2026 12:00

Merge remote-tracking branch 'origin/main' into feat/register_phpinfo…

bec8ae3

…_entries # Conflicts: # cli.go # cli_test.go # frankenphp.c

amend cli test

df91cab

hook frankenphp_module into cli execution too

1bffc57

make sure tests set display_errors=1 when they rely on it

c3950da

henderkes mentioned this pull request Sep 2, 2026

Possible use-after-free of the Go backing array of extensions retained by register_extensions #2629

Open

Merge branch 'main' into feat/register_phpinfo_entries

72c3a25

henderkes added 2 commits September 5, 2026 23:51

fix cli metadata without server runtime hooks

0296a89

restore extension registration hooks after cli execution

8019e98

henderkes added 4 commits September 6, 2026 00:15

respect module replacements in component version entries

083ba46

include main module in phpinfo module inventory

f8eabf6

test cli behavior across startup and shutdown

5ea3d90

test phpinfo module metadata and escaped rendering

c604428

henderkes requested a balanced review from Copilot September 5, 2026 22:24

Copilot started reviewing on behalf of henderkes September 5, 2026 22:25 View session

Copilot AI reviewed Sep 5, 2026

View reviewed changes

Comment thread caddy/caddy.go Outdated

caddy.RegisterModule(&FrankenPHPAdmin{})

// Report Caddy version in phpinfo()

simpleVersion, fullVersion := caddy.Version()

Comment thread cli_test.go Outdated

Comment thread frankenphp.go Outdated

henderkes added 6 commits September 6, 2026 00:40

reword comment to shut copilot up

acaad7e

Merge remote-tracking branch 'origin/main' into feat/register_phpinfo…

0df6262

…_entries # Conflicts: # frankenphp.go

fix: zero phpinfo arrays before Go pointer writes

78136e7

fix windows stream closing issue

2a4d1d3

fix linux amd64 race tests

44cfc28

satisfy go fmt

1b7a973

henderkes requested a balanced review from Copilot September 8, 2026 13:23

Copilot started reviewing on behalf of henderkes September 8, 2026 13:23 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

henderkes requested review from AlliBalliBaba and alexandre-daubois September 8, 2026 14:38

alexandre-daubois reviewed Sep 8, 2026

View reviewed changes

Comment thread frankenphp.c

Comment thread frankenphp.c Outdated

Comment thread caddy/caddy.go Outdated

suggestions

6d5a4fd

henderkes force-pushed the feat/register_phpinfo_entries branch from 632e412 to 6d5a4fd Compare September 8, 2026 17:21

henderkes requested a review from alexandre-daubois September 8, 2026 17:27

safety fix for userland closing non-duplicate streams

c64dd5b

AlliBalliBaba reviewed Sep 9, 2026

View reviewed changes

Comment thread watcher.go

henderkes added 5 commits September 9, 2026 16:23

pin instead of manual c memory management

873ca4a

switch to caddy's simpleVersion

d055b38

move phpinfo helpers into phpinfo.go

90a1fb0

centralize phpinfo module version lookup

f991a3a

lazily collect phpinfo metadata through minfo callback

2b6244b

Uh oh!

Conversation

henderkes commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henderkes commented Jul 29, 2026

Uh oh!

dunglas commented Jul 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

henderkes commented Aug 4, 2026

Uh oh!

dunglas commented Aug 4, 2026

Uh oh!

henderkes commented Aug 4, 2026

Uh oh!

henderkes commented Aug 13, 2026

Uh oh!

henderkes commented Aug 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlliBalliBaba Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henderkes Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

henderkes commented Jul 29, 2026 •

edited

Loading

AlliBalliBaba Sep 9, 2026 •

edited

Loading

henderkes Sep 9, 2026 •

edited

Loading