Skip to content

ocamlPackages.logs: 0.8.0 → 0.9.0; refactor to buildTokpkg#430848

Merged
vbgl merged 3 commits intoNixOS:masterfrom
toastal:ocaml-logs-0.9.0
Aug 12, 2025
Merged

ocamlPackages.logs: 0.8.0 → 0.9.0; refactor to buildTokpkg#430848
vbgl merged 3 commits intoNixOS:masterfrom
toastal:ocaml-logs-0.9.0

Conversation

@toastal
Copy link
Contributor

@toastal toastal commented Aug 4, 2025

#424216 like this but without changing the OCaml version checks. In fact, closes #424216. Bonus: we can mean what we say & unite with → RIGHTWARDS ARROW, not -> DASH + GREATER THAN (for @vbgl)!

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

1

Footnotes

  1. Please consider giving up MS GitHub or offering a non-proprietary, non-US-corporate-controlled mirror for this free software project. I wish to delete this Microsoft account in the future, but I need more projects like this to support alternative methods to send patches & contribute.

@toastal toastal requested review from marijanp and vbgl August 4, 2025 07:52
@nix-owners nix-owners bot requested a review from ulrikstrid August 4, 2025 07:53
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. labels Aug 4, 2025
@toastal
Copy link
Contributor Author

toastal commented Aug 4, 2025

Gonna rewrite this to use buildTopkgs

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Requires OCaml >= 4.14 now.

@toastal
Copy link
Contributor Author

toastal commented Aug 4, 2025

I pushed my rewrite WIP, but I will walk back & get both of these versions in

@toastal toastal changed the title ocamlPackages.logs: 0.8.0 → 0.9.0 ocamlPackages.logs: 0.8.0 → 0.9.0; refactor to buildTokpkg; add maintainer toastal Aug 4, 2025
@toastal toastal requested a review from sternenseemann August 4, 2025 09:28
@toastal
Copy link
Contributor Author

toastal commented Aug 4, 2025

@sternenseemann

Addressed versioning.

Now we can fallback to 0.8.0 still + the diff here is still less lines!

@toastal
Copy link
Contributor Author

toastal commented Aug 4, 2025

This would also be a good time to ask… cmdlinerSupport or withCmdliner?

@toastal toastal force-pushed the ocaml-logs-0.9.0 branch 7 times, most recently from 41b9745 to 83bb992 Compare August 4, 2025 11:27
@toastal
Copy link
Contributor Author

toastal commented Aug 4, 2025

Okay, I think I am satisfied with the commit history now so it’s easiest to review

One last thing, I think it will be easier to make sense using newlines in the shell args for grouping.

Copy link
Contributor

@marijanp marijanp left a comment

Choose a reason for hiding this comment

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

LGTM

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 5, 2025
@toastal
Copy link
Contributor Author

toastal commented Aug 5, 2025

@toastal
Copy link
Contributor Author

toastal commented Aug 5, 2025

@vbgl I could once again need help understand what inputs are buildInputs & what is propagatedBuildInputs

@toastal
Copy link
Contributor Author

toastal commented Aug 5, 2025

And adjacently: should we considering having like buildFlags on buildTopkg to avoid this sort of buildPhase override?

@toastal toastal force-pushed the ocaml-logs-0.9.0 branch 3 times, most recently from e6c8906 to 2349603 Compare August 5, 2025 15:23
@toastal
Copy link
Contributor Author

toastal commented Aug 8, 2025

@vbgl @ulrikstrid could you help resolve this argumentation with the direction y’all want OCaml packages to go?

@toastal toastal force-pushed the ocaml-logs-0.9.0 branch 5 times, most recently from b80b536 to a6767b9 Compare August 9, 2025 05:02
@toastal toastal changed the title ocamlPackages.logs: 0.8.0 → 0.9.0; refactor to buildTokpkg; add maintainer toastal ocamlPackages.logs: 0.8.0 → 0.9.0; refactor to buildTokpkg Aug 9, 2025
@toastal
Copy link
Contributor Author

toastal commented Aug 9, 2025

Since a month had past since #424216 & I wanted version 0.9.0 with no merge or acknowledgement from the maintainer, I had assumed this package was abandoned hence trying to update the code to match more contemporary style (& how I would have done it had I been the sole maintainer).

It's reasonably common, maybe not as common in code recently written, but I don't necessarily see a compelling reason to migrate existing code away from it.

In seeing a clash of that style & not wanting to fight, I do not wish to to co-maintain with @sternenseemann so I have removed myself from meta.maintainers & all attempts to make the style more in line with other more recent Nixpkgs packages. I did however:

  • move to buildTopkg which remove a lot of the no-longer-needed cruft related to manual topkg usage
  • have a switch to pick fall back to version 0.8.0 (following the pattern in ocamlPackages.eio & others)
  • continuing to use fetchurl but with the exact sha512 (over sha256+SRI) from OPAM match the exact tarball (meaning not the tarball contents as in fetchzip)

@toastal toastal requested review from sternenseemann and vbgl August 9, 2025 05:18
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Aug 10, 2025
@toastal toastal force-pushed the ocaml-logs-0.9.0 branch 2 times, most recently from edea37a to 1ab3810 Compare August 12, 2025 04:00
@toastal toastal requested a review from vbgl August 12, 2025 04:03
@vbgl vbgl merged commit 77664c1 into NixOS:master Aug 12, 2025
24 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants