deduplicate registration-time duplicates from match directives,

[henderkes]

fixes #2477

it's a really bad workaround, but I can't think of a better way without a major rework for the match matching logic

[AlliBalliBaba]

I think the issue comes from Provision() being called twice, seems like caddy detects 2 modules (only happens with php_server, not php.

[AlliBalliBaba]

Talking about these 2 lines, it creates an extra module instead of using the same one (= provision twice)

frankenphp/caddy/module.go

Line 586 in 3f56208

HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject(phpsrv, "handler", "php", nil)},

frankenphp/caddy/module.go

Line 671 in 3f56208

caddyconfig.JSONModuleObject(f, "handler", "php", nil),

I think there's a way to do named routes and invoke the same module twice instead of having separate modules, not sure how messy that is in comparison.

[AlliBalliBaba]

Thinking about it again, cleanest option is probably a separate handler directive.

Something like this:

// in caddy.go:Init()
httpcaddyfile.RegisterHandlerDirective("php_without_provision", parseCaddyfileWithoutProvision)`

// in module.go
func parseCaddyfileWithoutProvision(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
	m := &FrankenPHPModule{ skipProvision:true }
	err := m.UnmarshalCaddyfile(h.Dispenser)

	return m, err
}

func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
	if f.skipProvision {
		return nil // this module should skip the provision step
	}
	// ...
}

[henderkes]

Thinking about it again, cleanest option is probably a separate handler directive.

Something like this:

// in caddy.go:Init()
httpcaddyfile.RegisterHandlerDirective("php_without_provision", parseCaddyfileWithoutProvision)`

// in module.go
func parseCaddyfileWithoutProvision(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
	m := &FrankenPHPModule{ skipProvision:true }
	err := m.UnmarshalCaddyfile(h.Dispenser)

	return m, err
}

func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
	if f.skipProvision {
		return nil // this module should skip the provision step
	}
	// ...
}

That would break worker serving on match paths, unless we were to still run the provision, but skip the registration logic. But at that point we still need a route group and it's complexity is the same as this.

[AlliBalliBaba]

Hmm yeah.

That makes me think we might finally need something like a frankenphp.PhpServer and just have both modules reference that.

[henderkes]

I very much agree, but that's a larger change that'll inevitably dig up some other dirt. If you want to go for it now, we can close this one, otherwise we can add it as a longer-term goal.