From eacaa47df0a1bd31b7a0315b4fc51b09071b6d3a Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Tue, 20 Feb 2024 10:28:27 +0100 Subject: [PATCH 1/6] Scripts: inline install_build_deps.raw.sh --- debian-deps-build.Dockerfile | 1 - scripts/install_build_deps.raw.sh | 33 ------------------------------- scripts/install_build_deps.sh | 28 +++++++++++++++++++++++--- 3 files changed, 25 insertions(+), 37 deletions(-) delete mode 100755 scripts/install_build_deps.raw.sh diff --git a/debian-deps-build.Dockerfile b/debian-deps-build.Dockerfile index 0daad37da3c8..99f5e07dcf66 100644 --- a/debian-deps-build.Dockerfile +++ b/debian-deps-build.Dockerfile @@ -28,7 +28,6 @@ RUN opam init --bare --disable-sandboxing # we copy the mininum amount of files to use # the caching mechanism more efficiently COPY --link scripts/install_build_deps.sh root/tezos/scripts/ -COPY --link scripts/install_build_deps.raw.sh root/tezos/scripts/ COPY --link scripts/install_build_deps.rust.sh root/tezos/scripts/ COPY --link scripts/version.sh root/tezos/scripts/ COPY --link Makefile root/tezos/ diff --git a/scripts/install_build_deps.raw.sh b/scripts/install_build_deps.raw.sh deleted file mode 100755 index b3baef625c81..000000000000 --- a/scripts/install_build_deps.raw.sh +++ /dev/null @@ -1,33 +0,0 @@ -#!/bin/sh - -set -e - -script_dir="$(cd "$(dirname "$0")" && echo "$(pwd -P)/")" - -#shellcheck source=scripts/version.sh -. "$script_dir"/version.sh - -export OPAMYES="${OPAMYES:=true}" - -# install_build_deps.sh calls install_build_deps.rust.sh which checks whether -# Rust is installed with the right version and explains how to install it if -# needed, so here we only make opam acknowledge that we have a rust compiler -# we installed by our own. -# If we use opam depext, it will probably not install the right version. -OPAMASSUMEDEPEXTS=true opam install conf-rust conf-rust-2021 - -# Opam < 2.1 uses opam-depext as a plugin, later versions provide the option -# `--depext-only`: -case $(opam --version) in -2.0.*) - opam pin add -n -y octez-deps opam/virtual/ && opam depext octez-deps - opam pin remove octez-deps - ;; -*) opam install --depext-only opam/virtual/octez-deps.opam ;; -esac - -opam install opam/virtual/octez-deps.opam --deps-only --criteria="-notuptodate,-changed,-removed" - -if [ "$1" = "--tps" ]; then - opam install caqti-driver-postgresql -fi diff --git a/scripts/install_build_deps.sh b/scripts/install_build_deps.sh index dbb96066760e..726dca88927b 100755 --- a/scripts/install_build_deps.sh +++ b/scripts/install_build_deps.sh @@ -79,8 +79,7 @@ if [ "$(ocaml -vnum)" != "$ocaml_version" ]; then OPAMCLI=2.0 opam install --yes --unlock-base "ocaml-base-compiler.$ocaml_version" fi -# Must be done before install_build_deps.raw.sh because install_build_deps.raw.sh installs -# opam packages that depend on Rust. +# Must be done before using 'opam install' to install packages that depend on Rust. "$script_dir"/install_build_deps.rust.sh # Opam < 2.1 requires opam-depext as a plugin, later versions include it @@ -91,7 +90,30 @@ case $(opam --version) in ;; esac -"$script_dir"/install_build_deps.raw.sh "$1" +export OPAMYES="${OPAMYES:=true}" + +# install_build_deps.sh calls install_build_deps.rust.sh which checks whether +# Rust is installed with the right version and explains how to install it if +# needed, so here we only make opam acknowledge that we have a rust compiler +# we installed by our own. +# If we use opam depext, it will probably not install the right version. +OPAMASSUMEDEPEXTS=true opam install conf-rust conf-rust-2021 + +# Opam < 2.1 uses opam-depext as a plugin, later versions provide the option +# `--depext-only`: +case $(opam --version) in +2.0.*) + opam pin add -n -y octez-deps opam/virtual/ && opam depext octez-deps + opam pin remove octez-deps + ;; +*) opam install --depext-only opam/virtual/octez-deps.opam ;; +esac + +opam install opam/virtual/octez-deps.opam --deps-only --criteria="-notuptodate,-changed,-removed" + +if [ "$1" = "--tps" ]; then + opam install caqti-driver-postgresql +fi # add back the default repo if asked to or it was present in the first # place. we add the rank here even if it wasn't there just to be on -- GitLab From f3ba279641191da43d0c8c17d4ad81bd3aed8ffe Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Mon, 4 Mar 2024 10:34:16 +0100 Subject: [PATCH 2/6] Scripts: fix typos in comments --- scripts/install_build_deps.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/install_build_deps.sh b/scripts/install_build_deps.sh index 726dca88927b..b637279b6be3 100755 --- a/scripts/install_build_deps.sh +++ b/scripts/install_build_deps.sh @@ -94,8 +94,8 @@ export OPAMYES="${OPAMYES:=true}" # install_build_deps.sh calls install_build_deps.rust.sh which checks whether # Rust is installed with the right version and explains how to install it if -# needed, so here we only make opam acknowledge that we have a rust compiler -# we installed by our own. +# needed, so here we only make opam acknowledge that we have a Rust compiler +# we installed on our own. # If we use opam depext, it will probably not install the right version. OPAMASSUMEDEPEXTS=true opam install conf-rust conf-rust-2021 -- GitLab From c27249c7fc63072ff39910f69e4a2a29ce80fb4a Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Tue, 20 Feb 2024 10:39:38 +0100 Subject: [PATCH 3/6] Scripts: use the lock file --- debian-deps-build.Dockerfile | 2 +- scripts/install_build_deps.sh | 6 +++--- scripts/opam-check.sh | 4 ++-- scripts/update_opam_repo.sh | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/debian-deps-build.Dockerfile b/debian-deps-build.Dockerfile index 99f5e07dcf66..93ed40416d3a 100644 --- a/debian-deps-build.Dockerfile +++ b/debian-deps-build.Dockerfile @@ -31,7 +31,7 @@ COPY --link scripts/install_build_deps.sh root/tezos/scripts/ COPY --link scripts/install_build_deps.rust.sh root/tezos/scripts/ COPY --link scripts/version.sh root/tezos/scripts/ COPY --link Makefile root/tezos/ -COPY --link opam/virtual/octez-deps.opam root/tezos/opam/virtual/ +COPY --link opam/virtual/octez-deps.opam.locked root/tezos/opam/virtual/ COPY --link opam root/tezos/ WORKDIR root/tezos diff --git a/scripts/install_build_deps.sh b/scripts/install_build_deps.sh index b637279b6be3..0f636c636aa6 100755 --- a/scripts/install_build_deps.sh +++ b/scripts/install_build_deps.sh @@ -106,10 +106,10 @@ case $(opam --version) in opam pin add -n -y octez-deps opam/virtual/ && opam depext octez-deps opam pin remove octez-deps ;; -*) opam install --depext-only opam/virtual/octez-deps.opam ;; +*) opam install --depext-only opam/virtual/octez-deps.opam.locked ;; esac -opam install opam/virtual/octez-deps.opam --deps-only --criteria="-notuptodate,-changed,-removed" +opam install opam/virtual/octez-deps.opam.locked --deps-only --criteria="-notuptodate,-changed,-removed" if [ "$1" = "--tps" ]; then opam install caqti-driver-postgresql @@ -128,5 +128,5 @@ if [ -n "$dev" ]; then # enough (for [ppx_yojson_conv_lib] in particular), so we add a # minimal bound to ensure it won’t be picked by opam. # utop is constrained to avoid reinstalling in all the times. - opam install --yes opam/virtual/octez-deps.opam opam/virtual/octez-dev-deps.opam --deps-only --criteria="-changed,-removed" + opam install --yes opam/virtual/octez-deps.opam.locked opam/virtual/octez-dev-deps.opam --deps-only --criteria="-changed,-removed" fi diff --git a/scripts/opam-check.sh b/scripts/opam-check.sh index 0727b03e30b5..77b41ed4db4f 100755 --- a/scripts/opam-check.sh +++ b/scripts/opam-check.sh @@ -8,11 +8,11 @@ script_dir="$(cd "$(dirname "$0")" && echo "$(pwd -P)/")" echo "## Checking installed dependencies..." echo -if ! opam install opam/virtual/octez-deps.opam --deps-only --with-test --show-actions | grep "Nothing to do." > /dev/null 2>&1; then +if ! opam install opam/virtual/octez-deps.opam.locked --deps-only --with-test --show-actions | grep "Nothing to do." > /dev/null 2>&1; then echo echo 'Failure! Missing actions:' echo - opam install opam/virtual/octez-deps.opam --deps-only --with-test --show-actions + opam install opam/virtual/octez-deps.opam.locked --deps-only --with-test --show-actions echo # We really want literal backticks here, not command substitution. # shellcheck disable=SC2016 diff --git a/scripts/update_opam_repo.sh b/scripts/update_opam_repo.sh index 6136487cb7e2..8d57ed8b1a6a 100755 --- a/scripts/update_opam_repo.sh +++ b/scripts/update_opam_repo.sh @@ -61,7 +61,7 @@ git fetch --depth 1 origin "$full_opam_repository_tag" ## Adding the various tezos packages mkdir -p "$tmp_dir"/packages/octez-deps/octez-deps.dev -cp opam/virtual/octez-deps.opam "$tmp_dir"/packages/octez-deps/octez-deps.dev/opam +cp opam/virtual/octez-deps.opam.locked "$tmp_dir"/packages/octez-deps/octez-deps.dev/opam ## Filtering unrequired packages cd "$tmp_dir" -- GitLab From de5429507841cf59f3970b8b079ce81894e948a2 Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Tue, 20 Feb 2024 10:30:47 +0100 Subject: [PATCH 4/6] Scripts: make build-deps uses the public opam repository --- scripts/install_build_deps.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/install_build_deps.sh b/scripts/install_build_deps.sh index 0f636c636aa6..876742122143 100755 --- a/scripts/install_build_deps.sh +++ b/scripts/install_build_deps.sh @@ -32,10 +32,14 @@ fi opamswitch="$OPAMSWITCH" unset OPAMSWITCH +echo "Use opam repository commit: $full_opam_repository_tag" +opam_repository="https://github.com/ocaml/opam-repository.git#$full_opam_repository_tag" opam repository set-url tezos --dont-select "$opam_repository" || opam repository add tezos --dont-select "$opam_repository" > /dev/null 2>&1 -opam update --repositories --development +# Note: there should be no need to 'opam update' since 'opam repository add/set-url' +# should have fetched already and the repository cannot change since it is +# a specific commit. OPAMSWITCH="$opamswitch" -- GitLab From 4d90ed440decbcb94a92bc19eb7e5de99ed7ae07 Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Thu, 22 Feb 2024 14:18:14 +0100 Subject: [PATCH 5/6] Doc: update how to modify dependencies --- ...tributing-adding-a-new-opam-dependency.rst | 119 +++++++++++------- 1 file changed, 74 insertions(+), 45 deletions(-) diff --git a/docs/developer/contributing-adding-a-new-opam-dependency.rst b/docs/developer/contributing-adding-a-new-opam-dependency.rst index adbc2438d1d1..335745d685b7 100644 --- a/docs/developer/contributing-adding-a-new-opam-dependency.rst +++ b/docs/developer/contributing-adding-a-new-opam-dependency.rst @@ -2,7 +2,7 @@ How to add or update opam dependencies ====================================== When a merge request (MR) introduces a new dependency to an opam package, or -updates an existing dependency to an different version of an opam package, +updates an existing dependency to a different version of an opam package, additional steps must be taken in the development and merge process. This document explains those steps. @@ -13,16 +13,29 @@ Background ---------- The Octez project is built under a system that is somewhat stricter than -the default for OCaml project. Specifically, the Octez project maintains -a dedicated opam package repository that is a strict subset of the opam -default one; all binaries are built with dependencies from this subset -only. - -For this reason, adding or updating a dependency requires to work both -on `the main codebase `__ and on `the -dedicated opam package -repository `__. Moreover, work -between those two components must happen in a specific order. +the default for OCaml projects. The goal is to make sure that users, developers +and the CI use the exact same dependencies, with the exact same versions. +To this end: + +- the set of opam dependencies and their exact version number is stored in + :src:`an opam lock file `; +- the hash of the commit to use from the public opam repository is stored + in :src:`scripts/version.sh` in the variable ``full_opam_repository_tag``; +- ``make build-deps`` and ``make build-dev-deps`` use the lock file and the hash + to select dependencies; +- the CI uses Docker images that come with those dependencies pre-compiled. + +The Docker images for the CI are built by the CI of another repository, +the so-called `Tezos opam repository `__. +For legacy reasons, the Tezos opam repository is actually a subset +of the public opam repository containing the same dependencies as the lock file, +plus a few others such as ``odoc`` which is needed by the CI but not to build Octez. +Docker images are built from those package definitions. + +Adding, removing or updating dependencies thus requires to work both +on the `main codebase `__ and on +the `Tezos opam repository `__. +Moreover, work between those two components must happen in a specific order. The rest of this document explains the process from the point-of-view of a developer (you). The instructions below assume you have already @@ -30,16 +43,15 @@ a developer (you). The instructions below assume you have already but that you installed *development* dependencies (``make build-dev-deps`` instead of ``make build-deps``). - Local work ---------- -The simplest way of working locally (i.e., on your own machine) on the -Octez codebase, using a new dependency is to install it using ``opam``. +The simplest way of using a new dependency on the Octez codebase when working +locally (i.e., on your own machine) is to install it using ``opam``. Because you have used ``make build-dev-deps`` in order to install the Octez dependencies, you have access to the default opam repository in -addition to the dedicated one. +addition to the Tezos opam repository. **Install your dependency:** ``opam install foo`` @@ -58,15 +70,11 @@ as well as :src:`opam/virtual/octez-deps.opam`. You can work on your feature, using the types and values provided by your new dependency. - Making an MR ------------ -Even though you can compile and run code locally, the CI will fail. That is -because the CI runs not in an environment based on the content of the -:src:`opam/` directory, but instead based on pre-built Docker images built by -the CI of the dedicated ``opam-repository``. - +Even though you can compile and run code locally, the CI will likely fail. +This is because its set of available dependencies is now different from yours. You must follow the steps below in order to produce the necessary Docker images, allowing your work to eventually be merged. @@ -78,26 +86,44 @@ the ``master`` branch of (Note: this is not always necessary, but it is simpler for you to do so than to check whether it is necessary to do so.) -Second, still in your local copy of Octez, **execute the** -:src:`scripts/update_opam_repo.sh` **script**. This script uses the content of -your :src:`opam/` directory to create a file -called ``opam_repo.patch``. This file represents the diff between the current -dedicated opam repository and the dedicated opam repository that your MR -needs. - +Second, update the opam lock file. The safest way to do that is to +**execute the** :src:`scripts/update_opam_lock.sh` **script**. +It will ask opam to upgrade all Octez dependencies, +making sure that unwanted package versions are not selected for dependencies, +and will update the lock file accordingly. Note that the diff may include a few more changes than what you strictly need. Specifically, it might include some updates of some other dependencies. This is -not an issue in general but it might explain some changes unrelated to your -work. - -Third, **create an MR on the dedicated opam repository that includes +not an issue in general but it might explain some changes unrelated to your work. + +.. note:: + + If you do not wish to upgrade all dependencies, + you can also just run ``opam lock opam/virtual/octez-deps.opam`` + followed by ``mv octez-deps.opam.locked opam/virtual``, + or even edit the lock file manually. + Neither of these guarantees that packages are available in the commit + identified by ``full_opam_repository_tag`` of the public opam repository, + and even so, you may end up with unwanted versions of dependencies; + so you should review the resulting lock file even more carefully. + Editing the lock file manually is even less safe than running ``opam lock`` + as it does not guarantee that the set of dependencies is actually + a valid solution that the opam solver could have chosen. + +Third, still in your local copy of Octez, +**execute the** :src:`scripts/update_opam_repo.sh` **script**. +This script creates a file called ``opam_repo.patch``. +This file contains the difference between the current version of the +Tezos opam repository, and what it needs to be to take your new lock file +into account. + +Fourth, **create an MR on the Tezos opam repository that applies your patch.** This is the *opam repository MR*, its role is to prepare -the environment for your existing *Octez MR*. +the environment for the *Octez MR* that we will create below. In order to create the opam repository MR: - If you haven’t already done so, clone - `the dedicated opam repository `__. + `the Tezos opam repository `__. - Create a branch from the repository's ``master`` and switch to it. - Apply the patch generated by :src:`scripts/update_opam_repo.sh` (``git apply /opam_repo.path``). @@ -109,20 +135,21 @@ You can test the MR locally using the command ``OPAM_REPOSITORY_TAG= make build-deps``. This will rebuild the dependencies locally using the ```` of the opam-repository. -Fourth, back in your local copy of Octez, **update the variables in the** -:src:`.gitlab-ci.yml` **and** :src:`scripts/version.sh` **files**. Specifically, set -the ``build_deps_image_version`` and the ``opam_repository_tag`` variables -to the hash of your commit on the opam repository MR. Commit -this change with a title along the lines of “CI: use dependency -``foo``”. +Fifth, back in your local copy of Octez, **update the** ``opam_repository_tag`` **variable in the** +:src:`scripts/version.sh` **file**. Specifically, set it +to the hash of your commit on the opam repository MR. +Afterwards, you will also need to regenerate the GitLab CI configuration +by running ``make -C ci`` from the root of the repository. +Commit the change of ``scripts/version.sh`` and the GitLab configuration +with a title along the lines of “CI: use dependency ``foo``”. This commit will point the build scripts and CI to the modified opam-repository and the associated Docker images. Do note that the CI on your branch of Octez will only be able to run after the CI on your branch of opam-repository has completed. -Fifth, still in your local copy of Octez, **push these changes and open or -update the MR**. Make sure you add links referencing the opam-repository MR from +Finally, still in your local copy of Octez, **push these changes and open +an MR on the tezos/tezos project**. Make sure you add links referencing the opam-repository MR from the Octez MR and vice-versa. This gives the reviewers the necessary context to review. @@ -175,13 +202,15 @@ As a developer: - You propagate the changes to ``opam`` and ``dune`` files by running ``make -C manifest``. - You update the ``full_opam_repository_tag`` to the commit hash of a recent version of the public default opam repository. +- You update :src:`opam/virtual/octez-deps.opam.locked`, + for instance by executing :src:`scripts/update_opam_lock.sh`. - You execute :src:`scripts/update_opam_repo.sh`. - You open an opam repository MR from ``tezos/opam-repository:`` onto ``tezos/opam-repository:master`` that includes the generated patch. -- You update ``build_deps_image_version`` and ``opam_repository_tag`` - to the hash of the last commit of your opam repository MR. +- You update ``opam_repository_tag`` to the hash of the last commit of your opam repository MR + and regenerate the CI configuration. - You push the changes to your Octez MR. -- You update the description of your MRs to include links. +- You update the descriptions of your MRs to include links between them. As a merger: -- GitLab From 725688a72917653ee2121ad6ae3530e6fdcf315e Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Wed, 6 Mar 2024 12:02:23 +0100 Subject: [PATCH 6/6] Scripts: improve comment about opam version --- scripts/install_build_deps.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/install_build_deps.sh b/scripts/install_build_deps.sh index 876742122143..9d3540f24ad8 100755 --- a/scripts/install_build_deps.sh +++ b/scripts/install_build_deps.sh @@ -103,8 +103,8 @@ export OPAMYES="${OPAMYES:=true}" # If we use opam depext, it will probably not install the right version. OPAMASSUMEDEPEXTS=true opam install conf-rust conf-rust-2021 -# Opam < 2.1 uses opam-depext as a plugin, later versions provide the option -# `--depext-only`: +# Opam < 2.1 uses opam-depext as a plugin, later versions provide the option `--depext-only`. +# We assume Opam >= 2.0.0 (2.0.0 was released in 2018; Debian Buster already had Opam 2.0.3). case $(opam --version) in 2.0.*) opam pin add -n -y octez-deps opam/virtual/ && opam depext octez-deps -- GitLab