aboutsummaryrefslogtreecommitdiffstats
path: root/refs/files-backend.c (follow)
AgeCommit message (Collapse)AuthorFilesLines
2023-10-09files-backend.c: avoid stat in 'loose_fill_ref_dir'Victoria Dye1-9/+5
Modify the 'readdir' loop in 'loose_fill_ref_dir' to, rather than 'stat' a file to determine whether it is a directory or not, use 'get_dtype'. Currently, the loop uses 'stat' to determine whether each dirent is a directory itself or not in order to construct the appropriate ref cache entry. If 'stat' fails (returning a negative value), the dirent is silently skipped; otherwise, 'S_ISDIR(st.st_mode)' is used to check whether the entry is a directory. On platforms that include an entry's d_type in in the 'dirent' struct, this extra 'stat' check is redundant. We can use the 'get_dtype' method to extract this information on platforms that support it (i.e. where NO_D_TYPE_IN_DIRENT is unset), and derive it with 'stat' on platforms that don't. Because 'stat' is an expensive call, this confers a modest-but-noticeable performance improvement when iterating over large numbers of refs (approximately 20% speedup in 'git for-each-ref' in a 30k ref repo). Unlike other existing usage of 'get_dtype', the 'follow_symlinks' arg is set to 1 to replicate the existing handling of symlink dirents. This unfortunately requires calling 'stat' on the associated entry regardless of platform, but symlinks in the loose ref store are highly unlikely since they'd need to be created manually by a user. Note that this patch also changes the condition for skipping creation of a ref entry from "when 'stat' fails" to "when the d_type is anything other than DT_REG or DT_DIR". If a dirent's d_type is DT_UNKNOWN (either because the platform doesn't support d_type in dirents or some other reason) or DT_LNK, 'get_dtype' will try to derive the underlying type with 'stat'. If the 'stat' fails, the d_type will remain 'DT_UNKNOWN' and dirent will be skipped. However, it will also be skipped if it is any other valid d_type (e.g. DT_FIFO for named pipes, DT_LNK for a nested symlink). Git does not handle these properly anyway, so we can safely constrain accepted types to directories and regular files. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-21Merge branch 'tb/refs-exclusion-and-packed-refs'Junio C Hamano1-2/+3
Enumerating refs in the packed-refs file, while excluding refs that match certain patterns, has been optimized. * tb/refs-exclusion-and-packed-refs: ls-refs.c: avoid enumerating hidden refs where possible upload-pack.c: avoid enumerating hidden refs where possible builtin/receive-pack.c: avoid enumerating hidden references refs.h: implement `hidden_refs_to_excludes()` refs.h: let `for_each_namespaced_ref()` take excluded patterns revision.h: store hidden refs in a `strvec` refs/packed-backend.c: add trace2 counters for jump list refs/packed-backend.c: implement jump lists to avoid excluded pattern(s) refs/packed-backend.c: refactor `find_reference_location()` refs: plumb `exclude_patterns` argument throughout builtin/for-each-ref.c: add `--exclude` option ref-filter.c: parameterize match functions over patterns ref-filter: add `ref_filter_clear()` ref-filter: clear reachable list pointers after freeing ref-filter.h: provide `REF_FILTER_INIT` refs.c: rename `ref_filter`
2023-07-10refs: plumb `exclude_patterns` argument throughoutTaylor Blau1-2/+3
The subsequent patch will want to access an optional `excluded_patterns` array within `refs/packed-backend.c` that will cull out certain references matching any of the given patterns on a best-effort basis. To do so, the refs subsystem needs to be updated to pass this value across a number of different locations. Prepare for a future patch by introducing this plumbing now, passing NULLs at top-level APIs in order to make that patch less noisy and more easily readable. Signed-off-by: Taylor Blau <me@ttaylorr.co> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21repository: remove unnecessary include of path.hElijah Newren1-0/+1
This also made it clear that several .c files that depended upon path.h were missing a #include for it; add the missing includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21cache.h: remove this no-longer-used headerElijah Newren1-1/+1
Since this header showed up in some places besides just #include statements, update/clean-up/remove those other places as well. Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got away with violating the rule that all files must start with an include of git-compat-util.h (or a short-list of alternate headers that happen to include it first). This change exposed the violation and caused it to stop building correctly; fix it by having it include git-compat-util.h first, as per policy. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-13Merge branch 'jc/pack-ref-exclude-include'Junio C Hamano1-10/+17
"git pack-refs" learns "--include" and "--exclude" to tweak the ref hierarchy to be packed using pattern matching. * jc/pack-ref-exclude-include: pack-refs: teach pack-refs --include option pack-refs: teach --exclude option to exclude refs from being packed docs: clarify git-pack-refs --all will pack all refs
2023-05-12pack-refs: teach pack-refs --include optionJohn Cai1-8/+10
Allow users to be more selective over which refs to pack by adding an --include option to git-pack-refs. The existing options allow some measure of selectivity. By default git-pack-refs packs all tags. --all can be used to include all refs, and the previous commit added the ability to exclude certain refs with --exclude. While these knobs give the user some selection over which refs to pack, it could be useful to give more control. For instance, a repository may have a set of branches that are rarely updated and would benefit from being packed. --include would allow the user to easily include a set of branches to be packed while leaving everything else unpacked. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-12pack-refs: teach --exclude option to exclude refs from being packedJohn Cai1-6/+10
At GitLab, we have a system that creates ephemeral internal refs that don't live long before getting deleted. Having an option to exclude certain refs from a packed-refs file allows these internal references to be deleted much more efficiently. Add an --exclude option to the pack-refs builtin, and use the ref exclusions API to exclude certain refs from being packed into the final packed-refs file Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-24hash-ll.h: split out of hash.h to remove dependency on repository.hElijah Newren1-0/+1
hash.h depends upon and includes repository.h, due to the definition and use of the_hash_algo (defined as the_repository->hash_algo). However, most headers trying to include hash.h are only interested in the layout of the structs like object_id. Move the parts of hash.h that do not depend upon repository.h into a new file hash-ll.h (the "low level" parts of hash.h), and adjust other files to use this new header where the convenience inline functions aren't needed. This allows hash.h and object.h to be fairly small, minimal headers. It also exposes a lot of hidden dependencies on both path.h (which was brought in by repository.h) and repository.h (which was previously implicitly brought in by object.h), so also adjust other files to be more explicit about what they depend upon. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-24copy.h: move declarations for copy.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-11object-file.h: move declarations for object-file.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21write-or-die.h: move declarations for write-or-die.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21setup.h: move declarations for setup.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21environment.h: move declarations for environment.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21wrapper.h: move declarations for wrapper.c functions from cache.hElijah Newren1-1/+2
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21treewide: be explicit about dependence on gettext.hElijah Newren1-0/+1
Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23ident.h: move ident-related declarations out of cache.hElijah Newren1-0/+1
These functions were all defined in a separate ident.c already, so create ident.h and move the declarations into that file. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23cache.h: remove dependence on hex.h; make other files include it explicitlyElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-19refs: unify parse_worktree_ref() and ref_type()Han-Wen Nienhuys1-44/+36
The logic to handle worktree refs (worktrees/NAME/REF and main-worktree/REF) existed in two places: * ref_type() in refs.c * parse_worktree_ref() in worktree.c Collapse this logic together in one function parse_worktree_ref(): this avoids having to cross-check the result of parse_worktree_ref() and ref_type(). Introduce enum ref_worktree_type, which is slightly different from enum ref_type. The latter is a misleading name (one would think that 'ref_type' would have the symref option). Instead, enum ref_worktree_type only makes explicit how a refname relates to a worktree. From this point of view, HEAD and refs/bisect/abc are the same: they specify the current worktree implicitly. The files-backend must avoid packing refs/bisect/* and friends into packed-refs, so expose is_per_worktree_ref() separately. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-01git-compat-util.h: use "UNUSED", not "UNUSED(var)"Ævar Arnfjörð Bjarmason1-7/+7
As reported in [1] the "UNUSED(var)" macro introduced in 2174b8c75de (Merge branch 'jk/unused-annotation' into next, 2022-08-24) breaks coccinelle's parsing of our sources in files where it occurs. Let's instead partially go with the approach suggested in [2] of making this not take an argument. As noted in [1] "coccinelle" will ignore such tokens in argument lists that it doesn't know about, and it's less of a surprise to syntax highlighters. This undoes the "help us notice when a parameter marked as unused is actually use" part of 9b240347543 (git-compat-util: add UNUSED macro, 2022-08-19), a subsequent commit will further tweak the macro to implement a replacement for that functionality. 1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/ 2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19refs: mark unused virtual method parametersJeff King1-5/+5
The refs code uses various polymorphic types (e.g., loose vs packed ref_stores, abstracted iterators). Not every virtual function or callback needs all of its parameters. Let's mark the unused ones to quiet -Wunused-parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19refs: mark unused each_ref_fn parametersJeff King1-1/+3
Functions used with for_each_ref(), etc, need to conform to the each_ref_fn interface. But most of them don't need every parameter; let's annotate the unused ones to quiet -Wunused-parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-04-13Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"Junio C Hamano1-19/+7
This reverts commit 991b4d47f0accd3955d05927d5ce434e03ffbdb6, reversing changes made to bcd020f88e1e22f38422ac3f73ab06b34ec4bef1.
2022-03-29Merge branch 'ab/refs-various-fixes'Junio C Hamano1-32/+32
Code clean-up. * ab/refs-various-fixes: refs debug: add a wrapper for "read_symbolic_ref" packed-backend: remove stub BUG(...) functions misc *.c: use designated initializers for struct assignments refs: use designated initializers for "struct ref_iterator_vtable" refs: use designated initializers for "struct ref_storage_be"
2022-03-25Merge branch 'ps/fsync-refs'Junio C Hamano1-0/+1
Updates to refs traditionally weren't fsync'ed, but we can configure using core.fsync variable to do so. * ps/fsync-refs: core.fsync: new option to harden references
2022-03-17refs: use designated initializers for "struct ref_iterator_vtable"Ævar Arnfjörð Bjarmason1-6/+6
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-17refs: use designated initializers for "struct ref_storage_be"Ævar Arnfjörð Bjarmason1-26/+26
Change the definition of the three refs backends we currently carry to use designated initializers. The "= NULL" assignments being retained here are redundant, and could be removed, but let's keep them for clarity. All of these backends define almost all fields, so we're not saving much in terms of line count by omitting these, but e.g. for "refs_be_debug" it's immediately apparent that we're omitting "init" when comparing its assignment to the others. This is a follow-up to similar work merged in bd4232fac33 (Merge branch 'ab/struct-init', 2021-07-16), a4b9fb6a5cf (Merge branch 'ab/designated-initializers-more', 2021-10-18) and a30321b9eae (Merge branch 'ab/designated-initializers' into ab/designated-initializers-more, 2021-09-27). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-15core.fsync: new option to harden referencesPatrick Steinhardt1-0/+1
When writing both loose and packed references to disk we first create a lockfile, write the updated values into that lockfile, and on commit we rename the file into place. According to filesystem developers, this behaviour is broken because applications should always sync data to disk before doing the final rename to ensure data consistency [1][2][3]. If applications fail to do this correctly, a hard crash of the machine can easily result in corrupted on-disk data. This kind of corruption can in fact be easily observed with Git when the machine hard-resets shortly after writing references to disk. On machines with ext4, this will likely lead to the "empty files" problem: the file has been renamed, but its data has not been synced to disk. The result is that the reference is corrupt, and in the worst case this can lead to data loss. Implement a new option to harden references so that users and admins can avoid this scenario by syncing locked loose and packed references to disk before we rename them into place. [1]: https://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/ [2]: https://btrfs.wiki.kernel.org/index.php/FAQ (What are the crash guarantees of overwrite-by-rename) [3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/admin-guide/ext4.rst (see auto_da_alloc) Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-01refs/files-backend: optimize reading of symbolic refsPatrick Steinhardt1-6/+28
When reading references via `files_read_raw_ref()` we always consult both the loose reference, and if that wasn't found, we also consult the packed-refs file. While this makes sense to read a generic reference, it is wasteful in the case where we only care about symbolic references: the packed-refs backend does not support them, and thus it cannot ever return one for us. Special-case reading of symbolic references for the files backend such that we always skip asking the packed-refs backend. We use `refs_read_symbolic_ref()` extensively to determine whether we need to skip updating local symbolic references during a fetch, which is why the change results in a significant speedup when doing fetches in repositories with huge numbers of references. The following benchmark executes a mirror-fetch in a repository with about 2 million references via `git fetch --prune --no-write-fetch-head +refs/*:refs/*`: Benchmark 1: HEAD~ Time (mean ± σ): 68.372 s ± 2.344 s [User: 65.629 s, System: 8.786 s] Range (min … max): 65.745 s … 70.246 s 3 runs Benchmark 2: HEAD Time (mean ± σ): 60.259 s ± 0.343 s [User: 61.019 s, System: 7.245 s] Range (min … max): 60.003 s … 60.649 s 3 runs Summary 'HEAD' ran 1.13 ± 0.04 times faster than 'HEAD~' Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-01refs: add ability for backends to special-case reading of symbolic refsPatrick Steinhardt1-0/+1
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 <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-18Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'Junio C Hamano1-7/+19
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
2022-01-26refs API: remove "failure_errno" from refs_resolve_ref_unsafe()Ævar Arnfjörð Bjarmason1-22/+9
Remove the now-unused "failure_errno" parameter from the refs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its callers explicitly request the errno via an output parameter. As that series shows all but one caller ended up passing in a boilerplate "ignore_errno", since they only cared about whether the return value was NULL or not, i.e. if the ref could be resolved. There was one small issue with that series fixed with a follow-up in 31e39123695 (Merge branch 'ab/refs-errno-cleanup', 2022-01-14) a small bug in that series was fixed. After those two there was one caller left in sequencer.c that used the "failure_errno', but as of the preceding commit it uses a boilerplate "ignore_errno" instead. This leaves the public refs API without any use of "failure_errno" at all. We could still do with a bit of cleanup and generalization between refs.c and refs/files-backend.c before the "reftable" integration lands, but that's all internal to the reference code itself. So let's remove this output parameter. Not only isn't it used now, but it's unlikely that we'll want it again in the future. We'd like to slowly move the refs API to a more file-backend independent way of communicating error codes, having it use a "failure_errno" was only the first step in that direction. If this or any other function needs to communicate what specifically is wrong with the requested "refname" it'll be better to have the function set some output enum of well-defined error states than piggy-backend on "errno". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: skip hooks when deleting uncovered packed refsPatrick Steinhardt1-3/+6
When deleting refs from the loose-files refs backend, then we need to be careful to also delete the same ref from the packed refs backend, if it exists. If we don't, then deleting the loose ref would "uncover" the packed ref. We thus always have to queue up deletions of refs for both the loose and the packed refs backend. This is done in two separate transactions, where the end result is that the reference-transaction hook is executed twice for the deleted refs. This behaviour is quite misleading: it's exposing implementation details of how the files backend works to the user, in contrast to the logical updates that we'd really want to expose via the hook. Worse yet, whether the hook gets executed once or twice depends on how well-packed the repository is: if the ref only exists as a loose ref, then we execute it once, otherwise if it is also packed then we execute it twice. Fix this behaviour and don't execute the reference-transaction hook at all when refs in the packed-refs backend if it's driven by the files backend. This works as expected even in case the refs to be deleted only exist in the packed-refs backend because the loose-backend always queues refs in its own transaction even if they don't exist such that they can be locked for concurrent creation. And it also does the right thing in case neither of the backends has the ref because that would cause the transaction to fail completely. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: do not execute reference-transaction hook on packing refsPatrick Steinhardt1-2/+4
The reference-transaction hook is supposed to track logical changes to references, but it currently also gets executed when packing refs in a repository. This is unexpected and ultimately not all that useful: packing refs is not supposed to result in any user-visible change to the refs' state, and it ultimately is an implementation detail of how refs stores work. Fix this excessive execution of the hook when packing refs. Reported-by: Waleed Khan <me@waleedkhan.name> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: allow passing flags when beginning transactionsPatrick Steinhardt1-5/+5
We do not currently have any flags when creating reference transactions, but we'll add one to disable execution of the reference transaction hook in some cases. Allow passing flags to `ref_store_transaction_begin()` to prepare for this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: extract packed_refs_delete_refs() to allow control of transactionPatrick Steinhardt1-3/+10
When deleting loose refs, then we also have to delete the refs in the packed backend. This is done by calling `refs_delete_refs()`, which then uses the packed-backend's logic to delete refs. This doesn't allow us to exercise any control over the reference transaction which is being created in the packed backend, which is required in a subsequent commit. Extract a new function `packed_refs_delete_refs()`, which hosts most of the logic to delete refs except for creating the transaction itself. Like this, we can easily create the transaction in the files backend and thus exert more control over it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-14Merge branch 'ab/refs-errno-cleanup'Junio C Hamano1-2/+1
A brown-paper-bag fix on top of a topic that was merged during this cycle. * ab/refs-errno-cleanup: refs API: use "failure_errno", not "errno"
2022-01-13refs API: use "failure_errno", not "errno"Ævar Arnfjörð Bjarmason1-2/+1
Fix a logic error in refs_resolve_ref_unsafe() introduced in a recent series of mine to abstract the refs API away from errno. See 96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29)for that series. In that series introduction of "failure_errno" to refs_resolve_ref_unsafe came in ef18119dec8 (refs API: add a version of refs_resolve_ref_unsafe() with "errno", 2021-10-16). There we'd set "errno = 0" immediately before refs_read_raw_ref(), and then set "failure_errno" to "errno" if errno was non-zero afterwards. Then in the next commit 8b72fea7e91 (refs API: make refs_read_raw_ref() not set errno, 2021-10-16) we started expecting "refs_read_raw_ref()" to set "failure_errno". It would do that if refs_read_raw_ref() failed, but it wouldn't be the same errno. So we might set the "errno" here to any arbitrary bad value, and end up e.g. returning NULL when we meant to return the refname from refs_resolve_ref_unsafe(), or the other way around. Instrumenting this code will reveal cases where refs_read_raw_ref() will fail, and "errno" and "failure_errno" will be set to different values. In practice I haven't found a case where this scary bug changed anything in practice. The reason for that is that we'll not care about the actual value of "errno" here per-se, but only whether: 1. We have an errno 2. If it's one of ENOENT, EISDIR or ENOTDIR. See the adjacent code added in a1c1d8170db (refs_resolve_ref_unsafe: handle d/f conflicts for writes, 2017-10-06) I.e. if we clobber "failure_errno" with "errno", but it happened to be one of those three, and we'll clobber it with another one of the three we were OK. Perhaps there are cases where the difference ended up mattering, but I haven't found them. Instrumenting the test suite to fail if "errno" and "failure_errno" are different shows a lot of failures, checking if they're different *and* one is but not the other is outside that list of three "errno" values yields no failures. But let's fix the obvious bug. We should just stop paying attention to "errno" in refs_resolve_ref_unsafe(). In addition let's change the partial resetting of "errno" in files_read_raw_ref() to happen just before the "return", to ensure that any such bug will be more easily spotted in the future. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-10Merge branch 'ab/reflog-prep'Junio C Hamano1-24/+20
Code refactoring in the reflog part of refs API. * ab/reflog-prep: reflog + refs-backend: move "verbose" out of the backend refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN reflog: reduce scope of "struct rev_info" reflog expire: don't use lookup_commit_reference_gently() reflog expire: refactor & use "tip_commit" only for UE_NORMAL reflog expire: use "switch" over enum values reflog: change one->many worktree->refnames to use a string_list reflog expire: narrow scope of "cb" in cmd_reflog_expire() reflog delete: narrow scope of "cmd" passed to count_reflog_ent()
2021-12-22reflog + refs-backend: move "verbose" out of the backendÆvar Arnfjörð Bjarmason1-24/+20
Move the handling of the "verbose" flag entirely out of "refs/files-backend.c" and into "builtin/reflog.c". This allows the backend to stop knowing about the EXPIRE_REFLOGS_VERBOSE flag. The expire_reflog_ent() function shouldn't need to deal with the implementation detail of whether or not we're emitting verbose output, by doing this the --verbose output becomes backend-agnostic, so reftable will get the same output. I think the output is rather bad currently, and should e.g. be implemented with some better future mode of progress.[ch], but that's a topic for another improvement. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-22refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUNÆvar Arnfjörð Bjarmason1-2/+2
It's not possible for "cb->newlog" to be NULL if !EXPIRE_REFLOGS_DRY_RUN, since files_reflog_expire() would have error()'d and taken the "goto failure" branch if it couldn't open the file. By not using the "newlog" field private to "file-backend.c"'s "struct expire_reflog_cb", we can move this verbosity logging to "builtin/reflog.c" in a subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-22refs: centralize initialization of the base ref_store.Han-Wen Nienhuys1-4/+1
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-22refs: pass gitdir to packed_ref_store_createHan-Wen Nienhuys1-3/+2
This is consistent with the calling convention for ref backend creation, and avoids storing ".git/packed-refs" (the name of a regular file) in a variable called ref_store::gitdir. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-15Merge branch 'hn/allow-bogus-oid-in-ref-tests'Junio C Hamano1-23/+30
The test helper for refs subsystem learned to write bogus and/or nonexistent object name to refs to simulate error situations we want to test Git in. * hn/allow-bogus-oid-in-ref-tests: t1430: create valid symrefs using test-helper t1430: remove refs using test-tool refs: introduce REF_SKIP_REFNAME_VERIFICATION flag refs: introduce REF_SKIP_OID_VERIFICATION flag refs: update comment. test-ref-store: plug memory leak in cmd_delete_refs test-ref-store: parse symbolic flag constants test-ref-store: remove force-create argument for create-reflog
2021-12-10Merge branch 'hn/create-reflog-simplify'Junio C Hamano1-3/+2
A small simplification of API. * hn/create-reflog-simplify: refs: drop force_create argument of create_reflog API
2021-12-07refs: introduce REF_SKIP_OID_VERIFICATION flagHan-Wen Nienhuys1-21/+29
This lets the ref-store test helper write non-existent or unparsable objects into the ref storage. Use this to make t1006 and t3800 independent of the files storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07refs: update comment.Han-Wen Nienhuys1-2/+1
REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the `ref_update` flags"). Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-29Merge branch 'ab/refs-errno-cleanup'Junio C Hamano1-51/+102
The "remainder" of hn/refs-errno-cleanup topic. * ab/refs-errno-cleanup: (21 commits) refs API: post-migration API renaming [2/2] refs API: post-migration API renaming [1/2] refs API: don't expose "errno" in run_transaction_hook() refs API: make expand_ref() & repo_dwim_log() not set errno refs API: make resolve_ref_unsafe() not set errno refs API: make refs_ref_exists() not set errno refs API: make refs_resolve_refdup() not set errno refs tests: ignore ignore errno in test-ref-store helper refs API: ignore errno in worktree.c's find_shared_symref() refs API: ignore errno in worktree.c's add_head_info() refs API: make files_copy_or_rename_ref() et al not set errno refs API: make loose_fill_ref_dir() not set errno refs API: make resolve_gitlink_ref() not set errno refs API: remove refs_read_ref_full() wrapper refs/files: remove "name exist?" check in lock_ref_oid_basic() reflog tests: add --updateref tests refs API: make refs_rename_ref_available() static refs API: make parse_loose_ref_contents() not set errno refs API: make refs_read_raw_ref() not set errno refs API: add a version of refs_resolve_ref_unsafe() with "errno" ...
2021-11-22refs: drop force_create argument of create_reflog APIHan-Wen Nienhuys1-3/+2
There is only one caller, builtin/checkout.c, and it hardcodes force_create=1. This argument was introduced in abd0cd3a301 (refs: new public ref function: safe_create_reflog, 2015-07-21), which promised to immediately use it in a follow-on commit, but that never happened. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-25Merge branch 'jt/no-abuse-alternate-odb-for-submodules'Junio C Hamano1-5/+11
Follow through the work to use the repo interface to access submodule objects in-process, instead of abusing the alternate object database interface. * jt/no-abuse-alternate-odb-for-submodules: submodule: trace adding submodule ODB as alternate submodule: pass repo to check_has_commit() object-file: only register submodule ODB if needed merge-{ort,recursive}: remove add_submodule_odb() refs: peeling non-the_repository iterators is BUG refs: teach arbitrary repo support to iterators refs: plumb repo into ref stores