We read loose references in two steps. The code is roughly:
lstat()
if error ENOENT:
loose ref is missing; look for corresponding packed ref
else if S_ISLNK:
readlink()
if error:
report failure
else if S_ISDIR:
report failure
else
open()
if error:
report failure
read()
The problem is that the first filesystem call, to lstat(), is not
atomic with the second filesystem call, to readlink() or open().
Therefore it is possible for another process to change the file
between our two calls, for example:
* If the other process deletes the file, our second call will fail
with ENOENT, which we *should* interpret as "loose ref is missing;
look for corresponding packed ref". This can arise if the other
process is pack-refs; it might have just written a new packed-refs
file containing the old contents of the reference then deleted the
loose ref.
* If the other process changes a symlink into a plain file, our call
to readlink() will fail with EINVAL, which we *should* respond to by
trying to open() and read() the file.
The old code treats the reference as missing in both of these cases,
which is incorrect.
So instead, handle errors more selectively: if the result of
readline()/open() is a failure that is inconsistent with the result of
the previous lstat(), then something is fishy. In this case jump back
and start over again with a fresh call to lstat().
One race is still possible and undetected: another process could
change the file from a regular file into a symlink between the call to
lstat and the call to open(). The open() call would silently follow
the symlink and not know that something is wrong. This situation
could be detected in two ways:
* On systems that support O_NOFOLLOW, pass that option to the open().
* On other systems, call fstat() on the fd returned by open() and make
sure that it agrees with the stat info from the original lstat().
However, we don't use symlinks anymore, so this situation is unlikely.
Moreover, it doesn't appear that treating a symlink as a regular file
would have grave consequences; after all, this is exactly how the code
handles non-relative symlinks. So this commit leaves that race
unaddressed.
Note that this solves only the part of the race within
resolve_ref_unsafe. In the situation described above, we may still be
depending on a cached view of the packed-refs file; that race will be
dealt with in a future patch.
This problem was reported and diagnosed by Jeff King <peff@peff.net>,
and this solution is derived from his patch.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is only one "break" statement within the loop, which jumps to
the code after the loop that handles the case of a file that holds a
SHA-1. So move that code from below the loop into the if statement
where the break was previously located. This makes the logic flow
more local.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The nesting was getting a bit out of hand, and it's about to get
worse.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update reading and updating packed-refs file, correcting corner case
bugs.
* mh/packed-refs-various: (33 commits)
refs: handle the main ref_cache specially
refs: change do_for_each_*() functions to take ref_cache arguments
pack_one_ref(): do some cheap tests before a more expensive one
pack_one_ref(): use write_packed_entry() to do the writing
pack_one_ref(): use function peel_entry()
refs: inline function do_not_prune()
pack_refs(): change to use do_for_each_entry()
refs: use same lock_file object for both ref-packing functions
pack_one_ref(): rename "path" parameter to "refname"
pack-refs: merge code from pack-refs.{c,h} into refs.{c,h}
pack-refs: rename handle_one_ref() to pack_one_ref()
refs: extract a function write_packed_entry()
repack_without_ref(): write peeled refs in the rewritten file
t3211: demonstrate loss of peeled refs if a packed ref is deleted
refs: change how packed refs are deleted
search_ref_dir(): return an index rather than a pointer
repack_without_ref(): silence errors for dangling packed refs
t3210: test for spurious error messages for dangling packed refs
refs: change the internal reference-iteration API
refs: extract a function peel_entry()
...
Typing 'HEAD' is tedious, especially when we can use '@' instead.
The reason for choosing '@' is that it follows naturally from the
ref@op syntax (e.g. HEAD@{u}), except we have no ref, and no
operation, and when we don't have those, it makes sens to assume
'HEAD'.
So now we can use 'git show @~1', and all that goody goodness.
Until now '@' was a valid name, but it conflicts with this idea, so
let's make it invalid. Probably very few people, if any, used this name.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hold the ref_cache instance for the main repository in a dedicated,
statically-allocated instance to avoid the need for a function call
and a linked-list traversal when it is needed.
Suggested by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the callers convert submodule names into ref_cache pointers.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change pack_refs() to work with a file descriptor instead of a FILE*
(making the file-locking code less awkward) and use
write_packed_entry() to do the writing.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change pack_one_ref() to call peel_entry() rather than using its own
code for peeling references. Aside from sharing code, this lets it
take advantage of the optimization introduced by 6c4a060d7d.
Please note that we *could* use any peeled values that happen to
already be stored in the ref_entries, which would avoid some object
lookups for references that were already packed. But doing so would
also propagate any peeling errors across runs of "git pack-refs" and
give no way to recover from such errors. And "git pack-refs" isn't
run often enough that the performance cost is a problem. So instead,
add a new option to peel_entry() to force the entry to be re-peeled,
and call it with that option set.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Function do_not_prune() was redundantly checking REF_ISSYMREF, which
was already tested at the top of pack_one_ref(), so remove that check.
And the rest was trivial, so inline the function.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack_refs() was not using any of the extra features of for_each_ref(),
so change it to use do_for_each_entry(). This also gives it access to
the ref_entry and in particular its peeled field, which will be taken
advantage of in the next commit.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use a single struct lock_file for both pack_refs() and
repack_without_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make this function conform to the naming convention established in
65385ef7d4 for the rest of the refs.c file.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-refs.c doesn't contain much code, and the code it does contain is
closely related to reference handling. Moreover, there is some
duplication between pack_refs() and repack_without_ref(). Therefore,
merge pack-refs.c into refs.c and pack-refs.h into refs.h.
The code duplication will be addressed in future commits.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract the I/O code from the "business logic" in repack_ref_fn().
Later there will be another caller for this function.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a reference that existed in the packed-refs file is deleted, the
packed-refs file must be rewritten. Previously, the file was
rewritten without any peeled refs, even if the file contained peeled
refs when it was read. This was not a bug, because the packed-refs
file header didn't claim that the file contained peeled values. But
it had a performance cost, because the repository would lose the
benefit of having precomputed peeled references until pack-refs was
run again.
Teach repack_without_ref() to write peeled refs to the packed-refs
file (regardless of whether they were present in the old version of
the file).
This means that if the old version of the packed-refs file was not
fully peeled, then repack_without_ref() will have to peel references.
To avoid the expense of reading lots of loose references, we take two
shortcuts relative to pack-refs:
* If the peeled value of a reference is already known (i.e., because
it was read from the old version of the packed-refs file), then
output that peeled value again without any checks. This is the
usual code path and should avoid any noticeable overhead. (This is
different than pack-refs, which always re-peels references.)
* We don't verify that the packed ref is still current. It could be
that a packed references is overridden by a loose reference, in
which case the packed ref is no longer needed and might even refer
to an object that has been garbage collected. But we don't check;
instead, we just try to peel all references. If peeling is
successful, the peeled value is written out (even though it might
not be needed any more); if not, then the reference is silently
omitted from the output.
The extra overhead of peeling references in repack_without_ref()
should only be incurred the first time the packed-refs file is written
by a version of Git that knows about the "fully-peeled" attribute.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a function remove_ref(), which removes a single entry from a
reference cache.
Use this function to reimplement repack_without_ref(). The old
version iterated over all refs, packing all of them except for the one
to be deleted, then discarded the entire packed reference cache. The
new version deletes the doomed reference from the cache *before*
iterating.
This has two advantages:
* the code for writing packed-refs becomes simpler, because it doesn't
have to exclude one of the references.
* it is no longer necessary to discard the packed refs cache after
deleting a reference: symbolic refs cannot be packed, so packed
references cannot depend on each other, so the rest of the packed
refs cache remains valid after a reference is deleted.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change search_ref_dir() to return the index of the sought entry (or -1
on error) rather than a pointer to the entry. This will make it more
natural to use the function for removing an entry from the list.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Stop emitting an error message when deleting a packed reference if we
find another dangling packed reference that is overridden by a loose
reference. See the previous commit for a longer explanation of the
issue.
We have to be careful to make sure that the invalid packed reference
really *is* overridden by a loose reference; otherwise what we have
found is repository corruption, which we *should* report.
Please note that this approach is vulnerable to a race condition
similar to the race conditions already known to affect packed
references [1]:
* Process 1 tries to peel packed reference X as part of deleting
another packed reference. It discovers that X does not refer to a
valid object (because the object that it referred to has been
garbage collected).
* Process 2 tries to delete reference X. It starts by deleting the
loose reference X.
* Process 1 checks whether there is a loose reference X. There is not
(it has just been deleted by process 2), so process 1 reports a
spurious error "X does not point to a valid object!"
The worst case seems relatively harmless, and the fix is identical to
the fix that will be needed for the other race conditions (namely
holding a lock on the packed-refs file during *all* reference
deletions), so we leave the cleaning up of all of them as a future
project.
[1] http://thread.gmane.org/gmane.comp.version-control.git/211956
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Establish an internal API for iterating over references, which gives
the callback functions direct access to the ref_entry structure
describing the reference. (Do not change the iteration API that is
exposed outside of the module.)
Define a new internal callback signature
int each_ref_entry_fn(struct ref_entry *entry, void *cb_data)
Change do_for_each_ref_in_dir() and do_for_each_ref_in_dirs() to
accept each_ref_entry_fn callbacks, and rename them to
do_for_each_entry_in_dir() and do_for_each_entry_in_dirs(),
respectively. Adapt their callers accordingly.
Add a new function do_for_each_entry() analogous to do_for_each_ref()
but using the new callback style.
Change do_one_ref() into an each_ref_entry_fn that does some
bookkeeping and then calls a wrapped each_ref_fn.
Reimplement do_for_each_ref() in terms of do_for_each_entry(), using
do_one_ref() as an adapter.
Please note that the responsibility for setting current_ref remains in
do_one_ref(), which means that current_ref is *not* set when iterating
over references via the new internal API. This is not a disadvantage,
because current_ref is not needed by callers of the internal API (they
receive a pointer to the current ref_entry anyway). But more
importantly, this change prevents peel_ref() from returning invalid
results in the following scenario:
When iterating via the external API, the iteration always includes
both packed and loose references, and in particular never presents a
packed ref if there is a loose ref with the same name. The internal
API, on the other hand, gives the option to iterate over only the
packed references. During such an iteration, there is no check
whether the packed ref might be hidden by a loose ref of the same
name. But until now the packed ref was recorded in current_ref during
the iteration. So if peel_ref() were called with the reference name
corresponding to current ref, it would return the peeled version of
the packed ref even though there might be a loose ref that peels to a
different value. This scenario doesn't currently occur in the code,
but fix it to prevent things from breaking in a very confusing way in
the future.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Peel the entry, and as a side effect store the peeled value in the
entry. Use this function from two places in peel_ref(); a third
caller will be added soon.
Please note that this change can lead to ref_entries for unpacked refs
being peeled. This has no practical benefit but is harmless.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The old version was inconsistent: when a reference was
REF_KNOWS_PEELED but with a null peeled value, it returned non-zero
for the current reference but zero for other references. Change the
behavior for non-current references to match that of current_ref,
which is what callers expect. Document the behavior.
Current callers only call peel_ref() from within a for_each_ref-style
iteration and only for the current ref; therefore, the buggy code path
was never reached.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of just returning a success/failure bit, return an enumeration
value that explains the reason for any failure. This will come in
handy shortly.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is a nice, logical unit of work, and putting it in a function
removes the need to use a goto in peel_ref(). Soon it will also have
other uses.
The algorithm is unchanged.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is a nice unit of work and soon will be needed from multiple
locations.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of copying the reference's SHA1 into a caller-supplied
variable, just return the ref_entry itself (or NULL if there is no
such entry). This change will allow the function to be used from
elsewhere.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no way to drop out of the while loop. This code has been
dead since 432ad41e.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Document the bits that can appear in the "flags" parameter passed to
an each_ref_function and/or in the ref_entry::flag field.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An internal function used to implement "git checkout @{-1}" was
hard to use correctly.
* jc/reflog-reverse-walk:
refs.c: fix fread error handling
reflog: add for_each_reflog_ent_reverse() API
for_each_recent_reflog_ent(): simplify opening of a reflog file
for_each_reflog_ent(): extract a helper to process a single entry
Not that we do not actively encourage having annotated tags outside
refs/tags/ hierarchy, but they were not advertised correctly to the
ls-remote and fetch with recent version of Git.
* jk/fully-peeled-packed-ref:
pack-refs: add fully-peeled trait
pack-refs: write peeled entry for non-tags
use parse_object_or_die instead of die("bad object")
avoid segfaults on parse_object failure
fread returns the number of items read, with no special error return.
Commit 98f85ff (reflog: add for_each_reflog_ent_reverse() API -
2013-03-08) introduced a call to fread which checks for an error with
"nread < 0" which is tautological since nread is unsigned. The correct
check in this case (which tries to read a single item) is "nread != 1".
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Older versions of pack-refs did not write peel lines for
refs outside of refs/tags. This meant that on reading the
pack-refs file, we might set the REF_KNOWS_PEELED flag for
such a ref, even though we do not know anything about its
peeled value.
The previous commit updated the writer to always peel, no
matter what the ref is. That means that packed-refs files
written by newer versions of git are fine to be read by both
old and new versions of git. However, we still have the
problem of reading packed-refs files written by older
versions of git, or by other implementations which have not
yet learned the same trick.
The simplest fix would be to always unset the
REF_KNOWS_PEELED flag for refs outside of refs/tags that do
not have a peel line (if it has a peel line, we know it is
valid, but we cannot assume a missing peel line means
anything). But that loses an important optimization, as
upload-pack should not need to load the object pointed to by
refs/heads/foo to determine that it is not a tag.
Instead, we add a "fully-peeled" trait to the packed-refs
file. If it is set, we know that we can trust a missing peel
line to mean that a ref cannot be peeled. Otherwise, we fall
back to assuming nothing.
[commit message and tests by Jeff King <peff@peff.net>]
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git checkout -" is a short-hand for "git checkout @{-1}" and the
"@{nth}" notation for a negative number is to find nth previous
checkout in the reflog of the HEAD to determine the name of the
branch the user was on. We would want to find the nth most recent
reflog entry that matches "checkout: moving from X to Y" for this.
Unfortunately, reflog is implemented as an append-only file, and the
API to iterate over its entries, for_each_reflog_ent(), reads the
file in order, giving the entries from the oldest to newer. For the
purpose of finding nth most recent one, this API forces us to record
the last n entries in a rotating buffer and give the result out only
after we read everything. To optimize for a common case of finding
the nth most recent one for a small value of n, we also have a side
API for_each_recent_reflog_ent() that starts reading near the end of
the file, but it still has to read the entries in the "wrong" order.
The implementation of understanding @{-1} uses this interface.
This all becomes unnecessary if we add an API to let us iterate over
reflog entries in the reverse order, from the newest to older.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Split the logic that takes a single line of reflog entry in a
strbuf, parses the message, and calls the callback function out of
the loop into a separate helper function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow the server side to redact the refs/ namespace it shows to the
client.
Will merge to 'master'.
* jc/hidden-refs:
upload/receive-pack: allow hiding ref hierarchies
upload-pack: simplify request validation
upload-pack: share more code
A repository may have refs that are only used for its internal
bookkeeping purposes that should not be exposed to the others that
come over the network.
Teach upload-pack to omit some refs from its initial advertisement
by paying attention to the uploadpack.hiderefs multi-valued
configuration variable. Do the same to receive-pack via the
receive.hiderefs variable. As a convenient short-hand, allow using
transfer.hiderefs to set the value to both of these variables.
Any ref that is under the hierarchies listed on the value of these
variable is excluded from responses to requests made by "ls-remote",
"fetch", etc. (for upload-pack) and "push" (for receive-pack).
Because these hidden refs do not count as OUR_REF, an attempt to
fetch objects at the tip of them will be rejected, and because these
refs do not get advertised, "git push :" will not see local branches
that have the same name as them as "matching" ones to be sent.
An attempt to update/delete these hidden refs with an explicit
refspec, e.g. "git push origin :refs/hidden/22", is rejected. This
is not a new restriction. To the pusher, it would appear that there
is no such ref, so its push request will conclude with "Now that I
sent you all the data, it is time for you to update the refs. I saw
that the ref did not exist when I started pushing, and I want the
result to point at this commit". The receiving end will apply the
compare-and-swap rule to this request and rejects the push with
"Well, your update request conflicts with somebody else; I see there
is such a ref.", which is the right thing to do. Otherwise a push to
a hidden ref will always be "the last one wins", which is not a good
default.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Simplify ref_entry_cmp_sslice() by using strncmp() to compare the
length-limited key and a NUL-terminated entry. While we're at it,
retain the const attribute of the input pointers.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git pack-refs" that ran in parallel to another process that created
new refs had a nasty race.
* jk/repack-ref-racefix:
refs: do not use cached refs in repack_without_ref
When we delete a ref that is packed, we rewrite the whole
packed-refs file and simply omit the ref that no longer
exists. However, we base the rewrite on whatever happens to
be in our refs cache, not what is necessarily on disk. That
opens us up to a race condition if another process is
simultaneously packing the refs, as we will overwrite their
newly-made pack-refs file with our potentially stale data,
losing commits.
You can demonstrate the race like this:
# setup some repositories
git init --bare parent &&
(cd parent && git config core.logallrefupdates true) &&
git clone parent child &&
(cd child && git commit --allow-empty -m base)
# in one terminal, repack the refs repeatedly
cd parent &&
while true; do
git pack-refs --all
done
# in another terminal, simultaneously push updates to
# master, and create and delete an unrelated ref
cd child &&
while true; do
git push origin HEAD:newbranch &&
git commit --allow-empty -m foo
us=`git rev-parse master` &&
git push origin master &&
git push origin :newbranch &&
them=`git --git-dir=../parent rev-parse master` &&
if test "$them" != "$us"; then
echo >&2 "$them" != "$us"
exit 1
fi
done
In many cases the two processes will conflict over locking
the packed-refs file, and the deletion of newbranch will
simply fail. But eventually you will hit the race, which
happens like this:
1. We push a new commit to master. It is already packed
(from the looping pack-refs call). We write the new
value (let us call it B) to $GIT_DIR/refs/heads/master,
but the old value (call it A) remains in the
packed-refs file.
2. We push the deletion of newbranch, spawning a
receive-pack process. Receive-pack advertises all refs
to the client, causing it to iterate over each ref; it
caches the packed refs in memory, which points at the
stale value A.
3. Meanwhile, a separate pack-refs process is running. It
runs to completion, updating the packed-refs file to
point master at B, and deleting $GIT_DIR/refs/heads/master
which also pointed at B.
4. Back in the receive-pack process, we get the
instruction to delete :newbranch. We take a lock on
packed-refs (which works, as the other pack-refs
process has already finished). We then rewrite the
contents using the cached refs, which contain the stale
value A.
The resulting packed-refs file points master once again at
A. The loose ref which would override it to point at B was
deleted (rightfully) in step 3. As a result, master now
points at A. The only trace that B ever existed in the
parent is in the reflog: the final entry will show master
moving from A to B, even though the ref still points at A
(so you can detect this race after the fact, because the
next reflog entry will move from A to C).
We can fix this by invalidating the packed-refs cache after
we have taken the lock. This means that we will re-read the
packed-refs file, and since we have the lock, we will be
sure that what we read will be atomically up-to-date when we
write (it may be out of date with respect to loose refs, but
that is OK, as loose refs take precedence).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"update-ref -d --deref SYM" to delete a ref through a symbolic ref
that points to it did not remove it correctly.
* jh/update-ref-d-through-symref:
Fix failure to delete a packed ref through a symref
t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
When "update-ref -d --no-deref SYM" tried to delete a symbolic ref
SYM, it incorrectly locked the underlying reference pointed by SYM,
not the symbolic ref itself.
* rs/lock-correct-ref-during-delete:
refs: lock symref that is to be deleted, not its target
When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD'
to delete refs/heads/master), we would remove the loose ref, but a packed
version of the same ref would remain, the end result being that instead of
deleting refs/heads/master we would appear to reset it to its state as of
the last repack.
This patch fixes the issue, by making sure we pass the correct ref name
when invoking repack_without_ref() from within delete_ref().
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When delete_ref is called on a symref then it locks its target and then
either deletes the target or the symref, depending on whether the flag
REF_NODEREF was set in the parameter delopt.
Instead, simply pass the flag to lock_ref_sha1_basic, which will then
either lock the target or the symref, and delete the locked ref.
This reimplements part of eca35a25 (Fix git branch -m for symrefs.).
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of peel_ref is to dereference tags; if the base
object is not a tag, then we can return early without even
loading the object into memory.
This patch accomplishes that by checking sha1_object_info
for the type. For a packed object, we can get away with just
looking in the pack index. For a loose object, we only need
to inflate the first couple of header bytes.
This is a bit of a gamble; if we do find a tag object, then
we will end up loading the content anyway, and the extra
lookup will have been wasteful. However, if it is not a tag
object, then we save loading the object entirely. Depending
on the ratio of non-tags to tags in the input, this can be a
minor win or minor loss.
However, it does give us one potential major win: if a ref
points to a large blob (e.g., via an unannotated tag), then
we can avoid looking at it entirely.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The idea of the peel_ref function is to dereference tag
objects recursively until we hit a non-tag, and return the
sha1. Conceptually, it should return 0 if it is successful
(and fill in the sha1), or -1 if there was nothing to peel.
However, the current behavior is much more confusing. For a
regular loose ref, the behavior is as described above. But
there is an optimization to reuse the peeled-ref value for a
ref that came from a packed-refs file. If we have such a
ref, we return its peeled value, even if that peeled value
is null (indicating that we know the ref definitely does
_not_ peel).
It might seem like such information is useful to the caller,
who would then know not to bother loading and trying to peel
the object. Except that they should not bother loading and
trying to peel the object _anyway_, because that fallback is
already handled by peel_ref. In other words, the whole point
of calling this function is that it handles those details
internally, and you either get a sha1, or you know that it
is not peel-able.
This patch catches the null sha1 case internally and
converts it into a -1 return value (i.e., there is nothing
to peel). This simplifies callers, which do not need to
bother checking themselves.
Two callers are worth noting:
- in pack-objects, a comment indicates that there is a
difference between non-peelable tags and unannotated
tags. But that is not the case (before or after this
patch). Whether you get a null sha1 has to do with
internal details of how peel_ref operated.
- in show-ref, if peel_ref returns a failure, the caller
tries to decide whether to try peeling manually based on
whether the REF_ISPACKED flag is set. But this doesn't
make any sense. If the flag is set, that does not
necessarily mean the ref came from a packed-refs file
with the "peeled" extension. But it doesn't matter,
because even if it didn't, there's no point in trying to
peel it ourselves, as peel_ref would already have done
so. In other words, the fallback peeling is guaranteed
to fail.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we are asked to peel a ref to a sha1, we internally call
deref_tag, which will recursively parse each tagged object
until we reach a non-tag. This has the benefit that we will
verify our ability to load and parse the pointed-to object.
However, there is a performance downside: we may not need to
load that object at all (e.g., if we are listing peeled
simply listing peeled refs), or it may be a large object
that should follow a streaming code path (e.g., an annotated
tag of a large blob).
It makes more sense for peel_ref to choose the fast thing
rather than performing the extra check, for two reasons:
1. We will already sometimes short-circuit the tag parsing
in favor of a peeled entry from a packed-refs file. So
we are already favoring speed in some cases, and it is
not wise for a caller to rely on peel_ref to detect
corruption.
2. We already silently ignore much larger corruptions,
like a ref that points to a non-existent object, or a
tag object that exists but is corrupted.
2. peel_ref is not the right place to check for such a
database corruption. It is returning only the sha1
anyway, not the actual object. Any callers which use
that sha1 to load an object will soon discover the
corruption anyway, so we are really just pushing back
the discovery to later in the program.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Avoid unnecessary temporary allocations while looking for matching refs
inside refs API.
By René Scharfe (3) and Junio C Hamano (1)
* rs/refs-string-slice:
refs: do not create ref_entry when searching
refs: use strings directly in find_containing_dir()
refs: convert parameter of create_dir_entry() to length-limited string
refs: convert parameter of search_ref_dir() to length-limited string
The code to lazily read loose refs unnecessarily read the refs in a
subhierarchy by mistake when we free the data for the subhierarchy.
By Michael Haggerty
* mh/ref-api-lazy-loose:
free_ref_entry(): do not trigger reading of loose refs
The old code allowed many references to be efficiently added to a
single directory, because it just appended the references to the
containing directory unsorted without doing any searching (and
therefore without requiring any intermediate sorting). But the old
code was inefficient when a large number of subdirectories were added
to a directory, because the directory always had to be searched to see
if the new subdirectory already existed, and this search required the
directory to be sorted first. The same was repeated for every new
subdirectory, so the time scaled like O(N^2), where N is the number of
subdirectories within a single directory.
In practice, references are often added to the ref_cache in
lexicographic order, for example when reading the packed-refs file.
So build some intelligence into add_entry_to_dir() to optimize for the
case of references and/or subdirectories being added in lexicographic
order: if the existing entries were already sorted, and the new entry
comes after the last existing entry, then adjust ref_dir::sorted to
reflect the fact that the ref_dir is still sorted.
Thanks to Peff for pointing out the performance regression that
inspired this change.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The search_ref_dir() function is about looking up an existing ref_entry in
a sorted array of ref_entry stored in dir->entries, but it still allocates
a new ref_entry and frees it before returning. This is only because the
call to bsearch(3) was coded in a suboptimal way. Unlike the comparison
function given to qsort(3), the first parameter to its comparison function
does not need to point at an object that is shaped like an element in the
array.
Introduce a new comparison function that takes a counted string as the key
and an element in an array of ref_entry and give it to bsearch(), so that
we do not have to allocate a new ref_entry that we will never return to
the caller anyway.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the parameter subdirname of search_for_subdir() to a
length-limted string and then simply pass the interesting slice of the
refname from find_containing_dir(), thereby avoiding to duplicate the
string.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Do not call get_ref_dir() from within free_ref_entry(), because that
triggers the reading of loose refs, only for them to be freed
immediately.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refs API is updated to lazily read sub-hierarchies of refs/ namespace,
so that we do not have to grab everything from the filesystem when we
are only interested in listing branches, for example.
By Michael Haggerty (17) and Junio C Hamano (1)
* mh/ref-api-lazy-loose:
refs: fix find_containing_dir() regression
refs: read loose references lazily
read_loose_refs(): eliminate ref_cache argument
struct ref_dir: store a reference to the enclosing ref_cache
search_for_subdir(): return (ref_dir *) instead of (ref_entry *)
get_ref_dir(): add function for getting a ref_dir from a ref_entry
read_loose_refs(): rename function from get_ref_dir()
refs: wrap top-level ref_dirs in ref_entries
find_containing_dir(): use strbuf in implementation of this function
bisect: copy filename string obtained from git_path()
do_for_each_reflog(): use a strbuf to hold logfile name
do_for_each_reflog(): return early on error
get_ref_dir(): take the containing directory as argument
refs.c: extract function search_for_subdir()
get_ref_dir(): require that the dirname argument ends in '/'
get_ref_dir(): rename "base" parameter to "dirname"
get_ref_dir(): use a strbuf to hold refname
get_ref_dir(): return early if directory cannot be read
The function used to return NULL when asked to find the containing
directory for a ref that does not exist, allowing the caller to
omit iteration altogether. But a misconversion in an earlier change
"refs.c: extract function search_for_subdir()" started returning the
top-level directory entry, forcing callers to walk everything.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of reading the whole directory of loose references the first
time any are needed, only read them on demand, one directory at a
time.
Use a new ref_entry flag bit REF_INCOMPLETE to indicate that the entry
represents a REF_DIR that hasn't been read yet. Whenever any entries
from such a directory are needed, read all of the loose references
from that directory.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ref_cache can now be read from the ref_dir.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This means that a directory ref_entry contains all of the information
needed by read_loose_refs().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
That is what all the callers want, so give it to them.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert all accesses of a ref_dir within a ref_entry to use this
function. This function will later be responsible for reading loose
references from disk on demand.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The new name better describes the function's purpose, and also makes
the old name available for a more suitable purpose.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make it turtles all the way down. This affects the loose and packed
fields of ref_cache instances.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This simplifies the bookkeeping and allows an (artificial) restriction
on refname component length to be removed.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, the "dir" argument to get_ref_dir() was a pointer to the
top-level ref_dir. Change the function to expect a pointer to the
ref_dir corresponding to dirname. This allows entries to be added
directly to dir, without having to recurse through the reference trie
each time (i.e., we can use add_entry_to_dir() instead of add_ref()).
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This removes some conditional code and makes it consistent with the
way that direntry names are stored. Please note that this function is
never used on the top-level .git directory; it is always called for
directories at level .git/refs or deeper.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This simplifies the bookkeeping and allows an (artificial) restriction
on refname component length to be removed.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
More message strings marked for i18n.
By Nguyễn Thái Ngọc Duy (10) and Jonathan Nieder (1)
* nd/i18n:
help: replace underlining "help -a" headers using hyphens with a blank line
i18n: bundle: mark strings for translation
i18n: index-pack: mark strings for translation
i18n: apply: update say_patch_name to give translators complete sentence
i18n: apply: mark strings for translation
i18n: remote: mark strings for translation
i18n: make warn_dangling_symref() automatically append \n
i18n: help: mark strings for translation
i18n: mark relative dates for translation
strbuf: convenience format functions with \n automatically appended
Makefile: feed all header files to xgettext
This helps remove \n from translatable strings
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the base argument has a "/" chararacter, then only iterate over the
reference subdir whose name is the part up to the last "/".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Store references hierarchically in a tree that matches the
pseudo-directory structure of the reference names. Add a new kind of
ref_entry (with flag REF_DIR) to represent a whole subdirectory of
references. Sort ref_dirs one subdirectory at a time.
NOTE: the dirs can now be sorted as a side-effect of other function
calls. Therefore, it would be problematic to do something from a
each_ref_fn callback that could provoke the sorting of a directory
that is currently being iterated over (i.e., the directory containing
the entry that is being processed or any of its parents).
This is a bit far-fetched, because a directory is always sorted just
before being iterated over. Therefore, read-only accesses cannot
trigger the sorting of a directory whose iteration has already
started. But if a callback function would add a reference to a parent
directory of the reference in the iteration, then try to resolve a
reference under that directory, a re-sort could be triggered and cause
the iteration to work incorrectly.
Nevertheless...add a comment in refs.h warning against modifications
during iteration.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the more usual indexing idiom for clarity.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This purely textual change is in preparation for storing references
hierarchically, when the old ref_array structure will represent one
"directory" of references. Rename functions that deal with this
structure analogously, and also rename the structure's "refs" member
to "entries".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This change is obviously silly by itself, but it is a step towards
adding a second member to the union.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Return 0 (instead of -1) for zero-length components. Move the
interpretation of zero-length components as illegal to
check_refname_format().
This will make it easier to extend check_refname_format() to also
check whether directory names are valid.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a function free_ref_entry(). This function will become nontrivial
when ref_entry (soon) becomes polymorphic.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Save a bunch of lines of code and a couple of strlen() calls.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It costs a bit of boilerplate, but it means that the function can be
ignorant of how cached refs are stored.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract function do_for_each_ref_in_arrays() from do_for_each_ref().
The new function will be a useful building block for storing refs
hierarchically.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract function do_for_each_ref_in_array() from do_for_each_ref().
The new function will be a useful building block for storing refs
hierarchically.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Set and clear current_ref within do_one_ref() instead of setting it
here and leaving it to somebody else to clear it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reorder definitions in file: first check_refname_format() and helper
functions, then the functions for managing the ref_entry and ref_array
data structures, then ref_cache, then the more "business-logicky"
stuff. No code is changed.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The extra_refs provided a kludgy way to create fake references at a
global level in the hope that they would only affect some particular
code path. The last user of this API been rewritten, so strip this
stuff out before somebody else gets the bad idea of using it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a new function add_packed_ref() that adds a reference directly to
the in-memory packed reference cache. This will be useful for
creating local references while cloning.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Keep track of how many entries at the beginning of a ref_array are already
sorted. In sort_ref_array(), return early if the the array is already
sorted (i.e., if no new references has been appended to the end of the
list since the last call to sort_ref_array()).
Sort ref_arrays only when needed, namely in search_ref_array() and in
do_for_each_ref(). However, never call sort_ref_array() on the
extra_refs, because extra_refs can contain multiple entries with the same
name and because sort_ref_array() not only sorts, but de-dups its
contents.
This change is currently not useful, because entries are not added to
ref_arrays after they are created. But in a moment they will be...
Implementation note: we could store a binary "sorted" value instead of
an integer, but storing the number of sorted entries leaves the way
open for a couple of possible future optimizations:
* In sort_ref_array(), sort *only* the unsorted entries, then merge
them with the sorted entries. This should be faster if most of the
entries are already sorted.
* Teach search_ref_array() to do a binary search of any sorted
entries, and if unsuccessful do a linear search of any unsorted
entries. This would avoid the need to sort the list every time that
search_ref_array() is called, and (given some intelligence about how
often to sort) could significantly improve the speed in certain
hypothetical usage patterns.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* mh/ref-api:
add_ref(): take a (struct ref_entry *) parameter
create_ref_entry(): extract function from add_ref()
repack_without_ref(): remove temporary
resolve_gitlink_ref_recursive(): change to work with struct ref_cache
Pass a (ref_cache *) to the resolve_gitlink_*() helper functions
resolve_gitlink_ref(): improve docstring
get_ref_dir(): change signature
refs: change signatures of get_packed_refs() and get_loose_refs()
is_dup_ref(): extract function from sort_ref_array()
add_ref(): add docstring
parse_ref_line(): add docstring
is_refname_available(): remove the "quiet" argument
clear_ref_array(): rename from free_ref_array()
refs: rename parameters result -> sha1
refs: rename "refname" variables
struct ref_entry: document name member
Conflicts:
cache.h
refs.c
resolve_ref() may return a pointer to a shared buffer and can be
overwritten by the next resolve_ref() calls. Callers need to
pay attention, not to keep the pointer when the next call happens.
Rename with "_unsafe" suffix to warn developers (or reviewers) before
introducing new call sites.
This patch is generated using the following command
git grep -l 'resolve_ref(' -- '*.[ch]'|xargs sed -i 's/resolve_ref(/resolve_ref_unsafe(/g'
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Take a pointer to the ref_entry to add to the array, rather than
creating the ref_entry within the function. This opens the way to
having multiple kinds of ref_entries.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Separate the creation of the ref_entry from its addition to a ref_array.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
resolve_gitlink_ref() and resolve_gitlink_ref_recursive(), together,
basically duplicated the code in git_path_submodule(). So use that
function instead.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
And remove some redundant arguments from resolve_gitlink_packed_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change get_ref_dir() to take a (struct ref_cache *) in place of the
submodule name.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change get_packed_refs() and get_loose_refs() to take a (struct
ref_cache *) instead of the name of the submodule.
Change get_ref_dir() to take a submodule name (i.e., "" for the main
module) rather than a submodule pointer (i.e., NULL for the main
module) so that refs->name can be used as its argument. (In a moment
this function will also be changed to take a (struct ref_cache *),
too.)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Giving the function a name makes the code easier to understand.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
quiet was always set to 0, so get rid of it. Add a function docstring
for good measure.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rename the function since it doesn't actually free the array object
that is passed to it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Try consistently to use the name "sha1" for parameters to which a SHA1
will be stored.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Try to consistently use the variable name "refname" when referring to
a string that names a reference.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* nd/resolve-ref:
Copy resolve_ref() return value for longer use
Convert many resolve_ref() calls to read_ref*() and ref_exists()
Conflicts:
builtin/fmt-merge-msg.c
builtin/merge.c
refs.c
* jc/pull-signed-tag:
commit-tree: teach -m/-F options to read logs from elsewhere
commit-tree: update the command line parsing
commit: teach --amend to carry forward extra headers
merge: force edit and no-ff mode when merging a tag object
commit: copy merged signed tags to headers of merge commit
merge: record tag objects without peeling in MERGE_HEAD
merge: make usage of commit->util more extensible
fmt-merge-msg: Add contents of merged tag in the merge message
fmt-merge-msg: package options into a structure
fmt-merge-msg: avoid early returns
refs DWIMmery: use the same rule for both "git fetch" and others
fetch: allow "git fetch $there v1.0" to fetch a tag
merge: notice local merging of tags and keep it unwrapped
fetch: do not store peeled tag object names in FETCH_HEAD
Split GPG interface into its own helper library
Conflicts:
builtin/fmt-merge-msg.c
builtin/merge.c
The add_extra_ref() interface is used to add an extra-ref that is _not_
our ref for the purpose of helping auto-following of tags and reducing
object transfer from remote repository, and they are typically formatted
as a tagname followed by ^{} to make sure no valid refs match that
pattern. In other words, these entries are deliberately formatted not to
pass check-refname-format test.
A recent series however added a test unconditionally to the add_ref()
function that is called from add_extra_ref(). The check may be sensible
for other two callsites of the add_ref() interface, but definitely is
a wrong thing to do in add_extra_ref(). Disable it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
resolve_ref() may return a pointer to a static buffer, which is not
safe for long-term use because if another resolve_ref() call happens,
the buffer may be changed. Many call sites though do not care about
this buffer. They simply check if the return value is NULL or not.
Convert all these call sites to new wrappers to reduce resolve_ref()
calls from 57 to 34. If we change resolve_ref() prototype later on
to avoid passing static buffer out, this helps reduce changes.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git log frotz" can DWIM to "refs/remotes/frotz/HEAD", but in the remote
access context, "git fetch frotz" to fetch what the other side happened to
have fetched from what it calls 'frotz' (which may not have any relation
to what we consider is 'frotz') the last time would not make much sense,
so the fetch rules table did not include "refs/remotes/%.*s/HEAD".
When the user really wants to, "git fetch $there remotes/frotz/HEAD" would
let her do so anyway, so this is not about safety or security; it merely
is about confusion avoidance and discouraging meaningless usage.
Specifically, it is _not_ about ambiguity avoidance. A name that would
become ambiguous if we use the same rules table for both fetch and local
rev-parse would be ambiguous locally at the remote side.
So for the same reason as we added rule to allow "git fetch $there v1.0"
instead of "git fetch $there tags/v1.0" in the previous commit, here is a
bit longer rope for the users, which incidentally simplifies our code.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
You can already do so with "git fetch $there tags/v1.0" but if it is not
ambiguous there is no reason to force users to type more.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jc/broken-ref-dwim-fix:
resolve_ref(): report breakage to the caller without warning
resolve_ref(): expose REF_ISBROKEN flag
refs.c: move dwim_ref()/dwim_log() from sha1_name.c
* mh/ref-api:
clear_ref_cache(): inline function
write_ref_sha1(): only invalidate the loose ref cache
clear_ref_cache(): extract two new functions
clear_ref_cache(): rename parameter
invalidate_ref_cache(): expose this function in the refs API
invalidate_ref_cache(): take the submodule as parameter
invalidate_ref_cache(): rename function from invalidate_cached_refs()
629cd3a (resolve_ref(): emit warnings for improperly-formatted references,
2011-09-15) made resolve_ref() warn against files that are found in the
directories the ref dwimmery looks at. The intent may be good, but these
messages come from a wrong level of the API hierarchy.
Instead record the breakage in "flags" whose purpose is to explain the
result of the function to the caller, who is in a much better position to
make intelligent decision based on the information.
This updates sha1_name.c::dwim_ref() to warn against such a broken
candidate only when it does not appear directly below $GIT_DIR to restore
the traditional behaviour, as we know many files directly underneath
$GIT_DIR/ are not refs.
Warning against "git show config --" with "$GIT_DIR/config does not look
like a well-formed ref" does not make sense, and we may later tweak the
dwimmery not to even consider them as candidates, but that is a longer
term topic.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of keeping this as an internal API, let the callers find
out the reason why resolve_ref() returned NULL is not because there
was no such file in $GIT_DIR but because a file was corrupt.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2c5c66b (Merge branch 'jp/get-ref-dir-unsorted', 2011-10-10) merged a
topic that forked from the mainline before a new helper function
get_packed_refs() refactored code to read packed-refs file. The merge made
the call to the helper function with an incorrect argument. The parameter
to the function has to be a path to the submodule.
Fix the mismerge.
Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
clear_ref_cache() was only called from one place, so inline it
there.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since write_ref_sha1() can only write loose refs and cannot write
symbolic refs, there is no need for it to invalidate the packed ref
cache.
Suggested by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract two new functions from clear_cached_refs():
clear_loose_ref_cache() and clear_packed_ref_cache().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
...for consistency with the rest of this module.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make invalidate_ref_cache() an official part of the refs API. It is
currently a fact of life that code outside of refs.c mucks about with
references. This change gives such code a way of informing the refs
module that it should no longer trust its cache.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of invalidating the ref cache on an all-or-nothing basis,
invalidate the cache for a specific submodule.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is the cache that is being invalidated, not the references, and the
new name makes this unambiguous. Rename other items analogously:
* struct cached_refs -> struct ref_cache
* cached_refs (the variable) -> ref_cache
* clear_cached_refs() -> clear_ref_cache()
* create_cached_refs() -> create_ref_cache()
* get_cached_refs() -> get_ref_cache()
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Both dwim_ref()/dwim_log() functions are intimately related to the ref
parsing rules defined in refs.c and better fits there. Move them together
with substitute_branch_name(), a file scope static helper function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The commit message for c976d41 (git-branch: add options and tests for
branch renaming, 2006-11-28) mentions RENAME_REF but otherwise this is not
documented anywhere, and it does not appear in any of the tests.
Worse yet, the name of the actual file is "RENAMED-REF".
This was supposed to hold the commit object name at the tip of the branch
the most recent "branch -m/-M" renamed, but that is not necessary in order
to be able to recover from a mistake. Even when "branch -M A B" overwrites
an existing branch B, what is kept in RENAMED-REF is the commit at the tip
of the original branch A, not the commit B from the now-lost branch.
Just remove this unused "feature".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jp/get-ref-dir-unsorted:
refs.c: free duplicate entries in the ref array instead of leaking them
refs.c: abort ref search if ref array is empty
refs.c: ensure struct whose member may be passed to realloc is initialized
refs: Use binary search to lookup refs faster
Don't sort ref_list too early
Conflicts:
refs.c
* mh/check-ref-format-3: (23 commits)
add_ref(): verify that the refname is formatted correctly
resolve_ref(): expand documentation
resolve_ref(): also treat a too-long SHA1 as invalid
resolve_ref(): emit warnings for improperly-formatted references
resolve_ref(): verify that the input refname has the right format
remote: avoid passing NULL to read_ref()
remote: use xstrdup() instead of strdup()
resolve_ref(): do not follow incorrectly-formatted symbolic refs
resolve_ref(): extract a function get_packed_ref()
resolve_ref(): turn buffer into a proper string as soon as possible
resolve_ref(): only follow a symlink that contains a valid, normalized refname
resolve_ref(): use prefixcmp()
resolve_ref(): explicitly fail if a symlink is not readable
Change check_refname_format() to reject unnormalized refnames
Inline function refname_format_print()
Make collapse_slashes() allocate memory for its result
Do not allow ".lock" at the end of any refname component
Refactor check_refname_format()
Change check_ref_format() to take a flags argument
Change bad_ref_char() to return a boolean value
...
* mh/iterate-refs:
refs.c: make create_cached_refs() static
Retain caches of submodule refs
Store the submodule name in struct cached_refs
Allocate cached_refs objects dynamically
Change the signature of read_packed_refs()
Access reference caches only through new function get_cached_refs()
Extract a function clear_cached_refs()
The bsearch() implementation on IRIX 6.5 segfaults if it is passed NULL
for the base array argument even if number-of-elements is zero. So, let's
work around it by detecting an empty array and aborting early.
This is a useful optimization in its own right anyway, since we avoid a
useless allocation and initialization of the ref_entry when the ref array
is empty.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The variable "refs" is allocated on the stack but is not initialized. It
is passed to read_packed_refs(), and its struct members may eventually be
passed to add_ref() and ALLOC_GROW(). Since the structure has not been
initialized, its members may contain random non-zero values. So let's
initialize it.
The call sequence looks something like this:
resolve_gitlink_packed_ref(...) {
struct cached_refs refs;
...
read_packed_refs(f, &refs);
...
}
read_packed_refs(FILE*, struct cached_refs *cached_refs) {
...
add_ref(name, sha1, flag, &cached_refs->packed, &last);
...
}
add_ref(..., struct ref_array *refs, struct ref_entry **) {
...
ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
}
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In add_ref(), verify that the refname is formatted correctly before
adding it to the ref_list. Here we have to allow refname components
that start with ".", since (for example) the remote protocol uses
synthetic reference name ".have". So add a new REFNAME_DOT_COMPONENT
flag that can be passed to check_refname_format() to allow leading
dots.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Record information about resolve_ref(), hard-won via reverse
engineering, in a comment for future spelunkers.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
