Skip to content

Misc: install completion#2140

Merged
hhugo merged 9 commits intomasterfrom
cmdliner-cmp
Jan 15, 2026
Merged

Misc: install completion#2140
hhugo merged 9 commits intomasterfrom
cmdliner-cmp

Conversation

@hhugo
Copy link
Copy Markdown
Member

@hhugo hhugo commented Jan 13, 2026

No description provided.

@dbuenzli
Copy link
Copy Markdown
Contributor

The Windows errors were weird.

Just out of curiosity, why did you use the opam & dune instructions rather than just dune?

@hhugo
Copy link
Copy Markdown
Member Author

hhugo commented Jan 13, 2026

The Windows errors were weird.

Just out of curiosity, why did you use the opam & dune instructions rather than just dune?

I didn't know which one to choose. I wasn't sure the dune way was handling windows fine. Should I use the dune only instruction ?

@dbuenzli
Copy link
Copy Markdown
Contributor

I didn't know which one to choose. I wasn't sure the dune way was handling windows fine

I have no idea :-) But it seems cmdliner has problem deriving the tool name from the executable path on windows (see linked cmdliner issue) I suspect you can work around by replacing:

"_build/install/default/bin/js_of_ocaml.exe" {os-family = "windows"} 

with the following syntax to set the tool name (see the description of the argument in cmdliner install tool-support --help) :

"_build/install/default/bin/js_of_ocaml.exe:js_of_ocaml" {os-family = "windows"} 

@dbuenzli
Copy link
Copy Markdown
Contributor

The failure pattern is "interesting", but it seems the dune instructions trigger a bug in dune itself.

@WardBrian
Copy link
Copy Markdown

The failure pattern is "interesting", but it seems the dune instructions trigger a bug in dune itself.

The pattern used here is almost exactly what we use in stanc3.

Requesting dune build compiler/bin-js_of_ocaml/cmdliner-support compiler/bin-wasm_of_ocaml/cmdliner-support generates the expected files in the _build directory.

So, it seems like the issue comes specifically from specifying two share_root install targets with the (dir ...) atom.
Comment either one out and it will build, so at least we have a fairly specific bug to report...

@hhugo
Copy link
Copy Markdown
Member Author

hhugo commented Jan 15, 2026

As a workaround, this PR now lists the directory generated by cmdliner and generate an install-in-share.sexp file that can be used in the install stanza

(install
 (section share_root)
 (package js_of_ocaml-compiler)
 (files
  (include install-in-share.sexp)))

@hhugo hhugo merged commit cef7934 into master Jan 15, 2026
27 of 28 checks passed
@hhugo hhugo deleted the cmdliner-cmp branch January 15, 2026 16:35
@nojb
Copy link
Copy Markdown
Contributor

nojb commented Feb 9, 2026

Unfortunately, this PR does not seem enough to be able to build js_of_ocaml in a workspace (in a monorepo, without OPAM): ocaml/dune#13500.

@hhugo
Copy link
Copy Markdown
Member Author

hhugo commented Feb 9, 2026

Can you check with https://github.com/ocsigen/js_of_ocaml/tree/fix-completion ? The downside is that it could break if cmdliner changes the set of files it generate.

@nojb
Copy link
Copy Markdown
Contributor

nojb commented Feb 9, 2026

Can you check with https://github.com/ocsigen/js_of_ocaml/tree/fix-completion ? The downside is that it could break if cmdliner changes the set of files it generate.

I confirm it works, thanks!

@hhugo
Copy link
Copy Markdown
Member Author

hhugo commented Feb 10, 2026

Can you check with https://github.com/ocsigen/js_of_ocaml/tree/fix-completion ? The downside is that it could break if cmdliner changes the set of files it generate.

I confirm it works, thanks!

Do you need the fix the upcomming release ?

@nojb
Copy link
Copy Markdown
Contributor

nojb commented Feb 10, 2026

Can you check with https://github.com/ocsigen/js_of_ocaml/tree/fix-completion ? The downside is that it could break if cmdliner changes the set of files it generate.

I confirm it works, thanks!

Do you need the fix the upcomming release ?

If it is not too much work, that would be appreciated, yes. Otherwise, I will cherry-pick the fix. Thanks!

@hhugo
Copy link
Copy Markdown
Member Author

hhugo commented Feb 10, 2026

I wonder if I should regenerate the file before building with opam to improve compatibility with different version of cmdliner

jmid pushed a commit to ocaml/opam-repository that referenced this pull request Feb 18, 2026
CHANGES:

## Features/Changes
* Misc: install shell completion script generated by cmdliner (ocsigen/js_of_ocaml#2140)
* Compiler/wasm: omit code pointer from closures when not used (ocsigen/js_of_ocaml#2059, ocsigen/js_of_ocaml#2093)
* Compiler/wasm: number unboxing (ocsigen/js_of_ocaml#2069, ocsigen/js_of_ocaml#2101)
* Compiler/wasm: specialization of number comparisons and bigarray operations (ocsigen/js_of_ocaml#1954)
* Compiler/wasm: make the type of some Wasm primitives more precise (ocsigen/js_of_ocaml#2100)
* Compiler: reference unboxing (ocsigen/js_of_ocaml#1958)
* Compiler: js-parser: support import/export with attributes
* Compiler: js-parser: support 'using X = E' for resource management (ocsigen/js_of_ocaml#2143)
* Compiler: js-parser: support decorators
* Compiler: js-parser: support html-comments
* Compiler: avoid unnecessary boolean-to-integer conversions (ocsigen/js_of_ocaml#2168)
* Runtime: improved handling of NaNs (ocsigen/js_of_ocaml#2110)
* Lib: allow to reference values from the runtime (ocsigen/js_of_ocaml#2086)
* Lib: add `Dom_html.onload` for WASM-safe load handling (ocsigen/js_of_ocaml#1948)
* Runtime: make eval functions more robust (ocsigen/js_of_ocaml#2108)
* Compiler: added a constant sinking pass (ocsigen/js_of_ocaml#2167)

## Bug fixes
* Compiler: fix `Global_flow.do_escape` monotonicity
* Compiler: fix static eval of `caml_nativeint_to_int`
* Compiler: remove invalid conditional simplification
* Lib: fix `characterData.substringData` method name typo in Dom module
* Lib: fix various Dom_html bindings (submitEvent, mediaQueryListEvent, pointerEvent, element, inputElement, tableElement types and deprecations)
* Lib: fix `numberList` type in Dom_svg to use `number_t`
* Lib: fix `eventSource.url` type to use `js_string t`
* Lib: fix `file.lastModifiedDate` type to use `Js.date t`, add `lastModified`
* Lib: fix `Form.get_form_elements` infinite loop bug
* Lib: fix `position.timestamp` type in Geolocation module
* Lib: remove non-existent `setDay` and `setUTCDay` methods from `Js.date`
* Lib: fix `_ACTIVE_TEXTURE_` type in WebGL to use `textureUnit`
* Compiler: fix purity of comparison functions (again) (ocsigen/js_of_ocaml#2092)
* Compiler: fix inlining (ocsigen/js_of_ocaml#2107)
* Compiler: allow arrow functions in for loops
* Ppx: disable spurious warning for unused "self" in object literal (ocsigen/js_of_ocaml#2128)
* Ppx: fix labelled arguments for methods (ocsigen/js_of_ocaml#2126)
* Runtime/wasm: fix Unix.times (ocsigen/js_of_ocaml#2096)
* Runtime: runtime with target-env=browser should not rely on "require(..)" (ocsigen/js_of_ocaml#2129)
* Runtime: fix fake filesystem with path containing special regexp chars. (ocsigen/js_of_ocaml#2132)
* Runtime/wasm: fix unmarshalling of compressed data (ocsigen/js_of_ocaml#2141)
* Runtime: fix compilation of loops at start of exception handlers (ocsigen/js_of_ocaml#2151)
* Compiler: fix parallel renaming (ocsigen/js_of_ocaml#2156)
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.

4 participants