summaryrefslogtreecommitdiff
path: root/refs.h
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'gc/submodule-clone-update-with-branches' into jchTaylor Blau2022-11-181-1/+4
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git clone --recurse-submodules" and "git submodule update" learns to honor the "propagete branches" option. * gc/submodule-clone-update-with-branches: clone, submodule update: create and check out branches submodule--helper: remove update_data.suboid submodule update: refactor update targets submodule: return target of submodule symref t5617: drop references to remote-tracking branches submodule--helper clone: create named branch repo-settings: add submodule_propagate_branches clone: teach --detach option
| * submodule: return target of submodule symrefGlen Choo2022-10-301-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | resolve_gitlink_ref() can tell us which oid the submodule ref is pointing to, but in a future commit, we would also like to know the symbolic ref target if we are checking a symbolic ref. Teach resolve_gitlink_ref() to "return" the symbolic ref's target via an "out" parameter. This changes resolve_gitlink_ref()'s signature so that new callers trying to use the old signature will be stopped by the compiler. If we returned the target instead (just like refs_resolve_ref_unsafe()), we would be more consistent with refs_resolve_ref_unsafe(), but callers expecting the old signature will get the opposite return value from what they expect (since exit code 0 means success, but NULL pointer means failure). We should do this refactor once we think that nobody will try to use the old signature. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | refs: get rid of global list of hidden refsPatrick Steinhardt2022-11-171-2/+3
|/ | | | | | | | | | | | | | | | | We're about to add a new argument to git-rev-list(1) that allows it to add all references that are visible when taking `transfer.hideRefs` et al into account. This will require us to potentially parse multiple sets of hidden refs, which is not easily possible right now as there is only a single, global instance of the list of parsed hidden refs. Refactor `parse_hide_refs_config()` and `ref_is_hidden()` so that both take the list of hidden references as input and adjust callers to keep a local list, instead. This allows us to easily use multiple hidden-ref lists. Furthermore, it allows us to properly free this list before we exit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* refs: unify parse_worktree_ref() and ref_type()Han-Wen Nienhuys2022-09-191-7/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* refs: add array of ref namespacesDerrick Stolee2022-08-051-0/+46
| | | | | | | | | | | | | | | | | Git interprets different meanings to different refs based on their names. Some meanings are cosmetic, like how refs in 'refs/remotes/*' are colored differently from refs in 'refs/heads/*'. Others are more critical, such as how replace refs are interpreted. Before making behavior changes based on ref namespaces, collect all known ref namespaces into a array of ref_namespace_info structs. This array is indexed by the new ref_namespace enum for quick access. As of this change, this array is purely documentation. Future changes will add dependencies on this array. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"Junio C Hamano2022-04-131-7/+1
| | | | | This reverts commit 991b4d47f0accd3955d05927d5ce434e03ffbdb6, reversing changes made to bcd020f88e1e22f38422ac3f73ab06b34ec4bef1.
* refs: add ability for backends to special-case reading of symbolic refsPatrick Steinhardt2022-03-011-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Merge branch 'ps/fetch-atomic' into ps/fetch-mirror-optimJunio C Hamano2022-03-011-0/+14
|\ | | | | | | | | | | | | | | | | | | | | * 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 interface to iterate over queued transactional updatesPatrick Steinhardt2022-02-171-0/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'Junio C Hamano2022-02-181-1/+7
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | 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: allow skipping the reference-transaction hookPatrick Steinhardt2022-01-171-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The reference-transaction hook is executing whenever we prepare, commit or abort a reference transaction. While this is mostly intentional, in case of the files backend we're leaking the implementation detail that the store is in fact a composite store with one loose and one packed backend to the caller. So while we want to execute the hook for all logical updates, executing it for such implementation details is unexpected. Prepare for a fix by adding a new flag which allows to skip execution of the hook. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * refs: allow passing flags when beginning transactionsPatrick Steinhardt2022-01-171-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | 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>
* | refs API: remove "failure_errno" from refs_resolve_ref_unsafe()Ævar Arnfjörð Bjarmason2022-01-261-6/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | reflog + refs-backend: move "verbose" out of the backendÆvar Arnfjörð Bjarmason2021-12-221-2/+1
|/ | | | | | | | | | | | | | | | | | 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>
* Merge branch 'hn/allow-bogus-oid-in-ref-tests'Junio C Hamano2021-12-151-2/+14
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * refs: introduce REF_SKIP_REFNAME_VERIFICATION flagHan-Wen Nienhuys2021-12-071-2/+8
| | | | | | | | | | | | | | | | Use this flag with the test-helper in t1430, to avoid direct writes to the ref database. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * refs: introduce REF_SKIP_OID_VERIFICATION flagHan-Wen Nienhuys2021-12-071-1/+7
| | | | | | | | | | | | | | | | | | | | 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>
* | Merge branch 'jc/reflog-iterator-callback-doc'Junio C Hamano2021-12-151-1/+23
|\ \ | | | | | | | | | | | | | | | | | | | | | Document the parameters given to the reflog entry iterator callback functions. * jc/reflog-iterator-callback-doc: refs: document callback for reflog-ent iterators
| * | refs: document callback for reflog-ent iteratorsJunio C Hamano2021-11-281-1/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | refs_for_each_reflog_ent() and refs_for_each_reflog_ent_reverse() functions take a callback function that gets called with the details of each reflog entry. Its parameters were not documented beyond their names. Elaborate a bit on each of them. Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'hn/create-reflog-simplify'Junio C Hamano2021-12-101-2/+2
|\ \ \ | |_|/ |/| | | | | | | | | | | | | | A small simplification of API. * hn/create-reflog-simplify: refs: drop force_create argument of create_reflog API
| * | refs: drop force_create argument of create_reflog APIHan-Wen Nienhuys2021-11-221-2/+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>
* | | refs API: post-migration API renaming [2/2]Ævar Arnfjörð Bjarmason2021-10-161-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename the transitory refs_werrres_ref_unsafe() function to refs_resolve_ref_unsafe(), now that all callers of the old function have learned to pass in a "failure_errno" parameter. The coccinelle semantic patch added in the preceding commit works, but I couldn't figure out how to get spatch(1) to re-flow these argument lists (and sometimes make lines way too long), so this rename was done with: perl -pi -e 's/refs_werrres_ref_unsafe/refs_resolve_ref_unsafe/g' \ $(git grep -l refs_werrres_ref_unsafe -- '*.c') But after that "make contrib/coccinelle/refs.cocci.patch" comes up empty, so the result would have been the same. Let's remove that transitory semantic patch file, we won't need to retain it for any other in-flight changes, refs_werrres_ref_unsafe() only existed within this patch series. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | refs API: post-migration API renaming [1/2]Ævar Arnfjörð Bjarmason2021-10-161-13/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In preceding commits all callers of refs_resolve_ref_unsafe() were migrated to the transitory refs_werrres_ref_unsafe() function. As a first step in getting rid of it let's remove the old function from the public API (it went unused in a preceding commit). We then provide both a coccinelle rule to do the rename, and a macro to avoid breaking the existing callers. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | refs API: remove refs_read_ref_full() wrapperÆvar Arnfjörð Bjarmason2021-10-161-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the refs_read_ref_full() wrapper in favor of migrating various refs.c API users to the underlying refs_werrres_ref_unsafe() function. A careful reading of these callers shows that the callers of this function did not care about "errno", by moving away from the refs_resolve_ref_unsafe() wrapper we can be sure that nothing relies on it anymore. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | refs API: add a version of refs_resolve_ref_unsafe() with "errno"Ævar Arnfjörð Bjarmason2021-10-161-0/+12
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new refs_werrres_ref_unsafe() function, which is like refs_resolve_ref_unsafe() except that it explicitly saves away the "errno" to a passed-in parameter, the refs_resolve_ref_unsafe() then becomes a wrapper for it. In subsequent commits we'll migrate code over to it, before finally making "refs_resolve_ref_unsafe()" with an "errno" parameter the canonical version, so this this function exists only so that we can incrementally migrate callers, it will be going away in a subsequent commit. As the added comment notes has a rather tortured name to be the same length as "refs_resolve_ref_unsafe", to avoid churn as we won't need to re-indent the argument lists, similarly the documentation and structure of it in refs.h is designed to minimize a diff in a subsequent commit, where that documentation will be added to the new refs_resolve_ref_unsafe(). At the end of this migration the "meaningful errno" TODO item left in 76d70dc0c63 (refs.c: make resolve_ref_unsafe set errno to something meaningful on error, 2014-06-20) will be resolved. As can be seen from the use of refs_read_raw_ref() we'll also need to convert some functions that the new refs_werrres_ref_unsafe() itself calls to take this "failure_errno". That will be done in subsequent commits. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/ref-paranoia'Junio C Hamano2021-10-111-6/+3
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The ref iteration code used to optionally allow dangling refs to be shown, which has been tightened up. * jk/ref-paranoia: refs: drop "broken" flag from for_each_fullref_in() ref-filter: drop broken-ref code entirely ref-filter: stop setting FILTER_REFS_INCLUDE_BROKEN repack, prune: drop GIT_REF_PARANOIA settings refs: turn on GIT_REF_PARANOIA by default refs: omit dangling symrefs when using GIT_REF_PARANOIA refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flag refs-internal.h: reorganize DO_FOR_EACH_* flag documentation refs-internal.h: move DO_FOR_EACH_* flags next to each other t5312: be more assertive about command failure t5312: test non-destructive repack t5312: create bogus ref as necessary t5312: drop "verbose" helper t5600: provide detached HEAD for corruption failures t5516: don't use HEAD ref for invalid ref-deletion tests t7900: clean up some more broken refs
| * | refs: drop "broken" flag from for_each_fullref_in()Jeff King2021-09-271-6/+3
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | No callers pass in anything but "0" here. Likewise to our sibling functions. Note that some of them ferry along the flag, but none of their callers pass anything but "0" either. Nor is anybody likely to change that. Callers which really want to see all of the raw refs use for_each_rawref(). And anybody interested in iterating a subset of the refs will likely be happy to use the now-default behavior of showing broken refs, but omitting dangling symlinks. So we can get rid of this whole feature. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/retire-refs-unused-funcs'Junio C Hamano2021-10-061-2/+0
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code cleanup. * ab/retire-refs-unused-funcs: refs/ref-cache.[ch]: remove "incomplete" from create_dir_entry() refs/ref-cache.c: remove "mkdir" parameter from find_containing_dir() refs/ref-cache.[ch]: remove unused add_ref_entry() refs/ref-cache.[ch]: remove unused remove_entry_from_dir() refs.[ch]: remove unused ref_storage_backend_exists()
| * | refs.[ch]: remove unused ref_storage_backend_exists()Ævar Arnfjörð Bjarmason2021-09-281-2/+0
| |/ | | | | | | | | | | | | | | | | This function was added in 3dce444f178 (refs: add a backend method structure, 2016-09-04), but has never been used by anything. The only caller that might care uses find_ref_storage_backend() directly. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | refs API: remove OID argument to reflog_expire()Ævar Arnfjörð Bjarmason2021-08-251-4/+3
| | | | | | | | | | | | | | | | | | Since the the preceding commit the "oid" parameter to reflog_expire() is always NULL, but it was not cleaned up to reduce the size of the diff. Let's do that subsequent API and documentation cleanup now. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | reflog expire: don't lock reflogs using previously seen OIDÆvar Arnfjörð Bjarmason2021-08-251-1/+1
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | During reflog expiry, the cmd_reflog_expire() function first iterates over all reflogs in logs/*, and then one-by-one acquires the lock for each one and expires it. This behavior has been with us since this command was implemented in 4264dc15e1 ("git reflog expire", 2006-12-19). Change this to stop calling lock_ref_oid_basic() with the OID we saw when we looped over the logs, instead have it pass the OID it managed to lock. This mostly mitigates a race condition where e.g. "git gc" will fail in a concurrently updated repository because the branch moved since "git reflog expire --all" was started. I.e. with: error: cannot lock ref '<refname>': ref '<refname>' is at <OID-A> but expected <OID-B> This behavior of passing in an "oid" was needed for an edge-case that I've untangled in this and preceding commits though, namely that we needed this OID because we'd: 1. Lookup the reflog name/OID via dwim_log() 2. With that OID, lock the reflog 3. Later in builtin/reflog.c we use the OID we looked as input to lookup_commit_reference_gently(), assured that it's equal to the OID we got from dwim_log(). We can be sure that this change is safe to make because between dwim_log (step #1) and lock_ref_oid_basic (step #2) there was no other logic relevant to the OID or expiry run in the cmd_reflog_expire() caller. We can thus treat that code as a black box, before and after this change it would get an OID that's been locked, the only difference is that now we mostly won't be failing to get the lock due to the TOCTOU race[0]. That failure was purely an implementation detail in how the "current OID" was looked up, it was divorced from the locking mechanism. What do we mean with "mostly"? It mostly mitigates it because we'll still run into cases where the ref is locked and being updated as we want to expire it, and other git processes wanting to update the refs will in turn race with us as we expire the reflog. That remaining race can in turn be mitigated with the core.filesRefLockTimeout setting, see 4ff0f01cb7 ("refs: retry acquiring reference locks for 100ms", 2017-08-21). In practice if that value is high enough we'll probably never have ref updates or reflog expiry failing, since the clients involved will retry for far longer than the time any of those operations could take. See [1] for an initial report of how this impacted "git gc" and a large discussion about this change in early 2019. In particular patch looked good to Michael Haggerty, see his[2]. That message seems to not have made it to the ML archive, its content is quoted in full in my [3]. I'm leaving behind now-unused code the refs API etc. that takes the now-NULL "unused_oid" argument, and other code that can be simplified now that we never have on OID in that context, that'll be cleaned up in subsequent commits, but for now let's narrowly focus on fixing the "git gc" issue. As the modified assert() shows we always pass a NULL oid to reflog_expire() now. Unfortunately this sort of probabilistic contention is hard to turn into a test. I've tested this by running the following three subshells in concurrent terminals: ( rm -rf /tmp/git && git init /tmp/git && while true do head -c 10 /dev/urandom | hexdump >/tmp/git/out && git -C /tmp/git add out && git -C /tmp/git commit -m"out" done ) ( rm -rf /tmp/git-clone && git clone file:///tmp/git /tmp/git-clone && while git -C /tmp/git-clone pull do date done ) ( while git -C /tmp/git-clone reflog expire --all do date done ) Before this change the "reflog expire" would fail really quickly with the "but expected" error noted above. After this change both the "pull" and "reflog expire" will run for a while, but eventually fail because I get unlucky with core.filesRefLockTimeout (the "reflog expire" is in a really tight loop). As noted above that can in turn be mitigated with higher values of core.filesRefLockTimeout than the 100ms default. As noted in the commentary added in the preceding commit there's also the case of branches being racily deleted, that can be tested by adding this to the above: ( while git -C /tmp/git-clone branch topic master && git -C /tmp/git-clone branch -D topic do date done ) With core.filesRefLockTimeout set to 10 seconds (it can probably be a lot lower) I managed to run all four of these concurrently for about an hour, and accumulated ~125k commits, auto-gc's and all, and didn't have a single failure. The loops visibly stall while waiting for the lock, but that's expected and desired behavior. 0. https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use 1. https://lore.kernel.org/git/87tvg7brlm.fsf@evledraar.gmail.com/ 2. http://lore.kernel.org/git/b870a17d-2103-41b8-3cbc-7389d5fff33a@alum.mit.edu 3. https://lore.kernel.org/git/87pnqkco8v.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/ls-refs-optim'Junio C Hamano2021-02-051-0/+9
|\ | | | | | | | | | | | | | | | | | | The ls-refs protocol operation has been optimized to narrow the sub-hierarchy of refs/ it walks to produce response. * tb/ls-refs-optim: ls-refs.c: traverse prefixes of disjoint "ref-prefix" sets ls-refs.c: initialize 'prefixes' before using it refs: expose 'for_each_fullref_in_prefixes'
| * refs: expose 'for_each_fullref_in_prefixes'Taylor Blau2021-01-221-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This function was used in the ref-filter.c code to find the longest common prefix of among a set of refspecs, and then to iterate all of the references that descend from that prefix. A future patch will want to use that same code from ls-refs.c, so prepare by exposing and moving it to refs.c. Since there is nothing specific to the ref-filter code here (other than that it was previously the only caller of this function), this really belongs in the more generic refs.h header. The code moved in this patch is identical before and after, with the one exception of renaming some arguments to be consistent with other functions exposed in refs.h. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | refs: switch peel_ref() to peel_iterated_oid()Jeff King2021-01-211-10/+10
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The peel_ref() interface is confusing and error-prone: - it's typically used by ref iteration callbacks that have both a refname and oid. But since they pass only the refname, we may load the ref value from the filesystem again. This is inefficient, but also means we are open to a race if somebody simultaneously updates the ref. E.g., this: int some_ref_cb(const char *refname, const struct object_id *oid, ...) { if (!peel_ref(refname, &peeled)) printf("%s peels to %s", oid_to_hex(oid), oid_to_hex(&peeled); } could print nonsense. It is correct to say "refname peels to..." (you may see the "before" value or the "after" value, either of which is consistent), but mentioning both oids may be mixing before/after values. Worse, whether this is possible depends on whether the optimization to read from the current iterator value kicks in. So it is actually not possible with: for_each_ref(some_ref_cb); but it _is_ possible with: head_ref(some_ref_cb); which does not use the iterator mechanism (though in practice, HEAD should never peel to anything, so this may not be triggerable). - it must take a fully-qualified refname for the read_ref_full() code path to work. Yet we routinely pass it partial refnames from callbacks to for_each_tag_ref(), etc. This happens to work when iterating because there we do not call read_ref_full() at all, and only use the passed refname to check if it is the same as the iterator. But the requirements for the function parameters are quite unclear. Instead of taking a refname, let's instead take an oid. That fixes both problems. It's a little funny for a "ref" function not to involve refs at all. The key thing is that it's optimizing under the hood based on having access to the ref iterator. So let's change the name to make it clear why you'd want this function versus just peel_object(). There are two other directions I considered but rejected: - we could pass the peel information into the each_ref_fn callback. However, we don't know if the caller actually wants it or not. For packed-refs, providing it is essentially free. But for loose refs, we actually have to peel the object, which would be wasteful in most cases. We could likewise pass in a flag to the callback indicating whether the peeled information is known, but that complicates those callbacks, as they then have to decide whether to manually peel themselves. Plus it requires changing the interface of every callback, whether they care about peeling or not, and there are many of them. - we could make a function to return the peeled value of the current iterated ref (computing it if necessary), and BUG() otherwise. I.e.: int peel_current_iterated_ref(struct object_id *out); Each of the current callers is an each_ref_fn callback, so they'd mostly be happy. But: - we use those callbacks with functions like head_ref(), which do not use the iteration code. So we'd need to handle the fallback case there, anyway. - it's possible that a caller would want to call into generic code that sometimes is used during iteration and sometimes not. This encapsulates the logic to do the fast thing when possible, and fallback when necessary. The implementation is mostly obvious, but I want to call out a few things in the patch: - the test-tool coverage for peel_ref() is now meaningless, as it all collapses to a single peel_object() call (arguably they were pretty uninteresting before; the tricky part of that function is the fast-path we see during iteration, but these calls didn't trigger that). I've just dropped it entirely, though note that some other tests relied on the tags we created; I've moved that creation to the tests where it matters. - we no longer need to take a ref_store parameter, since we'd never look up a ref now. We do still rely on a global "current iterator" variable which _could_ be kept per-ref-store. But in practice this is only useful if there are multiple recursive iterations, at which point the more appropriate solution is probably a stack of iterators. No caller used the actual ref-store parameter anyway (they all call the wrapper that passes the_repository). - the original only kicked in the optimization when the "refname" pointer matched (i.e., not string comparison). We do likewise with the "oid" parameter here, but fall back to doing an actual oideq() call. This in theory lets us kick in the optimization more often, though in practice no current caller cares. It should never be wrong, though (peeling is a property of an object, so two refs pointing to the same object would peel identically). - the original took care not to touch the peeled out-parameter unless we found something to put in it. But no caller cares about this, and anyway, it is enforced by peel_object() itself (and even in the optimized iterator case, that's where we eventually end up). We can shorten the code and avoid an extra copy by just passing the out-parameter through the stack. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* get_default_branch_name(): prepare for showing some adviceJohannes Schindelin2020-12-131-2/+2
| | | | | | | | | | | | | | | | | We are about to introduce a message giving users running `git init` some advice about `init.defaultBranch`. This will necessarily be done in `repo_default_branch_name()`. Not all code paths want to show that advice, though. In particular, the `git clone` codepath _specifically_ asks for `init_db()` to be quiet, via the `INIT_DB_QUIET` flag. In preparation for showing users above-mentioned advice, let's change the function signature of `get_default_branch_name()` to accept the parameter `quiet`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jt/interpret-branch-name-fallback'Junio C Hamano2020-09-091-2/+10
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git status" has trouble showing where it came from by interpreting reflog entries that recordcertain events, e.g. "checkout @{u}", and gives a hard/fatal error. Even though it inherently is impossible to give a correct answer because the reflog entries lose some information (e.g. "@{u}" does not record what branch the user was on hence which branch 'the upstream' needs to be computed, and even if the record were available, the relationship between branches may have changed), at least hide the error to allow "status" show its output. * jt/interpret-branch-name-fallback: wt-status: tolerate dangling marks refs: move dwim_ref() to header file sha1-name: replace unsigned int with option struct
| * wt-status: tolerate dangling marksJonathan Tan2020-09-021-3/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a user checks out the upstream branch of HEAD, the upstream branch not being a local branch, and then runs "git status", like this: git clone $URL client cd client git checkout @{u} git status no status is printed, but instead an error message: fatal: HEAD does not point to a branch (This error message when running "git branch" persists even after checking out other things - it only stops after checking out a branch.) This is because "git status" reads the reflog when determining the "HEAD detached" message, and thus attempts to DWIM "@{u}", but that doesn't work because HEAD no longer points to a branch. Therefore, when calculating the status of a worktree, tolerate dangling marks. This is done by adding an additional parameter to dwim_ref() and repo_dwim_ref(). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * refs: move dwim_ref() to header fileJonathan Tan2020-09-021-1/+7
| | | | | | | | | | | | | | | | This makes it clear that dwim_ref() is just repo_dwim_ref() without the first parameter. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | refs: make refs_ref_exists publicHan-Wen Nienhuys2020-08-211-0/+2
| | | | | | | | | | | | | | This will be necessary to replace file existence checks for pseudorefs. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | argv-array: rename to strvecJeff King2020-07-281-2/+2
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The name "argv-array" isn't very good, because it describes what the data type can be used for (program argument arrays), not what it actually is (a dynamically-growing string array that maintains a NULL-terminator invariant). This leads to people being hesitant to use it for other cases where it would actually be a good fit. The existing name is also clunky to use. It's overly long, and the name often leads to saying things like "argv.argv" (i.e., the field names overlap with variable names, since they're describing the use, not the type). Let's give it a more neutral name. I settled on "strvec" because "vector" is the name for a dynamic array type in many programming languages. "strarray" would work, too, but it's longer and a bit more awkward to say (and don't we all say these things in our mind as we type them?). A more extreme direction would be a generic data structure which stores a NULL-terminated of _any_ type. That would be easy to do with void pointers, but we'd lose some type safety for the existing cases. Plus it raises questions about memory allocation and ownership. So I limited myself here to changing names only, and not semantics. If we do find a use for that more generic data type, we could perhaps implement it at a lower level and then provide type-safe wrappers around it for strings. But that can come later. This patch does the minimum to convert the struct and function names in the header and implementation, leaving a few things for follow-on patches: - files retain their original names for now - struct field names are retained for now - there's a preprocessor compat layer that lets most users remain the same for now. The exception is headers which made a manual forward declaration of the struct. I've converted them (and their dependent function declarations) here. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'js/default-branch-name'Junio C Hamano2020-07-061-0/+9
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The name of the primary branch in existing repositories, and the default name used for the first branch in newly created repositories, is made configurable, so that we can eventually wean ourselves off of the hardcoded 'master'. * js/default-branch-name: contrib: subtree: adjust test to change in fmt-merge-msg testsvn: respect `init.defaultBranch` remote: use the configured default branch name when appropriate clone: use configured default branch name when appropriate init: allow setting the default for the initial branch name via the config init: allow specifying the initial branch name for the new repository docs: add missing diamond brackets submodule: fall back to remote's HEAD for missing remote.<name>.branch send-pack/transport-helper: avoid mentioning a particular branch fmt-merge-msg: stop treating `master` specially
| * init: allow setting the default for the initial branch name via the configDon Goodman-Wilson2020-06-241-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We just introduced the command-line option `--initial-branch=<branch-name>` to allow initializing a new repository with a different initial branch than the hard-coded one. To allow users to override the initial branch name more permanently (i.e. without having to specify the name manually for each and every `git init` invocation), let's introduce the `init.defaultBranch` config setting. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Don Goodman-Wilson <don@goodman-wilson.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | refs.h: clarify reflog iteration orderHan-Wen Nienhuys2020-05-201-1/+17
|/ | | | | Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* log-tree: make ref_filter_match() a helper methodDerrick Stolee2020-04-161-12/+0
| | | | | | | | | | | | | | | | | | The ref_filter_match() method is defined in refs.h and implemented in refs.c, but is only used by add_ref_decoration() in log-tree.c. Move it into that file as a static helper method. The match_ref_pattern() comes along for the ride. While moving the code, also make a slight adjustment to have ref_filter_match() take a struct decoration_filter pointer instead of multiple string lists. This is non-functional, but will make a later change be much cleaner. The diff is easier to parse when using the --color-moved option. Reported-by: Junio C Hamano <gister@pobox.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: move doc to refs.hHeba Waly2019-11-181-0/+51
| | | | | | | | | | | | | Move the documentation from Documentation/technical/api-ref-iteration.txt to refs.h as it's easier for the developers to find the usage information beside the code instead of looking for it in another doc file. Also documentation/technical/api-ref-iteration.txt is removed because the information it has is now redundant and it'll be hard to keep it up to date and synchronized with the documentation in the header file. Signed-off-by: Heba Waly <heba.waly@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'nd/worktree-name-sanitization'Junio C Hamano2019-06-131-0/+6
|\ | | | | | | | | | | | | | | | | | | In recent versions of Git, per-worktree refs are exposed in refs/worktrees/<wtname>/ hierarchy, which means that worktree names must be a valid refname component. The code now sanitizes the names given to worktrees, to make sure these refs are well-formed. * nd/worktree-name-sanitization: worktree add: sanitize worktree names
| * worktree add: sanitize worktree namesNguyễn Thái Ngọc Duy2019-05-151-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Worktree names are based on $(basename $GIT_WORK_TREE). They aren't significant until 3a3b9d8cde (refs: new ref types to make per-worktree refs visible to all worktrees - 2018-10-21), where worktree name could be part of a refname and must follow refname rules. Update 'worktree add' code to remove special characters to follow these rules. In the future the user will be able to specify the worktree name by themselves if they're not happy with this dumb character substitution. Reported-by: Konstantin Kharlamov <hi-angel@yandex.ru> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'dl/no-extern-in-func-decl'Junio C Hamano2019-05-131-1/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | Mechanically and systematically drop "extern" from function declarlation. * dl/no-extern-in-func-decl: *.[ch]: manually align parameter lists *.[ch]: remove extern from function declarations using sed *.[ch]: remove extern from function declarations using spatch
| * | *.[ch]: remove extern from function declarations using spatchDenton Liu2019-05-051-1/+1
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There has been a push to remove extern from function declarations. Remove some instances of "extern" for function declarations which are caught by Coccinelle. Note that Coccinelle has some difficulty with processing functions with `__attribute__` or varargs so some `extern` declarations are left behind to be dealt with in a future patch. This was the Coccinelle patch used: @@ type T; identifier f; @@ - extern T f(...); and it was run with: $ git ls-files \*.{c,h} | grep -v ^compat/ | xargs spatch --sp-file contrib/coccinelle/noextern.cocci --in-place Files under `compat/` are intentionally excluded as some are directly copied from external sources and we should avoid churning them as much as possible. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | refs.c: remove the_repo from read_ref_at()Nguyễn Thái Ngọc Duy2019-04-081-1/+2
| | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>