kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
Patrick Steinhardt	9b67eb6fbe	refs: get rid of global list of hidden refs 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>	2 years ago
Patrick Steinhardt	5eeb9aa208	refs: fix memory leak when parsing hideRefs config When parsing the hideRefs configuration, we first duplicate the config value so that we can modify it. We then subsequently append it to the `hide_refs` string list, which is initialized with `strdup_strings` enabled. As a consequence we again reallocate the string, but never free the first duplicate and thus have a memory leak. While we never clean up the static `hide_refs` variable anyway, this is no excuse to make the leak worse by leaking every value twice. We are also about to change the way this variable will be handled so that we do indeed start to clean it up. So let's fix the memory leak by using the `string_list_append_nodup()` so that we pass ownership of the allocated string to `hide_refs`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>	2 years ago
Han-Wen Nienhuys	71e5473493	refs: unify parse_worktree_ref() and ref_type() 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>	2 years ago
Ævar Arnfjörð Bjarmason	5cf88fd8b0	git-compat-util.h: use "UNUSED", not "UNUSED(var)" 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 `9b24034754` (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>	2 years ago
Jeff King	02c3c59e62	hashmap: mark unused callback parameters Hashmap comparison functions must conform to a particular callback interface, but many don't use all of their parameters. Especially the void cmp_data pointer, but some do not use keydata either (because they can easily form a full struct to pass when doing lookups). Let's mark these to make -Wunused-parameter happy. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jeff King	c006e9fa59	refs: mark unused reflog callback parameters Functions used with for_each_reflog_ent() need to conform to a particular interface, but not every function needs all of the parameters. Mark the unused ones to make -Wunused-parameter happy. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jeff King	63e14ee2d6	refs: mark unused each_ref_fn parameters 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>	3 years ago
Derrick Stolee	97e61e0f9c	refs: use ref_namespaces for replace refs base The git_replace_ref_base global is used to store the value of the GIT_REPLACE_REF_BASE environment variable or the default of "refs/replace/". This is initialized within setup_git_env(). The ref_namespaces array is a new centralized location for information such as the ref namespace used for replace refs. Instead of having this namespace stored in two places, use the ref_namespaces array instead. For simplicity, create a local git_replace_ref_base variable wherever the global was previously used. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Derrick Stolee	b9342b3fd6	refs: add array of ref namespaces 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>	3 years ago
Derrick Stolee	b877e617e6	refs: allow "HEAD" as decoration filter The normalize_glob_ref() method was introduced in `65516f586b` (log: add option to choose which refs to decorate, 2017-11-21) to help with decoration filters such as --decorate-refs=<filter> and --decorate-refs-exclude=<filter>. The method has not been used anywhere else. At the moment, it is impossible to specify HEAD as a decoration filter since normalize_glob_ref() prepends "refs/" to the filter if it isn't already there. Allow adding HEAD as a decoration filter by allowing the exact string "HEAD" to not be prepended with "refs/". Add a test in t4202-log.sh that would previously fail since the HEAD decoration would exist in the output. It is sufficient to only cover "HEAD" here and not include other special refs like REBASE_HEAD. This is because HEAD is the only ref outside of refs/* that is added to the list of decorations. However, we may want to special-case these other refs in normalize_glob_ref() in the future. Leave a NEEDSWORK comment for now. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Derrick Stolee	d097a23bfa	clone: die() instead of BUG() on bad refs 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 <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Junio C Hamano	c6da34a610	Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'" This reverts commit `991b4d47f0`, reversing changes made to `bcd020f88e`.	3 years ago
Ævar Arnfjörð Bjarmason	5b8754043c	refs debug: add a wrapper for "read_symbolic_ref" In `cd475b3b03` (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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Patrick Steinhardt	cd475b3b03	refs: add ability for backends to special-case reading of symbolic refs 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>	3 years ago
Patrick Steinhardt	4f2ba2d06a	refs: add interface to iterate over queued transactional updates 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>	3 years ago
Ævar Arnfjörð Bjarmason	88c7b4c3c8	date API: create a date.h, split from cache.h Move the declaration of the date.c functions from cache.h, and adjust the relevant users to include the new date.h header. The show_ident_date() function belonged in pretty.h (it's defined in pretty.c), its two users outside of pretty.c didn't strictly need to include pretty.h, as they get it indirectly, but let's add it to them anyway. Similarly, the change to "builtin/{fast-import,show-branch,tag}.c" isn't needed as far as the compiler is concerned, but since they all use the "DATE_MODE()" macro we now define in date.h, let's have them include it. We could simply include this new header in "cache.h", but as this change shows these functions weren't common enough to warrant including in it in the first place. By moving them out of cache.h changes to this API will no longer cause a (mostly) full re-build of the project when "make" is run. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	ce14de03db	refs API: remove "failure_errno" from refs_resolve_ref_unsafe() Remove the now-unused "failure_errno" parameter from the refs_resolve_ref_unsafe() signature. In my recent `96f6623ada` (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 `31e3912369` (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>	3 years ago
Patrick Steinhardt	958fbc74e3	refs: allow skipping the reference-transaction hook 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>	3 years ago
Patrick Steinhardt	fbe73f61cb	refs: allow passing flags when beginning transactions 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>	3 years ago
Ævar Arnfjörð Bjarmason	cac15b3fb4	refs API: use "failure_errno", not "errno" 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 `96f6623ada` (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 `ef18119dec` (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 `8b72fea7e9` (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 `a1c1d8170d` (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>	3 years ago
Han-Wen Nienhuys	f9f7fd3b23	refs: centralize initialization of the base ref_store. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Neeraj Singh	ecd81dfc79	tmp-objdir: disable ref updates when replacing the primary odb When creating a subprocess with a temporary ODB, we set the GIT_QUARANTINE_ENVIRONMENT env var to tell child Git processes not to update refs, since the tmp-objdir may go away. Introduce a similar mechanism for in-process temporary ODBs when we call tmp_objdir_replace_primary_odb. Now both mechanisms set the disable_ref_updates flag on the odb, which is queried by the ref_transaction_prepare function. Peff's test case [1] was invoking ref updates via the cachetextconv setting. That particular code silently does nothing when a ref update is forbidden. See the call to notes_cache_put in fill_textconv where errors are ignored. [1] https://lore.kernel.org/git/YVOn3hDsb5pnxR53@coredump.intra.peff.net/ Reported-by: Jeff King <peff@peff.net> Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Han-Wen Nienhuys	3c966c7b4e	refs: introduce REF_SKIP_REFNAME_VERIFICATION flag 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>	3 years ago
Han-Wen Nienhuys	7b089120d9	refs: drop force_create argument of create_reflog API There is only one caller, builtin/checkout.c, and it hardcodes force_create=1. This argument was introduced in `abd0cd3a30` (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>	3 years ago
Jeff King	49f1eb3b34	refs: work around gcc-11 warning with REF_HAVE_NEW Using gcc-11 (or 12) to compile refs.o with -O3 results in: In file included from hashmap.h:4, from cache.h:6, from refs.c:5: In function ‘oidcpy’, inlined from ‘ref_transaction_add_update’ at refs.c:1065:3, inlined from ‘ref_transaction_update’ at refs.c:1094:2, inlined from ‘ref_transaction_verify’ at refs.c:1132:9: hash.h:262:9: warning: argument 2 null where non-null expected [-Wnonnull] 262 \| memcpy(dst->hash, src->hash, GIT_MAX_RAWSZ); \| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from git-compat-util.h:177, from cache.h:4, from refs.c:5: refs.c: In function ‘ref_transaction_verify’: /usr/include/string.h:43:14: note: in a call to function ‘memcpy’ declared ‘nonnull’ 43 \| extern void memcpy (void __restrict __dest, const void *__restrict __src, \| ^~~~~~ That call to memcpy() is in a conditional block that requires REF_HAVE_NEW to be set. But in ref_transaction_update(), we make sure it isn't set coming in: if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS) BUG("illegal flags 0x%x passed to ref_transaction_update()", flags); and then only set it if the variable isn't NULL: flags \|= (new_oid ? REF_HAVE_NEW : 0) \| (old_oid ? REF_HAVE_OLD : 0); So it should be impossible to reach that memcpy() with a NULL oid. But for whatever reason, gcc doesn't accept that hitting the BUG() means we won't go any further, even though it's marked with the noreturn attribute. And the conditional is correct; ALLOWED_FLAGS doesn't contain HAVE_NEW or HAVE_OLD, and you can even simplify it to check for those flags explicitly and the compiler still complains. We can work around this by just clearing the disallowed flags explicitly. This should be a noop because of the BUG() check, but it makes the compiler happy. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	f1da24ca5e	refs API: post-migration API renaming [2/2] 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>	3 years ago
Ævar Arnfjörð Bjarmason	25a33b3342	refs API: post-migration API renaming [1/2] 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>	3 years ago
Ævar Arnfjörð Bjarmason	4755d7dff7	refs API: don't expose "errno" in run_transaction_hook() In run_transaction_hook() we've checked errno since `6754159767` (refs: implement reference transaction hook, 2020-06-19), let's reset errno afterwards to make sure nobody using refs.c directly or indirectly relies on it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	6582bd31e3	refs API: make expand_ref() & repo_dwim_log() not set errno The use of these two is rather trivial, and it's easy to see none of their callers care about errno. So let's move them from refs_resolve_ref_unsafe() to refs_resolve_ref_unsafe_with_errno(), these were the last two callers, so we can get rid of that wrapper function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	ed90f04155	refs API: make resolve_ref_unsafe() not set errno Change the resolve_ref_unsafe() wrapper function to use the underlying refs_werrres_ref_unsafe() directly. From a reading of the callers I determined that the only one who cared about errno was a sequencer.c caller added in `e47c6cafcb` (commit: move print_commit_summary() to libgit, 2017-11-24), I'm migrating it to using refs_werrres_ref_unsafe() directly. This adds another "set errno" instance, but in this case it's OK and idiomatic. We are setting it just before calling die_errno(). We could have some hypothetical die_errno_var(&saved_errno, ...) here, but I don't think it's worth it. The problem with errno is subtle action at distance, not this sort of thing. We already use this pattern in a couple of places in wrapper.c Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	1e3ccb552f	refs API: make refs_ref_exists() not set errno Move refs_ref_exists from the legacy refs_resolve_ref_unsafe() to the new refs_werrres_ref_unsafe(). I have read its callers and determined that they don't care about errno being set, in particular: git grep -W -w -e refs_ref_exists -e ref_exists Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	f65bb9fb06	refs API: make refs_resolve_refdup() not set errno Move refs_resolve_refdup() from the legacy refs_resolve_ref_unsafe() to the new refs_werrres_ref_unsafe(). I have read its callers and determined that they don't care about errno being set. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	db7a3d25d6	refs API: make resolve_gitlink_ref() not set errno I have carefully read the upstream callers of resolve_gitlink_ref() and determined that they don't care about errno. So let's move away from the errno-setting refs_resolve_ref_unsafe() wrapper to refs_werrres_ref_unsafe(), and explicitly ignore the errno it sets for us. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	76887df014	refs API: remove refs_read_ref_full() wrapper 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>	3 years ago
Ævar Arnfjörð Bjarmason	c339ff690f	refs API: make refs_rename_ref_available() static Move the refs_rename_ref_available() function into "refs/files-backend.c". It is file-backend specific. This function was added in `5fe7d825da` (refs.c: pass a list of names to skip to is_refname_available, 2014-05-01) as rename_ref_available() and was only ever used in this one file-backend specific codepath. So let's move it there. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Han-Wen Nienhuys	df3458e957	refs API: make parse_loose_ref_contents() not set errno Change the parse_loose_ref_contents() function to stop setting "errno" and failure, and to instead pass up a "failure_errno" via a parameter. This requires changing its callers to do the same. The EINVAL error from parse_loose_ref_contents is used in files-backend to create a custom error message. In untangling this we discovered a tricky edge case. The refs_read_special_head() function was relying on parse_loose_ref_contents() setting EINVAL. By converting it to use "saved_errno" we can migrate away from "errno" in this part of the code entirely, and do away with an existing "save_errno" pattern, its only purpose was to not clobber the "errno" we previously needed at the end of files_read_raw_ref(). Let's assert that we can do that by not having files_read_raw_ref() itself operate on *failure_errno in addition to passing it on. Instead we'll assert that if we return non-zero we actually do set errno, thus assuring ourselves and callers that they can trust the resulting "failure_errno". 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>	3 years ago
Han-Wen Nienhuys	8b72fea7e9	refs API: make refs_read_raw_ref() not set errno Add a "failure_errno" to refs_read_raw_ref(), his allows refs_werrres_ref_unsafe() to pass along its "failure_errno", as a first step before its own callers are migrated to pass it further up the chain. We are leaving out out the refs_read_special_head() in refs_read_raw_ref() for now, as noted in a subsequent commit moving it to "failure_errno" will require some special consideration. 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>	3 years ago
Ævar Arnfjörð Bjarmason	ef18119dec	refs API: add a version of refs_resolve_ref_unsafe() with "errno" 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 `76d70dc0c6` (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>	3 years ago
Jonathan Tan	9bc45a2802	refs: teach arbitrary repo support to iterators Note that should_pack_ref() is called when writing refs, which is only supported for the_repository, hence the_repository is hardcoded there. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jonathan Tan	34224e14d6	refs: plumb repo into ref stores In preparation for the next 2 patches that adds (partial) support for arbitrary repositories to ref iterators, plumb a repository into all ref stores. There are no changes to program logic. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	98961e42f0	refs.[ch]: remove unused ref_storage_backend_exists() This function was added in `3dce444f17` (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>	3 years ago
Jeff King	67985e4e4a	refs: drop "broken" flag from for_each_fullref_in() 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>	3 years ago
Jeff King	5d1f5b8cd4	repack, prune: drop GIT_REF_PARANOIA settings Now that GIT_REF_PARANOIA is the default, we don't need to selectively enable it for destructive operations. In fact, it's harmful to do so, because it overrides any GIT_REF_PARANOIA=0 setting that the user may have provided (because they're trying to work around some corruption). With these uses gone, we can further clean up the ref_paranoia global, and make it a static variable inside the refs code. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jeff King	968f12fdac	refs: turn on GIT_REF_PARANOIA by default The original point of the GIT_REF_PARANOIA flag was to include broken refs in iterations, so that possibly-destructive operations would not silently ignore them (and would generally instead try to operate on the oids and fail when the objects could not be accessed). We already turned this on by default for some dangerous operations, like "repack -ad" (where missing a reachability tip would mean dropping the associated history). But it was not on for general use, even though it could easily result in the spreading of corruption (e.g., imagine cloning a repository which simply omits some of its refs because their objects are missing; the result quietly succeeds even though you did not clone everything!). This patch turns on GIT_REF_PARANOIA by default. So a clone as mentioned above would actually fail (upload-pack tells us about the broken ref, and when we ask for the objects, pack-objects fails to deliver them). This may be inconvenient when working with a corrupted repository, but: - we are better off to err on the side of complaining about corruption, and then provide mechanisms for explicitly loosening safety. - this is only one type of corruption anyway. If we are missing any other objects in the history that _aren't_ ref tips, then we'd behave similarly (happily show the ref, but then barf when we started traversing). We retain the GIT_REF_PARANOIA variable, but simply default it to "1" instead of "0". That gives the user an escape hatch for loosening this when working with a corrupt repository. It won't work across a remote connection to upload-pack (because we can't necessarily set environment variables on the remote), but there the client has other options (e.g., choosing which refs to fetch). As a bonus, this also makes ref iteration faster in general (because we don't have to call has_object_file() for each ref), though probably not noticeably so in the general case. In a repo with a million refs, it shaved a few hundred milliseconds off of upload-pack's advertisement; that's noticeable, but most repos are not nearly that large. The possible downside here is that any operation which iterates refs but doesn't ever open their objects may now quietly claim to have X when the object is corrupted (e.g., "git rev-list new-branch --not --all" will treat a broken ref as uninteresting). But again, that's not really any different than corruption below the ref level. We might have refs/heads/old-branch as non-corrupt, but we are not actively checking that we have the entire reachable history. Or the pointed-to object could even be corrupted on-disk (but our "do we have it" check would still succeed). In that sense, this is merely bringing ref-corruption in line with general object corruption. One alternative implementation would be to actually check for broken refs, and then _immediately die_ if we see any. That would cause the "rev-list --not --all" case above to abort immediately. But in many ways that's the worst of all worlds: - it still spends time looking up the objects an extra time - it still doesn't catch corruption below the ref level - it's even more inconvenient; with the current implementation of GIT_REF_PARANOIA for something like upload-pack, we can make the advertisement and let the client choose a non-broken piece of history. If we bail as soon as we see a broken ref, they cannot even see the advertisement. The test changes here show some of the fallout. A non-destructive "git repack -adk" now fails by default (but we can override it). Deleting a broken ref now actually tells the hooks the correct "before" state, rather than a confusing null oid. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jeff King	6d751be4b6	refs: omit dangling symrefs when using GIT_REF_PARANOIA Dangling symrefs aren't actually a corruption problem. It's perfectly fine for refs/remotes/origin/HEAD to point to an unborn branch. And in particular, if you are trying to establish reachability, a symref that points nowhere doesn't matter either way. Any ref it could point to will be examined during the rest of the traversal. It's possible that a symref pointing nowhere _could_ be a sign that the ref it was meant to point to was deleted accidentally (e.g., via corruption). But there is no particular reason to think that is true for any given case, and in the meantime, GIT_REF_PARANOIA kicking in automatically for some operations means they'll fail unnecessarily. So let's loosen it just a bit. The new test in t5312 shows off an example that is safe, but currently fails (and no longer does after this patch). Note that we don't do anything if the caller explicitly asked for DO_FOR_EACH_INCLUDE_BROKEN. In that case they may be looking for dangling symrefs themselves, and setting GIT_REF_PARANOIA should not _loosen_ things from what the caller asked for. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Jeff King	9aab952e85	refs-internal.h: reorganize DO_FOR_EACH_* flag documentation The documentation for the DO_FOR_EACH_* flags is sprinkled over the refs-internal.h file. We define the two flags in one spot, and then describe them in more detail far away from there, in the definitions of refs_ref_iterator_begin() and ref_iterator_advance_fn(). Let's try to organize this a bit better: - convert the #defines to an enum. This makes it clear that they are related, and that the enum shows the complete set of flags. - combine all descriptions for each flag in a single spot, next to the flag's definition - use the enum rather than a bare int for functions which take the flags. This helps readers realize which flags can be used. - clarify the mention of flags for ref_iterator_advance_fn(). It does not take flags itself, but is meant to depend on ones set up earlier. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	5e3aba33da	hook.[ch]: move find_hook() from run-command.c to hook.c Move the find_hook() function from run-command.c to a new hook.c library. This change establishes a stub library that's pretty pointless right now, but will see much wider use with Emily Shaffer's upcoming "configuration-based hooks" series. Eventually all the hook related code will live in hook.[ch]. Let's start that process by moving the simple find_hook() function over as-is. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Han-Wen Nienhuys	5b12e16bb1	refs: make errno output explicit for read_raw_ref_fn This makes it explicit how alternative ref backends should report errors in read_raw_ref_fn. read_raw_ref_fn needs to supply a credible errno for a number of cases. These are primarily: 1) The files backend calls read_raw_ref from lock_raw_ref, and uses the resulting error codes to create/remove directories as needed. 2) ENOENT should be translated in a zero OID, optionally with REF_ISBROKEN set, returning the last successfully resolved symref. This is necessary so read_raw_ref("HEAD") on an empty repo returns refs/heads/main (or the default branch du-jour), and we know on which branch to create the first commit. Make this information flow explicit by adding a failure_errno to the signature of read_raw_ref. All errnos from the files backend are still propagated unchanged, even though inspection suggests only ENOTDIR, EISDIR and ENOENT are relevant. 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>	4 years ago
Ævar Arnfjörð Bjarmason	cc40b5ce13	refs API: remove OID argument to reflog_expire() 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>	4 years ago
Ævar Arnfjörð Bjarmason	6f45ec88d2	refs: make repo_dwim_log() accept a NULL oid Change the repo_dwim_log() function initially added as dwim_log() in `eb3a48221f` (log --reflog: use dwim_log, 2007-02-09) to accept a NULL oid parameter. The refs_resolve_ref_unsafe() function it invokes already deals with it, but it didn't. This allows for a bit more clarity in a reflog-walk.c codepath added in `f2eba66d4d` (Enable HEAD@{...} and make it independent from the current branch, 2007-02-03). We'll shortly use this in builtin/reflog.c as well. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago

1 2 3 4 5 ...

1070 Commits (9ea1378d046d764642718f1b070689072bde4601)