summaryrefslogtreecommitdiff
path: root/refs/files-backend.c
AgeCommit message (Collapse)AuthorFilesLines
2018-11-26files-backend.c: fix build error on SolarisLibravatar Nguyễn Thái Ngọc Duy1-1/+2
This function files_reflog_path returns void, which usually means "return;" not returning "void value" from another function. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-22refs: new ref types to make per-worktree refs visible to all worktreesLibravatar Nguyễn Thái Ngọc Duy1-0/+28
One of the problems with multiple worktree is accessing per-worktree refs of one worktree from another worktree. This was sort of solved by multiple ref store, where the code can open the ref store of another worktree and has access to the ref space of that worktree. The problem with this is reporting. "HEAD" in another ref space is also called "HEAD" like in the current ref space. In order to differentiate them, all the code must somehow carry the ref store around and print something like "HEAD from this ref store". But that is not feasible (or possible with a _lot_ of work). With the current design, we pass a reference around as a string (so called "refname"). Extending this design to pass a string _and_ a ref store is a nightmare, especially when handling extended SHA-1 syntax. So we do it another way. Instead of entering a separate ref space, we make refs from other worktrees available in the current ref space. So "HEAD" is always HEAD of the current worktree, but then we can have "worktrees/blah/HEAD" to denote HEAD from a worktree named "blah". This syntax coincidentally matches the underlying directory structure which makes implementation a bit easier. The main worktree has to be treated specially because well... it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main-worktree/HEAD" instead of "worktrees/main/HEAD" because "main" could be just another secondary worktree. This patch also makes it possible to specify refs from one worktree in another one, e.g. git log worktrees/foo/HEAD Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-07Add a place for (not) sharing stuff between worktreesLibravatar Nguyễn Thái Ngọc Duy1-3/+11
When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes (*), have a generic rule: - Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory. - Inside refs/, which is shared by default except refs/bisect, add refs/worktree/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code. (*) And it may also include stuff from external commands which will have no way to modify common/per-worktree rules. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "oidcmp() != 0" to "!oideq()"Libravatar Jeff King1-1/+1
This is the flip side of the previous two patches: checking for a non-zero oidcmp() can be more strictly expressed as inequality. Like those patches, we write "!= 0" in the coccinelle transformation, which covers by isomorphism the more common: if (oidcmp(E1, E2)) As with the previous two patches, this patch can be achieved almost entirely by running "make coccicheck"; the only differences are manual line-wrap fixes to match the original code. There is one thing to note for anybody replicating this, though: coccinelle 1.0.4 seems to miss the case in builtin/tag.c, even though it's basically the same as all the others. Running with 1.0.7 does catch this, so presumably it's just a coccinelle bug that was fixed in the interim. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "oidcmp() == 0" to oideq()Libravatar Jeff King1-2/+2
Using the more restrictive oideq() should, in the long run, give the compiler more opportunities to optimize these callsites. For now, this conversion should be a complete noop with respect to the generated code. The result is also perhaps a little more readable, as it avoids the "zero is equal" idiom. Since it's so prevalent in C, I think seasoned programmers tend not to even notice it anymore, but it can sometimes make for awkward double negations (e.g., we can drop a few !!oidcmp() instances here). This patch was generated almost entirely by the included coccinelle patch. This mechanical conversion should be completely safe, because we check explicitly for cases where oidcmp() is compared to 0, which is what oideq() is doing under the hood. Note that we don't have to catch "!oidcmp()" separately; coccinelle's standard isomorphisms make sure the two are treated equivalently. I say "almost" because I did hand-edit the coccinelle output to fix up a few style violations (it mostly keeps the original formatting, but sometimes unwraps long lines). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-15Merge branch 'jk/size-t'Libravatar Junio C Hamano1-1/+1
Code clean-up to use size_t/ssize_t when they are the right type. * jk/size-t: strbuf_humanise: use unsigned variables pass st.st_size as hint for strbuf_readlink() strbuf_readlink: use ssize_t strbuf: use size_t for length in intermediate variables reencode_string: use size_t for string lengths reencode_string: use st_add/st_mult helpers
2018-08-02Merge branch 'sb/object-store-lookup'Libravatar Junio C Hamano1-1/+1
lookup_commit_reference() and friends have been updated to find in-core object for a specific in-core repository instance. * sb/object-store-lookup: (32 commits) commit.c: allow lookup_commit_reference to handle arbitrary repositories commit.c: allow lookup_commit_reference_gently to handle arbitrary repositories tag.c: allow deref_tag to handle arbitrary repositories object.c: allow parse_object to handle arbitrary repositories object.c: allow parse_object_buffer to handle arbitrary repositories commit.c: allow get_cached_commit_buffer to handle arbitrary repositories commit.c: allow set_commit_buffer to handle arbitrary repositories commit.c: migrate the commit buffer to the parsed object store commit-slabs: remove realloc counter outside of slab struct commit.c: allow parse_commit_buffer to handle arbitrary repositories tag: allow parse_tag_buffer to handle arbitrary repositories tag: allow lookup_tag to handle arbitrary repositories commit: allow lookup_commit to handle arbitrary repositories tree: allow lookup_tree to handle arbitrary repositories blob: allow lookup_blob to handle arbitrary repositories object: allow lookup_object to handle arbitrary repositories object: allow object_as_type to handle arbitrary repositories tag: add repository argument to deref_tag tag: add repository argument to parse_tag_buffer tag: add repository argument to lookup_tag ...
2018-08-02Merge branch 'bc/object-id'Libravatar Junio C Hamano1-2/+2
Conversion from uchar[40] to struct object_id continues. * bc/object-id: pretty: switch hard-coded constants to the_hash_algo sha1-file: convert constants to uses of the_hash_algo log-tree: switch GIT_SHA1_HEXSZ to the_hash_algo->hexsz diff: switch GIT_SHA1_HEXSZ to use the_hash_algo builtin/merge-recursive: make hash independent builtin/merge: switch to use the_hash_algo builtin/fmt-merge-msg: make hash independent builtin/update-index: simplify parsing of cacheinfo builtin/update-index: convert to using the_hash_algo refs/files-backend: use the_hash_algo for writing refs sha1-name: use the_hash_algo when parsing object names strbuf: allocate space with GIT_MAX_HEXSZ commit: express tree entry constants in terms of the_hash_algo hex: switch to using the_hash_algo tree-walk: replace hard-coded constants with the_hash_algo cache: update object ID functions for the_hash_algo
2018-07-24pass st.st_size as hint for strbuf_readlink()Libravatar Jeff King1-1/+1
When we initially added the strbuf_readlink() function in b11b7e13f4 (Add generic 'strbuf_readlink()' helper function, 2008-12-17), the point was that we generally have a _guess_ as to the correct size based on the stat information, but we can't necessarily trust it. Over the years, a few callers have grown up that simply pass in 0, even though they have the stat information. Let's have them pass in their hint for consistency (and in theory efficiency, since it may avoid an extra resize/syscall loop, but neither location is probably performance critical). Note that st.st_size is actually an off_t, so in theory we need xsize_t() here. But none of the other callsites use it, and since this is just a hint, it doesn't matter either way (if we wrap we'll simply start with a too-small hint and then eventually complain when we cannot allocate the memory). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-16refs/files-backend: use the_hash_algo for writing refsLibravatar brian m. carlson1-2/+2
In order to ensure we write the correct amount, use the_hash_algo to find the correct number of bytes for the current hash. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-10convert log_ref_write_fd() to use strbufLibravatar Ben Peart1-19/+10
Since we don't care about how many bytes were written, simplify the return value logic. log_ref_write_fd() was written long before strbuf was fleshed out. Remove the old manual buffer management code and replace it with strbuf(). Also update copy_reflog_msg() which is called only by log_ref_write_fd() to use strbuf as it keeps things consistent. Signed-off-by: Ben Peart <Ben.Peart@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29object: add repository argument to parse_objectLibravatar Stefan Beller1-1/+1
Add a repository argument to allow the callers of parse_object to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-30Merge branch 'js/use-bug-macro'Libravatar Junio C Hamano1-10/+10
Developer support update, by using BUG() macro instead of die() to mark codepaths that should not happen more clearly. * js/use-bug-macro: BUG_exit_code: fix sparse "symbol not declared" warning Convert remaining die*(BUG) messages Replace all die("BUG: ...") calls by BUG() ones run-command: use BUG() to report bugs, not die() test-tool: help verifying BUG() code paths
2018-05-30Merge branch 'ma/lockfile-cleanup'Libravatar Junio C Hamano1-1/+1
Code clean-up to adjust to a more recent lockfile API convention that allows lockfile instances kept on the stack. * ma/lockfile-cleanup: lock_file: move static locks into functions lock_file: make function-local locks non-static refs.c: do not die if locking fails in `delete_pseudoref()` refs.c: do not die if locking fails in `write_pseudoref()` t/helper/test-write-cache: clean up lock-handling
2018-05-10lock_file: make function-local locks non-staticLibravatar Martin Ågren1-1/+1
Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack. (This applies even if a user returns early, leaving a locked lock behind.) These `struct lock_file`s are local to their respective functions and we can drop their staticness. For good measure, I have inspected these sites and come to believe that they always release the lock, with the possible exception of bailing out using `die()` or `exit()` or by returning from a `cmd_foo()`. As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08Merge branch 'sb/object-store-replace'Libravatar Junio C Hamano1-4/+0
The effort to pass the repository in-core structure throughout the API continues. This round deals with the code that implements the refs/replace/ mechanism. * sb/object-store-replace: replace-object: allow lookup_replace_object to handle arbitrary repositories replace-object: allow do_lookup_replace_object to handle arbitrary repositories replace-object: allow prepare_replace_object to handle arbitrary repositories refs: allow for_each_replace_ref to handle arbitrary repositories refs: store the main ref store inside the repository struct replace-object: add repository argument to lookup_replace_object replace-object: add repository argument to do_lookup_replace_object replace-object: add repository argument to prepare_replace_object refs: add repository argument to for_each_replace_ref refs: add repository argument to get_main_ref_store replace-object: check_replace_refs is safe in multi repo environment replace-object: eliminate replace objects prepared flag object-store: move lookup_replace_object to replace-object.h replace-object: move replace_map to object store replace_object: use oidmap
2018-05-06Replace all die("BUG: ...") calls by BUG() onesLibravatar Johannes Schindelin1-10/+10
In d8193743e08 (usage.c: add BUG() function, 2017-05-12), a new macro was introduced to use for reporting bugs instead of die(). It was then subsequently used to convert one single caller in 588a538ae55 (setup_git_env: convert die("BUG") to BUG(), 2017-05-12). The cover letter of the patch series containing this patch (cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not terribly clear why only one call site was converted, or what the plan is for other, similar calls to die() to report bugs. Let's just convert all remaining ones in one fell swoop. This trick was performed by this invocation: sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-12refs: store the main ref store inside the repository structLibravatar Stefan Beller1-4/+0
This moves the 'main_ref_store', which was a global variable in refs.c into the repository struct. This patch does not deal with the parts in the refs subsystem which deal with the submodules there. A later patch needs to get rid of the submodule exposure in the refs API, such as 'get_submodule_ref_store(path)'. Acked-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-30refs: use chdir_notify to update cached relative pathsLibravatar Jeff King1-0/+6
Commit f57f37e2e1 (files-backend: remove the use of git_path(), 2017-03-26) introduced a regression when a relative $GIT_DIR is used in a working tree: - when we initialize the ref backend, we make a copy of get_git_dir(), which may be relative - later, we may call setup_work_tree() and chdir to the root of the working tree - further calls to the ref code will use the stored git directory, but relative paths will now point to the wrong place The new test in t1501 demonstrates one such instance (the bug causes us to write the ref update to the nonsense "relative/relative/.git"). Since setup_work_tree() now uses chdir_notify, we can just ask it update our relative paths when necessary. Reported-by: Rafael Ascensao <rafa.almas@gmail.com> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-13Merge branch 'mr/packed-ref-store-fix'Libravatar Junio C Hamano1-2/+1
Crash fix for a corner case where an error codepath tried to unlock what it did not acquire lock on. * mr/packed-ref-store-fix: files_initial_transaction_commit(): only unlock if locked
2018-01-19files_initial_transaction_commit(): only unlock if lockedLibravatar Mathias Rav1-2/+1
Running git clone --single-branch --mirror -b TAGNAME previously triggered the following error message: fatal: multiple updates for ref 'refs/tags/TAGNAME' not allowed. This error condition is handled in files_initial_transaction_commit(). 42c7f7ff9 ("commit_packed_refs(): remove call to `packed_refs_unlock()`", 2017-06-23) introduced incorrect unlocking in the error path of this function, which changes the error message to fatal: BUG: packed_refs_unlock() called when not locked Move the call to packed_refs_unlock() above the "cleanup:" label since the unlocking should only be done in the last error path. Signed-off-by: Mathias Rav <m@git.strova.dk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-06Merge branch 'mh/avoid-rewriting-packed-refs' into maintLibravatar Junio C Hamano1-1/+17
Recent update to the refs infrastructure implementation started rewriting packed-refs file more often than before; this has been optimized again for most trivial cases. * mh/avoid-rewriting-packed-refs: files-backend: don't rewrite the `packed-refs` file unnecessarily t1409: check that `packed-refs` is not rewritten unnecessarily
2017-11-15Merge branch 'mh/tidy-ref-update-flags'Libravatar Junio C Hamano1-38/+94
Code clean-up in refs API implementation. * mh/tidy-ref-update-flags: refs: update some more docs to use "oid" rather than "sha1" write_packed_entry(): take `object_id` arguments refs: rename constant `REF_ISPRUNING` to `REF_IS_PRUNING` refs: rename constant `REF_NODEREF` to `REF_NO_DEREF` refs: tidy up and adjust visibility of the `ref_update` flags ref_transaction_add_update(): remove a check ref_transaction_update(): die on disallowed flags prune_ref(): call `ref_transaction_add_update()` directly files_transaction_prepare(): don't leak flags to packed transaction
2017-11-15Merge branch 'mh/avoid-rewriting-packed-refs'Libravatar Junio C Hamano1-1/+17
Recent update to the refs infrastructure implementation started rewriting packed-refs file more often than before; this has been optimized again for most trivial cases. * mh/avoid-rewriting-packed-refs: files-backend: don't rewrite the `packed-refs` file unnecessarily t1409: check that `packed-refs` is not rewritten unnecessarily
2017-11-06Merge branch 'bc/object-id'Libravatar Junio C Hamano1-57/+55
Conversion from uchar[20] to struct object_id continues. * bc/object-id: (25 commits) refs/files-backend: convert static functions to object_id refs: convert read_raw_ref backends to struct object_id refs: convert peel_object to struct object_id refs: convert resolve_ref_unsafe to struct object_id worktree: convert struct worktree to object_id refs: convert resolve_gitlink_ref to struct object_id Convert remaining callers of resolve_gitlink_ref to object_id sha1_file: convert index_path and index_fd to struct object_id refs: convert reflog_expire parameter to struct object_id refs: convert read_ref_at to struct object_id refs: convert peel_ref to struct object_id builtin/pack-objects: convert to struct object_id pack-bitmap: convert traverse_bitmap_commit_list to object_id refs: convert dwim_log to struct object_id builtin/reflog: convert remaining unsigned char uses to object_id refs: convert dwim_ref and expand_ref to struct object_id refs: convert read_ref and read_ref_full to object_id refs: convert resolve_refdup and refs_resolve_refdup to struct object_id Convert check_connected to use struct object_id refs: update ref transactions to use struct object_id ...
2017-11-06refs: update some more docs to use "oid" rather than "sha1"Libravatar Michael Haggerty1-10/+9
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06refs: rename constant `REF_ISPRUNING` to `REF_IS_PRUNING`Libravatar Michael Haggerty1-9/+9
Underscores are cheap, and help readability. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06refs: rename constant `REF_NODEREF` to `REF_NO_DEREF`Libravatar Michael Haggerty1-20/+20
Even after working with this code for years, I still see this constant name as "ref node ref". Rename it to make it's meaning clearer. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06refs: tidy up and adjust visibility of the `ref_update` flagsLibravatar Michael Haggerty1-0/+45
The constants used for `ref_update::flags` were rather disorganized: * The definitions in `refs.h` were not close to the functions that used them. * Maybe constants were defined in `refs-internal.h`, making them visible to the whole refs module, when in fact they only made sense for the files backend. * Their documentation wasn't very consistent and partly still referred to sha1s rather than oids. * The numerical values followed no rational scheme Fix all of these problems. The main functional improvement is that some constants' visibility is now limited to `files-backend.c`. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06ref_transaction_add_update(): remove a checkLibravatar Michael Haggerty1-1/+6
We want to make `REF_ISPRUNING` internal to the files backend. For this to be possible, `ref_transaction_add_update()` mustn't know about it. So move the check that `REF_ISPRUNING` is only used with `REF_NODEREF` from this function to `files_transaction_prepare()`. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06prune_ref(): call `ref_transaction_add_update()` directlyLibravatar Michael Haggerty1-9/+16
`prune_ref()` needs to use the `REF_ISPRUNING` flag, but we want to make that flag private to the files backend. So instead of calling `ref_transaction_delete()`, which is a public function and therefore shouldn't allow the `REF_ISPRUNING` flag, change `prune_ref()` to call `ref_transaction_add_update()`, which is private to the refs module. (Note that we don't need any of the other services provided by `ref_transaction_delete()`.) This allows us to change `ref_transaction_update()` to reject the `REF_ISPRUNING` flag. Do so by adjusting `REF_TRANSACTION_UPDATE_ALLOWED_FLAGS`. Also add parentheses to its definition to avoid potential future mishaps. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06files_transaction_prepare(): don't leak flags to packed transactionLibravatar Michael Haggerty1-2/+2
The files backend uses `ref_update::flags` for several internal flags. But those flags have no meaning to the packed backend. So when adding updates for the packed-refs transaction, only use flags that make sense to the packed backend. `REF_NODEREF` is part of the public interface, and it's logically what we want, so include it. In fact it is actually ignored by the packed backend (which doesn't support symbolic references), but that's its own business. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-30files-backend: don't rewrite the `packed-refs` file unnecessarilyLibravatar Michael Haggerty1-1/+17
Even when we are deleting references, we needn't overwrite the `packed-refs` file if the references that we are deleting only exist as loose references. Implement this optimization as follows: * Add a function `is_packed_transaction_needed()`, which checks whether a given packed-refs transaction actually needs to be carried out (i.e., it returns false if the transaction obviously wouldn't have any effect). This function must be called while holding the `packed-refs` lock to avoid races. * Change `files_transaction_prepare()` to check whether the packed-refs transaction is actually needed. If not, squelch it, but continue holding the `packed-refs` lock until the end of the transaction to avoid races. This fixes a mild regression caused by dc39e09942 (files_ref_store: use a transaction to update packed refs, 2017-09-08). Before that commit, unnecessary rewrites of `packed-refs` were suppressed by `repack_without_refs()`. But the transaction-based writing introduced by that commit didn't perform that optimization. Note that the pre-dc39e09942 code still had to *read* the whole `packed-refs` file to determine that the rewrite could be skipped, so the performance for the cases that the write could be elided was `O(N)` in the number of packed references both before and after dc39e09942. But after that commit the constant factor increased. This commit reimplements the optimization of eliding unnecessary `packed-refs` rewrites. That, plus the fact that since cfa2e29c34 (packed_ref_store: get rid of the `ref_cache` entirely, 2017-03-17) we don't necessarily have to read the whole `packed-refs` file at all, means that deletes of one or a few loose references can now be done with `O(n lg N)` effort, where `n` is the number of loose references being deleted and `N` is the total number of packed references. This commit fixes two tests in t1409. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-28Merge branch 'bc/object-id' into baseLibravatar Michael Haggerty1-57/+55
2017-10-26Merge branch 'mh/ref-locking-fix'Libravatar Junio C Hamano1-1/+1
Transactions to update multiple references that involves a deletion was quite broken in an error codepath and did not abort everything correctly. * mh/ref-locking-fix: files_transaction_prepare(): fix handling of ref lock failure t1404: add a bunch of tests of D/F conflicts
2017-10-25files_transaction_prepare(): fix handling of ref lock failureLibravatar Michael Haggerty1-1/+1
Since dc39e09942 (files_ref_store: use a transaction to update packed refs, 2017-09-08), failure to lock a reference has been handled incorrectly by `files_transaction_prepare()`. If `lock_ref_for_update()` fails in the lock-acquisition loop of that function, it sets `ret` then breaks out of that loop. Prior to dc39e09942, that was OK, because the only thing following the loop was the cleanup code. But dc39e09942 added another blurb of code between the loop and the cleanup. That blurb sometimes resets `ret` to zero, making the cleanup code think that the locking was successful. Specifically, whenever * One or more reference deletions have been processed successfully in the lock-acquisition loop. (Processing the first such reference causes a packed-ref transaction to be initialized.) * Then `lock_ref_for_update()` fails for a subsequent reference. Such a failure can happen for a number of reasons, such as the old SHA-1 not being correct, lock contention, etc. This causes a `break` out of the lock-acquisition loop. * The `packed-refs` lock is acquired successfully and `ref_transaction_prepare()` succeeds for the packed-ref transaction. This has the effect of resetting `ret` back to 0, and making the cleanup code think that lock acquisition was successful. In that case, any reference updates that were processed prior to breaking out of the loop would be carried out (loose and packed), but the reference that couldn't be locked and any subsequent references would silently be ignored. This can easily cause data loss if, for example, the user was trying to push a new name for an existing branch while deleting the old name. After the push, the branch could be left unreachable, and could even subsequently be garbage-collected. This problem was noticed in the context of deleting one reference and creating another in a single transaction, when the two references D/F conflict with each other, like git update-ref --stdin <<EOF delete refs/foo create refs/foo/bar HEAD EOF This triggers the above bug because the deletion is processed successfully for `refs/foo`, then the D/F conflict causes `lock_ref_for_update()` to fail when `refs/foo/bar` is processed. In this case the transaction *should* fail, but instead it causes `refs/foo` to be deleted without creating `refs/foo`. This could easily result in data loss. The fix is simple: instead of just breaking out of the loop, jump directly to the cleanup code. This fixes some tests in t1404 that were added in the previous commit. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs/files-backend: convert static functions to object_idLibravatar brian m. carlson1-28/+28
Convert several static functions to take pointers to struct object_id. Change the relevant parameters to write_packed_entry to be const, as we don't modify them. Rename lock_ref_sha1_basic to lock_ref_oid_basic to reflect its new argument. Update the docstring for verify lock to account for the new parameter name, and note additionally that the old_oid may be NULL. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: convert read_raw_ref backends to struct object_idLibravatar brian m. carlson1-6/+7
Convert the unsigned char * parameter to struct object_id * for files_read_raw_ref and packed_read_raw_ref. Update the documentation. Switch from using get_sha1_hex and a hard-coded 40 to using parse_oid_hex. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: convert resolve_ref_unsafe to struct object_idLibravatar brian m. carlson1-4/+4
Convert resolve_ref_unsafe to take a pointer to struct object_id by converting one remaining caller to use struct object_id, removing the temporary NULL pointer check in expand_ref, converting the declaration and definition, and applying the following semantic patch: @@ expression E1, E2, E3, E4; @@ - resolve_ref_unsafe(E1, E2, E3.hash, E4) + resolve_ref_unsafe(E1, E2, &E3, E4) @@ expression E1, E2, E3, E4; @@ - resolve_ref_unsafe(E1, E2, E3->hash, E4) + resolve_ref_unsafe(E1, E2, E3, E4) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: convert reflog_expire parameter to struct object_idLibravatar brian m. carlson1-6/+3
reflog_expire already used struct object_id internally, but it did not take it as a parameter. Adjust the parameter (and the callers) to pass a pointer to struct object_id instead of a pointer to unsigned char. Remove the temporary inserted earlier as it is no longer required. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: convert read_ref and read_ref_full to object_idLibravatar brian m. carlson1-5/+5
All but two of the call sites already have parameters using the hash parameter of struct object_id, so convert them to take a pointer to the struct directly. Also convert refs_read_refs_full, the underlying implementation. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: update ref transactions to use struct object_idLibravatar brian m. carlson1-6/+6
Update the ref transaction code to use struct object_id. Remove one NULL pointer check which was previously inserted around a dereference; since we now pass a pointer to struct object_id directly through, the code we're calling handles this for us. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: convert delete_ref and refs_delete_ref to struct object_idLibravatar brian m. carlson1-1/+1
Convert delete_ref and refs_delete_ref to take a pointer to struct object_id. Update the documentation accordingly, including referring to null_oid in lowercase, as it is not a #define constant. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs/files-backend: convert struct ref_to_prune to object_idLibravatar brian m. carlson1-3/+3
Change the member of this struct to be a struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-05Merge branch 'rs/resolve-ref-optional-result'Libravatar Junio C Hamano1-2/+1
Code clean-up. * rs/resolve-ref-optional-result: refs: pass NULL to resolve_refdup() if hash is not needed refs: pass NULL to refs_resolve_refdup() if hash is not needed
2017-10-03Merge branch 'mh/mmap-packed-refs'Libravatar Junio C Hamano1-45/+10
Operations that do not touch (majority of) packed refs have been optimized by making accesses to packed-refs file lazy; we no longer pre-parse everything, and an access to a single ref in the packed-refs does not touch majority of irrelevant refs, either. * mh/mmap-packed-refs: (21 commits) packed-backend.c: rename a bunch of things and update comments mmapped_ref_iterator: inline into `packed_ref_iterator` ref_cache: remove support for storing peeled values packed_ref_store: get rid of the `ref_cache` entirely ref_store: implement `refs_peel_ref()` generically packed_read_raw_ref(): read the reference from the mmapped buffer packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator` read_packed_refs(): ensure that references are ordered when read packed_ref_cache: keep the `packed-refs` file mmapped if possible packed-backend.c: reorder some definitions mmapped_ref_iterator_advance(): no peeled value for broken refs mmapped_ref_iterator: add iterator over a packed-refs file packed_ref_cache: remember the file-wide peeling state read_packed_refs(): read references with minimal copying read_packed_refs(): make parsing of the header line more robust read_packed_refs(): only check for a header at the top of the file read_packed_refs(): use mmap to read the `packed-refs` file die_unterminated_line(), die_invalid_line(): new functions packed_ref_cache: add a backlink to the associated `packed_ref_store` prefix_ref_iterator: break when we leave the prefix ...
2017-10-03Merge branch 'jk/read-in-full'Libravatar Junio C Hamano1-1/+1
Code clean-up to prevent future mistakes by copying and pasting code that checks the result of read_in_full() function. * jk/read-in-full: worktree: check the result of read_in_full() worktree: use xsize_t to access file size distinguish error versus short read from read_in_full() avoid looking at errno for short read_in_full() returns prefer "!=" when checking read_in_full() result notes-merge: drop dead zero-write code files-backend: prefer "0" for write_in_full() error check
2017-10-03Merge branch 'sd/branch-copy'Libravatar Junio C Hamano1-8/+38
"git branch" learned "-c/-C" to create a new branch by copying an existing one. * sd/branch-copy: branch: fix "copy" to never touch HEAD branch: add a --copy (-c) option to go with --move (-m) branch: add test for -m renaming multiple config sections config: create a function to format section headers
2017-10-01refs: pass NULL to refs_resolve_refdup() if hash is not neededLibravatar René Scharfe1-2/+1
This gets us rid of a write-only variable. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-28Merge branch 'rs/resolve-ref-optional-result'Libravatar Junio C Hamano1-2/+1
Code clean-up. * rs/resolve-ref-optional-result: refs: pass NULL to resolve_ref_unsafe() if hash is not needed refs: pass NULL to refs_resolve_ref_unsafe() if hash is not needed refs: make sha1 output parameter of refs_resolve_ref_unsafe() optional