odoc: Put support files in their own directory #6913

jonludlam · 2023-01-21T12:14:37Z

odoc 2.2 added math support, which comes with extra javascript, css and font files. Rather than add these as explicit targets, this commit changes the invocation of odoc support-files to output into the dir _build/default/_doc/_html/_odoc_support as a directory target.

Signed-off-by: Jon Ludlam jon@recoil.org

src/dune_rules/odoc.ml

Alizter · 2023-01-21T13:07:46Z

We also do something similar for coqdoc in coq/coq_rules if it helps. IIRC we have to register the directory target somewhere else as well as create it.

jonludlam · 2023-02-06T16:54:40Z

Having taken a look at the coqdoc rules, I've added the new directory to the 'directory_targets' field of the rules - is this right? Also, does that now mean I can get rid of the rmdir?

Alizter · 2023-02-06T17:20:56Z

Here is the PR for coqdoc: https://github.com/ocaml/dune/pull/5695/files

jonludlam · 2023-02-06T17:39:56Z

Gotcha, so I at least missed the Action_builder.With_targets.add_directories ~directory_targets - and with that I need to remove the Target that I had before in my Command.run args. Also you've got no rmdir so I'll get rid of that too. Thanks!

rgrinberg · 2023-02-06T19:52:41Z

src/dune_rules/odoc.ml

+      ; A "--support-uri"
+      ; A "_odoc_support"
+      ; A "--theme-uri"
+      ; A "_odoc_support"


Does this mean we're dropping support for old versions of odoc? We should add that to the conflicts section then.

No, odoc has supported those arguments since 2.0.0, so that's unchanged. Admittedly I haven't tested it yet though!

rgrinberg · 2023-02-06T19:53:31Z

src/dune_rules/odoc.ml

@@ -353,6 +351,10 @@ let setup_html sctx (odoc_file : odoc_artefact) =
      "html-generate" ~flags_for:None
      [ A "-o"
      ; Path (Path.build (Paths.html_root ctx))
+      ; A "--support-uri"
+      ; A "_odoc_support"


Also, why is this not passed as a path? E.g. Path (odoc_support ctx)

It can be as long as it'll work in all contexts - a bare argument like this is taken as a relative path from the output directory of the html-generate command. Does dune ever do anything smart with these paths like resolve them relative to the current dir or the like?

The rules are all context specific, so I don't see why not. When generating the command, dune will do Path.reach from the path from which the command is ran. So it should be a relative path in the end anyway.

Hrm, actually it feels a bit misleading to me, as Path.reach isn't the right thing to do, even if we arrange for it to produce the correct results in this instance.

Why isn't it the right thing to do?

I'd be more comfortable making it explicit - ie, Path.reach Paths.html_dir Paths.support_dir or something maybe?

although even that might not be right - I wouldn't want a ./ to get put on the front, for example.

Looking at the docs I see:

--support-uri=URI Where to look for support files (e.g. `URI/highlite.pack.js'). Relative URIs are resolved using `--output-dir' as a target.

And paths are certainly valid URI's.

although even that might not be right - I wouldn't want a ./ to get put on the front, for example.

Does that actually break it? I would hope not.

Acutally, I suppose it should be A (Path.reach support_dir ~from:index_html) to be fully correct.

although even that might not be right - I wouldn't want a ./ to get put on the front, for example.

Does that actually break it? I would hope not.

Ah probably not, and if so, that's an odoc bug :-) I've gone for this approach.

src/dune_rules/odoc.ml

rgrinberg

Looks good. Can you update CHANGES?

I'll let @emillon approve and merge.

jonludlam · 2023-02-07T08:25:06Z

Done.

src/dune_rules/odoc.ml

emillon · 2023-02-07T13:47:10Z

Extra question - what do you want to do about #1556? Is this still something that is relevant or is not anymore now that we have directory targets?

jonludlam · 2023-02-07T16:43:37Z

The approach in #1556 was what I was initially looking at. however, dynamic targets like that are, I understand, a bit tricky in dune. It seemed to me that using the directory target neatly fixes the problem that #1556 was addressing, so I believe this PR essentially fixes that issue - or at least renders it obsolete.

odoc 2.2 added math support, which comes with extra javascript, css and font files. Rather than add these as explicit targets, this commit changes the invocation of `odoc support-files` to output into the dir `_build/default/_doc/_html/_odoc_support` as a directory target. Signed-off-by: Jon Ludlam <jon@recoil.org>

Signed-off-by: Jon Ludlam <jon@recoil.org>

emillon · 2023-02-08T14:23:14Z

Thanks for the answers. I think it's good to go!

@jonludlam

…, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0~alpha2) CHANGES: - Fix handling of support files generated by odoc. (ocaml/dune#6913, @jonludlam)

@gridbugs

…, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0) CHANGES: - Allow running `$ dune exec` in watch mode (with the `-w` flag). In watch mode, `$ dune exec` the executed binary whenever it is recompiled. (ocaml/dune#6966, @gridbugs) - `coqdep` is now called once per theory, instead of one time per Coq file. This should significantly speed up some builds, as `coqdep` startup time is often heavy (ocaml/dune#7048, @Alizter, @ejgallego) - Add `map_workspace_root` dune-project stanza to allow disabling of mapping of workspace root to `/workspace_root`. (ocaml/dune#6988, fixes ocaml/dune#6929, @richardlford) - Fix handling of support files generated by odoc. (ocaml/dune#6913, @jonludlam) - Fix parsing of OCaml errors that contain code excerpts with `...` in them. (ocaml/dune#7008, @rgrinberg) - Pre-emptively clear screen in watch mode (ocaml/dune#6987, fixes ocaml/dune#6884, @rgrinberg) - Fix cross compilation configuration when a context with targets is itself a host of another context (ocaml/dune#6958, fixes ocaml/dune#6843, @rgrinberg) - Fix parsing of the `<=` operator in *blang* expressions of `dune` files. Previously, the operator would be interpreted as `<`. (ocaml/dune#6928, @tatchi) - Fix `--trace-file` output. Dune now emits a single *complete* event for every executed process. Unterminated *async* events are no longer written. (ocaml/dune#6892, @rgrinberg) - Fix preprocessing with `staged_pps` (ocaml/dune#6748, fixes ocaml/dune#6644, @rgrinberg) - Use colored output with MDX when Dune colors are enabled. (ocaml/dune#6462, @MisterDA) - Make `dune describe workspace` return consistent dependencies for executables and for libraries. By default, compile-time dependencies towards PPX-rewriters are from now not taken into account (but runtime dependencies always are). Compile-time dependencies towards PPX-rewriters can be taken into account by providing the `--with-pps` flag. (ocaml/dune#6727, fixes ocaml/dune#6486, @esope) - Print missing newline after `$ dune exec`. (ocaml/dune#6821, fixes ocaml/dune#6700, @rgrinberg, @Alizter) - Fix binary corruption when installing or promoting in parallel (ocaml/dune#6669, fixes ocaml/dune#6668, @edwintorok) - Use colored output with GCC and Clang when compiling C stubs. The flag `-fdiagnostics-color=always` is added to the `:standard` set of flags. (ocaml/dune#4083, @MisterDA) - Fix the parsing of decimal and hexadecimal escape literals in `dune`, `dune-package`, and other dune s-expression based files (ocaml/dune#6710, @shym) - Report an error if `dune init ...` would create a "dune" file in a location which already contains a "dune" directory (ocaml/dune#6705, @gridbugs) - Fix the parsing of alerts. They will now show up in diagnostics correctly. (ocaml/dune#6678, @rginberg) - Fix the compilation of modules generated at link time when `implicit_transitive_deps` is enabled (ocaml/dune#6642, @rgrinberg) - Allow `$ dune utop` to load libraries defined in data only directories defined using `(subdir ..)` (ocaml/dune#6631, @rgrinberg) - Format dune files when they are named `dune-file`. This occurs when we enable the alternative file names project option. (ocaml/dune#6566, @rgrinberg) - Move `$ dune ocaml-merlin -dump-config=$dir` to `$ dune ocaml merlin dump-config $dir`. (ocaml/dune#6547, @rgrinberg) - Allow compilation rules to be impacted by `(env ..)` stanzas that modify the environment or set binaries. (ocaml/dune#6527, @rgrinberg) - Coq native mode is now automatically detected by Dune starting with Coq lang 0.7. `(mode native)` has been deprecated in favour of detection from the configuration of Coq. (ocaml/dune#6409, @Alizter) - Print "Leaving Directory" whenever "Entering Directory" is printed. (ocaml/dune#6419, fixes ocaml/dune#138, @cpitclaudel, @rgrinberg) - Allow `$ dune ocaml dump-dot-merlin` to run in watch mode. Also this command shouldn't print "Entering Directory" mesages. (ocaml/dune#6497, @rgrinberg) - `dune clean` should no longer fail under Windows due to the inability to remove the `.lock` file. Also, bring the implementation of the global lock under Windows closer to that of Unix. (ocaml/dune#6523, @nojb) - Remove "Entering Directory" messages for `$ dune install`. (ocaml/dune#6513, @rgrinberg) - Stop passing `-q` flag in `dune coq top`, which allows for `.coqrc` to be loaded. (ocaml/dune#6848, fixes ocaml/dune#6847, @Alizter) - Fix missing dependencies when detecting the kind of C compiler we're using (ocaml/dune#6610, fixes ocaml/dune#6415, @emillon) - Allow `(include_subdirs qualified)` for OCaml projects. (ocaml/dune#6594, fixes ocaml/dune#1084, @rgrinberg) - Accurately determine merlin configuration for all sources selected with `copy#` and `copy_files#`. The old heuristic of looking for a module in parent directories is removed (ocaml/dune#6594, @rgrinberg) - Fix inline tests with *js_of_ocaml* and whole program compilation mode enabled (ocaml/dune#6645, @hhugo) - Fix *js_of_ocaml* separate compilation rules when `--enable=effects` ,`--enable=use-js-string` or `--toplevel` is used. (ocaml/dune#6714, ocaml/dune#6828, ocaml/dune#6920, @hhugo) - Fix *js_of_ocaml* separate compilation in presence of linkall (ocaml/dune#6832, ocaml/dune#6916, @hhugo) - Remove spurious build dir created when running `dune init proj ...` (ocaml/dune#6707, fixes ocaml/dune#5429, @gridbugs) - Allow `--sandbox` to affect `ocamldep` invocations. Previously, they were wrongly marked as incompatible (ocaml/dune#6749, @rgrinberg) - Validate the command line arguments for `$ dune ocaml top-module`. This command requires one positional argument (ocaml/dune#6796, fixes ocaml/dune#6793, @rgrinberg) - Add a `dune cache size` command for displaying the size of the cache (ocaml/dune#6638, @Alizter) - Fix dependency cycle when installing files to the bin section with `glob_files` (ocaml/dune#6764, fixes ocaml/dune#6708, @gridbugs) - Handle "Too many links" errors when using Dune cache on Windows (ocaml/dune#6993, @nojb) - Allow the `cinaps` stanza to set a custom alias. By default, if the alias is not set then the cinaps actions will be attached to both `@cinaps` and `@runtest` (ocaml/dune#6991, @rgrinberg) - Add `(using ctypes 0.3)`. When used, paths in `(ctypes)` are interpreted relative to where the stanza is defined. (ocaml/dune#6883, fixes ocaml/dune#5325, @emillon) - Auto-detect `dune-workspace` files as `dune` files in Emacs (ocaml/dune#7061, @ilankri) - Add native support for polling mode on Windows (ocaml/dune#7010, @yams-yams, @nojb)

hhugo · 2023-03-20T17:31:27Z

Mostly FYI, I was not able to dig further yet, but it seems that this change somehow broke the js_of_ocaml online documentation.

On http://ocsigen.org/js_of_ocaml/5.1.0/api/index.html, odoc.css is not found. Yet, it is available in https://github.com/ocsigen/js_of_ocaml/blob/gh-pages/5.1.0/api/_odoc_support/odoc.css.

After searching more, it could be an issue with github pages and directories with leading underscores.

hhugo · 2023-03-21T07:58:36Z

I can confirm that creating an empty .nojekyll file at the root fixes the issue.

jonludlam commented Jan 21, 2023

View reviewed changes

src/dune_rules/odoc.ml Outdated Show resolved Hide resolved

jonludlam force-pushed the odoc-support-files-dir branch from 3932a09 to 6698408 Compare February 6, 2023 16:53

rgrinberg reviewed Feb 6, 2023

View reviewed changes

src/dune_rules/odoc.ml Outdated Show resolved Hide resolved

rgrinberg reviewed Feb 6, 2023

View reviewed changes

src/dune_rules/odoc.ml Outdated Show resolved Hide resolved

rgrinberg added the odoc label Feb 6, 2023

rgrinberg approved these changes Feb 7, 2023

View reviewed changes

emillon reviewed Feb 7, 2023

View reviewed changes

src/dune_rules/odoc.ml Show resolved Hide resolved

rgrinberg linked an issue Feb 7, 2023 that may be closed by this pull request

odoc: use upcoming support-files-targets command #1556

Closed

jonludlam added 6 commits February 8, 2023 12:13

Set directory targets for the _odoc_support dir

370d8fc

Signed-off-by: Jon Ludlam <jon@recoil.org>

More directory rules fixes for odoc

2adb08e

Signed-off-by: Jon Ludlam <jon@recoil.org>

Formatting

239d21d

Signed-off-by: Jon Ludlam <jon@recoil.org>

Factor out constant

e6616e8

Signed-off-by: Jon Ludlam <jon@recoil.org>

Update CHANGES

8098bd1

Signed-off-by: Jon Ludlam <jon@recoil.org>

jonludlam force-pushed the odoc-support-files-dir branch from 5304be8 to 8098bd1 Compare February 8, 2023 12:13

emillon merged commit 1d5ce4a into ocaml:main Feb 8, 2023

emillon mentioned this pull request Feb 9, 2023

[new release] xdg, stdune, ordering, ocamlc-loc, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0~alpha2) ocaml/opam-repository#23279

Closed

emillon mentioned this pull request Feb 17, 2023

[new release] xdg, stdune, ordering, ocamlc-loc, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0) ocaml/opam-repository#23349

Merged

hhugo mentioned this pull request Mar 21, 2023

Bad interaction between _odoc_supports directory and github pages / jekyll #7364

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

odoc: Put support files in their own directory #6913

odoc: Put support files in their own directory #6913

jonludlam commented Jan 21, 2023

Alizter commented Jan 21, 2023

jonludlam commented Feb 6, 2023

Alizter commented Feb 6, 2023

jonludlam commented Feb 6, 2023

rgrinberg Feb 6, 2023

jonludlam Feb 6, 2023

rgrinberg Feb 6, 2023

jonludlam Feb 6, 2023

rgrinberg Feb 6, 2023

jonludlam Feb 6, 2023

rgrinberg Feb 6, 2023

jonludlam Feb 6, 2023

jonludlam Feb 6, 2023

rgrinberg Feb 6, 2023

rgrinberg Feb 6, 2023

jonludlam Feb 6, 2023

rgrinberg left a comment

jonludlam commented Feb 7, 2023

emillon commented Feb 7, 2023

jonludlam commented Feb 7, 2023

emillon commented Feb 8, 2023

hhugo commented Mar 20, 2023

hhugo commented Mar 21, 2023

odoc: Put support files in their own directory #6913

odoc: Put support files in their own directory #6913

Conversation

jonludlam commented Jan 21, 2023

Alizter commented Jan 21, 2023

jonludlam commented Feb 6, 2023

Alizter commented Feb 6, 2023

jonludlam commented Feb 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgrinberg left a comment

Choose a reason for hiding this comment

jonludlam commented Feb 7, 2023

emillon commented Feb 7, 2023

jonludlam commented Feb 7, 2023

emillon commented Feb 8, 2023

hhugo commented Mar 20, 2023

hhugo commented Mar 21, 2023