Skip to content
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

feat: enable resolve_root_symlink by default #546

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

dunglas
Copy link
Owner

@dunglas dunglas commented Feb 2, 2024

Not having this by default seems to break Laravel apps using public/storage/.

As you know well this part and Laravel, would you mind to review this @francislavoie? Thank you!

caddy/caddy.go Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
@francislavoie
Copy link
Contributor

The reason we didn't enable it by default is to avoid a tiny performance hit from the extra syscall which may be wasteful if the root is never a symlink.

Felt better to default to having the best performance rather than best compat because it's not everyone that will use symlinks as a deployment strategy and if they do it's trivial to enable.

I have no issue with this being on by default for FrankenPHP.

dunglas and others added 2 commits February 2, 2024 14:48
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
@dunglas
Copy link
Owner Author

dunglas commented Feb 2, 2024

@francislavoie ok! Let's have better compatibility with Laravel and the like (probably helps with Symfony too) and let users create a custom Caddyfile for performance optimizations.

@dunglas dunglas merged commit ae95851 into main Feb 2, 2024
39 checks passed
@dunglas dunglas deleted the feat/resolve_root_symlink-true branch February 2, 2024 14:58
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.

None yet

2 participants