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

chore: compile without nosql's support for Postgres and MySQL #6655

Merged
merged 6 commits into from
Oct 22, 2024

Conversation

dunglas
Copy link
Collaborator

@dunglas dunglas commented Oct 22, 2024

I had issues with nosql that only supports old versions of Badger while upgrading Mercure dependencies: dunglas/mercure#960. While working on this, I figured out that we could disable compilation of Badger support for nosql (already done by Caddy), but also of MySQL and Postgres.

This reduce the build time and the binary size.
This also has the nice side effect of fixing #6613 because this dependency will not be included in the binary at all anymore.

When #6097 will be finished, we may need to add back support for MySQL and Postgres, but that's not sure because the new version of nosql may provide adapters in separate modules, that users will cherry-pick (that would be the best option IMHO).

Closes #6613.

@francislavoie
Copy link
Member

francislavoie commented Oct 22, 2024

Yeah I agree we should do this for now. I really don't like that it's necessary. I hope Smallstep modularizes it sooner rather than later so this isn't necessary. It would be nicer if they were just fully gone from the dependency tree unless directly imported. (FYI @hslatman @azazeal, just so you know that it also affects Caddy's plugin authors as well)

@dunglas this will also need a small change in xcaddy as well since it adds the nobadger tag by default

@mohammed90
Copy link
Member

I don't mind this at the moment, until upstream refactoring is finalized and the #6097 is complete sometime in the future.

@mholt mholt merged commit eaaa2e5 into caddyserver:master Oct 22, 2024
33 checks passed
@francislavoie
Copy link
Member

Looking at the CI artifacts, it doesn't seem like the build output reduced in size at all (or at least is within 100kb difference).

@francislavoie
Copy link
Member

francislavoie commented Oct 22, 2024

Hmm, tried building locally with the tags and:

$ go build -tags=nobadger,nomysql,nopgx
$ ls -la caddy
56650655

$ go build -tags=nobadger
$ ls -la caddy
59840067

So it seems to be about 3MB smaller actually. Maybe I'm being misled by these: (before: https://github.com/caddyserver/caddy/actions/runs/11467177533, after: https://github.com/caddyserver/caddy/actions/runs/11468346497)

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.

Remove MLP-2.0 dependency
4 participants