Recent changes in git-kv breaks yocaml_git

[reynir]

I was updating blog.robur.coop, but index.html and feed.xml refused to update. After instrumenting yocaml with some extra logging in Action.need_update I found that it decided the target was newer. In fact so new that it was just created even if that did not correspond with the mtime in the git repository.

Likely, it is related to a recent change in git-kv that attempts to improve the semantics of change_and_push:
https://git.robur.coop/robur/git-kv/pulls/2

Namely, inside a change_and_push it's not-so-straightforward to give an accurate last_modified so "now" is returned. And since yocaml_git executes inside a change_and_push everything will look new /o\

[reynir]

I tried with the following patch (and a lot of extra logging), but I still get "now" as the mtime. I don't understand.

diff --git a/plugins/yocaml_git/yocaml_git.ml b/plugins/yocaml_git/yocaml_git.ml
index 99125ac..2d0774a 100644
--- a/plugins/yocaml_git/yocaml_git.ml
+++ b/plugins/yocaml_git/yocaml_git.ml
@@ -26,6 +26,13 @@ let run (module Source : Required.SOURCE) (module Clock : Mirage_clock.PCLOCK)
   let* context = match context with `SSH -> Ssh.context () in
   let* store = Git_kv.connect context remote in
   let module Store = Git_kv.Make (Clock) in
+  let module Store = struct
+    include Store
+    (* last_modified and change_and_push have a weird interaction;
+       so we show the old last_modified *)
+    let last_modified _new_store key = last_modified store key
+  end
+  in
   Store.change_and_push ?author ?author_email:email ?message store (fun store ->
       let module Config = struct
         let store = store

[xhtmlboi]

The logic used to calculate the time to use is a bit tricky (particularly because of the dynamic dependencies), so I'm looking into it as soon as possible! Thanks for the feedback!

[reynir]

I am pretty sure the bug is in git-kv.

When the remote has the following commit I get a timestamp for index.html that says today: https://git.robur.coop/reynir/blog.robur.coop/commit/a46df08b2cfcef1a4efa7525064addf956e09047

As you can see the commit is from two months ago.

To my surprise it doesn't seem to help to override last_modified to ignore the store value passed and instead use the "old" store value. I otherwise thought that would be a workaround.

[xhtmlboi]

Hm, looking at the issue mentioned, are we sure it's a problem on the YOCaml side? I have the impression that Hannes' suggestion (use the time of the last commit) seems a reasonable fix? Unless you have a direct idea on the YOCaml side, so as not to involve modifications on your side?

[xhtmlboi]

I am pretty sure the bug is in git-kv.

Ok. But as I think yocaml-git is one of the coolest features of YOCaml, if there's anything we can do to make it as smooth as possible, I'll happily do it!

[reynir]

So it turns out a change broke Git_kv.last_modified so it always returns "now". Obviously, this is a problem for yocaml.

https://git.robur.coop/robur/git-kv/pulls/10

In general the semantics of last_modified in git-kv are not ideal. On a clean git state it will return the date from the latest commit (or HEAD rather) even if the file in question was last modified several commits ago. And when in a change_and_push it will return "now" even if no files were changed (I think) or only unrelated files are changed.

Now I am curious what is the desired behavior for yocaml? Is it fine to just return the timestamp of the latest (clean) commit? Or is it required to report an accurate timestamp (e.g. timestamp of latest commit that touched that file)? And what about change_and_push - do we want accurate timestamps of staged changes?

[gr-im]

From my point of view, both approaches are satisfying, although I actually feel that the latest (report an accurate timestamp (e.g. timestamp of latest commit that touched that file)) would be ideal.

[reynir]

A fix has been merged in git-kv and git-kv.0.1.2 with the fix has been submitted to opam-repository: ocaml/opam-repository#27160

The fix is the easiest one to implement which is, on a clean commit, to report the date of the commit. Likely, you will want something like the change here to observe the changes as they are on the remote instead of staged changes (which will report a somewhat misleading timestamp): #61 (comment)

A more accurate timestamp will not happen this year I think ;-) happy holidays

[reynir]

With git-kv pinned to v0.1.2 and more-or-less the above patch I could update blog.robur.coop with updated index.html and feed.xml 🥳

[xvw]

Good news. Should we be more strict on the git-kv constraint scheme or can we close?

[reynir]

Happy new year!

I think we need a work around as in here: #61 (comment)

The reason is because we do everything in a change_and_push and then timestamps are just made up (still) /o\ so if we override last_modified to look at the old git-kv object we can get the remote timestamps. I think those semantics are probably "good enough". And I guess we should conflict on the previous release at least as it breaks yocaml.