Commit Graph

98 Commits

Author SHA1 Message Date
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>
2022-09-19 11:11:11 -07:00
Æ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>
2022-09-01 10:49:48 -07:00
Jeff King
7718827a2d refs: mark unused virtual method parameters
The refs code uses various polymorphic types (e.g., loose vs packed
ref_stores, abstracted iterators). Not every virtual function or
callback needs all of its parameters. Let's mark the unused ones to
quiet -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19 12:18:55 -07:00
Junio C Hamano
c6da34a610 Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"
This reverts commit 991b4d47f0, reversing
changes made to bcd020f88e.
2022-04-13 15:51:33 -07:00
Junio C Hamano
3d8046a820 Merge branch 'ab/refs-various-fixes'
Code clean-up.

* ab/refs-various-fixes:
  refs debug: add a wrapper for "read_symbolic_ref"
  packed-backend: remove stub BUG(...) functions
  misc *.c: use designated initializers for struct assignments
  refs: use designated initializers for "struct ref_iterator_vtable"
  refs: use designated initializers for "struct ref_storage_be"
2022-03-29 12:22:02 -07:00
Junio C Hamano
6e1a8952e9 Merge branch 'ps/fsync-refs'
Updates to refs traditionally weren't fsync'ed, but we can
configure using core.fsync variable to do so.

* ps/fsync-refs:
  core.fsync: new option to harden references
2022-03-25 16:38:25 -07:00
Ævar Arnfjörð Bjarmason
ca40893a41 packed-backend: remove stub BUG(...) functions
Remove the stub BUG(...) functions previously used by the "struct
ref_storage_be refs_be_packed" backend.

We never call any functions in the packed backend by using it as a
"normal" primary ref store, instead we'll always initialize a "files"
backend ref-store.

It will then via the "packed_ref_store" member of "struct
files_ref_store" call selected functions in the "packed" backend, and
we'll in addition call others via wrappers in refs.c.

So while these would arguably give us *slightly* more meaningful error
messages we'll NULL the missing members in the initializer anyway, so
we'll reliably get a segfault if we're ever changing the backend and
having it call something it doesn't have.

So there's no need for this verbose boilerplate, and as shown in a
subsequent commit it might even lead to some confusion about the
packed backend being a "real" backend. Let's make it clear that it's
not.

As an aside, this also fixes a warning emitted by SunCC in at least
versions 12.5 and 12.6 of Oracle Developer Studio:

    "refs/packed-backend.c", line 1599: warning: Function has no return statement : packed_create_symref
    "refs/packed-backend.c", line 1606: warning: Function has no return statement : packed_rename_ref)
    "refs/packed-backend.c", line 1613: warning: Function has no return statement : packed_copy_ref
    "refs/packed-backend.c", line 1648: warning: Function has no return statement : packed_create_reflog

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-17 10:38:05 -07:00
Ævar Arnfjörð Bjarmason
e2f8acb6a0 refs: use designated initializers for "struct ref_iterator_vtable"
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-17 10:36:11 -07:00
Ævar Arnfjörð Bjarmason
32bff617c6 refs: use designated initializers for "struct ref_storage_be"
Change the definition of the three refs backends we currently carry to
use designated initializers.

The "= NULL" assignments being retained here are redundant, and could
be removed, but let's keep them for clarity. All of these backends
define almost all fields, so we're not saving much in terms of line
count by omitting these, but e.g. for "refs_be_debug" it's immediately
apparent that we're omitting "init" when comparing its assignment to
the others.

This is a follow-up to similar work merged in bd4232fac3 (Merge
branch 'ab/struct-init', 2021-07-16), a4b9fb6a5c (Merge branch
'ab/designated-initializers-more', 2021-10-18) and a30321b9ea (Merge
branch 'ab/designated-initializers' into
ab/designated-initializers-more, 2021-09-27).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-17 10:36:04 -07:00
Patrick Steinhardt
bc22d845c4 core.fsync: new option to harden references
When writing both loose and packed references to disk we first create a
lockfile, write the updated values into that lockfile, and on commit we
rename the file into place. According to filesystem developers, this
behaviour is broken because applications should always sync data to disk
before doing the final rename to ensure data consistency [1][2][3]. If
applications fail to do this correctly, a hard crash of the machine can
easily result in corrupted on-disk data.

This kind of corruption can in fact be easily observed with Git when the
machine hard-resets shortly after writing references to disk. On
machines with ext4, this will likely lead to the "empty files" problem:
the file has been renamed, but its data has not been synced to disk. The
result is that the reference is corrupt, and in the worst case this can
lead to data loss.

Implement a new option to harden references so that users and admins can
avoid this scenario by syncing locked loose and packed references to
disk before we rename them into place.

[1]: https://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/
[2]: https://btrfs.wiki.kernel.org/index.php/FAQ (What are the crash guarantees of overwrite-by-rename)
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/admin-guide/ext4.rst (see auto_da_alloc)

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-15 13:30:58 -07:00
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>
2022-03-01 10:13:46 -08:00
Junio C Hamano
991b4d47f0 Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'
Because a deletion of ref would need to remove it from both the
loose ref store and the packed ref store, a delete-ref operation
that logically removes one ref may end up invoking ref-transaction
hook twice, which has been corrected.

* ps/avoid-unnecessary-hook-invocation-with-packed-refs:
  refs: skip hooks when deleting uncovered packed refs
  refs: do not execute reference-transaction hook on packing refs
  refs: demonstrate excessive execution of the reference-transaction hook
  refs: allow skipping the reference-transaction hook
  refs: allow passing flags when beginning transactions
  refs: extract packed_refs_delete_refs() to allow control of transaction
2022-02-18 13:53:27 -08:00
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>
2022-01-17 11:01:44 -08:00
Patrick Steinhardt
69840cc0f7 refs: extract packed_refs_delete_refs() to allow control of transaction
When deleting loose refs, then we also have to delete the refs in the
packed backend. This is done by calling `refs_delete_refs()`, which
then uses the packed-backend's logic to delete refs. This doesn't allow
us to exercise any control over the reference transaction which is being
created in the packed backend, which is required in a subsequent commit.

Extract a new function `packed_refs_delete_refs()`, which hosts most of
the logic to delete refs except for creating the transaction itself.
Like this, we can easily create the transaction in the files backend
and thus exert more control over it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17 11:01:44 -08:00
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>
2021-12-22 13:51:38 -08:00
Han-Wen Nienhuys
99f0d97b73 refs: pass gitdir to packed_ref_store_create
This is consistent with the calling convention for ref backend creation, and
avoids storing ".git/packed-refs" (the name of a regular file) in a variable called
ref_store::gitdir.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-22 13:51:37 -08:00
Junio C Hamano
b8148376a2 Merge branch 'hn/create-reflog-simplify'
A small simplification of API.

* hn/create-reflog-simplify:
  refs: drop force_create argument of create_reflog API
2021-12-10 14:35:13 -08:00
Junio C Hamano
96f6623ada Merge branch 'ab/refs-errno-cleanup'
The "remainder" of hn/refs-errno-cleanup topic.

* ab/refs-errno-cleanup: (21 commits)
  refs API: post-migration API renaming [2/2]
  refs API: post-migration API renaming [1/2]
  refs API: don't expose "errno" in run_transaction_hook()
  refs API: make expand_ref() & repo_dwim_log() not set errno
  refs API: make resolve_ref_unsafe() not set errno
  refs API: make refs_ref_exists() not set errno
  refs API: make refs_resolve_refdup() not set errno
  refs tests: ignore ignore errno in test-ref-store helper
  refs API: ignore errno in worktree.c's find_shared_symref()
  refs API: ignore errno in worktree.c's add_head_info()
  refs API: make files_copy_or_rename_ref() et al not set errno
  refs API: make loose_fill_ref_dir() not set errno
  refs API: make resolve_gitlink_ref() not set errno
  refs API: remove refs_read_ref_full() wrapper
  refs/files: remove "name exist?" check in lock_ref_oid_basic()
  reflog tests: add --updateref tests
  refs API: make refs_rename_ref_available() static
  refs API: make parse_loose_ref_contents() not set errno
  refs API: make refs_read_raw_ref() not set errno
  refs API: add a version of refs_resolve_ref_unsafe() with "errno"
  ...
2021-11-29 15:41:45 -08:00
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>
2021-11-22 11:01:25 -08:00
Junio C Hamano
162a13b855 Merge branch 'jt/no-abuse-alternate-odb-for-submodules'
Follow through the work to use the repo interface to access
submodule objects in-process, instead of abusing the alternate
object database interface.

* jt/no-abuse-alternate-odb-for-submodules:
  submodule: trace adding submodule ODB as alternate
  submodule: pass repo to check_has_commit()
  object-file: only register submodule ODB if needed
  merge-{ort,recursive}: remove add_submodule_odb()
  refs: peeling non-the_repository iterators is BUG
  refs: teach arbitrary repo support to iterators
  refs: plumb repo into ref stores
2021-10-25 16:06:56 -07:00
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>
2021-10-16 11:17:02 -07:00
Jonathan Tan
8788195c88 refs: peeling non-the_repository iterators is BUG
There is currently no support for peeling the current ref of an iterator
iterating over a non-the_repository ref store, and none is needed. Thus,
for now, BUG() if that happens.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-08 15:06:06 -07:00
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>
2021-10-08 15:06:05 -07:00
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>
2021-10-08 15:06:05 -07:00
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>
2021-08-25 13:30:26 -07:00
Æ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>
2021-08-25 13:27:37 -07:00
Ævar Arnfjörð Bjarmason
881aebffcf refs/packet: add missing BUG() invocations to reflog callbacks
In e0cc8ac820 (packed_ref_store: make class into a subclass of
`ref_store`, 2017-06-23) a die() was added to packed_create_reflog(),
but not to any of the other reflog callbacks, let's do that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-19 19:06:38 -07:00
Junio C Hamano
bd4232fac3 Merge branch 'ab/struct-init'
Code cleanup around struct_type_init() functions.

* ab/struct-init:
  string-list.h users: change to use *_{nodup,dup}()
  string-list.[ch]: add a string_list_init_{nodup,dup}()
  dir.[ch]: replace dir_init() with DIR_INIT
  *.c *_init(): define in terms of corresponding *_INIT macro
  *.h: move some *_INIT to designated initializers
2021-07-16 17:42:53 -07:00
Ævar Arnfjörð Bjarmason
bc40dfb10a string-list.h users: change to use *_{nodup,dup}()
Change all in-tree users of the string_list_init(LIST, BOOL) API to
use string_list_init_{nodup,dup}(LIST) instead.

As noted in the preceding commit let's leave the now-unused
string_list_init() wrapper in-place for any in-flight users, it can be
removed at some later date.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-01 12:32:22 -07:00
Han-Wen Nienhuys
617480d75b refs: make explicit that ref_iterator_peel returns boolean
Use -1 as error return value throughout.

This removes spurious differences in the GIT_TRACE_REFS output, depending on the
ref storage backend active.

Before, the cached ref_iterator (but only that iterator!) would return
peel_object() output directly. No callers relied on the peel_status values
beyond success/failure. All calls to these functions go through
peel_iterated_oid(), which returns peel_object() as a fallback, but also
squashing the error values.

The iteration interface already passes REF_ISSYMREF and REF_ISBROKEN through the
flags argument, so the additional error values in enum peel_status provide no
value.

The ref iteration interface provides a separate peel() function because certain
formats (eg. packed-refs and reftable) can store the peeled object next to the
tag SHA1. Passing the peeled SHA1 as an optional argument to each_ref_fn maps
more naturally to the implementation of ref databases. Changing the code in this
way is left for a future refactoring.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-20 07:54:12 +09:00
René Scharfe
ca56dadb4b use CALLOC_ARRAY
Add and apply a semantic patch for converting code that open-codes
CALLOC_ARRAY to use it instead.  It shortens the code and infers the
element size automatically.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13 16:00:09 -08:00
Han-Wen Nienhuys
5085aef4c8 refs: move gitdir into base ref_store
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-19 14:08:04 -07:00
Sun Chao
a613d4f817 pack-refs: always refresh after taking the lock file
When a packed ref is deleted, the whole packed-refs file is
rewritten to omit the ref that no longer exists. However if another
gc command is running and calls `pack-refs --all` simultaneously,
there is a chance that a ref that was just updated lose the newly
created commits.

Through these steps, losing commits on newly updated refs can be
demonstrated:

  # step 1: compile git without `USE_NSEC` option
  Some kernel releases do enable it by default while some do
  not. And if we compile git without `USE_NSEC`, it will be easier
  demonstrated by the following steps.

  # step 2: setup a repository and add the first commit
  git init repo &&
  (cd repo &&
   git config core.logallrefupdates true &&
   git commit --allow-empty -m foo)

  # step 3: in one terminal, repack the refs repeatedly
  cd repo &&
  while true
  do
    git pack-refs --all
  done

  # step 4: in another terminal, simultaneously update the
  # master with update-ref, and create and delete an
  # unrelated ref also with update-ref
  cd repo &&
  while true
  do
    us=$(git commit-tree -m foo -p HEAD HEAD^{tree}) &&
    git update-ref refs/heads/newbranch $us &&
    git update-ref refs/heads/master $us &&
    git update-ref -d refs/heads/newbranch &&
    them=$(git rev-parse master) &&
    if test "$them" != "$us"
    then
      echo >&2 "lost commit: $us"
      exit 1
    fi
    # eye candy
    printf .
  done

Though we have the packed-refs lock file and loose refs lock
files to avoid updating conflicts, a ref will lost its newly
commits if racy stat-validity of `packed-refs` file happens
(which is quite same as the racy-git described in
`Documentation/technical/racy-git.txt`), the following
specific set of operations demonstrates the problem:

  1. Call `pack-refs --all` to pack all the loose refs to
     packed-refs, and let say the modify time of the
     packed-refs is DATE_M.

  2. Call `update-ref` to update a new commit to master while
     it is already packed.  the old value (let us call it
     OID_A) remains in the packed-refs file and write the new
     value (let us call it OID_B) to $GIT_DIR/refs/heads/master.

  3. Call `update-ref -d` within the same DATE_M from the 1th
     step to delete a different ref newbranch which is packed
     in the packed-refs file. It check newbranch's oid from
     packed-refs file without locking it.

     Meanwhile it keeps a snapshot of the packed-refs file in
     memory and record the file's attributes with the snapshot.
     The oid of master in the packed-refs's snapshot is OID_A.

  4. Call a new `pack-refs --all` to pack the loose refs, the
     oid of master in packe-refs file is OID_B, and the loose
     refs $GIT_DIR/refs/heads/master is removed. Let's say
     the `pack-refs --all` is very quickly done and the new
     packed-refs file's modify time is still DATE_M, and it
     has the same file size, even the same inode.

  5. 3th step now goes on after checking the newbranch, it
     begin to rewrite the packed-refs file. After get the
     lock file of packed-ref file, it checks it's on-disk
     file attributes with the snapshot, suck as the timestamp,
     the file size and the inode value. If they are both the
     same values, and the snapshot is not refreshed.

     Because the loose ref of master is removed by 4th step,
     `update-ref -d` will updates the new packed-ref to disk
     which contains master with the oid OID_A. So now the
     newly commit OID_B of master is lost.

The best path forward is just always refreshing after take
the lock file of `packed-refs` file. Traditionally we avoided
that because refreshing it implied parsing the whole file.
But these days we mmap it, so it really is just an extra
open()/mmap() and a quick read of the header. That doesn't seem
like an outrageous cost to pay when we're already taking the lock.

Signed-off-by: Sun Chao <sunchao9@huawei.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Sun Chao <sunchao9@huawei.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-02 09:59:05 -07:00
brian m. carlson
49d166081b refs/packed-backend: express constants using the_hash_algo
Switch uses of GIT_SHA1_HEXSZ to use the_hash_algo so that they are
appropriate for the any given hash length.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-15 12:53:15 +09:00
Jeff King
9001dc2a74 convert "oidcmp() != 0" to "!oideq()"
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-29 11:32:49 -07:00
Stefan Beller
09427e8366 refs/packed-backend.c: close fd of empty file
Signed-off-by: Stefan Beller <sbeller@google.com>
Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-01 10:49:53 +09:00
Johannes Schindelin
033abf97fc Replace all die("BUG: ...") calls by BUG() ones
In d8193743e0 (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 588a538ae5
(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-05-06 19:06:13 +09:00
Jeff King
fb9c2d2703 refs: use chdir_notify to update cached relative paths
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-03-30 12:50:03 -07:00
Junio C Hamano
9db22910f7 Merge branch 'kg/packed-ref-cache-fix'
Avoid mmapping small files while using packed refs (especially ones
with zero size, which would cause later munmap() to fail).

* kg/packed-ref-cache-fix:
  packed_ref_cache: don't use mmap() for small files
  load_contents(): don't try to mmap an empty file
  packed_ref_iterator_begin(): make optimization more general
  find_reference_location(): make function safe for empty snapshots
  create_snapshot(): use `xmemdupz()` rather than a strbuf
  struct snapshot: store `start` rather than `header_len`
2018-02-15 14:55:42 -08:00
Kim Gybels
ba41a8b600 packed_ref_cache: don't use mmap() for small files
Take a hint from commit ea68b0ce9f (hash-object: don't use mmap() for
small files, 2010-02-21) and use read() instead of mmap() for small
packed-refs files.

Signed-off-by: Kim Gybels <kgybels@infogroep.be>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24 12:55:26 -08:00
Michael Haggerty
01caf20d57 load_contents(): don't try to mmap an empty file
We don't actually create zero-length `packed-refs` files, but they are
valid and we should handle them correctly. The old code `xmmap()`ed
such files, which led to an error when `munmap()` was called. So, if
the `packed-refs` file is empty, leave the snapshot at its zero values
and return 0 without trying to read or mmap the file.

Returning 0 also makes `create_snapshot()` exit early, which avoids
the technically undefined comparison `NULL < NULL`.

Reported-by: Kim Gybels <kgybels@infogroep.be>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24 12:55:26 -08:00
Michael Haggerty
f34242975f packed_ref_iterator_begin(): make optimization more general
We can return an empty iterator not only if the `packed-refs` file is
missing, but also if it is empty or if there are no references whose
names succeed `prefix`. Optimize away those cases as well by moving
the call to `find_reference_location()` higher in the function and
checking whether the determined start position is the same as
`snapshot->eof`. (This is possible now because the previous commit
made `find_reference_location()` robust against empty snapshots.)

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24 12:55:26 -08:00
Michael Haggerty
4a14f8d093 find_reference_location(): make function safe for empty snapshots
This function had two problems if called for an empty snapshot (i.e.,
`snapshot->start == snapshot->eof == NULL`):

* It checked `NULL < NULL`, which is undefined by C (albeit highly
  unlikely to fail in the real world).

* (Assuming the above comparison behaved as expected), it returned
  NULL when `mustexist` was false, contrary to its docstring.

Change the check and fix the docstring.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24 12:55:26 -08:00
Michael Haggerty
27a41841ec create_snapshot(): use xmemdupz() rather than a strbuf
It's lighter weight.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24 12:55:26 -08:00
Michael Haggerty
4a2854f77c struct snapshot: store start rather than header_len
Store a pointer to the start of the actual references within the
`packed-refs` contents rather than storing the length of the header.
This is more convenient for most users of this field.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24 12:55:26 -08:00
Junio C Hamano
02abc6be8e Merge branch 'mh/avoid-rewriting-packed-refs' into maint
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-12-06 09:08:50 -08:00
Junio C Hamano
a97222978a Merge branch 'mh/tidy-ref-update-flags'
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-15 12:14:29 +09:00
Junio C Hamano
ffb0b5762e Merge branch 'mh/avoid-rewriting-packed-refs'
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-15 12:14:27 +09:00
Michael Haggerty
78fb457968 refs: update some more docs to use "oid" rather than "sha1"
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06 10:31:08 +09:00
Michael Haggerty
4170188262 write_packed_entry(): take object_id arguments
Change `write_packed_entry()` to take `struct object_id *` rather than
`unsigned char *` arguments.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06 10:31:08 +09:00