Add option `learn-ocaml build serve --serve-during-build` and fix related minor issues

[erikmd]

• Kind: feature

• Close #594

Description

Good news, @AltGr @yurug !

I was able (with Louis' advice from last week) to implement #594.

Regarding the naming of the CLI command, I now propose --serve-during-build (better than my initial try, --fork-replace)

Each commit comes with a detailed description (or motivation), in particular note that:

• I fixed a minor issue (that originated in the --replace PR) due to the fact alpine's default lsof supports no option; unlike the lsof package;
• I fixed another minor issue regarding a missing flush in Learnocaml_server.kill_running;
• and I changed the formatting of the displayed version (→ if you don't object, it will print Learnocaml v1.1.0 running instead of Learnocaml v.1.1.0 running, which was making less sense given our current git tag convention).

I thoroughly tested the new option as summarized in the message of the main commit https://github.com/ocaml-sf/learn-ocaml/commit/f0108a34ae70ab45d22c21252b51e1bfb8a6c5ca but as suggested by @AltGr, I didn't turn the option on by default in the Dockerfile.

Regarding performance, I ran an experiment on learn-ocaml-corpus endowed with the following docker-compose.yml:

docker-compose.yml

```yaml services: lo-test: image: learn-ocaml # build using ( sudo make docker-images ) volumes: - ../learn-ocaml-corpus:/repository - lo-test-serve-during-build-sync:/sync ports: - '8080:8080' restart: unless-stopped environment: - 'LEARNOCAML_SERVE_DURING_BUILD=true' volumes: lo-test-serve-during-build-sync: driver: local ```

along with the following script:

shell session

``` $ sudo make docker-images $ sudo docker compose up -d $ sudo docker compose logs -t lo-test $ sudo docker compose stop $ touch ../learn-ocaml-corpus/exercises/fpottier/*/test.ml # touch 26 exercises $ sudo docker compose restart $ sudo docker compose logs -t lo-test ```

And I got:

• (around 10' to build the docker image locally)
• 54s to build learn-ocaml-corpus (offline)
• (<1s to start the server)
• (immediate stop)
• (<1s to restart the child server from the previous www)
• 19s to rebuild learn-ocaml-corpus/exercises/fpottier (online)
• (<1s to kill the child server and start the main one)

So I believe the effort was worth it for this quite standard use case :)

Now, let's wait for the CI ! and let me know if you have review comments.

And last hint: we shouldn't squash-merge this PR but rather do a regular merge (to preserve the atomic commits).

Checklist

• [x] Read the CONTRIBUTING.md guide and:
  • [x] Use Atomic Commits so each commit gathers a single logical change
  • [x] Use Conventional Commits regarding commit messages (needed by our release toolchain)
• [ ] Add/update tests
• [ ] Add/update documentation

Note to maintainers

• Read this wiki page.
• Make sure the PR has a milestone.
• Assign yourself before merging.
• [x] Either do a regular merge:
  • for PRs containing several commits following conventional-commits,
  • or for PRs containing 1 commit shared with a later PR (to preserve the SHA1)
• [ ] Or do a squash-merge:
  • for PRs containing only 1 commit (not shared with a later PR),
  • or for PRs containing several commits that need not be kept in the history;
  • → Update the commit message header with a conventional-commit type,
  • → Add a footer Close #… if a related issue exists.

[erikmd]

Hi @AltGr @yurug, I believe I addressed all your suggestions.

[erikmd]

@AltGr

while I agree that code-wise they are pretty different, from a user-perspective, they're both options concerning the server though (as the name implies!) -- and with similar use-cases so it would be best to have them closer to each other.

We could cheat by keeping this organisation in the code, but hardcoding ~docs to still put it in the SERVER section of the manpage (COMMON OPTIONS implicitely means the union of the commands, here we're dealing with the intersection :) )

Very good point, thanks! I fixed it as suggested.

Thanks again for your advice and help!

[erikmd]

With this last tweak (and rebase), CI is green.

@AltGr @yurug If you don't object, I will merge the PR today at 16:00 or so.