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

update to OCaml 4.14.1 #3656

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

update to OCaml 4.14.1 #3656

wants to merge 23 commits into from

Conversation

avsm
Copy link
Member

@avsm avsm commented Jan 3, 2023

No description provided.

@avsm
Copy link
Member Author

avsm commented Jan 10, 2023

Couple of things broken here:

@avsm
Copy link
Member Author

avsm commented Jan 12, 2023

This failure is a new one on me...

image

Also @samoht @emillon I'm not sure where to report ocaml-ci bugs these days, but it's now impossible to cut-and-paste from the logs as it puts line numbers and Markdown all over the place, so I had to screenshot it instead.

@emillon
Copy link
Contributor

emillon commented Jan 12, 2023

Not sure what's going on here, but it's possible it's the _build directory caching (done by ocaml-ci) that goes in the way. In other terms, the compiler got upgraded but ocaml-ci reuses the existing _build directory.

@avsm
Copy link
Member Author

avsm commented Jan 12, 2023

Hm, dune will detect a changed compiler and just recompile everything though, as the digests wont match.

@tmcgilchrist
Copy link

Also @samoht @emillon I'm not sure where to report ocaml-ci bugs these days, but it's now impossible to cut-and-paste from the logs as it puts line numbers and Markdown all over the place, so I had to screenshot it instead.

Hi @avsm you can report any ocaml-ci related bugs at https://github.com/ocurrent/ocaml-ci/issues/.
The issue with cut-and-paste is covered by ocurrent/ocaml-ci#654 and various options for fixes are being worked on. Please do report any bugs or issues you find with the new ocaml-ci. We value your input and want to make ocaml-ci better.

@tmcgilchrist
Copy link

Running a local build using docker:

git clone --recursive "https://github.com/realworldocaml/book.git" && cd "book" && git fetch origin "refs/pull/3656/head" && git reset --hard fc64a884
cat > Dockerfile <<'END-OF-DOCKERFILE'
FROM ocaml/opam@sha256:6309bd23e6e42f06e4f74354bb7969749b996af2263c70432f803d894a357129
# debian-11-4.14_opam-2.1
USER 1000:1000
ENV CLICOLOR_FORCE="1"
ENV OPAMCOLOR="always"
WORKDIR /src
RUN sudo ln -f /usr/bin/opam-2.1 /usr/bin/opam
RUN opam init --reinit -ni
WORKDIR /src
RUN sudo chown opam /src
RUN cd ~/opam-repository && (git cat-file -e e0171a799fbb4458db77a10f76a6279727aac521 || git fetch origin master) && git reset -q --hard e0171a799fbb4458db77a10f76a6279727aac521 && git log --no-decorate -n1 --oneline && opam update -u
ENV DEPS="base-bigarray.base base-threads.base base-unix.base conf-pkg-config.2 dune.3.6.1 ocaml.4.14.1 ocaml-base-compiler.4.14.1 ocaml-config.2 ocaml-options-vanilla.1 opam-monorepo.0.3.5"
ENV CI="true"
ENV OCAMLCI="true"
RUN opam update --depexts && opam install --cli=2.1 --depext-only -y  $DEPS
RUN opam install $DEPS
WORKDIR /src
RUN sudo chown opam /src
COPY --chown=1000:1000 dune-project rwo.opam.locked /src/
RUN opam monorepo depext --yes --lock ./rwo.opam.locked
RUN opam install --yes --ignore-pin-depends --deps-only ./rwo.opam.locked
RUN opam exec -- opam monorepo pull
COPY --chown=1000:1000 . /src/
RUN opam exec -- dune build @install
RUN opam exec -- dune runtest
END-OF-DOCKERFILE
docker build .

I get this locking error, without any ocaml-ci involved:

#22 163.8 Entering directory 'correct/hello'
#22 164.2 File "book/testing/examples/dune.inc", line 67, characters 0-291:
#22 164.2 67 | (rule
#22 164.2 68 |  (alias manual_property_test)
#22 164.2 69 |  (locks /global)
#22 164.2 ....
#22 164.2 76 |   (package typerep))
#22 164.2 77 |  (action
#22 164.2 78 |   (system "dune build @all @runtest --root ./correct/manual_property_test")))
#22 164.2 (cd _build/default/book/testing/examples && /bin/sh -c 'dune build @all @runtest --root ./correct/manual_property_test')
#22 164.2 Entering directory 'correct/manual_property_test'
#22 164.2 Error: A running dune (pid: 12651) instance has locked the build directory.
#22 164.2 If this is not the case, please delete _build/.lock

@avsm
Copy link
Member Author

avsm commented Jan 20, 2023

Thanks very much Tim for checking it's not a ci issue. I'm not really sure how to resolve this issue now as I've run out of time to debug the internals of the RWO build. We have some locking problem from dune-in-dune invocations, and now a new (locks) field in mdx that was added to fix it, but I've clearly missed one somewhere. @emillon might you have any suggestions of how to proceed?

I'm starting to get the impression that the build has gotten overcomplex, and that we might be better served just by having separate builds of the tools (just binaries), the examples (just separate dune builds) and the book markdown promotions.

@emillon
Copy link
Contributor

emillon commented Jan 20, 2023

The new locking feature in dune acquires a lock whenever something is happening in _build in order to prevent 2 dune processes racing with the same data directory. The current implementation is a bit too eager when locking so it sometimes errors when it could proceed. For example, I think that dune exec can not be executed if dune build -w is running, even if dune exec does not need to build anything and build -w is waiting for something to happen. These restrictions are being lifted but it's possible that 3.6.1 is not clever enough.
You're right that the work on (locks) is supposed to prevent that - one lock per build directory should ensure that we're never in the situation where two dunes are started in the same directory.
@Leonidas-from-XIV I think you added these locks, can you have a look at this?
In terms of debugging strategy, I would try trace new dune processes and log their pid/cwd and match that against the error message. Looks like execsnoop does not support logging cwd out of the box but that might be easy to add. opensnoop on _build directories might work. Or strace -f the main process but I'm not sure it's easy to get the working directory from this.
(actually, one option might also to pass a different --build-dir to the dunes that race, but there won't be any sharing between these so it should just be for small examples)

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