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

caddy: 2.7.6 -> 2.8.4 #316076

Merged
merged 1 commit into from
Jun 3, 2024
Merged

caddy: 2.7.6 -> 2.8.4 #316076

merged 1 commit into from
Jun 3, 2024

Conversation

Nanotwerp
Copy link
Contributor

@Nanotwerp Nanotwerp commented May 31, 2024

Description of changes

Updates caddy from version 2.7.6 to version 2.8.4. This commit also adds the nobadger build tag, which prevents the badger-db implementation of NoSQL from being compiled. This reduces the binary size of caddy, and is being used by caddy's upstream ever since 2.8.0.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4010

flokli
flokli previously requested changes May 31, 2024
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

I'm not sure we should actively migrate code over to another code formatter. At least, I'd leave it up to the individual package maintainers for now.

Squashing it into the same commit that does a bump makes this very hard to review. Please make it two separate commits at least, or leave the reformatting up to the maintainers, or a tree wide formatting, if that happens at some point.

@Nanotwerp
Copy link
Contributor Author

Nanotwerp commented May 31, 2024

I'm not sure we should actively migrate code over to another code formatter. At least, I'd leave it up to the individual package maintainers for now.

Squashing it into the same commit that does a bump makes this very hard to review. Please make it two separate commits at least, or leave the reformatting up to the maintainers, or a tree wide formatting, if that happens at some point.

Done! I made them separate commits. I apologize for the inconvenience.

@flokli
Copy link
Contributor

flokli commented May 31, 2024

Thanks! Bump changes LGTM, but let's have the package maintainers decide about the PR.

pkgs/by-name/ca/caddy/package.nix Show resolved Hide resolved
pkgs/by-name/ca/caddy/package.nix Show resolved Hide resolved
Copy link
Member

@emilylange emilylange left a comment

Choose a reason for hiding this comment

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

Oh and while you are at it, could you also please add at the very least two links to each of the version's release notes as hinted in our contributing guidelines?

https://github.com/NixOS/nixpkgs/blob/342ac0a9895ef24ab2725d829da96670abae2f09/pkgs/README.md#commit-conventions


https://github.com/caddyserver/caddy/releases/tag/v2.8.0

https://github.com/caddyserver/caddy/releases/tag/v2.8.1

@Nanotwerp Nanotwerp force-pushed the caddy-up branch 3 times, most recently from c6570fb to 10f8d96 Compare May 31, 2024 18:54
@Nanotwerp Nanotwerp requested a review from emilylange May 31, 2024 18:55
@emilylange
Copy link
Member

CI check fails because of a trailing comma and still missing the release note links in the commit message.

It's fine if you only change the CI check failure. I can squash-merge your commit and fix the commit message that way.

@Nanotwerp Nanotwerp force-pushed the caddy-up branch 2 times, most recently from f3a59d2 to 19110f0 Compare May 31, 2024 19:07
@Nanotwerp
Copy link
Contributor Author

CI check fails because of a trailing comma and still missing the release note links in the commit message.

It's fine if you only change the CI check failure. I can squash-merge your commit and fix the commit message that way.

All done! Sorry for the formatting issues and thousands of force-pushes!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1707

@Nanotwerp Nanotwerp changed the title caddy: 2.7.6 -> 2.8.1 caddy: 2.7.6 -> 2.8.2 Jun 2, 2024
@Nanotwerp
Copy link
Contributor Author

Bumped up to 2.8.2.

@Nanotwerp Nanotwerp force-pushed the caddy-up branch 2 times, most recently from 893df42 to 9377dd3 Compare June 2, 2024 06:11
@Nanotwerp Nanotwerp force-pushed the caddy-up branch 2 times, most recently from e964834 to 5584b1a Compare June 2, 2024 06:13
@emilylange
Copy link
Member

emilylange commented Jun 2, 2024

You previously had a vendorHash mismatch, but you seem to have fixed it as part of the 2.8.2. That's good.
Now it actually builds :)

However, 2.8.3 will get released soon, so I'd propose to wait for it first.

We could merge 2.8.2 but that would leave us with an unnecessarily buggy version 🫣

and thousands of force-pushes!

No worries, even thousands of force-pushes are fine.
Arguably GitHub does not handle it all too well, but nixpkgs decided to use force-pushes anyway, so that just how it is :)

@fin444
Copy link
Contributor

fin444 commented Jun 2, 2024

https://github.com/caddyserver/caddy/releases/tag/v2.8.4

@emilylange emilylange changed the title caddy: 2.7.6 -> 2.8.2 caddy: 2.7.6 -> 2.8.3 Jun 2, 2024
@emilylange emilylange changed the title caddy: 2.7.6 -> 2.8.3 caddy: 2.7.6 -> 2.8.4 Jun 2, 2024
@Nanotwerp
Copy link
Contributor Author

Updated to 2.8.4!

Copy link
Member

@emilylange emilylange left a comment

Choose a reason for hiding this comment

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

How about the following commit message instead:

caddy: 2.7.6 -> 2.8.4

Starting with v2.8.0, Caddy recommends building with the `nobadger`
build tag.

The build tag makes go skip the compilation of the badger-db
implementation from smallstep/nosql, as it isn't used by Caddy and thus
slightly shrinks the resulting binary size.

https://github.com/caddyserver/caddy/releases/tag/v2.8.4

https://github.com/caddyserver/caddy/releases/tag/v2.8.3

https://github.com/caddyserver/caddy/releases/tag/v2.8.2

https://github.com/caddyserver/caddy/releases/tag/v2.8.1

https://github.com/caddyserver/caddy/releases/tag/v2.8.0

diff: https://github.com/caddyserver/caddy/compare/v2.7.6...v2.8.4

pkgs/by-name/ca/caddy/package.nix Outdated Show resolved Hide resolved
@flokli
Copy link
Contributor

flokli commented Jun 3, 2024

Please check the failing CI output, you now introduced trailing whitespace.

Starting with v2.8.0, Caddy recommends building with the `nobadger`
build tag.

The build tag makes go skip the compilation of the badger-db
implementation from smallstep/nosql, as it isn't used by Caddy and thus
slightly shrinks the resulting binary size.

https://github.com/caddyserver/caddy/releases/tag/v2.8.4

https://github.com/caddyserver/caddy/releases/tag/v2.8.3

https://github.com/caddyserver/caddy/releases/tag/v2.8.2

https://github.com/caddyserver/caddy/releases/tag/v2.8.1

https://github.com/caddyserver/caddy/releases/tag/v2.8.0

diff: caddyserver/caddy@v2.7.6...v2.8.4
@Nanotwerp
Copy link
Contributor Author

Please check the failing CI output, you now introduced trailing whitespace.

Fixed. The text editor I use needs to get EditorConfig support QUICKLY.

@emilylange emilylange merged commit dedd8eb into NixOS:master Jun 3, 2024
24 checks passed
@Nanotwerp Nanotwerp deleted the caddy-up branch June 4, 2024 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants