sequencer: signal failed ff as an aborted, not a conflicted merge

`do_pick_commit` handles three situations if it is not fast-forwarding.
In order for `do_pick_commit` to identify the situation, it examines the
return value of the selected merge command.

1. return value 0 stands for a clean merge
2. 1 is passed in case of a failed merge due to conflict
3. any other return value means that the merge did not even start

So far, the sequencer returns 1 in case of a failed fast-forward, which
would mean "failed merge due to conflict". However, a fast-forward
either starts and succeeds or does not start at all. In particular, it
cannot fail in between like a merge with a dirty index due to conflicts.

In order to signal the three possible situations (not only success and
failure to complete) after a pick through porcelain commands such as
`cherry-pick`, exit with a return value that is neither 0 nor 1. 128 was
chosen in line with the other situations in which the sequencer
encounters an error. In such situations, the sequencer returns a
negative value and `cherry-pick` translates this into a call to `die`.
`die` then terminates the process with exit status 128.

Signed-off-by: Fabian Ruch <bafain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git potty: restore environments after alias expansion

Commit 4ad8332 (t0001: test git init when run via an alias -
2010-11-26) noted breakages when running init via alias. The problem
is for alias to be used, $GIT_DIR must be searched, but 'init' and
'clone' are not happy with that. So we start a new process like an
external command, with clean environment in this case. Env variables
that are set by command line (e.g. "git --git-dir=.. ") are kept.

This should also fix autocorrecting a command typo to "init" because
it's the same problem: aliases are read, then "init" is unhappy with
$GIT_DIR already set up because of that.

Reminded-by: David Turner <dturner@twopensource.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: do not accidentally pack kept objects by default

Commit ee34a2b (repack: add `repack.packKeptObjects` config
var, 2014-03-03) added a flag which could duplicate kept
objects, but did not mean to turn it on by default. Instead,
the option is tied by default to the decision to write
bitmaps, like:

  if (pack_kept_objects < 0)
	  pack_kept_objects = write_bitmap;

after which we expect pack_kept_objects to be a boolean 0 or
1.  However, that assignment neglects that write_bitmap is
_also_ a tri-state with "-1" as the default, and with
neither option given, we accidentally turn the option on.

This patch is the minimal fix to restore the desired
behavior for the default state. Further patches will fix the
more complicated cases.

Note the update to t7700. It failed to turn on bitmaps,
meaning we were actually confirming the wrong behavior!

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: respect pack.writebitmaps

The config option to turn on bitmaps is read all the way
down in the plumbing of pack-objects. This makes it hard for
other options in the porcelain of repack to make decisions
based on the bitmap setting. For example,
repack.packKeptObjects tries to kick in by default only when
bitmaps are turned on. But it can't do so reliably because
it doesn't yet know whether we are using bitmaps.

This patch teaches repack to respect pack.writebitmaps. It
means we pass a redundant command-line flag to pack-objects,
but that's OK; it shouldn't affect the outcome.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: s/write_bitmap/&s/ in code

The config name is "writeBitmaps", so the internal variable
missing the plural is unnecessarily confusing to write.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-objects: stop respecting pack.writebitmaps

The handling of the pack.writebitmaps config option
originally happened in pack-objects, which is quite
low-level. It would make more sense for drivers of
pack-objects to read the config, and then manipulate
pack-objects with command-line options.

Recently, repack learned to do so, making the low-level read
of pack.writebitmaps redundant here. Other callers, like
upload-pack, would not generally want to write bitmaps
anyway.

This could be considered a regression for somebody who is
driving pack-objects themselves outside of repack and
expects the config option to be used. However, such users
seem rather unlikely given how new the bitmap code is (and
the fact that they would basically be reimplementing repack
in the first place).

Note that we do not do anything with pack.writeBitmapHashCache
here. That option is not about "do we write bimaps", but
rather "when we are writing bitmaps, how do we do it?". You
would want that to kick in anytime you decide to write them,
similar to how pack.indexVersion is used.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: simplify handling of --write-bitmap-index

We previously needed to pass --no-write-bitmap-index
explicitly to pack-objects to override its reading of
pack.writebitmaps from the config. Now that it no longer
does so, we can assume that bitmaps are off by default, and
only turn them on when necessary. This also lets us avoid a
confusing tri-state flag for write_bitmaps.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: introduce repack.writeBitmaps config option

We currently have pack.writeBitmaps, which originally
operated at the pack-objects level. This should really have
been a repack.* option from day one. Let's give it the more
sensible name, but keep the old version as a deprecated
synonym.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
docs: Explain the purpose of fetch's and pull's <refspec> parameter.

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit_tree: take a pointer/len pair rather than a const strbuf

While strbufs are pretty common throughout our code, it is
more flexible for functions to take a pointer/len pair than
a strbuf. It's easy to turn a strbuf into such a pair (by
dereferencing its members), but less easy to go the other
way (you can strbuf_attach, but that has implications about
memory ownership).

This patch teaches commit_tree (and its associated callers
and sub-functions) to take such a pair for the commit
message rather than a strbuf.  This makes passing the buffer
around slightly more verbose, but means we can get rid of
some dangerous strbuf_attach calls in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
replace dangerous uses of strbuf_attach

It is not a good idea to strbuf_attach an arbitrary pointer
just because a function you are calling wants a strbuf.
Attaching implies a transfer of memory ownership; if anyone
were to modify or release the resulting strbuf, we would
free() the pointer, leading to possible problems:

  1. Other users of the original pointer might access freed
     memory.

  2. The pointer might not be the start of a malloc'd
     area, so calling free() on it in the first place would
     be wrong.

In the two cases modified here, we are fortunate that nobody
touches the strbuf once it is attached, but it is an
accident waiting to happen.  Since the previous commit,
commit_tree and friends take a pointer/buf pair, so we can
just do away with the strbufs entirely.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
alloc: include any-object allocations in alloc_report

When 2c1cbec (Use proper object allocators for unknown
object nodes too, 2007-04-16), added a special "any_object"
allocator, it never taught alloc_report to report on it. To
do so we need to add an extra type argument to the REPORT
macro, as that commit did for DEFINE_ALLOCATOR.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit: push commit_index update into alloc_commit_node

Whenever we create a commit object via lookup_commit, we
give it a unique index to be used with the commit-slab API.
The theory is that any "struct commit" we create would
follow this code path, so any such struct would get an
index. However, callers could use alloc_commit_node()
directly (and get multiple commits with index 0).

Let's push the indexing into alloc_commit_node so that it's
hard for callers to get it wrong.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
do not create "struct commit" with xcalloc

In both blame and merge-recursive, we sometimes create a
"fake" commit struct for convenience (e.g., to represent the
HEAD state as if we would commit it). By allocating
ourselves rather than using alloc_commit_node, we do not
properly set the "index" field of the commit. This can
produce subtle bugs if we then use commit-slab on the
resulting commit, as we will share the "0" index with
another commit.

We can fix this by using alloc_commit_node() to allocate.
Note that we cannot free the result, as it is part of our
commit allocator. However, both cases were already leaking
the allocated commit anyway, so there's nothing to fix up.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
logmsg_reencode: return const buffer

The return value from logmsg_reencode may be either a newly
allocated buffer or a pointer to the existing commit->buffer.
We would not want the caller to accidentally free() or
modify the latter, so let's mark it as const.  We can cast
away the constness in logmsg_free, but only once we have
determined that it is a free-able buffer.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
sequencer: use logmsg_reencode in get_message

This simplifies the code, as logmsg_reencode handles the
reencoding for us in a single call. It also means we learn
logmsg_reencode's trick of pulling the buffer from disk when
commit->buffer is NULL (we currently just silently return!).
It is doubtful this matters in practice, though, as
sequencer operations would not generally turn off
save_commit_buffer.

Note that we may be fixing a bug here. The existing code
does:

  if (same_encoding(to, from))
	  reencode_string(buf, to, from);

That probably should have been "!same_encoding".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pull: do not abuse 'break' inside a shell 'case'

It is not C. The code would break under mksh when 'pull.ff' is set:

  $ git pull
  /usr/lib/git-core/git-pull[67]: break: can't break
  Already up-to-date.

Signed-off-by: Jacek Konieczny <jajcus@jajcus.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'jc/rev-parse-argh-dashed-multi-words' into maint

* jc/rev-parse-argh-dashed-multi-words:
  update-index: fix segfault with missing --cacheinfo argument
Sync with maint

* maint:
  pull: do not abuse 'break' inside a shell 'case'
t7700: drop explicit --no-pack-kept-objects from .keep test

We want to make sure that the default behavior of git-repack,
without any options, continues to treat .keep files as it
always has. Adding an explicit --no-pack-kept-objects, as
ee34a2b did, is a much less interesting test, and prevented
us from noticing the bug fixed by 64d3dc9 (repack: do not
accidentally pack kept objects by default, 2014-06-10).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>