git/refs.c, branch v2.37.2

clone: die() instead of BUG() on bad refs

2022-04-25T18:05:28Z

When cloning directly from a local repository, we load a list of refs based on scanning the $GIT_DIR/refs/ directory of the "server" repository. If files exist in that directory that do not parse as hexadecimal hashes, then the ref array used by write_remote_refs() ends up with some entries with null OIDs. This causes us to hit a BUG() statement in ref_transaction_create(): BUG: create called without valid new_oid This BUG() call used to be a die() until 033abf97f (Replace all die("BUG: ...") calls by BUG() ones, 2018-05-02). Before that, the die() was added by f04c5b552 (ref_transaction_create(): check that new_sha1 is valid, 2015-02-17). The original report for this bug [1] mentioned that this problem did not exist in Git 2.27.0. The failure bisects unsurprisingly to 968f12fda (refs: turn on GIT_REF_PARANOIA by default, 2021-09-24). When GIT_REF_PARANOIA is enabled, this case always fails as far back as I am able to successfully compile and test the Git codebase. [1] https://github.com/git-for-windows/git/issues/3781 There are two approaches to consider here. One would be to remove this BUG() statement in favor of returning with an error. There are only two callers to ref_transaction_create(), so this would have a limited impact. The other approach would be to add special casing in 'git clone' to avoid this faulty input to the method. While I originally started with changing 'git clone', I decided that modifying ref_transaction_create() was a more complete solution. This prevents failing with a BUG() statement when we already have a good way to report an error (including a reason for that error) within the method. Both callers properly check the return value and die() with the error message, so this is an appropriate direction. The added test helps check against a regression, but does check that our intended error message is handled correctly. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"

2022-04-13T22:51:33Z

This reverts commit 991b4d47f0accd3955d05927d5ce434e03ffbdb6, reversing changes made to bcd020f88e1e22f38422ac3f73ab06b34ec4bef1.

refs debug: add a wrapper for "read_symbolic_ref"

2022-03-17T17:40:14Z

In cd475b3b038 (refs: add ability for backends to special-case reading of symbolic refs, 2022-03-01) when the "read_symbolic_ref" callback was added we'd fall back on "refs_read_raw_ref" if there wasn't any backend implementation of "read_symbolic_ref". As discussed in the preceding commit this would only happen if we were running the "debug" backend, e.g. in the "setup for ref completion" test in t9902-completion.sh with: GIT_TRACE_REFS=1 git fetch --no-tags other Let's improve the trace output, but and also eliminate the now-redundant refs_read_raw_ref() fallback case. As noted in the preceding commit the "packed" backend will never call refs_read_symbolic_ref() (nor is it ever going to). For any future backend such as reftable it's OK to ask that they either implement this (or a wrapper) themselves. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

Merge branch 'ps/fetch-mirror-optim'

2022-03-17T00:53:07Z

Various optimization for "git fetch". * ps/fetch-mirror-optim: refs/files-backend: optimize reading of symbolic refs remote: read symbolic refs via `refs_read_symbolic_ref()` refs: add ability for backends to special-case reading of symbolic refs fetch: avoid lookup of commits when not appending to FETCH_HEAD upload-pack: look up "want" lines via commit-graph

Merge branch 'ps/fetch-atomic'

2022-03-13T22:56:16Z

"git fetch" can make two separate fetches, but ref updates coming from them were in two separate ref transactions under "--atomic", which has been corrected. * ps/fetch-atomic: fetch: make `--atomic` flag cover pruning of refs fetch: make `--atomic` flag cover backfilling of tags refs: add interface to iterate over queued transactional updates fetch: report errors when backfilling tags fails fetch: control lifecycle of FETCH_HEAD in a single place fetch: backfill tags before setting upstream fetch: increase test coverage of fetches

refs: add ability for backends to special-case reading of symbolic refs

2022-03-01T18:13:46Z

Reading of symbolic and non-symbolic references is currently treated the same in reference backends: we always call `refs_read_raw_ref()` and then decide based on the returned flags what type it is. This has one downside though: symbolic references may be treated different from normal references in a backend from normal references. The packed-refs backend for example doesn't even know about symbolic references, and as a result it is pointless to even ask it for one. There are cases where we really only care about whether a reference is symbolic or not, but don't care about whether it exists at all or may be a non-symbolic reference. But it is not possible to optimize for this case right now, and as a consequence we will always first check for a loose reference to exist, and if it doesn't, we'll query the packed-refs backend for a known-to-not-be-symbolic reference. This is inefficient and requires us to search all packed references even though we know to not care for the result at all. Introduce a new function `refs_read_symbolic_ref()` which allows us to fix this case. This function will only ever return symbolic references and can thus optimize for the scenario layed out above. By default, if the backend doesn't provide an implementation for it, we just use the old code path and fall back to `read_raw_ref()`. But in case the backend provides its own, more efficient implementation, we will use that one instead. Note that this function is explicitly designed to not distinguish between missing references and non-symbolic references. If it did, we'd be forced to always search the packed-refs backend to see whether the symbolic reference the user asked for really doesn't exist, or if it exists as a non-symbolic reference. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ps/fetch-atomic' into ps/fetch-mirror-optim

2022-03-01T18:11:00Z

* ps/fetch-atomic: fetch: make `--atomic` flag cover pruning of refs fetch: make `--atomic` flag cover backfilling of tags refs: add interface to iterate over queued transactional updates fetch: report errors when backfilling tags fails fetch: control lifecycle of FETCH_HEAD in a single place fetch: backfill tags before setting upstream fetch: increase test coverage of fetches

Merge branch 'ab/date-mode-release'

2022-02-25T23:47:36Z

Plug (some) memory leaks around parse_date_format(). * ab/date-mode-release: date API: add and use a date_mode_release() date API: add basic API docs date API: provide and use a DATE_MODE_INIT date API: create a date.h, split from cache.h cache.h: remove always unused show_date_human() declaration

Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'

2022-02-18T21:53:27Z

Because a deletion of ref would need to remove it from both the loose ref store and the packed ref store, a delete-ref operation that logically removes one ref may end up invoking ref-transaction hook twice, which has been corrected. * ps/avoid-unnecessary-hook-invocation-with-packed-refs: refs: skip hooks when deleting uncovered packed refs refs: do not execute reference-transaction hook on packing refs refs: demonstrate excessive execution of the reference-transaction hook refs: allow skipping the reference-transaction hook refs: allow passing flags when beginning transactions refs: extract packed_refs_delete_refs() to allow control of transaction

refs: add interface to iterate over queued transactional updates

2022-02-17T19:19:44Z

There is no way for a caller to see whether a reference update has already been queued up for a given reference transaction. There are multiple alternatives to provide this functionality: - We may add a function that simply tells us whether a specific reference has already been queued. If implemented naively then this would potentially be quadratic in runtime behaviour if this question is asked repeatedly because we have to iterate over all references every time. The alternative would be to add a hashmap of all queued reference updates to speed up the lookup, but this adds overhead to all callers. - We may add a flag to `ref_transaction_add_update()` that causes it to skip duplicates, but this has the same runtime concerns as the first alternative. - We may add an interface which lets callers collect all updates which have already been queued such that he can avoid re-adding them. This is the most flexible approach and puts the burden on the caller, but also allows us to not impact any of the existing callsites which don't need this information. This commit implements the last approach: it allows us to compute the map of already-queued updates once up front such that we can then skip all subsequent references which are already part of this map. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano