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

switch odoc to version 2.4.0 #168

Closed
wants to merge 2 commits into from

Conversation

EmileTrotignon
Copy link

This switches the odoc version used to version 2.4.0. This is the version of odoc that supports search.

The intend being this is to be a step towards using sherlodoc on ocaml. This will be an improvement over the current search because there is fuzzy type search like hoogle, and because it is tailored and tested for ocaml.

This will also allow updating the online version of sherlodoc, which needs as input the odocl file generated by this CI. The new version of sherlodoc is only compatible with odoc.2.4.0. As the format of the odocl file is not backward-compatible, the odocl files currently generated are not suitable to be consumed by the new sherlodoc, which has loads of new features.

I am not sure my change is correct, I have run the tests and they pass, but maybe more testing is required. Odoc itself should be backwards compatible.

@tmcgilchrist
Copy link
Member

Thanks for the contribution.

The process for upgrading odoc requires coordination with voodoo to ensure the same version is used. See ocaml-doc/voodoo#134 for an example of what to change.

After that is in place, it can be deployed to staging pipeline (https://staging.docs.ci.ocamllabs.io) to validate the changes and check that package versions build correctly.

Then https://github.com/ocaml/ocaml.org needs modifications to ensure it can read the new voodoo/odoc artefacts. I am not familiar with the changes required there cc @sabine.

Finally the changes get deployed to https://docs.ci.ocaml.org live and once an epoch is generated then a coordinated deploy of ocaml.org is required.

I can help with getting things deployed to staging and validating the changes there.

@sabine
Copy link
Contributor

sabine commented Dec 19, 2023

As long as the odoc upgrade doesn't change the odoc html --as-json format, no changes to OCaml.org should be necessary.

But indeed, testing first locally on voodoo with odoc 2.4.0 and then on staging.docs.ci.ocamllabs.io will be great.

@EmileTrotignon
Copy link
Author

I am in vacation for a while, so I will work on this when I come back.

@EmileTrotignon
Copy link
Author

Hey @tmcgilchrist .
Guillaume has a PR on voodoo that works with odoc 2.4 ocaml-doc/voodoo#128
I have tested it on ocaml.org and it required a small patch because the .html.json have two extra fields, but apart from that it looks fine.

@sabine
Copy link
Contributor

sabine commented Jan 12, 2024

The odoc pin on the staging docs pipeline should be upgraded according to this PR.

Only after the voodoo staging branch builds package docs successfully on the staging pipeline, this PR should be merged into the main branch.

@mtelvers
Copy link
Member

Isn't that just a case of pushing this PR to the staging branch?

@sabine
Copy link
Contributor

sabine commented Jan 14, 2024

@mtelvers Yes, but I think neither of @EmileTrotignon or I have rights to push on this repo.

@mtelvers
Copy link
Member

@sabine I have pushed it to staging

@sabine
Copy link
Contributor

sabine commented Jan 15, 2024

@mtelvers thanks!

@mtelvers
Copy link
Member

There seem to be lots of errors like this: https://staging.docs.ci.ocamllabs.io/job/2024-01-14/232445-voodoo-do-52799a

@sabine
Copy link
Contributor

sabine commented Jan 15, 2024

@EmileTrotignon @gpetiot are we pinning the right odoc version?

@tmcgilchrist
Copy link
Member

tmcgilchrist commented Jan 15, 2024 via email

@gpetiot gpetiot mentioned this pull request Jan 15, 2024
@gpetiot
Copy link

gpetiot commented Jan 15, 2024

odoc is expecting the same version of odoc-parser (the package is not released separately anymore).

I'm trying to reproduce locally but ocaml-docs-ci does not build locally (fresh install, switch, etc.), I get this error:

File "vendor/ocurrent/lib_web/query.ml", line 35, characters 87-91:
35 |         [input ~a:[a_input_type `Checkbox; a_name "id"; a_value job_id; a_autocomplete `Off] ()]
                                                                                            ^^^^
Error: This expression has type [> `Off ]
       but an expression was expected of type bool

(probably hiding something else)

@mtelvers
Copy link
Member

mtelvers commented Jan 15, 2024

A quick fix to that compiling issue is to replace the two occurrences of 'Off with false.

The actual fix is to upgrade TyXML to 4.6.0.

@mtelvers
Copy link
Member

Can you edit ocaml-docs-ci.opam and update the pin-depends sections to this

pin-depends: [
  ["ocaml-version.dev" "git+https://github.com/ocurrent/ocaml-version#496b14cdf826a737841526b753ebad8c3656a85a"]
  ["current.dev" "./vendor/ocurrent"]
  ["current_git.dev" "./vendor/ocurrent"]
  ["current_github.dev" "./vendor/ocurrent"]
  ["current_web.dev" "./vendor/ocurrent"]
  ["current_ocluster.dev" "./vendor/ocluster"]
  ["ocluster-api.dev" "./vendor/ocluster"]
]

Then, create a new switch:

opam switch create . 4.14.1 --deps-only --with-test

@gpetiot
Copy link

gpetiot commented Jan 16, 2024

Naive question but how is it supposed to be tested locally? Running docker-compose -f docker-compose.yml up runs the whole day without coming to an end or failing until the containers are stopped with Ctrl+C. And the online CI seems to only return errors 500.

@sabine
Copy link
Contributor

sabine commented Jan 18, 2024

As far as I understood, testing the docs-ci locally is nontrivial because of the keys involved, that's why the staging environment exists.

Is there anything I can do to help this progress? It would be great to get the staging branch of voodoo tested and deployed.

@gpetiot
Copy link

gpetiot commented Jan 19, 2024

Let's wait for odoc.2.4.1 to be available on opam, as it's apparently necessary to have eio compile.

@sabine
Copy link
Contributor

sabine commented Jan 19, 2024

In that case, can we revert the staging branch to be identical to the main branch?

This would allow me to test and deploy the bugfix for wrong library names on ocaml.org that people are waiting for.

@mtelvers
Copy link
Member

Done

@sabine
Copy link
Contributor

sabine commented Jan 19, 2024

Thanks, I'll update voodoo staging and it will probably be picked up tomorrow by staging pipeline

@sabine
Copy link
Contributor

sabine commented Jan 28, 2024

This can be closed since #169 removes the pinning of odoc by the pipeline.

@tmattio tmattio closed this Jan 29, 2024
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.

6 participants