search

ocurrent / opam-repo-ci

An OCurrent pipeline for testing submissions to opam-repository
Apache License 2.0
20 stars 22 forks source link

Move all vendored git submodule to pin-depend #320

Closed shonfeder closed 1 month ago

shonfeder commented 4 months ago

Closes tarides/infrastructure#320

Replaces all vendored git-submodules with pin-depends.

This enable us to:

Due to a quirk in ocaml-ci, we cannot just pin to master, so updates here will require a find/replace of the hash of the package to be updated. This is only slightly more cumbersome than a git sub-module update. I'd like to fix this in a followup.

shonfeder commented 4 months ago

The CI failures are just debian-12-4.14_x86_32_opam-2.1 (failed: Failed: Build failed), which are not be filtered out as I'd expect given https://github.com/ocurrent/ocaml-ci/pull/932, even tho it is being filtered out for ocaml-ci itself! :disappointed: -- but that is besides the point.

mtelvers commented 3 months ago

The CI failures are just debian-12-4.14_x86_32_opam-2.1 (failed: Failed: Build failed), which are not be filtered out as I'd expect given ocurrent/ocaml-ci#932, even tho it is being filtered out for ocaml-ci itself! 😞 -- but that is besides the point.

There are some CI failures on debian-12 which aren't related to the x86_32 issue. e.g. https://ocaml.ci.dev/github/ocurrent/opam-repo-ci/commit/d8ac120af81c2d22f03000ec3dc170c1d12faf21/variant/debian-12-5.2_arm64_opam-2.1.

I think some will go away with a dune build --auto-promote

mtelvers commented 3 months ago

I also note that this change significantly increases the build time on my system.

$ git checkout master && rm -rf _build/ && time dune build
Switched to branch 'master'
Your branch is up to date with 'upstream/master'.
schema.capnp --> schema.mli schema.ml
ocurrent.capnp --> ocurrent.mli ocurrent.ml
kj/filesystem-disk-unix.c++:1690: warning: PWD environment variable doesn't match current directory; pwd = /home/mtelvers/opam-repo-ci

real    0m5.771s
user    0m22.141s
sys 0m5.079s

Using this PR.

$ git checkout pr320 && rm -rf _build/ && time dune build
Switched to branch 'pr320'
schema.capnp --> schema.mli schema.ml
ocurrent.capnp --> ocurrent.mli ocurrent.ml
kj/filesystem-disk-unix.c++:1690: warning: PWD environment variable doesn't match current directory; pwd = /home/mtelvers/opam-repo-ci

real    0m26.479s
user    0m41.993s
sys 0m6.208s
shonfeder commented 3 months ago

@mtelvers I am not seeing long build times. But maybe it if from the var file @benmandrew mentioned. I've excluded that directory from the dune dirs again, maybe that will sort things out?

Worth noting I guess that the CI systems don't seem to showing long build times afaict?

There are some CI failures on debian-12 which aren't related to the x86_32 issue. e.g. https://ocaml.ci.dev/github/ocurrent/opam-repo-ci/commit/d8ac120af81c2d22f03000ec3dc170c1d12faf21/variant/debian-12-5.2_arm64_opam-2.1.

I think some will go away with a dune build --auto-promote

Is it possible those are results from an earlier commit? Later commits aren't showing these testing failures, and I don't know how that test could be failing on just one platform?

I do see some debian-12s were failing with package installation trouble, but that is not recurring on rebuild, so seems like it may be flakiness?

mtelvers commented 3 months ago

Yes, I did comment on @benmandrew thread that the var exclusion for Dune was indeed the issue.

CI looks clean with the current build.

shonfeder commented 3 months ago

Are you all OK with me merging this, or any concerns that it may not deploy correctly etc.? Anything else you'd suggest to doing to reduce the chance of any outage from deploying this change?

mtelvers commented 3 months ago

Looks good to me. I'd just wait for a quiet moment and deploy it. Any running jobs will be cancelled when the docker container is replaced.

shonfeder commented 3 months ago

Ok! Will do. Thanks :)

shonfeder commented 3 months ago

This is stalled out due to:

shonfeder commented 1 month ago

I've become more convinced that a mono-repoization is gonna be more useful here. In any case, until the limitation with ocaml-ci is fixed, this will create more (rather than less) work to maintain.