Skip to content

cohttp: 5.3.1 -> 6.1.1 and init cohttp-eio #411517

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

cohttp: 5.3.1 -> 6.1.1 and init cohttp-eio #411517

wants to merge 4 commits into from

Conversation

marijanp
Copy link
Contributor

@marijanp marijanp commented May 27, 2025 •
edited
Loading

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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.

@marijanp marijanp requested review from vbgl and toastal May 27, 2025 20:10
@github-actions github-actions bot added the 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. label May 27, 2025
@marijanp marijanp added 8.has: package (new) This PR adds a new package 8.has: package (update) This PR updates a package to a newer version labels May 27, 2025
@nix-owners nix-owners bot requested a review from ulrikstrid May 27, 2025 20:11
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 27, 2025
toastal
toastal approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@toastal toastal left a comment

Choose a reason for hiding this comment

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

generally looks good to me

@vbgl
Copy link
Contributor

vbgl commented May 28, 2025

@ofborg build ocamlPackages.graphql-cohttp

vbgl
vbgl requested changes May 28, 2025
View reviewed changes
Copy link
Contributor

@vbgl vbgl left a comment

Choose a reason for hiding this comment

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

This breaks a few things.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label May 28, 2025
@ulrikstrid
Copy link
Member

This change to curly fixes the build of that (and dune-release)

    substituteInPlace test/test_curly.ml \
      --replace "let body_header b = [\"content-length\", string_of_int (String.length b)]" \
                "let body_header b = [\"connection\", \"keep-alive\"; \"content-length\", string_of_int (String.length b)]"

Are there more broken packages because of this @vbgl?

@marijanp
Copy link
Contributor Author

@ulrikstrid I will add a commit.

@marijanp marijanp requested review from vbgl and toastal August 22, 2025 10:03
@marijanp marijanp self-assigned this Aug 22, 2025
@marijanp
Copy link
Contributor Author

@ofborg build ocamlPackages.graphql-cohttp

I've patched the respective source file

@marijanp
Copy link
Contributor Author

@ofborg build ocamlPackages.graphql-cohttp

@marijanp marijanp changed the title cohttp: 5.3.1 -> 6.1.0 and init cohttp-eio cohttp: 5.3.1 -> 6.1.1 and init cohttp-eio Aug 22, 2025
@ulrikstrid
Copy link
Member

Looking at this a bit closer (just building random packages that depends on cohttp) it seems like this breaks a couple of more packages. ocsigen_server, cohttp-lwt-jsoo (just to try a random cohttp package) and the resto packages.

The resto packages could be dropped, they seem to be unmaintained upstream and I added them to allow packaging Tezos. But ocsigen_server and the other cohttp packages needs to work.

@ulrikstrid ulrikstrid mentioned this pull request Aug 26, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulrikstrid ulrikstrid Awaiting requested review from ulrikstrid

@vbgl vbgl Awaiting requested review from vbgl

@toastal toastal Awaiting requested review from toastal

Requested changes must be addressed to merge this pull request.

Assignees

@marijanp marijanp

Labels
6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 8.has: package (new) This PR adds a new package 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marijanp @vbgl @ulrikstrid @toastal @wegank