summaryrefslogtreecommitdiff
path: root/builtin/difftool.c
AgeCommit message (Collapse)AuthorFilesLines
2018-08-17Merge branch 'rs/parse-opt-lithelp'Libravatar Junio C Hamano1-2/+2
The parse-options machinery learned to refrain from enclosing placeholder string inside a "<bra" and "ket>" pair automatically without PARSE_OPT_LITERAL_ARGHELP. Existing help text for option arguments that are not formatted correctly have been identified and fixed. * rs/parse-opt-lithelp: parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP shortlog: correct option help for -w send-pack: specify --force-with-lease argument help explicitly pack-objects: specify --index-version argument help explicitly difftool: remove angular brackets from argument help add, update-index: fix --chmod argument help push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets
2018-08-03difftool: remove angular brackets from argument helpLibravatar René Scharfe1-2/+2
Parseopt wraps arguments in a pair of angular brackets by default, signifying that the user needs to replace it with a value of the documented type. Remove the pairs from the option definitions to duplication and confusion. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-02Merge branch 'jm/cache-entry-from-mem-pool'Libravatar Junio C Hamano1-3/+3
For a large tree, the index needs to hold many cache entries allocated on heap. These cache entries are now allocated out of a dedicated memory pool to amortize malloc(3) overhead. * jm/cache-entry-from-mem-pool: block alloc: add validations around cache_entry lifecyle block alloc: allocate cache entries from mem_pool mem-pool: fill out functionality mem-pool: add life cycle management functions mem-pool: only search head block for available space block alloc: add lifecycle APIs for cache_entry structs read-cache: teach make_cache_entry to take object_id read-cache: teach refresh_cache_entry to take istate
2018-07-18Merge branch 'sb/object-store-grafts'Libravatar Junio C Hamano1-0/+1
The conversion to pass "the_repository" and then "a_repository" throughout the object access API continues. * sb/object-store-grafts: commit: allow lookup_commit_graft to handle arbitrary repositories commit: allow prepare_commit_graft to handle arbitrary repositories shallow: migrate shallow information into the object parser path.c: migrate global git_path_* to take a repository argument cache: convert get_graft_file to handle arbitrary repositories commit: convert read_graft_file to handle arbitrary repositories commit: convert register_commit_graft to handle arbitrary repositories commit: convert commit_graft_pos() to handle arbitrary repositories shallow: add repository argument to is_repository_shallow shallow: add repository argument to check_shallow_file_for_update shallow: add repository argument to register_shallow shallow: add repository argument to set_alternate_shallow_file commit: add repository argument to lookup_commit_graft commit: add repository argument to prepare_commit_graft commit: add repository argument to read_graft_file commit: add repository argument to register_commit_graft commit: add repository argument to commit_graft_pos object: move grafts to object parser object-store: move object access functions to object-store.h
2018-07-03block alloc: add lifecycle APIs for cache_entry structsLibravatar Jameson Miller1-3/+3
It has been observed that the time spent loading an index with a large number of entries is partly dominated by malloc() calls. This change is in preparation for using memory pools to reduce the number of malloc() calls made to allocate cahce entries when loading an index. Add an API to allocate and discard cache entries, abstracting the details of managing the memory backing the cache entries. This commit does actually change how memory is managed - this will be done in a later commit in the series. This change makes the distinction between cache entries that are associated with an index and cache entries that are not associated with an index. A main use of cache entries is with an index, and we can optimize the memory management around this. We still have other cases where a cache entry is not persisted with an index, and so we need to handle the "transient" use case as well. To keep the congnitive overhead of managing the cache entries, there will only be a single discard function. This means there must be enough information kept with the cache entry so that we know how to discard them. A summary of the main functions in the API is: make_cache_entry: create cache entry for use in an index. Uses specified parameters to populate cache_entry fields. make_empty_cache_entry: Create an empty cache entry for use in an index. Returns cache entry with empty fields. make_transient_cache_entry: create cache entry that is not used in an index. Uses specified parameters to populate cache_entry fields. make_empty_transient_cache_entry: create cache entry that is not used in an index. Returns cache entry with empty fields. discard_cache_entry: A single function that knows how to discard a cache entry regardless of how it was allocated. Signed-off-by: Jameson Miller <jamill@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03read-cache: teach make_cache_entry to take object_idLibravatar Jameson Miller1-2/+2
Teach make_cache_entry function to take object_id instead of a SHA-1. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-01Merge branch 'nd/use-opt-int-set-f'Libravatar Junio C Hamano1-5/+4
Code simplification. * nd/use-opt-int-set-f: Use OPT_SET_INT_F() for cmdline option specification
2018-05-30Merge branch 'ma/lockfile-cleanup'Libravatar Junio C Hamano1-1/+1
Code clean-up to adjust to a more recent lockfile API convention that allows lockfile instances kept on the stack. * ma/lockfile-cleanup: lock_file: move static locks into functions lock_file: make function-local locks non-static refs.c: do not die if locking fails in `delete_pseudoref()` refs.c: do not die if locking fails in `write_pseudoref()` t/helper/test-write-cache: clean up lock-handling
2018-05-24Use OPT_SET_INT_F() for cmdline option specificationLibravatar Nguyễn Thái Ngọc Duy1-5/+4
The only thing these commands need is extra parseopt flag which can be passed in by OPT_SET_INT_F() and it is a bit more compact than full struct initialization. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-16object-store: move object access functions to object-store.hLibravatar Stefan Beller1-0/+1
This should make these functions easier to find and cache.h less overwhelming to read. In particular, this moves: - read_object_file - oid_object_info - write_object_file As a result, most of the codebase needs to #include object-store.h. In this patch the #include is only added to files that would fail to compile otherwise. It would be better to #include wherever identifiers from the header are used. That can happen later when we have better tooling for it. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-10lock_file: make function-local locks non-staticLibravatar Martin Ågren1-1/+1
Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack. (This applies even if a user returns early, leaving a locked lock behind.) These `struct lock_file`s are local to their respective functions and we can drop their staticness. For good measure, I have inspected these sites and come to believe that they always release the lock, with the possible exception of bailing out using `die()` or `exit()` or by returning from a `cmd_foo()`. As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-11exec_cmd: rename to use dash in file nameLibravatar Stefan Beller1-1/+1
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Signed-off-by: Stefan Beller <sbeller@google.com>
2018-03-14sha1_file: convert read_sha1_file to struct object_idLibravatar brian m. carlson1-1/+1
Convert read_sha1_file to take a pointer to struct object_id and rename it read_object_file. Do the same for read_sha1_file_extended. Convert one use in grep.c to use the new function without any other code change, since the pointer being passed is a void pointer that is already initialized with a pointer to struct object_id. Update the declaration and definitions of the modified functions, and apply the following semantic patch to convert the remaining callers: @@ expression E1, E2, E3; @@ - read_sha1_file(E1.hash, E2, E3) + read_object_file(&E1, E2, E3) @@ expression E1, E2, E3; @@ - read_sha1_file(E1->hash, E2, E3) + read_object_file(E1, E2, E3) @@ expression E1, E2, E3, E4; @@ - read_sha1_file_extended(E1.hash, E2, E3, E4) + read_object_file_extended(&E1, E2, E3, E4) @@ expression E1, E2, E3, E4; @@ - read_sha1_file_extended(E1->hash, E2, E3, E4) + read_object_file_extended(E1, E2, E3, E4) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-07read-cache: leave lock in right state in `write_locked_index()`Libravatar Martin Ågren1-1/+0
If the original version of `write_locked_index()` returned with an error, it didn't roll back the lockfile unless the error occured at the very end, during closing/committing. See commit 03b866477 (read-cache: new API write_locked_index instead of write_index/write_cache, 2014-06-13). In commit 9f41c7a6b (read-cache: close index.lock in do_write_index, 2017-04-26), we learned to close the lock slightly earlier in the callstack. That was mostly a side-effect of lockfiles being implemented using temporary files, but didn't cause any real harm. Recently, commit 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05) introduced a subtle bug. If the temporary file is deleted (i.e., the lockfile is rolled back), the tempfile-pointer in the `struct lock_file` will be left dangling. Thus, an attempt to reuse the lockfile, or even just to roll it back, will induce undefined behavior -- most likely a crash. Besides not crashing, we clearly want to make things consistent. The guarantees which the lockfile-machinery itself provides is A) if we ask to commit and it fails, roll back, and B) if we ask to close and it fails, do _not_ roll back. Let's do the same for consistency. Do not delete the temporary file in `do_write_index()`. One of its callers, `write_locked_index()` will thereby avoid rolling back the lock. The other caller, `write_shared_index()`, will delete its temporary file anyway. Both of these callers will avoid undefined behavior (crashing). Teach `write_locked_index(..., COMMIT_LOCK)` to roll back the lock before returning. If we have already succeeded and committed, it will be a noop. Simplify the existing callers where we now have a superfluous call to `rollback_lockfile()`. That should keep future readers from wondering why the callers are inconsistent. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-20sha1_file: convert index_fd to struct object_idLibravatar Patryk Obara1-1/+1
Convert all remaining callers as well. Signed-off-by: Patryk Obara <patryk.obara@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-05builtin/difftool.c: drop hashmap_cmp_fn castLibravatar Stefan Beller1-15/+22
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-30hashmap.h: compare function has access to a data fieldLibravatar Stefan Beller1-9/+15
When using the hashmap a common need is to have access to caller provided data in the compare function. A couple of times we abuse the keydata field to pass in the data needed. This happens for example in patch-ids.c. This patch changes the function signature of the compare function to have one more void pointer available. The pointer given for each invocation of the compare function must be defined in the init function of the hashmap and is just passed through. Documentation of this new feature is deferred to a later patch. This is a rather mechanical conversion, just adding the new pass-through parameter. However while at it improve the naming of the fields of all compare functions used by hashmaps by ensuring unused parameters are prefixed with 'unused_' and naming the parameters what they are (instead of 'unused' make it 'unused_keydata'). Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-24Merge branch 'bw/config-h'Libravatar Junio C Hamano1-0/+1
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-15config: don't include config.h by defaultLibravatar Brandon Williams1-0/+1
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-05-08difftool: address a couple of resource/memory leaksLibravatar Johannes Schindelin1-10/+23
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-04-13difftool: fix use-after-freeLibravatar Johannes Schindelin1-2/+5
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-03-30difftool: avoid strcpyLibravatar Jeff King1-16/+15
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-15difftool: handle modified symlinks in dir-diff modeLibravatar David Aguilar1-5/+46
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-02-06difftool: fix bug when printing usageLibravatar David Aguilar1-4/+4
"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-01-25difftool: hack around -Wzero-length-format warningLibravatar Jeff King1-1/+1
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-19difftool: retire the scripted versionLibravatar Johannes Schindelin1-41/+0
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-19difftool: implement the functionality in the builtinLibravatar Johannes Schindelin1-1/+671
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-17difftool: add a skeleton for the upcoming builtinLibravatar Johannes Schindelin1-0/+63
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>