Граф коммитов

334 Коммитов

Автор SHA1 Сообщение Дата
Han-Wen Nienhuys 63c0567365 refs: move REF_LOG_ONLY to refs-internal.h
REF_LOG_ONLY is used in the transaction preparation: if a symref is involved in
a transaction, the referent of the symref should be updated, and the symref
itself should only be updated in the reflog.

Other ref backends will need to duplicate this logic too, so move it to a
central place.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-08 15:51:07 -07: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
Han-Wen Nienhuys e39620f07e refs: split off reading loose ref data in separate function
This prepares for handling FETCH_HEAD (which is not a regular ref)
separately from the ref backend.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-19 14:08:03 -07:00
Han-Wen Nienhuys 25429fed5c refs: move the logic to add \t to reflog to the files backend
523fa69c (reflog: cleanse messages in the refs.c layer, 2020-07-10)
centralized reflog normalizaton.  However, the normalizaton added a
leading "\t" to the message. This is an artifact of the reflog
storage format in the files backend, so it should be added there.

Routines that parse back the reflog (such as grab_nth_branch_switch)
expect the "\t" to not be in the message, so without this fix, git
with reftable cannot process the "@{-1}" syntax.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-31 10:21:51 -07:00
Junio C Hamano 523fa69c36 reflog: cleanse messages in the refs.c layer
Regarding reflog messages:

 - We expect that a reflog message consists of a single line.  The
   file format used by the files backend may add a LF after the
   message as a delimiter, and output by commands like "git log -g"
   may complete such an incomplete line by adding a LF at the end,
   but philosophically, the terminating LF is not a part of the
   message.

 - We however allow callers of refs API to supply a random sequence
   of NUL terminated bytes.  We cleanse caller-supplied message by
   squashing a run of whitespaces into a SP, and by trimming trailing
   whitespace, before storing the message.  This is how we tolerate,
   instead of erring out, a message with LF in it (be it at the end,
   in the middle, or both).

Currently, the cleansing of the reflog message is done by the files
backend, before the log is written out.  This is sufficient with the
current code, as that is the only backend that writes reflogs.  But
new backends can be added that write reflogs, and we'd want the
resulting log message we would read out of "log -g" the same no
matter what backend is used, and moving the code to do so to the
generic layer is a way to do so.

An added benefit is that the "cleansing" function could be updated
later, independent from individual backends, to e.g. allow
multi-line log messages if we wanted to, and when that happens, it
would help a lot to ensure we covered all bases if the cleansing
function (which would be updated) is called from the generic layer.

Side note: I am not interested in supporting multi-line reflog
messages right at the moment (nobody is asking for it), but I
envision that instead of the "squash a run of whitespaces into a SP
and rtrim" cleansing, we can %urlencode problematic bytes in the
message *AND* append a SP at the end, when a new version of Git that
supports multi-line and/or verbatim reflog messages writes a reflog
record.  The reading side can detect the presense of SP at the end
(which should have been rtrimmed out if it were written by existing
versions of Git) as a signal that decoding %urlencode recovers the
original reflog message.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-10 13:53:37 -07:00
Patrick Steinhardt edc30691e5 refs: fix segfault when aborting empty transaction
When cleaning up a transaction that has no updates queued, then the
transaction's backend data will not have been allocated. We correctly
handle this for the packed backend, where the cleanup function checks
whether the backend data has been allocated at all -- if not, then there
is nothing to clean up. For the files backend we do not check this and
as a result will hit a segfault due to dereferencing a `NULL` pointer
when cleaning up such a transaction.

Fix the issue by checking whether `backend_data` is set in the files
backend, too.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-30 10:34:11 -07:00
Junio C Hamano 145136a95a C: use skip_prefix() to avoid hardcoded string length
We often skip an optional prefix in a string with a hardcoded
constant, e.g.

	if (starts_with(string, "prefix"))
		string += 6;

which is less error prone when written

	skip_prefix(string, "prefix", &string);

Note that this changes a few error messages from "git reflog expire
--expire=nonsense.timestamp", which used to complain by saying

    '--expire=nonsense.timestamp' is not a valid timestamp

but with this change, we say

    'nonsense.timestamp' is not a valid timestamp

which is more technically correct (the string with --expire= as
a prefix obviously cannot be a valid timestamp, but the error is
about the part of the input without that prefix).

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-31 13:03:45 -08:00
René Scharfe e0ae2447d6 refs: pass NULL to refs_read_ref_full() because object ID is not needed
refs_read_ref_full() wraps refs_resolve_ref_unsafe(), which handles a
NULL oid pointer of callers not interested in the resolved object ID.
Pass NULL from files_copy_or_rename_ref() to clarify that it is one
such caller.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-11 13:48:42 -08:00
René Scharfe 9b7b0295f9 dir-iterator: release strbuf after use
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-07 12:25:35 -07:00
Matheus Tavares fa1da7d2ee dir-iterator: add flags parameter to dir_iterator_begin
Add the possibility of giving flags to dir_iterator_begin to initialize
a dir-iterator with special options.

Currently possible flags are:
- DIR_ITERATOR_PEDANTIC, which makes dir_iterator_advance abort
immediately in the case of an error, instead of keep looking for the
next valid entry;
- DIR_ITERATOR_FOLLOW_SYMLINKS, which makes the iterator follow
symlinks and include linked directories' contents in the iteration.

These new flags will be used in a subsequent patch.

Also add tests for the flags' usage and adjust refs/files-backend.c to
the new dir_iterator_begin signature.

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11 13:52:15 -07:00
Matheus Tavares 3012397e03 dir-iterator: refactor state machine model
dir_iterator_advance() is a large function with two nested loops. Let's
improve its readability factoring out three functions and simplifying
its mechanics. The refactored model will no longer depend on
level.initialized and level.dir_state to keep track of the iteration
state and will perform on a single loop.

Also, dir_iterator_begin() currently does not check if the given string
represents a valid directory path. Since the refactored model will have
to stat() the given path at initialization, let's also check for this
kind of error and make dir_iterator_begin() return NULL, on failures,
with errno appropriately set. And add tests for this new behavior.

Improve documentation at dir-iteration.h and code comments at
dir-iterator.c to reflect the changes and eliminate possible
ambiguities.

Finally, adjust refs/files-backend.c to check for now possible
dir_iterator_begin() failures.

Original-patch-by: Daniel Ferreira <bnmvco@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11 13:52:15 -07:00
Junio C Hamano 2e08c892a7 Merge branch 'jk/refs-double-abort'
A corner case bug in the refs API has been corrected.

* jk/refs-double-abort:
  refs/files-backend: don't look at an aborted transaction
  refs/files-backend: handle packed transaction prepare failure
2019-04-16 19:28:11 +09:00
Junio C Hamano 917f2cd1c2 Merge branch 'nd/rewritten-ref-is-per-worktree'
"git rebase" uses the refs/rewritten/ hierarchy to store its
intermediate states, which inherently makes the hierarchy per
worktree, but it didn't quite work well.

* nd/rewritten-ref-is-per-worktree:
  Make sure refs/rewritten/ is per-worktree
  files-backend.c: reduce duplication in add_per_worktree_entries_to_dir()
  files-backend.c: factor out per-worktree code in loose_fill_ref_dir()
2019-04-10 02:14:23 +09:00
Jeff King d3322eb28b refs/files-backend: don't look at an aborted transaction
When deleting refs, we hold packed-refs.lock and prepare a packed
transaction to drop the refs from the packed-refs file. If it turns out
that we don't need to rewrite the packed refs (e.g., because none of the
deletions were present in the file), then we abort the transaction.

If that abort succeeds, then the transaction struct will have been
freed, and we set our local pointer to NULL so we don't look at it
again.

However, if it fails, then the struct will _still_ have been freed
(because ref_transaction_abort() always frees). But we don't clean up
the pointer, and will jump to our cleanup code, which will try to abort
it again, causing a use-after-free.

It's actually impossible for this to trigger in practice, since
packed_transaction_abort() will never return anything but success. But
let's fix it anyway, since that's more than we should assume about the
packed-refs code (after all, we are already bothering to check for an
error result which cannot be triggered).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-22 15:52:54 +09:00
Jeff King 249e8dc73e refs/files-backend: handle packed transaction prepare failure
In files_transaction_prepare(), if we have to delete some refs, we use a
subordinate packed_transaction to do so. It's rare for that
sub-transaction's prepare step to fail, since we hold the packed-refs
lock. But if it does, we trigger a BUG() due to these steps:

  - we've attached the packed transaction to the files transaction as
    backend_data->packed_transaction

  - when the prepare step fails, the packed transaction cleans itself
    up, putting itself into the CLOSED state

  - the error value from preparing the packed transaction lets us know
    in files_transaction_prepare() that we should also clean up and
    return an error. We call files_transaction_cleanup(), which tries to
    abort backend_data->packed_transaction. Since it's already CLOSED,
    that triggers an assertion in ref_transaction_abort().

We can fix that by disconnecting the packed transaction from the outer
files transaction, and then free-ing (not aborting!) it ourselves.

A few other options/alternatives I considered:

  - we could just make it a noop to abort a CLOSED transaction. But that
    seems less safe, since clearly this code expects (and enforces) a
    particular set of state transitions.

  - we could have files_transaction_cleanup() selectively call abort()
    vs free() based on the state of the on the packed transaction.
    That's basically a more restricted version of the above, but also
    potentially unsafe.

  - instead of disconnecting backend_data->packed_transaction on error,
    we could wait to install it until we successfully prepare. That
    might make the flow a little simpler, but it introduces a hassle.
    Earlier parts of files_transaction_prepare() that encounter an error
    will jump to the cleanup label, and expect that cleaning up the
    outer transaction will clean up the packed transaction, too. We'd
    have to adjust those sites to clean up the packed transaction.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-22 15:52:49 +09:00
Nguyễn Thái Ngọc Duy b9317d55a3 Make sure refs/rewritten/ is per-worktree
a9be29c981 (sequencer: make refs generated by the `label` command
worktree-local, 2018-04-25) adds refs/rewritten/ as per-worktree
reference space. Unfortunately (my bad) there are a couple places that
need update to make sure it's really per-worktree.

 - add_per_worktree_entries_to_dir() is updated to make sure ref listing
   look at per-worktree refs/rewritten/ instead of per-repo one [1]

 - common_list[] is updated so that git_path() returns the correct
   location. This includes "rev-parse --git-path".

This mess is created by me. I started trying to fix it with the
introduction of refs/worktree, where all refs will be per-worktree
without special treatments. Unfortunate refs/rewritten came before
refs/worktree so this is all we can do.

This also fixes logs/refs/worktree not being per-worktree.

[1] note that ref listing still works sometimes. For example, if you
    have .git/worktrees/foo/refs/rewritten/bar AND the directory
    .git/worktrees/refs/rewritten, refs/rewritten/bar will show up.
    add_per_worktree_entries_to_dir() is only needed when the directory
    .git/worktrees/refs/rewritten is missing.

Reported-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-08 11:57:47 +09:00
Nguyễn Thái Ngọc Duy 90d31ff5d4 files-backend.c: reduce duplication in add_per_worktree_entries_to_dir()
This function is duplicated to handle refs/bisect/ and refs/worktree/
and a third prefix is coming. Time to clean up.

This also fixes incorrect "refs/worktrees/" length in this code. The
correct length is 14 not 11. The test in the next patch will also cover
this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-08 11:57:47 +09:00
Nguyễn Thái Ngọc Duy 09e65645e3 files-backend.c: factor out per-worktree code in loose_fill_ref_dir()
This is the first step for further cleaning up and extending this
function.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-08 11:57:47 +09:00
Jeff King 10dee40ed3 files-backend: drop refs parameter from split_symref_update()
This parameter was added in fcc42ea0c9 (split_symref_update(): add a
files_ref_store argument, 2016-09-04) without comment, but never used.
The splitting is purely mechanical, and doesn't depend on the particular
ref-store. Let's drop this parameter in the name of simplicity.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-14 15:26:15 -08:00
Nguyễn Thái Ngọc Duy 46c0eb5843 files-backend.c: fix build error on Solaris
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-11-26 15:22:58 +09:00
Nguyễn Thái Ngọc Duy 3a3b9d8cde refs: new ref types to make per-worktree refs visible to all worktrees
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-22 13:32:29 +09:00
Nguyễn Thái Ngọc Duy 8aff1a9ca5 Add a place for (not) sharing stuff between worktrees
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-10-07 08:21:18 +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
Jeff King 4a7e27e957 convert "oidcmp() == 0" to oideq()
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-29 11:32:49 -07:00
Junio C Hamano 7d020f5a78 Merge branch 'jk/size-t'
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-15 15:08:25 -07:00
Junio C Hamano 3a2a1dc170 Merge branch 'sb/object-store-lookup'
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-02 15:30:42 -07:00
Junio C Hamano 37aac3e408 Merge branch 'bc/object-id'
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-08-02 15:30:39 -07:00
Jeff King 765b496dc6 pass st.st_size as hint for strbuf_readlink()
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-24 10:19:29 -07:00
brian m. carlson 2ae2e2a1ca refs/files-backend: use the_hash_algo for writing refs
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-16 14:27:39 -07:00
Ben Peart 80a6c2073b convert log_ref_write_fd() to use strbuf
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-07-10 14:22:44 -07:00
Stefan Beller 109cd76dd3 object: add repository argument to parse_object
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-06-29 10:43:38 -07:00
Junio C Hamano 50f08db594 Merge branch 'js/use-bug-macro'
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-30 14:04:07 +09:00
Junio C Hamano 2f76ebc93c Merge branch 'ma/lockfile-cleanup'
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-30 14:04:05 +09:00
Martin Ågren b227586831 lock_file: make function-local locks non-static
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-10 14:54:45 +09:00
Junio C Hamano 174774cd51 Merge branch 'sb/object-store-replace'
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-08 15:59:21 +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
Stefan Beller 64a741619d refs: store the main ref store inside the repository struct
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-04-12 11:38:56 +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 2b72ea0a48 Merge branch 'mr/packed-ref-store-fix'
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-02-13 13:39:11 -08:00
Mathias Rav 81fcb698e0 files_initial_transaction_commit(): only unlock if locked
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>
2018-01-19 14:16:56 -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
Junio C Hamano e7e456f500 Merge branch 'bc/object-id'
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-06 14:24: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 acedcde76d refs: rename constant `REF_ISPRUNING` to `REF_IS_PRUNING`
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-06 10:31:08 +09:00
Michael Haggerty 91774afcc3 refs: rename constant `REF_NODEREF` to `REF_NO_DEREF`
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-06 10:31:08 +09:00
Michael Haggerty 5ac95fee3d refs: tidy up and adjust visibility of the `ref_update` flags
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-06 10:31:07 +09:00
Michael Haggerty 62c72d1fd0 ref_transaction_add_update(): remove a check
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-06 10:31:07 +09:00
Michael Haggerty b00f3cfa92 prune_ref(): call `ref_transaction_add_update()` directly
`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-06 10:31:07 +09:00