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

Bug: learn-ocaml build serve --replace doesn't recompile the cmo as expected #583

Closed
erikmd opened this issue Feb 12, 2024 · 2 comments · Fixed by #584, #572 or ocaml/opam-repository#25251

Comments

@erikmd
Copy link
Member

erikmd commented Feb 12, 2024

Related user(s):

@AltGr, @erikmd, @yurug

Related issue(s) or PR(s):

Related project scope(s):

build

Bug description:

learn-ocaml build serve --replace never recompiles the cmo's

To reproduce:

learn-ocaml build --repo=demo-repository serve
^C
echo '# Update !' >> demo-repository/exercises/demo2/descr.md
# …more edits…
learn-ocaml build --repo=demo-repository serve --replace

Learnocaml v.0.16.0 running.
Updating app at ./www.temp
demo2                    (no changes)
demo                     (no changes)
Learnocaml server v.0.16.0 starting on port 8080

Expected behavior:

Learnocaml v.0.16.0 running.
Updating app at ./www
demo                     (no changes)
demo2                        [OK]
Learnocaml server v.0.16.0 starting on port 8080

learn-ocaml --version

No response

git describe --long --always --abbrev=40 --tags

No response

What OS do you use?

GNU/Linux

What OS version/distribution do you use?

Debian 11

What browser(s) do you use with learn-ocaml?

Firefox

What browser(s) version did you used to reproduce the issue?

No response

Screenshots (if need be):

No response

Additional context:

No response

@AltGr
Copy link
Collaborator

AltGr commented Feb 12, 2024

Argh, indeed that's pretty annoying. :(
After looking into it I can diagnose it as follows:

  • rebuilding the exercises is based only on timestamp comparisons (à la make)
  • when --replace is activated, the build dir is first copied to a temporary place, which updates the timestamps

After a quick test, it seems that adding -p (which is POSIX) at https://github.com/ocaml-sf/learn-ocaml/blob/master/src/utils/lwt_utils.ml#L43 (ie replace "-PR" with "-pPR") is enough to solve the issue.

@AltGr
Copy link
Collaborator

AltGr commented Feb 12, 2024

Thanks a lot @erikmd for seriously doing the testing! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment