On 25/07/22 01:20PM, Patrick Steinhardt wrote: > The `REF_HAVE_OLD` flag indicates whether a given ref update has its old > object ID set. If so, the value of that field is used to verify whteher s/whteher/whether/ > the current state of the reference matches this expected state. It is > thus an important part of mitigating races with a concurrent process > that updates the same set of references. > > When writing reflogs though we explicitly unset that flag. This is a > sensible thing to do: the old state of reflog entry updates may not > necessarily match the current on-disk state of its accompanying ref, but > it's only intended to signal what old object ID we want to write into > the new reflog entry. For example when migrating refs we end up writing > many reflog entries for a single reference, and most likely those reflog > entries will have many different old object IDs. > > But unsetting this flag also removes a useful signal, namely that the > caller _did_ provide an old object ID for a given reflog entry. This > signal is useful to determine whether we have to resolve the refname > manually to figure out the current state, or whether we should just go > with what the caller has provided. > > This actually causes real issues when migrating reflogs, as we don't > know to actually use the caller-provided old object ID when writing > those entries. Instead, reflog entries simply end up with the all-zero > object ID. Ok, if I'm understanding this correctly, the `REF_HAVE_OLD` flag is also required to actually record a provided old OID in the reflog entry. If it is not set, a NUL OID is recorded instead. > Stop unsetting the flag so that we can use it as this described signal, > which we'll do in a subsequent commit. Skip checking the old object ID > for log-only updates so that we don't expect it to match the current > on-disk state. Just to clarify, when migrating reflogs, are these operations always marked with `REF_LOG_ONLY`? The comment for that flag states: Used as a flag in ref_update::flags when we want to log a ref update but not actually perform it. This is used when a symbolic ref update is split up. I might be misunderstanding this though. -Justin
