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: allow fs embedding with --embed #160

Merged
merged 5 commits into from
Dec 18, 2023
Merged

feat: allow fs embedding with --embed #160

merged 5 commits into from
Dec 18, 2023

Conversation

mohammed90
Copy link
Member

@mohammed90 mohammed90 commented Dec 12, 2023

This PR adds the ability to embed a folder into the compiled Caddy by copying the entire tree passed via --embed (or Builder#EmbedDir struct field) that's specified into a directory named files under the temporary build directory and synthesize a Caddy module in the same dir (a copy of github.com/mholt/caddy-embed). This function relies on Go's embed package for embedding capability, thus it abides by the rules listed in its documentation.

I couldn't figure out to how to do multiple folders to allow the file server to do serve different roots, hence it's marked Experimental 🤷🏼

Resolves #130
Resolves mholt/caddy-embed#1

@mholt
Copy link
Member

mholt commented Dec 12, 2023

Oh neat!! This is cool, I'll give it a look soon :)

@francislavoie
Copy link
Member

I'm not sure how embedding works, but can we have like a dir: kind of prefix for multiple dirs?

@mohammed90
Copy link
Member Author

I'm not sure how embedding works, but can we have like a dir: kind of prefix for multiple dirs?

Now that I stepped back and thought about it, the current implementation can support it if the user provides the dirs already nested. In other words, if they embed sites which contains:

  • sites/example.com
  • sites/example.net

They can do

example.com , example.net
 file_server {
    fs embedded
    root {host}
}

If the user specifies multiple --embed, we build the nesting similarly instead of the user building it beforehand.

One thing I'd like to address is to somehow expose the embedded FS tree so users can inspect their fat binaries. That can be in a follow-up.

@francislavoie
Copy link
Member

francislavoie commented Dec 12, 2023

Can we allow the user to pass multiple --embed and if so nest them ourselves with those directory names? Maybe by default we should ask the user to provide a directory name. We could use a syntax like --embed embed-name:./path/to/dir or something.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks, this could be a neat/useful enhancement!

internal/utils/io.go Outdated Show resolved Hide resolved
@mohammed90
Copy link
Member Author

Can we allow the user to pass multiple --embed and if so nest them ourselves with those directory names? Maybe by default we should ask the user to provide a directory name. We could use a syntax like --embed embed-name:./path/to/dir or something.

Done. You can now specify multiple --embed name1:path/to/dir or --embed path/to/dir, and we'll take charge of the nesting. If no name is specified, the embedded dir content is placed at the root of the tree.

@francislavoie
Copy link
Member

francislavoie commented Dec 13, 2023

Nice! 👏

So I assume this works?

$ xcaddy build --embed foo:./sites/foo --embed bar:./sites/bar

With config:

foo.localhost {
	root * /foo
	file_server {
		fs embedded
	}
}

bar.localhost {
	root * /bar
	file_server {
		fs embedded
	}
}

Correct me if I'm wrong 🤔

We'll also need to cover this in the README.md

Also I think we're missing support for fs for try_files in Caddy? We might need to figure that out so that the FS is defined in the request context (like root is a var) so it gets used automatically throughout.

environment.go Outdated Show resolved Hide resolved
@mohammed90
Copy link
Member Author

mohammed90 commented Dec 14, 2023

So I assume this works?

Yep, yep, and yep.

Also I think we're missing support for fs for try_files in Caddy?

Hm... perhaps, I haven't inspected the extenet of that particular change in Caddy. We can create an issue to track it.

Edit:
try_files supports an embedded FS.

https://github.com/caddyserver/caddy/blob/b16aba5c27ace2c6ada9c2525f44329bf6dfcd37/modules/caddyhttp/fileserver/matcher.go#L264-L277

@mholt
Copy link
Member

mholt commented Dec 18, 2023

This is neat!! Is it ready to go?

@mohammed90
Copy link
Member Author

This is neat!! Is it ready to go?

Yep!

@mholt mholt merged commit 2887af6 into master Dec 18, 2023
8 checks passed
@mholt mholt deleted the embed-fs branch December 18, 2023 20:22
@mholt
Copy link
Member

mholt commented Dec 18, 2023

Thanks for working on this :) Let's give it a try

@francislavoie
Copy link
Member

try_files supports an embedded FS.

https://caddyserver.com/docs/caddyfile/directives/try_files doesn't though I think (easy fix though).

And I think we should use a var or whatever to bubble it down to other modules similarly to root. Usability will be not great from a config standpoint until we do that.

@korpa
Copy link

korpa commented Dec 30, 2023

I really like this feature. But sadly for example directories like _app (used by svelte) are not included. Would it be possible to support files and dirs starting with underscores as well?

See golang/go#43854

@korpa
Copy link

korpa commented Dec 30, 2023

Furthermore I wonder if I could use embedded files with cli command only instead of using Caddyfile:

Some sort of caddy files_server --embed --listen :8080

@mohammed90
Copy link
Member Author

Would it be possible to support files and dirs starting with underscores as well?

It may be possible to include a condition/flag to have the all: prefix for the pattern, but I'm not sure what the best UX is for that. I don't want to introduce another flag that depends on this flag. I also don't want to add embedded flags to this pattern <[alias]:path/to/dir>.... You can always embed the directory with a name that does not start with an underscore then rewrite the request path to have the underscore. How does this sound?

Furthermore I wonder if I could use embedded files with cli command only instead of using Caddyfile:

Some sort of caddy files_server --embed --listen :8080

We cannot add a flag to a command that's defined elsewhere (i.e. the file-server module in this case). I personally believe one must bite the bullet at some point and switch to config files, i.e. Caddyfile, rather than craft long lines of commands and flags. If we are to add a flag, it'd be --fs rather than --embed, but again this should be discussed in the caddyserver/caddy repo, not here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for embedding static files at build-time Consider moving to xcaddy project
4 participants