Commit Graph

11 Commits

Author SHA1 Message Date
Junio C Hamano
f31d23a399 Merge branch 'bw/config-h'
Fix configuration codepath to pay proper attention to commondir
that is used in multi-worktree situation, and isolate config API
into its own header file.

* bw/config-h:
  config: don't implicitly use gitdir or commondir
  config: respect commondir
  setup: teach discover_git_directory to respect the commondir
  config: don't include config.h by default
  config: remove git_config_iter
  config: create config.h
2017-06-24 14:28:41 -07:00
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Johannes Schindelin
5f3296c069 difftool: address a couple of resource/memory leaks
This change plugs a couple of memory leaks and makes sure that the file
descriptor is closed in run_dir_diff().

Spotted by Coverity.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08 12:18:19 +09:00
Johannes Schindelin
882add136f difftool: fix use-after-free
The left and right base directories were pointed to the buf field of
two strbufs, which were subject to change.

A contrived test case shows the problem where a file with a long enough
name to force the strbuf to grow is up-to-date (hence the code path is
used where the work tree's version of the file is reused), and then a
file that is not up-to-date needs to be written (hence the code path is
used where checkout_entry() uses the previously recorded base_dir that
is invalid by now).

Let's just copy the base_dir strings for use with checkout_entry(),
never touch them until the end, and release them then. This is an easily
verifiable fix (as opposed to the next-obvious alternative: to re-set
base_dir after every loop iteration).

This fixes https://github.com/git-for-windows/git/issues/1124

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-13 17:53:08 -07:00
Jeff King
0730dd4ffb difftool: avoid strcpy
In order to checkout files, difftool reads "diff --raw"
output and feeds the names to checkout_entry(). That
function requires us to have a "struct cache_entry". And
because that struct uses a FLEX_ARRAY for the name field, we
have to actually copy in our new name.

The current code allocates a single re-usable cache_entry
that can hold a name up to PATH_MAX, and then copies
filenames into it using strcpy(). But there's no guarantee
that incoming names are smaller than PATH_MAX. They've come
from "diff --raw" output which might be diffing between two
trees (and hence we'd be subject to the PATH_MAX of some
other system, or even none at all if they were created
directly via "update-index").

We can fix this by using make_cache_entry() to create a
correctly-sized cache_entry for each name. This incurs an
extra allocation per file, but this is negligible compared
to actually writing out the file contents.

To make this simpler, we can push this procedure into a new
helper function. Note that we can also get rid of the "len"
variables for src_path and dst_path (and in fact we must, as
the compiler complains that they are unused).

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-30 13:02:16 -07:00
David Aguilar
18ec800512 difftool: handle modified symlinks in dir-diff mode
Detect the null object ID for symlinks in dir-diff so that difftool can
detect when symlinks are modified in the worktree.

Previously, a null symlink object ID would crash difftool.
Handle null object IDs as unknown content that must be read from
the worktree.

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-15 12:08:10 -07:00
David Aguilar
d81345ce09 difftool: fix bug when printing usage
"git difftool -h" reports an error:

	fatal: BUG: setup_git_env called without repository

Defer repository setup so that the help option processing happens before
the repository is initialized.

Add tests to ensure that the basic usage works inside and outside of a
repository.

Signed-off-by: David Aguilar <davvid@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-06 10:13:48 -08:00
Jeff King
94d3997ecc difftool: hack around -Wzero-length-format warning
Building with "gcc -Wall" will complain that the format in:

  warning("")

is empty. Which is true, but the warning is over-eager. We
are calling the function for its side effect of printing
"warning:", even with an empty string.

Our DEVELOPER Makefile knob disables the warning, but not
everybody uses it. Let's silence the warning in the code so
that nobody reports it or tries to "fix" it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-25 13:28:34 -08:00
Johannes Schindelin
019678d6b1 difftool: retire the scripted version
It served its purpose, but now we have a builtin difftool. Time for the
Perl script to enjoy Florida.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-19 13:23:43 -08:00
Johannes Schindelin
03831ef7b5 difftool: implement the functionality in the builtin
This patch gives life to the skeleton added in the previous patch.

The motivation for converting the difftool is that Perl scripts are not at
all native on Windows, and that `git difftool` therefore is pretty slow on
that platform, when there is no good reason for it to be slow.

In addition, Perl does not really have access to Git's internals. That
means that any script will always have to jump through unnecessary
hoops, and it will often need to perform unnecessary work (e.g. when
reading the entire config every time `git config` is called to query a
single config value).

The current version of the builtin difftool does not, however, make full
use of the internals but instead chooses to spawn a couple of Git
processes, still, to make for an easier conversion. There remains a lot
of room for improvement, left later.

Note: to play it safe, the original difftool is still called unless the
config setting difftool.useBuiltin is set to true.

The reason: this new, experimental, builtin difftool was shipped as part
of Git for Windows v2.11.0, to allow for easier large-scale testing, but
of course as an opt-in feature.

The speedup is actually more noticable on Linux than on Windows: a quick
test shows that t7800-difftool.sh runs in (2.183s/0.052s/0.108s)
(real/user/sys) in a Linux VM, down from  (6.529s/3.112s/0.644s), while on
Windows, it is (36.064s/2.730s/7.194s), down from (47.637s/2.407s/6.863s).
The culprit is most likely the overhead incurred from *still* having to
shell out to mergetool-lib.sh and difftool--helper.sh.

Still, it is an improvement.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-19 13:22:36 -08:00
Johannes Schindelin
be8a90e59c difftool: add a skeleton for the upcoming builtin
This adds a builtin difftool that still falls back to the legacy Perl
version, which has been renamed to `legacy-difftool`.

The idea is that the new, experimental, builtin difftool immediately hands
off to the legacy difftool for now, unless the config variable
difftool.useBuiltin is set to true.

This feature flag will be used in the upcoming Git for Windows v2.11.0
release, to allow early testers to opt-in to use the builtin difftool and
flesh out any bugs.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-17 13:32:47 -08:00