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>
This code is about to be moved, so name the function more
distinctively.
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 test that demonstrates that the peeled values recorded in
packed-refs are lost if a packed ref is deleted. (The code in
repack_without_ref() doesn't even attempt to write peeled refs.) This
will be fixed in a moment.
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>
A packed reference can be overridden by a loose reference, in which
case the packed reference is obsolete and is never used. The object
pointed to by such a reference can be garbage collected. Since
d66da478f2, this could lead to the emission of a spurious error
message:
error: refs/heads/master does not point to a valid object!
The error is generated by repack_without_ref() if there is an obsolete
dangling packed reference in packed-refs when the packed-refs file has
to be rewritten due to the deletion of another packed reference. Add
a failing test demonstrating this problem and some passing tests of
related scenarios.
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>
* rs/pp-user-info-without-extra-allocation:
pretty: remove intermediate strbufs from pp_user_info()
pretty: simplify output line length calculation in pp_user_info()
pretty: simplify input line length calculation in pp_user_info()
* tr/remote-tighten-commandline-parsing:
remote: 'show' and 'prune' can take more than one remote
remote: check for superfluous arguments in 'git remote add'
remote: add a test for extra arguments, according to docs
When using "git subtree push" to split out a subtree and push it to a
remote repository, we do not detect if the split command fails which
causes the LHS of the refspec to be empty, deleting the remote branch.
Fix this by pulling the result of the split command into a variable so
that we can die if the command fails.
Reported-by: Steffen Jaeckel <steffen.jaeckel@stzedn.de>
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bazaar doesn't seem to be tested for multiple usage of branches, so
resources seem to be leaked all over. Let's try to minimize this by
accessing the Branch objects only when needed.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This way we don't need to store the list of all the revisions, which
doesn't seem to be very memory efficient with bazaar's design, for
whatever reason.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
No need to manually count the revisions, and also, this would help to
iterate more properly.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We might not want all the branches. And branch handling in bazaar is
rather tricky, so it's safer to simply specify them.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The official method is incredibly inefficient and slow.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
So that we don't end up with '<None>', and also synchronize it with the
one from remote-hg.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This way all the remotes share the same data, so adding multiple
remotes, or renaming them doesn't create extra overhead.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When branches have '/' in their name (aka. sub-branches), bazaar seems
to choke while creating the new directory.
Also, git cannot have both 'foo' and 'foo/bar'.
So let's replace slashes with a plus sign.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In bazaar, a repository can contain multiple branches, and previously we
were supporting only one branch at a time. Now we fetch them all.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There should be no functional changes. Basically we want to reserve the
'repo' variable.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
