Skip to content

Conversation

@stepbrobd
Copy link
Contributor

Thanks for making dolmen!

I maintain dolmen lsp over at nixpkgs and while updating linol from 0.6 to 0.10 NixOS/nixpkgs#399596, I hit a couple type errors and added a patch fixing those

I thought this might save you some time if you ever want to bump linol to 0.10 so here I am :)

Copy link
Owner

@Gbury Gbury left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fixes ! And sorry for answering so late.

@stepbrobd stepbrobd force-pushed the linol branch 2 times, most recently from 8050cf9 to 4e8d28e Compare August 4, 2025 19:48
Copy link
Owner

@Gbury Gbury left a comment

Choose a reason for hiding this comment

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

Just waiting for CI to finish and I'll merge.

Thanks a lot for maintaining the nixpkg version of dolmen lsp ! It's always great to hear that there are users of dolmen !

@stepbrobd
Copy link
Contributor Author

I'll try and fix those CI failures tomorrow

@Gbury
Copy link
Owner

Gbury commented Aug 4, 2025

Oh, looks like there are a few build errors. From a quick look, it seems to be an error in linol, where a type definition for json values is not the same as the one from the version of yojson installed. It should be a simple matter of restricting the versions of yojson that linol depends on. As a quick fix for dolmen you can also add yojson as a dependency with the right version constraints (and a comment explaining the situation).

@stepbrobd
Copy link
Contributor Author

stepbrobd commented Aug 5, 2025

Quick question, given that (implicit_transitive_deps true) is in dune-project, do you think it'd be better to change it to false and declare all dependencies explicitly? The CI failures are caused by linol depending on 2.* version of yojson while 3.0.0 is fetched. I'm unsure if we can override transitive dependency's version without vendoring (setting "yojson" { < "3.0.0" } in dolmen_lsp.opam probably won't change the version fetched to build linol)

@Gbury
Copy link
Owner

Gbury commented Aug 5, 2025

I'm fine with setting (implicit_transitive_deps false), but note that this only affects the dune build, and does not affect the choices opam makes when choosing which versions of packages to install.

Adding "yojson" { < "3.0.0" } will make it so that when a user tries to install dolmen_lsp, a 2.x version of yojson will be installed, which should make linol build correctly. Actually, that is a bug in linol (see this comment ), but putting the bound on yojson in the dolmen_lsp opam file (with a comment) is fine for now I'd say.

@stepbrobd
Copy link
Contributor Author

Gosh I'm so used to using Nix to manage all my dependencies, I have no clue how opam works now 🤣 I think CI should pass now

@Gbury Gbury merged commit 914ebe1 into Gbury:master Aug 5, 2025
20 checks passed
@stepbrobd stepbrobd deleted the linol branch August 5, 2025 12:27
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.

2 participants