write_pack_data() passes bundle_fd to start_command() to be used as
the stdout of pack-objects. But start_command() closes its stdout if
it is > 1. This is a problem if bundle_fd is the fd of a lock_file,
because commit_lock_file() will also try to close the fd.
So the old code suppressed commit_lock_file()'s usual behavior of
closing the file descriptor by setting the lock_file object's fd field
to -1.
But this is not really kosher. Code here shouldn't be mutating fields
within the lock_file object.
Instead, duplicate the file descriptor before passing it to
write_pack_data(). Then that function can close its copy without
closing the copy held in the lock_file object.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There was one continue statement without an accompanying `free(ref)`.
Instead of adding that, replace all the free&&continue with a goto
just after writing the refs, where we'd do the free anyway and then
reloop.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bundle_create() function has a number of logical steps:
process the input, write the refs, and write the packfile.
Recent commits split the first and third into separate
sub-functions. It's worth splitting the middle step out,
too, if only because it makes the progression of the steps
more obvious.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The new helper compute_and_write_prerequistes() is ugly, but it
cannot be avoided. Ideally we should avoid a function that computes
and does I/O at the same time, but the prerequisites lines in the
output needs the human readable title only to help the recipient of
the bundle. The code copies them straight from the rev-list output
and immediately discards as no other internal computation needs that
information.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The create_bundle() function, while it does one single logical
thing, takes a rather large implementation to do so.
Let's start separating what it does into smaller steps to make it
easier to see what is going on. This is a first step to separate
out the actual pack-data generation, after the earlier part of the
function figures out which part of the history to place in the
bundle.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call child_process_init() instead of zeroing the memory of variables of
type struct child_process by hand before use because the former is both
clearer and shorter.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
resolve_ref_unsafe takes a boolean argument for reading (a nonexistent ref
resolves successfully for writing but not for reading). Change this to be
a flags field instead, and pass the new constant RESOLVE_REF_READING when
we want this behaviour.
While at it, swap two of the arguments in the function to put output
arguments at the end. As a nice side effect, this ensures that we can
catch callers that were unaware of the new API so they can be audited.
Give the wrapper functions resolve_refdup and read_ref_full the same
treatment for consistency.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Free the buffer returned by read_sha1_file() even if no valid tagger
line is found.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the interface declaration for the functions in lockfile.c from
cache.h to a new file, lockfile.h. Add #includes where necessary (and
remove some redundant includes of cache.h by files that already
include builtin.h).
Move the documentation of the lock_file state diagram from lockfile.c
to the new header file.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git bundle create" with date-range specification were meant to
exclude tags outside the range
* lf/bundle-exclusion:
bundle: fix exclusion of annotated tags
Most struct child_process variables are cleared using memset first after
declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to
initialize them statically instead. That's shorter, doesn't require a
function call and is slightly more readable (especially given that we
already have STRBUF_INIT, ARGV_ARRAY_INIT etc.).
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In commit c9a42c4 (bundle: allow rev-list options to exclude annotated
tags, 2009-01-02), support for excluding annotated tags outside the
specified date range was added. However, the wrong order of parameters
was chosen when calling memchr().
Fix this by swapping the character to search for with the maximum length
parameter. Also cover this behavior with an additional test.
Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the existing argv_array member instead of providing our own. This
way the argv_array is cleared after use automatically for us; it was
leaking before.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Attempts to show where a single-strand-of-pearls break in "git log"
output.
* nd/log-show-linear-break:
log: add --show-linear-break to help see non-linear history
object.h: centralize object flag allocation
While the field "flags" is mainly used by the revision walker, it is
also used in many other places. Centralize the whole flag allocation to
one place for a better overview (and easier to move flags if we have
too).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We invented hashcpy() to keep the abstraction of "object name"
behind it. Use it instead of calling memcpy() with hard-coded
20-byte length when moving object names between pieces of memory.
Leave ppc/sha1.c as-is, because the function is about the SHA-1 hash
algorithm whose output is and will always be 20 bytes.
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Helped-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Sun He <sunheehnus@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of hand-crafted arrays to manage command line arguments
we create internally, use argv-array helpers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, the memory management of the object_array_entry::name
field was inconsistent and undocumented. object_array_entries are
ultimately created by a single function, add_object_array_with_mode(),
which has an argument "const char *name". This function used to
simply set the name field to reference the string pointed to by the
name parameter, and nobody on the object_array side ever freed the
memory. Thus, it assumed that the memory for the name field would be
managed by the caller, and that the lifetime of that string would be
at least as long as the lifetime of the object_array_entry. But
callers were inconsistent:
* Some passed pointers to constant strings or argv entries, which was
OK.
* Some passed pointers to newly-allocated memory, but didn't arrange
for the memory ever to be freed.
* Some passed the return value of sha1_to_hex(), which is a pointer to
a statically-allocated buffer that can be overwritten at any time.
* Some passed pointers to refnames that they received from a
for_each_ref()-type iteration, but the lifetimes of such refnames is
not guaranteed by the refs API.
Bring consistency to this mess by changing object_array to make its
own copy for the object_array_entry::name field and free this memory
when an object_array_entry is deleted from the array.
Many callers were passing the empty string as the name parameter, so
as a performance optimization, treat the empty string specially.
Instead of making a copy, store a pointer to a statically-allocated
empty string to object_array_entry::name. When deleting such an
entry, skip the free().
Change the callers that were already passing copies to
add_object_array_with_mode() to either skip the copy, or (if the
memory needed to be allocated anyway) freeing the memory itself.
A part of this commit effectively reverts
70d26c6e76 read_revisions_from_stdin: make copies for handle_revision_arg
because the copying introduced by that commit (which is still
necessary) is now done at a deeper level.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While explicitly stating that the commit message in a prerequisite
line is optional, we required all lines with 40 or more characters
to contain a space after the object name, bailing out if a line
consisted of an object name only. This was to allow bundling a
history to a commit without an message, but the code forgot that it
already called rtrim() to remove that whitespace.
As a workaround, only check for SP when the line has more than 40
characters.
Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many call-sites of parse_object assume that they will get a
non-NULL return value; this is not the case if we encounter
an error while parsing the object.
This patch adds a wrapper function around parse_object that
handles dying automatically, and uses it anywhere we
immediately try to access the return value as a non-NULL
pointer (i.e., anywhere that we would currently segfault).
This wrapper may also be useful in other places. The most
obvious one is code like:
o = parse_object(sha1);
if (!o)
die(...);
However, these should not be mechanically converted to
parse_object_or_die, as the die message is sometimes
customized. Later patches can address these sites on a
case-by-case basis.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These slightly improve the reading flow by making it obvious that a list
follows.
Also, make the wording of both headings consistent by changing "contains
%d ref(s)" to "contains this ref"/"contains these %d refs".
Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A more informative message for "complete" bundles was added in commit
8c3710fd30 (tweak "bundle verify" of a complete history, 2012-06-04).
However, the prerequisites ref list is currently read *after* we
check if it equals zero, which means we never actually use the
number of prerequisite refs to decide when to print the newly
introduced message. The code incorrectly uses the number of
references recorded in the bundle instead.
Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A bundle that records a complete history without prerequiste is a
useful way to sneakernet the sources of your configuration files
under your home directory, etc. E.g.
$ GIT_DIR=/srv/git/homesrc.git git bundle create x.bndl HEAD master
Running "git bundle verify" on such a "complete" bundle, however,
gives somewhat a funny output.
$ git bundle verify x.bndl
The bundle contains 2 refs
b2611f37ebc7ed6435a72d77fbc5f8b48a7d7146 HEAD
b2611f37ebc7ed6435a72d77fbc5f8b48a7d7146 refs/heads/master
The bundle requires these 0 refs
x.bndl is okay
Reword "requires these 0 refs" to say "The bundle records a complete
history" instead.
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
After running rev-list --boundary to retrieve the list of boundary
commits, "git bundle create" runs its own revision walk. If in this
stage git encounters an unfamiliar option, it writes a message with an
unbalanced quotation mark:
error: unrecognized argument: --foo'
Drop the stray quote to match the "unrecognized argument: %s" message
used elsewhere and save translators some work.
This is mostly a futureproofing measure: for now, the "rev-list
--boundary" command catches most strange arguments on its own and the
above message is not seen unless you try something esoteric like "git
bundle create test.bundle --header HEAD".
Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git bundle" did not record boundary commits correctly when there
are many of them.
By Thomas Rast
* tr/maint-bundle-boundary:
bundle: keep around names passed to add_pending_object()
t5510: ensure we stay in the toplevel test dir
t5510: refactor bundle->pack conversion
By Thomas Rast
* tr/maint-bundle-boundary:
bundle: keep around names passed to add_pending_object()
t5510: ensure we stay in the toplevel test dir
t5510: refactor bundle->pack conversion
* tr/maint-bundle-long-subject:
t5704: match tests to modern style
strbuf: improve strbuf_get*line documentation
bundle: use a strbuf to scan the log for boundary commits
bundle: put strbuf_readline_fd in strbuf.c with adjustments
The 'name' field passed to add_pending_object() is used to later
deduplicate in object_array_remove_duplicates().
git-bundle had a bug in this area since 18449ab (git-bundle: avoid
packing objects which are in the prerequisites, 2007-03-08): it passed
the name of each boundary object in a static buffer. In other words,
all that object_array_remove_duplicates() saw was the name of the
*last* added boundary object.
The recent switch to a strbuf in bc2fed4 (bundle: use a strbuf to scan
the log for boundary commits, 2012-02-22) made this slightly worse: we
now free the buffer at the end, so it is not even guaranteed that it
still points into addressable memory by the time object_array_remove_
duplicates looks at it. On the plus side however, it was now
detectable by valgrind.
The fix is easy: pass a copy of the string to add_pending_object.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* tr/maint-bundle-long-subject:
t5704: match tests to modern style
strbuf: improve strbuf_get*line documentation
bundle: use a strbuf to scan the log for boundary commits
bundle: put strbuf_readline_fd in strbuf.c with adjustments
The first part of the bundle header contains the boundary commits, and
could be approximated by
# v2 git bundle
$(git rev-list --pretty=oneline --boundary <ARGS> | grep ^-)
git-bundle actually spawns exactly this rev-list invocation, and does
the grepping internally.
There was a subtle bug in the latter step: it used fgets() with a
1024-byte buffer. If the user has sufficiently long subjects (e.g.,
by not adhering to the git oneline-subject convention in the first
place), the 'oneline' format can easily overflow the buffer. fgets()
then returns the rest of the line in the next call(s). If one of
these remaining parts started with '-', git-bundle would mistakenly
insert it into the bundle thinking it was a boundary commit.
Fix it by using strbuf_getwholeline() instead, which handles arbitrary
line lengths correctly.
Note that on the receiving side in parse_bundle_header() we were
already using strbuf_getwholeline_fd(), so that part is safe.
Reported-by: Jannis Pohlmann <jannis.pohlmann@codethink.co.uk>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The comment even said that it should eventually go there. While at
it, match the calling convention and name of the function to the
strbuf_get*line family. So it now is strbuf_getwholeline_fd.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint:
docs: describe behavior of relative submodule URLs
fix hang in git fetch if pointed at a 0 length bundle
Documentation: read-tree --prefix works with existing subtrees
Add MYMETA.json to perl/.gitignore
git-repo if interupted at the exact wrong time will generate zero
length bundles- literal empty files. git-repo is wrong here, but
git fetch shouldn't effectively spin loop if pointed at a zero
length bundle.
Signed-off-by: Brian Harring <ferringb@chromium.org>
Helped-by: Johannes Sixt
Helped-by: Nguyen Thai Ngoc Duy
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
* rs/pending:
commit: factor out clear_commit_marks_for_object_array
checkout: use leak_pending flag
bundle: use leak_pending flag
bisect: use leak_pending flag
revision: add leak_pending flag
checkout: use add_pending_{object,sha1} in orphan check
revision: factor out add_pending_sha1
checkout: check for "Previous HEAD" notice in t2020
Conflicts:
builtin/checkout.c
revision.c
Move most of the code from read_bundle_header() to parse_bundle_header()
that takes a file descriptor that is already opened for reading, and make
the former responsible only for opening the file and noticing errors.
As a logical consequence of this, is_bundle() helper function can be
implemented as a non-complaining variant of read_bundle_header() that
does not return an open file descriptor, and can be used to tighten
the check used to decide the use of bundle transport in transport_get()
function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We wished that "git bundle" to eventually learn to read from a network
socket which is not seekable. The current code opens with fopen(), reads
the file halfway and run ftell(), and reopens the same file with open()
and seeks, to skip the header.
This patch by itself does not reach that goal yet, but I think it is a
right step in that direction.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Factor out the code to clear the commit marks for a whole struct
object_array from builtin/checkout.c into its own exported function
clear_commit_marks_for_object_array and use it in bisect and bundle
as well. It handles tags and commits and ignores objects of any
other type.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of creating a copy of the list of pending objects, copy the
struct object_array that points to it, turn on leak_pending, and thus
cause prepare_revision_walk to leave it to us. And free it once
we're done.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
