Skip to content

Source compatibility with newer cmdliner versions - #6854

Merged
kit-ty-kate merged 1 commit into
ocaml:masterfrom
WardBrian:cmdliner-2-src-compat
Jul 16, 2026
Merged

Source compatibility with newer cmdliner versions#6854
kit-ty-kate merged 1 commit into
ocaml:masterfrom
WardBrian:cmdliner-2-src-compat

Conversation

@WardBrian

Copy link
Copy Markdown
Contributor

Related to issues #6424, #6849, #6853

This PR makes the necessary code changes to allow compilation under cmdliner 2.0 while keeping the existing vendored version of 1.3.0.

These changes are mostly find-and-replace type things, but they would allow (at some future point):

  • Updating the vendor to 2.1.0+
  • Progressive enhancement from that point (e.g., replacing of the Arg.conv' calls this PR ads with 2.0's Arg.Conv.make, which would also allow us to specify completion handlers)

To verify, change the vendor_cmdliner.sh script to

#!/bin/sh

set -euo pipefail

cd src/core/cmdliner
rm -rf *.ml *.mli dune

git clone https://github.com/dbuenzli/cmdliner tmp-vendor
git -C tmp-vendor switch --detach v2.1.0

mv tmp-vendor/src/*.{ml,mli} .
rm -rf tmp-vendor

mv cmdliner.ml opamCmdliner.ml
mv cmdliner.mli opamCmdliner.mli

cat > dune << EOF
(library
 (name opamCmdliner)
 (public_name opam-core.cmdliner)
 (flags :standard -w -27-32-35-50))
EOF

run it, and re-build.

@kit-ty-kate kit-ty-kate added this to the 2.6.0~alpha1 milestone Jan 30, 2026

@kit-ty-kate kit-ty-kate left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks a lot! After some minor reformatting to avoid unrelated changes and simplify the diff it looks great.

Note for the remaining reviewer (@rjbou): the github "Hide whitespace" setting makes the diff straightforward

@kit-ty-kate
kit-ty-kate requested a review from rjbou January 30, 2026 17:15
@WardBrian

Copy link
Copy Markdown
Contributor Author

Thanks!

I wasn't sure if this kind of thing needed a CHANGES entry -- should I just ignore the CI run for that, or do you have a recommendation for what section I should put it under ("Internal"?)

@kit-ty-kate

Copy link
Copy Markdown
Member

I wasn't sure if this kind of thing needed a CHANGES entry -- should I just ignore the CI run for that, or do you have a recommendation for what section I should put it under ("Internal"?)

you can ignore the CI, that's no problem, we'll do it before the last rebase/final merge. But if you want to do it before that, this one should probably go under Internal in the master_changes.md, indeed.

@kit-ty-kate
kit-ty-kate force-pushed the cmdliner-2-src-compat branch from 306a1ee to ede7372 Compare July 14, 2026 23:02

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It worth having operators or small function name defined for OpamCmdliner.Arg.conv* in OpamArgTools and used everywhere.

@kit-ty-kate

Copy link
Copy Markdown
Member

It worth having operators or small function name defined for OpamCmdliner.Arg.conv* in OpamArgTools and used everywhere.

what would be the point?

@WardBrian

Copy link
Copy Markdown
Contributor Author

It worth having operators or small function name defined for OpamCmdliner.Arg.conv* in OpamArgTools and used everywhere.

Many of these are replaced in #6868 when completions are added

@rjbou

rjbou commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

It worth having operators or small function name defined for OpamCmdliner.Arg.conv* in OpamArgTools and used everywhere.

what would be the point?

to simplify the code, the use of these function. I find it simpler to write/read an converter operator and the main variable/function call, that a plain text wrapper.

Many of these are replaced in #6868 when completions are added

Fair enough. I prefer operators/tiny functions name for this kind of usage, but it's a personal opinion, we'll see how it goes.

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks!

@kit-ty-kate

Copy link
Copy Markdown
Member

Ignoring windows CI failure (#7030).
Thanks!

@kit-ty-kate
kit-ty-kate merged commit b268cf7 into ocaml:master Jul 16, 2026
40 of 41 checks passed
@WardBrian
WardBrian deleted the cmdliner-2-src-compat branch July 23, 2026 20:42
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.

3 participants