Add a lock file for the alt-ergo-lib #774
Conversation
Makefile
Outdated
| lock: | ||
| opam lock ./alt-ergo-lib.opam -w | ||
| # Remove OCaml compiler constraints | ||
| sed -i '/\"ocaml/d' ./alt-ergo-lib.opam.locked |
There was a problem hiding this comment.
We shouldn't be blanket removing packages that start with "ocaml because there are non-compiler-related libraries whose name starts with ocaml (e.g. ocamlgraph).
In our case, it removes the lock for ocaml-compiler-libs, ocamlbuild and ocamlfind which I believe are independent of the compiler version.
I think the following is better:
| sed -i '/\"ocaml/d' ./alt-ergo-lib.opam.locked | |
| sed -i '/"ocaml"\|"ocaml-base-compiler"\|"ocaml-system"\|"ocaml-config"/d' ./alt-ergo-lib.opam.locked |
| @@ -234,6 +234,11 @@ archi: $(EXTRA_DIR)/ocamldot/ocamldot | |||
| echo "}" >> archi.dot | |||
| dot -Tpdf archi.dot > archi.pdf | |||
|
|
|||
| lock: | |||
| opam lock ./alt-ergo-lib.opam -w | |||
There was a problem hiding this comment.
Maybe we should dune build ./alt-ergo-lib.opam first?
Makefile
Outdated
| @@ -234,6 +234,11 @@ archi: $(EXTRA_DIR)/ocamldot/ocamldot | |||
| echo "}" >> archi.dot | |||
| dot -Tpdf archi.dot > archi.pdf | |||
|
|
|||
| lock: clean lib | |||
There was a problem hiding this comment.
Why do we need clean and lib dependencies?
(Especially clean, I don't understand why we need that)
There was a problem hiding this comment.
I clean the _build directory to be sure the binary compiles with the actual opam switch and I add the lib to check if the binary compiles as we ask me. The command dune build ./alt-ergo-lib.opam doesn't exist.
There was a problem hiding this comment.
I only suggested to make sure that alt-ergo-lib.opam is up-to-date before locking it because it seemed sensible. Making sure that the library compiles makes sense as well, although I am not sure either of these things are necessary, I'm fine having them or not as you prefer.
However we shouldn't clean the build directory without a reason, and I don't see any reason to do it here -- if we want to check that the library builds, we can just do that, without clearing the build cache.
Not sure what you mean by "The command dune bulid ./alt-ergo-lib.opam doesn't exist." however, it should rebuild alt-ergo-lib.opam from the dune-project.
$ file alt-ergo-lib.opam
alt-ergo-lib.opam: cannot open `alt-ergo-lib.opam' (No such file or directory)
$ dune build ./alt-ergo-lib.opam
$ file alt-ergo-lib.opam
alt-ergo-lib.opam: ASCII textThere was a problem hiding this comment.
Sorry, I didn't know that dune build ./alt-ergo-lib.opam rebuild only the opam file. I thought the command has no effect at all when I tried it.
I think we should force to recompile the binary before creating the lock file. I agree cleaning everything is not great. I did that because I have absolutely no faith in Dune to rebuild correctly the binary after updating the opam switch.
There was a problem hiding this comment.
In fact the CI will fail in any case. So we can only rebuild the opam file ;)
This PR add a lock file for the alt-ergo-lib in order to improve the reproducibility of the building process.
I also add a target
lockin the makefile to update the lock file.