use child_process_init() to initialize struct child_process variables

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>
cache-tree: avoid infinite loop on zero-entry tree

The loop in cache-tree's update_one iterates over all the
entries in the index. For each one, we find the cache-tree
subtree which represents our path (creating it if
necessary), and then recurse into update_one again. The
return value we get is the number of index entries that
belonged in that subtree. So for example, with entries:

    a/one
    a/two
    b/one

We start by processing the first entry, "a/one".  We would
find the subtree for "a" and recurse into update_one. That
would then handle "a/one" and "a/two", and return the value
2. The parent function then skips past the 2 handled
entries, and we continue by processing "b/one".

If the recursed-into update_one ever returns 0, then we make
no forward progress in our loop. We would process "a/one"
over and over, infinitely.

This should not happen normally. Any subtree we create must
have at least one path in it (the one that we are
processing!). However, we may also reuse a cache-tree entry
we found in the on-disk index. For the same reason, this
should also never have zero entries. However, certain buggy
versions of libgit2 could produce such bogus cache-tree
records. The libgit2 bug has since been fixed, but it does
not hurt to protect ourselves against bogus input coming
from the on-disk data structures.

Note that this is not a die("BUG") or assert, because it is
not an internal bug, but rather a corrupted on-disk
structure. It's possible that we could even recover from it
(by throwing out the bogus cache-tree entry), but it is not
worth the effort; the important thing is that we report an
error instead of looping infinitely.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
bundle: split out a helper function to create pack data

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>
bundle: split out a helper function to compute and write prerequisites

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>
bundle: split out ref writing from bundle_create

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>
fetch: load all default config at startup

When we start the git-fetch program, we call git_config to
load all config, but our callback only processes the
fetch.prune option; we do not chain to git_default_config at
all.

This means that we may not load some core configuration
which will have an effect. For instance, we do not load
core.logAllRefUpdates, which impacts whether or not we
create reflogs in a bare repository.

Note that I said "may" above. It gets even more exciting. If
we have to transfer actual objects as part of the fetch,
then we call fetch_pack as part of the same process. That
function loads its own config, which does chain to
git_default_config, impacting global variables which are
used by the rest of fetch. But if the fetch is a pure ref
update (e.g., a new ref which is a copy of an old one), we
skip fetch_pack entirely. So we get inconsistent results
depending on whether or not we have actual objects to
transfer or not!

Let's just load the core config at the start of fetch, so we
know we have it (we may also load it again as part of
fetch_pack, but that's OK; it's designed to be idempotent).

Our tests check both cases (with and without a pack). We
also check similar behavior for push for good measure, but
it already works as expected.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ignore stale directories when checking reflog existence

When we update a ref, we have two rules for whether or not
we actually update the reflog:

  1. If the reflog already exists, we will always append to
     it.

  2. If log_all_ref_updates is set, we will create a new
     reflog file if necessary.

We do the existence check by trying to open the reflog file,
either with or without O_CREAT (depending on log_all_ref_updates).
If it fails, then we check errno to see what happened.

If we were not using O_CREAT and we got ENOENT, the file
doesn't exist, and we return success (there isn't a reflog
already, and we were not told to make a new one).

If we get EISDIR, then there is likely a stale directory
that needs to be removed (e.g., there used to be "foo/bar",
it was deleted, and the directory "foo" was left. Now we
want to create the ref "foo"). If O_CREAT is set, then we
catch this case, try to remove the directory, and retry our
open. So far so good.

But if we get EISDIR and O_CREAT is not set, then we treat
this as any other error, which is not right. Like ENOENT,
EISDIR is an indication that we do not have a reflog, and we
should silently return success (we were not told to create
it). Instead, the current code reports this as an error, and
we fail to update the ref at all.

Note that this is relatively unlikely to happen, as you
would have to have had reflogs turned on, and then later
turned them off (it could also happen due to a bug in fetch,
but that was fixed in the previous commit). However, it's
quite easy to fix: we just need to treat EISDIR like ENOENT
for the non-O_CREAT case, and silently return (note that
this early return means we can also simplify the O_CREAT
case).

Our new tests cover both cases (O_CREAT and non-O_CREAT).
The first one already worked, of course.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
line-log: fix crash when --first-parent is used

line-log tries to access all parents of a commit, but only the first
parent has been loaded if "--first-parent" is specified, resulting
in a crash.

Limit the number of parents to one if "--first-parent" is specified.

Reported-by: Eric N. Vander Weele <ericvw@gmail.com>
Signed-off-by: Tzvetan Mikov <tmikov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
gitignore.txt: fix spelling of "backslash"

Signed-off-by: Ben North <ben@redfrontdoor.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
docs/credential-store: s/--store/--file/

The option name "--store" was used early in development, but
never even made it into an applied patch, let alone a
released version of git. I forgot to update the matching
documentation at the time, though.

Noticed-by: Jesse Hopkins <jesse.hopkins@lmco.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'jk/cache-tree-protect-from-broken-libgit2'

The code to use cache-tree trusted the on-disk data too much
and fell into an infinite loop.

* jk/cache-tree-protect-from-broken-libgit2:
  cache-tree: avoid infinite loop on zero-entry tree
Merge branch 'rs/use-child-process-init-more'

* rs/use-child-process-init-more:
  bundle: split out ref writing from bundle_create
  bundle: split out a helper function to compute and write prerequisites
  bundle: split out a helper function to create pack data
  use child_process_init() to initialize struct child_process variables
Merge branch 'jk/fetch-reflog-df-conflict'

Corner-case bugfixes for "git fetch" around reflog handling.

* jk/fetch-reflog-df-conflict:
  ignore stale directories when checking reflog existence
  fetch: load all default config at startup
Merge branch 'tm/line-log-first-parent'

"git log --first-parent -L..." used to crash.

* tm/line-log-first-parent:
  line-log: fix crash when --first-parent is used
Merge branch 'nd/gitignore-trailing-whitespace'

Documentation update.

* nd/gitignore-trailing-whitespace:
  gitignore.txt: fix spelling of "backslash"
Merge branch 'maint'

* maint:
  docs/credential-store: s/--store/--file/
make "struct counted_string" available everywhere

This is a handy convenience struct to pair a pointer and a
length. It can be used when you need to pass both through a
single void pointer. It's already defined and used in
remote.c, but we could make use of it elsewhere.
hashmap: split comparison into two functions

When a hashmap is initialized, the caller gives it a
comparison function to disambiguate entries with the same
hash. Ideally, the comparison function for a hash
containing "struct foo" would just take two "struct foo *"
poitners and tell the hash whether they're equal.

However, when looking up a value, it's not practical to ask
that the caller already have a fully formed "struct foo" to
pass.  They might have the data in a different form, and it
may be costly to create a "struct foo" (e.g., if it contains
a FLEX_ARRAY and we would have to allocate a whole struct).
For that reason, hashmap provides an extra "keydata"
parameter, which gets passed to the comparator. You can pass
a half-formed "struct foo", and then pass the rest of the
data via "keydata".

This ends up making comparators confusing to write, as you
are really covering two cases: one in which you are really
comparing two entries, and one in which you are comparing a
key in arbitrary format to existing entries. In the latter
case, you have a half-formed "struct foo", and you can only
look at certain entries (and have to replace them with
values from "keydata").

Instead, we can provide two comparison functions. One
which will take two "struct foo" pointers and compare their
values, and one which will take a "struct foo" and another
arbitrary opaque data type and compare them.

The upside is that the comparators are much easier to
understand. There are two downsides, though:

  1. It's more verbose. You're writing two functions instead
     of one.  Often one can be written in terms of the other,
     or it's trivial to share a common implementation, so
     this is really just a matter of lines of code, not of
     increased complexity (in fact, the complexity should
     hopefully be less).

  2. The key data type must encapsulate all of the data.
     In cases where "struct foo" contains a FLEX_ARRAY and
     "keydata" is a pointer to the bytes that would be found
     in the array, you can do something like:

       struct foo key;
       hashmap_entry_init(&key, memhash(data, len));
       key.len = len;
       hashmap_get(map, &key, data);

     In other words, you really care comparing the "struct
     foo", but you're just "augmenting" it with the extra
     data in keydata (and the comparator must know how to
     apply that augmentation; it looks at foo.len, but
     "keydata" instead of foo->data).

     But with a separate structure, you may need an extra
     data type. For example, you need some way of binding
     the data and length into a single pointer, like:

       struct hashmap_entry entry;
       struct counted_string key;

       hashmap_entry_init(&key, memhash(data, len));
       key.str = data;
       key.len = len;
       hashmap_get(map, &entry, &key);

     In most cases (in fact all that are currently present)
     this "counted_string" type is sufficient, and can be
     reused without having to define a new type.

This patch makes such a split. While we are there, it also
flips the boolean sense of the equality functions. Calling
these functions "foo_cmp" does not follow our usual pattern,
which would imply that "foo_cmp" can be used both for
equality tests (as "!foo_cmp") and for ordering (i.e., that
the sign bit of foo_cmp's return is useful). Some
comparators do provide that, but others do not. Instead,
let's call it "foo_equals", which reinforces that it's a
pure equality test.

It also tends to make the comparator code itself easier to
follow, as we do not need extra negations inside it. We can
write:

  return a->len == b->len && !memcmp(a->data, b->data, a->len);

instead of:

  return a->len != b->len || memcmp(a->data, b->data, a->len);

All of the existing callers are updated.