ocurrent / solver-service

An OCluster service for solving opam dependencies
Apache License 2.0
12 stars 7 forks source link

oldest_commits_with: fix caching #35

Closed emillon closed 1 year ago

emillon commented 1 year ago

The current version has an important issue: oldest_commit_with always returns the commit equals to from. In other words, the most recent commit of opam-repository. This is correct but destroys caching because every opam-repository push will trigger a rebuild of all ocaml-ci repositories.

The reason is that the paths argument to that function is []. In turn, this is caused by is_path_in_repo always returning false. There are two issues in that function:

Removing this filtering makes the function return an older commit (and it is possible to observe that paths are being passed to git log, for example using execsnoop).

emillon commented 1 year ago

(note that I'm not sure about the correctness of the fix - in particular whether some partitioning is necessary)

moyodiallo commented 1 year ago

I introduced this bug during the merge of the 2 solvers, from OCaml-multicore-CI and OCaml-CI. The differences are introduced by this commit https://github.com/ocurrent/ocaml-multicore-ci/commit/975a2605ea8b6119fa7fae3b512ede581728215c.

This PR could be a good start to harmonize things or doing them better.