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

pkg: relax version constraints for ocamlformat dev tool #11019

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

gridbugs
Copy link
Collaborator

Previously dune would install the exact version of ocamlformat specified in the .ocamlformat file in the project's root. This prevents alternative distributions of the ocamlformat package from being used, as these typically append a suffix to the version number.

The motivation for this change is supporting a binary dev tools distribution where the ocamlformat package is named like "ocamlformat.0.26.2+binary", and we want that package to be installable when the .ocamlformat file of a project specifies "version=0.26.2".

@gridbugs gridbugs force-pushed the relax-ocamlformat-version-constraint branch 2 times, most recently from a1c7252 to 1ecdbcd Compare October 18, 2024 06:24
Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

Seems sensible to me, I would assume ocamlformat.0.26.2.1should qualify if one requests 0.26.2.


Read more at: https://opam.ocaml.org/doc/Manual.html#Version-ordering
*)
let max_version = String.concat ~sep:"" [ min_version; "___" ] in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let max_version = String.concat ~sep:"" [ min_version; "___" ] in
let max_version = min_version ^ "___" in

@moyodiallo
Copy link
Collaborator

moyodiallo commented Oct 18, 2024

Would you like to add a test ?

@gridbugs gridbugs force-pushed the relax-ocamlformat-version-constraint branch 2 times, most recently from 98940f5 to 4ccfdbb Compare October 21, 2024 05:27
@gridbugs
Copy link
Collaborator Author

Added a test

Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

LGTM! Nice and clear test.

Previously dune would install the exact version of ocamlformat specified
in the .ocamlformat file in the project's root. This prevents
alternative distributions of the ocamlformat package from being used, as
these typically append a suffix to the version number.

The motivation for this change is supporting a binary dev tools
distribution where the ocamlformat package is named like
"ocamlformat.0.26.2+binary", and we want that package to be installable
when the .ocamlformat file of a project specifies "version=0.26.2".

Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
@gridbugs gridbugs force-pushed the relax-ocamlformat-version-constraint branch from 4ccfdbb to 0544313 Compare October 22, 2024 01:38
@gridbugs gridbugs merged commit bd29b15 into ocaml:main Oct 22, 2024
25 of 27 checks passed
anmonteiro pushed a commit to anmonteiro/dune that referenced this pull request Nov 17, 2024
Previously dune would install the exact version of ocamlformat specified
in the .ocamlformat file in the project's root. This prevents
alternative distributions of the ocamlformat package from being used, as
these typically append a suffix to the version number.

The motivation for this change is supporting a binary dev tools
distribution where the ocamlformat package is named like
"ocamlformat.0.26.2+binary", and we want that package to be installable
when the .ocamlformat file of a project specifies "version=0.26.2".

Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
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.

3 participants