Commit Graph

45586 Commits

Author SHA1 Message Date
Richard Hansen
c1b0d3a010 mergetool: take the "-O" out of $orderfile
This will make it easier for a future commit to convert a relative
orderfile pathname to either absolute or relative to the top-level
directory.  It also improves code readability.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
b9ebb65926 t7610: add test case for rerere+mergetool+subdir bug
If rerere is enabled and mergetool is run from a subdirectory,
mergetool always prints "No files need merging".  Add an expected
failure test case for this situation.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
bd9714f253 t7610: spell 'git reset --hard' consistently
Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
fef6c06d64 t7610: don't assume the checked-out commit
Always check out the required commit at the beginning of the test so
that a failure in a previous test does not cause the test to work off
of the wrong commit.

This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
61b76d2de3 t7610: always work on a test-specific branch
Create and use a test-specific branch when the test might create a
commit.  This is not always necessary for correctness, but it improves
debuggability by ensuring a commit created by test #N shows up on the
testN branch, not the branch for test #N-1.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
e866ff851a t7610: delete some now-unnecessary 'git reset --hard' lines
Tests now always run 'git reset --hard' at the end (even if they
fail), so it's no longer necessary to run 'git reset --hard' at the
beginning of a test.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
c3ad3126b8 t7610: run 'git reset --hard' after each test to clean up
Use test_when_finished to run 'git reset --hard' after each test so
that the repository is left in a saner state for the next test.

This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
b696ac9aaf t7610: don't rely on state from previous test
If the repository must be in a particular state (beyond what is
already done by the 'setup' test case) before the test can run, make
the necessary repository changes in the test script even if it means
duplicating some lines of code from the previous test case.

This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
614eb27f02 t7610: use test_when_finished for cleanup tasks
This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:20 -08:00
Richard Hansen
157acfcf35 t7610: move setup code to the 'setup' test case
Multiple test cases depend on these hunks, so move them to the 'setup'
test case.  This is a step toward making the tests more independent so
that if one test fails it doesn't cause subsequent tests to fail.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:19 -08:00
Richard Hansen
ecfdf0bd2f t7610: update branch names to match test number
Rename the testNN branches so that NN matches the test number.  This
should make it easier to troubleshoot test issues.  Use $test_count to
keep this future-proof.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:19 -08:00
Richard Hansen
11873b438f rev-parse doc: pass "--" to rev-parse in the --prefix example
The "--" argument avoids "ambiguous argument: unknown revision or
path not in the working tree" errors when a pathname argument refers
to a non-existent file.

The "--" passed explicitly to set was removed because rev-parse
outputs the "--" argument that it is given.

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 13:22:19 -08:00
Stefan Beller
384f1a167b unpack-trees: factor progress setup out of check_updates
This makes check_updates shorter and easier to understand.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 11:53:33 -08:00
Stefan Beller
c4bfc7728b unpack-trees: remove unneeded continue
The continue is the last statement in the loop, so not needed.
This situation arose in 700e66d66 (2010-07-30, unpack-trees: let
read-tree -u remove index entries outside sparse area) when statements
after the continue were removed.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 11:51:19 -08:00
Stefan Beller
30ac275b1c unpack-trees: move checkout state into check_updates
The checkout state was introduced via 16da134b1f
(read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to
refactor the checkout state was done in b56aa5b268 (unpack-trees: pass
checkout state explicitly to check_updates(), 2016-09-13), but we can
go even further.

The `struct checkout state` is not used in unpack_trees apart from
initializing it, so move it into the function that makes use of it,
which is `check_updates`.

Signed-off-by: Stefan Beller <sbeller@google.com>
Reviewed-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 11:51:14 -08:00
Richard Hansen
314caebe21 .mailmap: record canonical email for Richard Hansen
When I changed employers my work address changed from rhansen@bbn.com
to hansenr@google.com.  Rather than map my old work address to my new,
map them both to my permanent personal email address.  (I will still
use my work address in commits I submit so that my employer gets some
credit.)

Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10 11:13:11 -08:00
Stefan Beller
2d81c48fa7 pathspec: give better message for submodule related pathspec error
Every once in a while someone complains to the mailing list to have
run into this weird assertion[1]. The usual response from the mailing
list is link to old discussions[2], and acknowledging the problem
stating it is known.

This patch accomplishes two things:

  1. Switch assert() to die("BUG") to give a more readable message.

  2. Take one of the cases where we hit a BUG and turn it into a normal
     "there was something wrong with the input" message.

     This assertion triggered for cases where there wasn't a programming
     bug, but just bogus input. In particular, if the user asks for a
     pathspec that is inside a submodule, we shouldn't assert() or
     die("BUG"); we should tell the user their request is bogus.

     The only reason we did not check for it, is the expensive nature
     of such a check, so callers avoid setting the flag
     PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE. However when we die due
     to bogus input, the expense of CPU cycles spent outweighs the user
     wondering what went wrong, so run that check unconditionally before
     dying with a more generic error message.

Note: There is a case (e.g. "git -C submodule add .") in which we call
strip_submodule_slash_expensive, as git-add requests it via the flag
PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE, but the assert used to
trigger nevertheless, because the flag PATHSPEC_LITERAL was not set,
such that we executed

	if (item->nowildcard_len < prefixlen)
		item->nowildcard_len = prefixlen;

and prefixlen was not adapted (e.g. it was computed from "submodule/")
So in the die_inside_submodule_path function we also need handle paths,
that were stripped before, i.e. are the exact submodule path. This
is why the conditions in die_inside_submodule_path are slightly
different than in strip_submodule_slash_expensive.

[1] https://www.google.com/search?q=item-%3Enowildcard_len
[2] http://git.661346.n2.nabble.com/assert-failed-in-submodule-edge-case-td7628687.html
    https://www.spinics.net/lists/git/msg249473.html

Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 15:19:22 -08:00
Jeff King
46df6906f3 execv_dashed_external: wait for child on signal death
When you hit ^C to interrupt a git command going to a pager,
this usually leaves the pager running. But when a dashed
external is in use, the pager ends up in a funny state and
quits (but only after eating one more character from the
terminal!). This fixes it.

Explaining the reason will require a little background.

When git runs a pager, it's important for the git process to
hang around and wait for the pager to finish, even though it
has no more data to feed it. This is because git spawns the
pager as a child, and thus the git process is the session
leader on the terminal. After it dies, the pager will finish
its current read from the terminal (eating the one
character), and then get EIO trying to read again.

When you hit ^C, that sends SIGINT to git and to the pager,
and it's a similar situation.  The pager ignores it, but the
git process needs to hang around until the pager is done. We
addressed that long ago in a3da882120 (pager: do
wait_for_pager on signal death, 2009-01-22).

But when you have a dashed external (or an alias pointing to
a builtin, which will re-exec git for the builtin), there's
an extra process in the mix. For instance, running:

  $ git -c alias.l=log l

will end up with a process tree like:

  git (parent)
    \
     git-log (child)
      \
       less (pager)

If you hit ^C, SIGINT goes to all of them. The pager ignores
it, and the child git process will end up in wait_for_pager().
But the parent git process will die, and the usual EIO
trouble happens.

So we really want the parent git process to wait_for_pager(),
but of course it doesn't know anything about the pager at
all, since it was started by the child.  However, we can
have it wait on the git-log child, which in turn is waiting
on the pager. And that's what this patch does.

There are a few design decisions here worth explaining:

  1. The new feature is attached to run-command's
     clean_on_exit feature. Partly this is convenience,
     since that feature already has a signal handler that
     deals with child cleanup.

     But it's also a meaningful connection. The main reason
     that dashed externals use clean_on_exit is to bind the
     two processes together. If somebody kills the parent
     with a signal, we propagate that to the child (in this
     instance with SIGINT, we do propagate but it doesn't
     matter because the original signal went to the whole
     process group). Likewise, we do not want the parent
     to go away until the child has done so.

     In a traditional Unix world, we'd probably accomplish
     this binding by just having the parent execve() the
     child directly. But since that doesn't work on Windows,
     everything goes through run_command's more spawn-like
     interface.

  2. We do _not_ automatically waitpid() on any
     clean_on_exit children. For dashed externals this makes
     sense; we know that the parent is doing nothing but
     waiting for the child to exit anyway. But with other
     children, it's possible that the child, after getting
     the signal, could be waiting on the parent to do
     something (like closing a descriptor). If we were to
     wait on such a child, we'd end up in a deadlock. So
     this errs on the side of caution, and lets callers
     enable the feature explicitly.

  3. When we send children the cleanup signal, we send all
     the signals first, before waiting on any children. This
     is to avoid the case where one child might be waiting
     on another one to exit, causing a deadlock. We inform
     all of them that it's time to die before reaping any.

     In practice, there is only ever one dashed external run
     from a given process, so this doesn't matter much now.
     But it future-proofs us if other callers start using
     the wait_after_clean mechanism.

There's no automated test here, because it would end up racy
and unportable. But it's easy to reproduce the situation by
running the log command given above and hitting ^C.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 13:41:40 -08:00
Jeff King
246f0edec0 execv_dashed_external: stop exiting with negative code
When we try to exec a git sub-command, we pass along the
status code from run_command(). But that may return -1 if we
ran into an error with pipe() or execve(). This tends to
work (and end up as 255 due to twos-complement wraparound
and truncation), but in general it's probably a good idea to
avoid negative exit codes for portability.

We can easily translate to the normal generic "128" code we
get when syscalls cause us to die.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 13:41:35 -08:00
Jeff King
2b296c93d4 execv_dashed_external: use child_process struct
When we run a dashed external, we use the one-liner
run_command_v_opt() to do so. Let's switch to using a
child_process struct, which has two advantages:

  1. We can drop all of the allocation and cleanup code for
     building our custom argv array, and just rely on the
     builtin argv_array (at the minor cost of doing a few
     extra mallocs).

  2. We have access to the complete range of child_process
     options, not just the ones that the "_opt()" form can
     forward.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 13:41:33 -08:00
Brandon Williams
0b9864aa28 real_path: set errno when max number of symlinks is exceeded
Set errno to ELOOP when the maximum number of symlinks is exceeded, as
would be done by other symlink-resolving functions.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 11:26:10 -08:00
Brandon Williams
7aeb81f1de real_path: prevent redefinition of MAXSYMLINKS
The macro 'MAXSYMLINKS' is already defined on macOS and Linux in
<sys/param.h>.  If 'MAXSYMLINKS' has already been defined, use the value
defined by the OS otherwise default to a value of 32 which is more
inline with what is allowed by many systems.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 11:25:57 -08:00
Steven Penny
aa38ad2b24 Makefile: put LIBS after LDFLAGS for imap-send
This matches up with the targets git-%, git-http-fetch, git-http-push
and git-remote-testsvn. It must be done this way in Cygwin else lcrypto
cannot find lgdi32 and lws2_32.

Signed-off-by: Steven Penny <svnpenn@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 06:31:52 -08:00
Steven Penny
7c44b33f8b Makefile: POSIX windres
When environment variable POSIXLY_CORRECT is set, the
"input -o output" syntax is not supported.

  http://cygwin.com/ml/cygwin/2017-01/msg00036.html

Use "-i input -o output" syntax instead.

Signed-off-by: Steven Penny <svnpenn@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 01:56:22 -08:00
Pranit Bauva
c6f44e1da5 t9813: avoid using pipes
The exit code of the upstream in a pipe is ignored thus we should avoid
using it. By writing out the output of the git command to a file, we can
test the exit codes of both the commands.

Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-09 01:54:47 -08:00
Brandon Williams
27ec42826e pathspec: rename prefix_pathspec to init_pathspec_item
Give a more relevant name to the prefix_pathspec function as it does
more than just prefix a pathspec element.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:18 -08:00
Brandon Williams
4f1bf4d2b4 pathspec: small readability changes
A few small changes to improve readability.  This is done by grouping related
assignments, adding blank lines, ensuring lines are <80 characters, and
adding additional comments.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:18 -08:00
Brandon Williams
5590215b13 pathspec: create strip submodule slash helpers
Factor out the logic responsible for stripping the trailing slash on
pathspecs referencing submodules into its own function.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:18 -08:00
Brandon Williams
1b6112c527 pathspec: create parse_element_magic helper
Factor out the logic responsible for the magic in a pathspec element
into its own function.

Also avoid calling into the parsing functions when
`PATHSPEC_LITERAL_PATH` is specified since it causes magic to be
ignored and all paths to be treated as literals.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:18 -08:00
Brandon Williams
8881fde013 pathspec: create parse_long_magic function
Factor out the logic responsible for parsing long magic into its own
function.  As well as hoist the prefix check logic outside of the inner
loop as there isn't anything that needs to be done after matching
"prefix:".

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:18 -08:00
Brandon Williams
b4bebdce83 pathspec: create parse_short_magic function
Factor out the logic responsible for parsing short magic into its own
function.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
db7e85988f pathspec: factor global magic into its own function
Create helper functions to read the global magic environment variables
in additon to factoring out the global magic gathering logic into its
own function.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
5d8f084a5d pathspec: simpler logic to prefix original pathspec elements
The logic used to prefix an original pathspec element with 'prefix'
magic is more general purpose and can be used for more than just short
magic.  Remove the extra code paths and rename 'prefix_short_magic' to
'prefix_magic' to better indicate that it can be used in more general
situations.

Also, slightly change the logic which decides when to prefix the
original element in order to prevent a pathspec of "." from getting
converted to "" (empty string).

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
2aee5849c9 pathspec: always show mnemonic and name in unsupported_magic
For better clarity, always show the mnemonic and name of the unsupported
magic being used.  This lets users have a more clear understanding of
what magic feature isn't supported.  And if they supplied a mnemonic,
the user will be told what its corresponding name is which will allow
them to more easily search the man pages for that magic type.

This also avoids passing an extra parameter around the pathspec
initialization code.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
93f3ddb2a1 pathspec: remove unused variable from unsupported_magic
Removed unused variable 'n' from the 'unsupported_magic()' function.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
8aee769fa1 pathspec: copy and free owned memory
The 'original' string entry in a pathspec_item is only duplicated some
of the time, instead always make a copy of the original and take
ownership of the memory.

Since both 'match' and 'original' string entries in a pathspec_item are
owned by the pathspec struct, they need to be freed when clearing the
pathspec struct (in 'clear_pathspec()') and duplicated when copying the
pathspec struct (in 'copy_pathspec()').

Also change the type of 'match' and 'original' to 'char *' in order to
more explicitly show the ownership of the memory.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
34305f7753 pathspec: remove the deprecated get_pathspec function
Now that all callers of the old 'get_pathspec' interface have been
migrated to use the new pathspec struct interface it can be removed
from the codebase.

Since there are no more users of the '_raw' field in the pathspec struct
it can also be removed.  This patch also removes the old functionality
of modifying the const char **argv array that was passed into
parse_pathspec.  Instead the constructed 'match' string (which is a
pathspec element with the prefix prepended) is only stored in its
corresponding pathspec_item entry.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
e1e24edc1a ls-tree: convert show_recursive to use the pathspec struct interface
Convert 'show_recursive()' to use the pathspec struct interface from
using the '_raw' entry in the pathspec struct.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
966de3028b dir: convert fill_directory to use the pathspec struct interface
Convert 'fill_directory()' to use the pathspec struct interface from
using the '_raw' entry in the pathspec struct.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
e1b8c7bdc0 dir: remove struct path_simplify
Teach simplify_away() and exclude_matches_pathspec() to handle struct
pathspec directly, eliminating the need for the struct path_simplify.

Also renamed the len parameter to pathlen in exclude_matches_pathspec()
to match the parameter names used in simplify_away().

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Brandon Williams
2ec87741b2 mv: remove use of deprecated 'get_pathspec()'
Convert the 'internal_copy_pathspec()' function to 'prefix_path()'
instead of using the deprecated 'get_pathspec()' interface.  Also,
rename 'internal_copy_pathspec()' to 'internal_prefix_pathspec()' to be
more descriptive of what the funciton is actually doing.

In addition to this, fix a memory leak caused by only duplicating some
of the pathspec elements.  Instead always duplicate all of the the
pathspec elements as an intermediate step (with modificationed based on
the passed in flags).  This way the intermediate strings can then be
freed after getting the result from 'prefix_path()'.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-08 18:04:17 -08:00
Jeff King
4e76832984 blame: output porcelain "previous" header for each file
It's possible for content currently found in one file to
have originated in two separate files, each of which may
have been modified in some single older commit.  The
--porcelain output generates an incorrect "previous" header
in this case, whereas --line-porcelain gets it right.  The
problem is that the porcelain output tries to omit repeated
details of commits, and treats "previous" as a property of
the commit, when it is really a property of the blamed block
of lines.

Let's look at an example. In a case like this, you might see
this output from --line-porcelain:

  SOME_SHA1 1 1 1
  author ...
  committer ...
  previous SOME_SHA1^ file_one
  filename file_one
          ...some line content...
  SOME_SHA1 2 1 1
  author ...
  committer ...
  previous SOME_SHA1^ file_two
  filename file_two
          ...some different content....

The "filename" fields tell us that the two lines are from
two different files. But notice that the filename also
appears in the "previous" field, which tells us where to
start a re-blame. The second content line never appeared in
file_one at all, so we would obviously need to re-blame from
file_two (or possibly even some other file, if had just been
renamed to file_two in SOME_SHA1).

So far so good. Now here's what --porcelain looks like:

  SOME_SHA1 1 1 1
  author ...
  committer ...
  previous SOME_SHA1^ file_one
  filename file_one
          ...some line content...
  SOME_SHA1 2 1 1
  filename file_two
          ...some different content....

We've dropped the author and committer fields from the
second line, as they would just be repeats.  But we can't
omit "filename", because it depends on the actual block of
blamed lines, not just the commit. This is handled by
emit_porcelain_details(), which will show the filename
either if it is the first mention of the commit _or_ if the
commit has multiple paths in it.

But we don't give "previous" the same handling. It's written
inside emit_one_suspect_detail(), which bails early if we've
already seen that commit. And so the output above is wrong;
a reader would assume that the correct place to re-blame
line two is from file_one, but that's obviously nonsense.

Let's treat "previous" the same as "filename", and show it
fresh whenever we know we are in a confusing case like this.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:34:54 -08:00
Jeff King
ed58d8088b blame: handle --no-abbrev
You can already ask blame for full sha1s with "-l" or with
"--abbrev=40". But for consistency with other parts of Git,
we should support "--no-abbrev".

Worse, blame already accepts --no-abbrev, but it's totally
broken. When we see --no-abbrev, the abbrev variable is set
to 0, which is then used as a printf precision. For regular
sha1s, that means we print nothing at all (which is very
wrong). For boundary commits we decrement it to "-1", which
printf interprets as "no limit" (which is almost correct,
except it misses the 39-length magic explained in the
previous commit).

Let's detect --no-abbrev and behave as if --abbrev=40 was
given.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:34:54 -08:00
Jeff King
91229834c2 blame: fix alignment with --abbrev=40
The blame command internally adds 1 to any requested sha1
abbreviation length, and then subtracts it when outputting a
boundary commit. This lets regular and boundary sha1s line
up visually, but it misses one corner case.

When the requested length is 40, we bump the value to 41.
But since we only have 40 characters, that's all we can show
(fortunately the truncation is done by a printf precision
field, so it never tries to read past the end of the
buffer).  So a normal sha1 shows 40 hex characters, and a
boundary sha1 shows "^" plus 40 hex characters. The result
is misaligned.

The "-l" option to show long sha1s gets around this by
skipping the "abbrev" variable entirely and just always
using GIT_SHA1_HEXSZ.  This avoids the "+1" issue, but it
does mean that boundary commits only have 39 characters
printed.  This is somewhat odd, but it does look good
visually: the results are aligned and left-justified. The
alternative would be to allocate an extra column that would
contain either an extra space or the "^" boundary marker.

As this is by definition the human-readable view, it's
probably not that big a deal either way (and of course
--porcelain, etc, correctly produce correct 40-hex sha1s).
But for consistency, this patch teaches --abbrev=40 to
produce the same output as "-l" (always left-aligned, with
40-hex for normal sha1s, and "^" plus 39-hex for
boundaries).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:34:54 -08:00
Jeff King
356b8ecff1 rebase--interactive: count squash commits above 10 correctly
We generate the squash commit message incrementally running
a sed script once for each commit. It parses "This is
a combination of <N> commits" from the first line of the
existing message, adds one to <N>, and uses the result as
the number of our current message.

Since f2d17068fd (i18n: rebase-interactive: mark comments of
squash for translation, 2016-06-17), the first line may be
localized, and sed uses a pretty liberal regex, looking for:

  /^#.*([0-9][0-9]*)/

The "[0-9][0-9]*" tries to match double digits, but it
doesn't quite work.  The first ".*" is greedy, so if you
have:

  This is a combination of 10 commits.

it will eat up "This is a combination of 1", leaving "0" to
match the first "[0-9]" digit, and then skipping the
optional match of "[0-9]*".

As a result, the count resets every 10 commits, and a
15-commit squash would end up as:

  # This is a combination of 5 commits.
  # This is the 1st commit message:
  ...
  # This is the commit message :
  ... and so on ..
  # This is the commit message :
  ...
  # This is the commit message :
  ...
  # This is the commit message :
  ... etc, up to 5 ...

We can fix this by making the ".*" less greedy. Instead of
depending on ".*?" working portably, we can just limit the
match to non-digit characters, which accomplishes the same
thing.

Reported-by: Brandon Tolsch <btolsch@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:26:53 -08:00
Kyle Meyer
b10731f43d branch_get_push: do not segfault when HEAD is detached
Move the detached HEAD check from branch_get_push_1() to
branch_get_push() to avoid setting branch->push_tracking_ref when
branch is NULL.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:20:07 -08:00
Jeff King
965cba2e7e archive-zip: load userdiff config
Since 4aff646d17 (archive-zip: mark text files in archives,
2015-03-05), the zip archiver will look at the userdiff
driver to decide whether a file is text or binary. This
usually doesn't need to look any further than the attributes
themselves (e.g., "-diff", etc). But if the user defines a
custom driver like "diff=foo", we need to look at
"diff.foo.binary" in the config. Prior to this patch, we
didn't actually load it.

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 18:49:30 -08:00
Johannes Schindelin
9574901c02 giteveryday: unbreak rendering with AsciiDoctor
The "giteveryday" document has a callout list that contains a code
block. This is not a problem for AsciiDoc, but AsciiDoctor sadly was
explicitly designed *not* to render this correctly [*1*]. The symptom is
an unhelpful

	line 322: callout list item index: expected 1 got 12
	line 325: no callouts refer to list item 1
	line 325: callout list item index: expected 2 got 13
	line 327: no callouts refer to list item 2

In Git for Windows, we rely on the speed improvement of AsciiDoctor (on
this developer's machine, `make -j15 html` takes roughly 30 seconds with
AsciiDoctor, 70 seconds with AsciiDoc), therefore we need a way to
render this correctly.

The easiest way out is to simplify the callout list, as suggested by
AsciiDoctor's author, even while one may very well disagree with him
that a code block hath no place in a callout list.

*1*: https://github.com/asciidoctor/asciidoctor/issues/1478

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 14:03:40 -08:00
Johannes Schindelin
16615f0fbc mingw: add a regression test for pushing to UNC paths
On Windows, there are "UNC paths" to access network (AKA shared)
folders, of the form \\server\sharename\directory. This provides a
convenient way for Windows developers to share their Git repositories
without having to have a dedicated server.

Git for Windows v2.11.0 introduced a regression where pushing to said
UNC paths no longer works, although fetching and cloning still does, as
reported here: https://github.com/git-for-windows/git/issues/979

This regression was fixed in 7814fbe3f1 (normalize_path_copy(): fix
pushing to //server/share/dir on Windows, 2016-12-14).

Let's make sure that it does not regress again, by introducing a test
that uses so-called "administrative shares": disk volumes are
automatically shared under certain circumstances, e.g.  the C: drive is
shared as \\localhost\c$. The test needs to be skipped if the current
directory is inaccessible via said administrative share, of course.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 13:39:29 -08:00
Stefan Beller
3c87878490 contrib: remove gitview
gitview did not have meaningful contributions since 2007, which gives the
impression it is either a mature or dead project.

In both cases we should not carry it in git.git as the README for contrib
states we only want to carry experimental things to give early exposure.

Recently a security vulnerability was reported by Javantea, so the decision
to either fix the issue or remove the code in question becomes a bit
more urgent.

Reported-by: Javantea <jvoss@altsci.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Acked-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 13:22:54 -08:00