summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'nd/travis-gcc-8'Junio C Hamano2018-05-302-0/+6
|\ | | | | | | | | | | | | | | Developer support. Use newer GCC on one of the builds done at TravisCI.org to get more warnings and errors diagnosed. * nd/travis-gcc-8: travis-ci: run gcc-8 on linux-gcc jobs
| * travis-ci: run gcc-8 on linux-gcc jobsNguyễn Thái Ngọc Duy2018-05-212-0/+6
| | | | | | | | | | | | | | | | | | | | Switch from gcc-4.8 to gcc-8. Newer compilers come with more warning checks (usually in -Wextra). Since -Wextra is enabled in developer mode (which is also enabled in travis), this lets travis report more warnings before other people do it. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'nd/pack-struct-commit'Junio C Hamano2018-05-301-1/+1
|\ \ | | | | | | | | | | | | | | | | | | Memory optimization. * nd/pack-struct-commit: commit.h: rearrange 'index' to shrink struct commit
| * | commit.h: rearrange 'index' to shrink struct commitNguyễn Thái Ngọc Duy2018-05-131-1/+1
| |/ | | | | | | | | | | | | | | | | | | | | On linux 64-bit architecture, pahole finds that there's a 4 bytes padding after 'index'. Moving it to the end reduces this struct's size from 72 to 64 bytes (because of another 4 bytes padding after graph_pos). On linux 32-bit, the struct size remains 52 bytes like before. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ma/create-pseudoref-with-null-old-oid'Junio C Hamano2018-05-302-5/+77
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git update-ref A B" is supposed to ensure that ref A does not yet exist when B is a NULL OID, but this check was not done correctly for pseudo-refs outside refs/ hierarchy, e.g. MERGE_HEAD. * ma/create-pseudoref-with-null-old-oid: refs: handle zero oid for pseudorefs t1400: add tests around adding/deleting pseudorefs refs.c: refer to "object ID", not "sha1", in error messages
| * | refs: handle zero oid for pseudorefsMartin Ågren2018-05-132-5/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | According to the documentation, it is possible to "specify 40 '0' or an empty string as <oldvalue> to make sure that the ref you are creating does not exist." But in the code for pseudorefs, we do not implement this, as demonstrated by the failing tests added in the previous commit. If we fail to read the old ref, we immediately die. But a failure to read would actually be a good thing if we have been given the zero oid. With the zero oid, allow -- and even require -- the ref-reading to fail. This implements the "make sure that the ref ... does not exist" part of the documentation and fixes both failing tests from the previous commit. Since we have a `strbuf err` for collecting errors, let's use it and signal an error to the caller instead of dying hard. Reported-by: Rafael Ascensão <rafa.almas@gmail.com> Helped-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | t1400: add tests around adding/deleting pseudorefsMartin Ågren2018-05-131-0/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I have not been able to find any tests around adding pseudorefs using `git update-ref`. Add some as outlined in this table (original design by Michael Haggerty; modified and extended by me): Pre-update value | ref-update old OID | Expected result -------------------|----------------------|---------------- missing | value | reject missing | none given | accept set | none given | accept set | correct value | accept set | wrong value | reject missing | zero | accept * set | zero | reject * The tests marked with a * currently fail, despite git-update-ref(1) claiming that it is possible to "specify 40 '0' or an empty string as <oldvalue> to make sure that the ref you are creating does not exist." These failing tests will be fixed in the next commit. It is only natural to test deletion as well. Test deletion without an old OID, with a correct one and with an incorrect one. Suggested-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | refs.c: refer to "object ID", not "sha1", in error messagesMartin Ågren2018-05-131-2/+4
| |/ | | | | | | | | | | | | | | | | | | | | | | | | We have two error messages that complain about the "sha1". Because we are about to touch one of these sites and add some tests, let's first modernize the messages to say "object ID" instead. While at it, make the second one use `error()` instead of `warning()`. After printing the message, we do not continue, but actually drop the lock and return -1 without deleting the pseudoref. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/unavailable-can-be-missing'Junio C Hamano2018-05-301-40/+50
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up to turn history traversal more robust in a semi-corrupt repository. * jk/unavailable-can-be-missing: mark_parents_uninteresting(): avoid most allocation mark_parents_uninteresting(): replace list with stack mark_parents_uninteresting(): drop missing object check mark_tree_contents_uninteresting(): drop missing object check
| * | mark_parents_uninteresting(): avoid most allocationJeff King2018-05-131-19/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 941ba8db57 (Eliminate recursion in setting/clearing marks in commit list, 2012-01-14) used a clever double-loop to avoid allocations for single-parent chains of history. However, it did so only when following parents of parents (which was an uncommon case), and _always_ incurred at least one allocation to populate the list of pending parents in the first place. We can turn this into zero-allocation in the common case by iterating directly over the initial parent list, and then following up on any pending items we might have discovered. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | mark_parents_uninteresting(): replace list with stackJeff King2018-05-131-24/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The mark_parents_uninteresting() function uses two loops: an outer one to process our queue of pending parents, and an inner one to process first-parent chains. This is a clever optimization from 941ba8db57 (Eliminate recursion in setting/clearing marks in commit list, 2012-01-14) to limit the number of linked-list allocations when following single-parent chains. Unfortunately, this makes the result a little hard to read. Let's replace the list with a stack. Then we don't have to worry about doing this double-loop optimization, as we'll just reuse the top element of the stack as we pop/push. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | mark_parents_uninteresting(): drop missing object checkJeff King2018-05-131-12/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We allow UNINTERESTING objects in a traversal to be unavailable. As part of this, mark_parents_uninteresting() checks whether we have a particular uninteresting parent; if not, we will mark it as "parsed" so that later code skips it. This code is redundant and even a little bit harmful, so let's drop it. It's redundant because when our parse_object() call in add_parents_to_list() fails, we already quietly skip UNINTERESTING parents. This redundancy is a historical artifact. The mark_parents_uninteresting() protection is from 454fbbcde3 (git-rev-list: allow missing objects when the parent is marked UNINTERESTING, 2005-07-10). Much later, aeeae1b771 (revision traversal: allow UNINTERESTING objects to be missing, 2009-01-27) covered more cases by making the actual parse more gentle. As an aside, even if this weren't redundant, it would be insufficient. The gentle parsing handles both missing and corrupted objects, whereas the has_object_file() check we're getting rid of covers only missing ones. And the code we're dropping is harmful for two reasons: 1. We spend extra time on the object lookup, even though we don't actually need the information at this point (and will just repeat that lookup later when we parse for the common case that we _do_ have the object). 2. It "lies" about the commit by setting the parsed flag, even though we didn't load any useful data into the struct. This shouldn't matter for the UNINTERESTING case, but we may later clear our flags and do another traversal in the same process. While pretty unlikely, it's possible that we could then look at the same commit without the UNINTERESTING flag, in which case we'd produce the wrong result (we'd think it's a commit with no parents, when in fact we should probably die due to the missing object). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | mark_tree_contents_uninteresting(): drop missing object checkJeff King2018-05-131-4/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's generally acceptable for UNINTERESTING objects in a traversal to be unavailable (e.g., see aeeae1b771). When marking trees UNINTERESTING, we access the object database twice: once to check if the object is missing (and return quietly if it is), and then again to actually parse it. We can instead just try to parse; if that fails, we can then return quietly. That halves the effort we spend on locating the object. Note that this isn't _exactly_ the same as the original behavior, as the parse failure could be due to other problems than a missing object: it could be corrupted, in which case the original code would have died. But the new behavior is arguably better, as it covers the object being unavailable for any reason. We'll also still issue a warning to stderr in such a case. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'bp/status-rename-config'Junio C Hamano2018-05-306-1/+192
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git status" learned to honor a new status.renames configuration to skip rename detection, which could be useful for those who want to do so without disabling the default rename detection done by the "git diff" command. * bp/status-rename-config: add status config and command line options for rename detection
| * | | add status config and command line options for rename detectionBen Peart2018-05-138-2/+194
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After performing a merge that has conflicts git status will, by default, attempt to detect renames which causes many objects to be examined. In a virtualized repo, those objects do not exist locally so the rename logic triggers them to be fetched from the server. This results in the status call taking hours to complete on very large repos vs seconds with this patch. Add a new config status.renames setting to enable turning off rename detection during status and commit. This setting will default to the value of diff.renames. Add a new config status.renamelimit setting to to enable bounding the time spent finding out inexact renames during status and commit. This setting will default to the value of diff.renamelimit. Add --no-renames command line option to status that enables overriding the config setting from the command line. Add --find-renames[=<n>] command line option to status that enables detecting renames and optionally setting the similarity index. Reviewed-by: Elijah Newren <newren@gmail.com> Original-Patch-by: Alejandro Pauly <alpauly@microsoft.com> Signed-off-by: Ben Peart <Ben.Peart@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'js/use-bug-macro'Junio C Hamano2018-05-3069-206/+214
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Developer support update, by using BUG() macro instead of die() to mark codepaths that should not happen more clearly. * js/use-bug-macro: BUG_exit_code: fix sparse "symbol not declared" warning Convert remaining die*(BUG) messages Replace all die("BUG: ...") calls by BUG() ones run-command: use BUG() to report bugs, not die() test-tool: help verifying BUG() code paths
| * | | | BUG_exit_code: fix sparse "symbol not declared" warningRamsay Jones2018-05-102-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | Convert remaining die*(BUG) messagesJohannes Schindelin2018-05-064-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These were not caught by the previous commit, as they did not match the regular expression. While at it, remove the localization from one instance: we never want BUG() messages to be translated, as they target Git developers, not the end user (hence it would be quite unhelpful to not only burden the translators, but then even end up with a bug report in a language that no core Git contributor understands). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | Replace all die("BUG: ...") calls by BUG() onesJohannes Schindelin2018-05-0667-190/+190
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In d8193743e08 (usage.c: add BUG() function, 2017-05-12), a new macro was introduced to use for reporting bugs instead of die(). It was then subsequently used to convert one single caller in 588a538ae55 (setup_git_env: convert die("BUG") to BUG(), 2017-05-12). The cover letter of the patch series containing this patch (cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not terribly clear why only one call site was converted, or what the plan is for other, similar calls to die() to report bugs. Let's just convert all remaining ones in one fell swoop. This trick was performed by this invocation: sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | run-command: use BUG() to report bugs, not die()Johannes Schindelin2018-05-061-13/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The slightly misleading name die_bug() of the function intended to report a bug is actually called always, and only reports a bug if the passed-in parameter `err` is non-zero. It uses die_errno() to report the bug, to helpfully include the error message corresponding to `err`. However, as these messages indicate bugs, we really should use BUG(). And as BUG() is a macro to be able to report the exact file and line number, we need to convert die_bug() to a macro instead of only replacing the die_errno() by a call to BUG(). While at it, use a name more indicative of the purpose: CHECK_BUG(). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | test-tool: help verifying BUG() code pathsJohannes Schindelin2018-05-062-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we call BUG(), we signal via SIGABRT that something bad happened, dumping cores if so configured. In some setups these coredumps are redirected to some central place such as /proc/sys/kernel/core_pattern, which is a good thing. However, when we try to verify in our test suite that bugs are caught in certain code paths, we do *not* want to clutter such a central place with unnecessary coredumps. So let's special-case the test helpers (which we use to verify such code paths) so that the BUG() calls will *not* call abort() but exit with a special-purpose exit code instead. Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'rs/no-null-ptr-arith-in-fast-export'Junio C Hamano2018-05-301-4/+3
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up to avoid non-standard-conformant pointer arithmetic. * rs/no-null-ptr-arith-in-fast-export: fast-export: avoid NULL pointer arithmetic
| * | | | | fast-export: avoid NULL pointer arithmeticRené Scharfe2018-05-101-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Clang 6 reports the following warning, which is turned into an error in a DEVELOPER build: builtin/fast-export.c:162:28: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic] return ((uint32_t *)NULL) + mark; ~~~~~~~~~~~~~~~~~~ ^ 1 error generated. The compiler is correct, and the error message speaks for itself. There is no need for any undefined operation -- just cast mark to void * or uint32_t after an intermediate cast to uintptr_t. That encodes the integer value into a pointer and later decodes it as intended. While at it remove an outdated comment -- intptr_t has been used since ffe659f94d (parse-options: make some arguments optional, add callbacks), committed in October 2007. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'nd/repo-clear-keep-the-index'Junio C Hamano2018-05-301-1/+2
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | the_repository->index is not a allocated piece of memory but repo_clear() indiscriminately attempted to free(3) it, which has been corrected. * nd/repo-clear-keep-the-index: repository: fix free problem with repo_clear(the_repository)
| * | | | | | repository: fix free problem with repo_clear(the_repository)Nguyễn Thái Ngọc Duy2018-05-101-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | the_repository is special. One of the special things about it is that it does not allocate a new index_state object like submodules but points to the global the_index variable instead. As a global variable, the_index cannot be free()'d. Add an exception for this in repo_clear(). In the future perhaps we would be able to allocate the_repository's index on heap too. Then we can revert this. the_repository->index remains pointed to a clean the_index even after repo_clear() so that it could still be used next time (e.g. in a crazy use case where a dev switches repo in the same process). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ma/lockfile-cleanup'Junio C Hamano2018-05-3018-38/+32
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | | | | lock_file: move static locks into functionsMartin Ågren2018-05-106-11/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.) Each of these `struct lock_file`s is used from within a single function. Move them into the respective functions to make the scope clearer and drop the 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. After this commit, the remaining occurrences of "static struct lock_file" are locks that are used from within different functions. That is, they need to remain static. (Short of more intrusive changes like passing around pointers to non-static locks.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | lock_file: make function-local locks non-staticMartin Ågren2018-05-1010-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | | | | refs.c: do not die if locking fails in `delete_pseudoref()`Martin Ågren2018-05-101-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After taking the lock we check whether we got it and die otherwise. But since we take the lock using `LOCK_DIE_ON_ERROR`, we would already have died. Considering the choice between dropping the dead code and dropping the flag, let's go for option number three: Drop the flag, write an error instead of dying, then return -1. This function already returns -1 for another error, so the caller (or rather, its callers) should be able to handle this. There is some inconsistency around how we handle errors in this function and elsewhere in this file, but let's take this small step towards gentle error-reporting now and leave the rest for another time. While at it, make the lock non-static and reduce its scope. (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.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | refs.c: do not die if locking fails in `write_pseudoref()`Martin Ågren2018-05-101-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we could not take the lock, we add an error to the `strbuf err` and return. However, this code is dead. The reason is that we take the lock using `LOCK_DIE_ON_ERROR`. Drop the flag to allow our more gentle error-handling to actually kick in. We could instead just drop the dead code and die here. But everything is prepared for gently propagating the error, so let's do that instead. There is similar dead code in `delete_pseudoref()`, but let's save that for the next patch. While at it, make the lock non-static. (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.) Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | t/helper/test-write-cache: clean up lock-handlingMartin Ågren2018-05-101-9/+5
| | |_|_|_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Die in case writing the index fails, so that the caller can notice (instead of, say, being impressed by how performant the writing is). While at it, note that after opening a lock with `LOCK_DIE_ON_ERROR`, we do not need to worry about whether we succeeded. Also, we can move the `struct lock_file` into the function and drop the staticness. (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.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'sg/t6500-no-redirect-of-stdin'Junio C Hamano2018-05-301-2/+0
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Test cleanup. * sg/t6500-no-redirect-of-stdin: t6050-replace: don't disable stdin for the whole test script
| * | | | | | | t6050-replace: don't disable stdin for the whole test scriptSZEDER Gábor2018-05-091-2/+0
| |/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test script 't6050-replace.sh' starts off with redirecting the whole test script's stdin from /dev/null. This redirection has been there since the test script was introduced in a3e8267225 (replace_object: add a test case, 2009-01-23), but the commit message doesn't explain why it was deemed necessary. AFAICT, it has never been necessary, and t6050 runs just fine and succeeds even without it, not only the current version but past versions as well. Besides being unnecessary, this redirection is also harmful, as it prevents the test helper functions 'test_pause' and 'debug' from working properly in t6050, because we can't enter any commands to the shell and the debugger, respectively. So let's remove that redirection. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'bp/merge-rename-config'Junio C Hamano2018-05-308-14/+69
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With merge.renames configuration set to false, the recursive merge strategy can be told not to spend cycles trying to find renamed paths and merge them accordingly. * bp/merge-rename-config: merge: pass aggressive when rename detection is turned off merge: add merge.renames config setting merge: update documentation for {merge,diff}.renameLimit
| * | | | | | | merge: pass aggressive when rename detection is turned offBen Peart2018-05-081-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Set aggressive flag in git_merge_trees() when rename detection is turned off. This allows read_tree() to auto resolve more cases that would have otherwise been handled by the rename detection. Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Ben Peart <benpeart@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | merge: add merge.renames config settingBen Peart2018-05-087-12/+64
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add the ability to control rename detection for merge via a config setting. This setting behaves the same and defaults to the value of diff.renames but only applies to merge. Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Ben Peart <benpeart@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | merge: update documentation for {merge,diff}.renameLimitBen Peart2018-05-082-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Update the documentation to better indicate that the renameLimit setting is ignored if rename detection is turned off via command line options or config settings. Signed-off-by: Ben Peart <benpeart@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'js/sequencer-and-root-commits'Junio C Hamano2018-05-306-35/+276
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The implementation of "git rebase -i --root" has been updated to use the sequencer machinery more. * js/sequencer-and-root-commits: rebase --rebase-merges: root commits can be cousins, too rebase --rebase-merges: a "merge" into a new root is a fast-forward sequencer: allow introducing new root commits rebase -i --root: let the sequencer handle even the initial part sequencer: learn about the special "fake root commit" handling sequencer: extract helper to update active_cache_tree
| * | | | | | | | rebase --rebase-merges: root commits can be cousins, tooJohannes Schindelin2018-05-062-1/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reported by Wink Saville: when rebasing with no-rebase-cousins, we will want to refrain from rebasing all of them, even when they are root commits. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | rebase --rebase-merges: a "merge" into a new root is a fast-forwardJohannes Schindelin2018-05-062-0/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a user provides a todo list containing something like reset [new root] merge my-branch let's do the same as if pulling into an orphan branch: simply fast-forward. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | sequencer: allow introducing new root commitsJohannes Schindelin2018-05-062-12/+62
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the context of the new --rebase-merges mode, which was designed specifically to allow for changing the existing branch topology liberally, a user may want to extract commits into a completely fresh branch that starts with a newly-created root commit. This is now possible by inserting the command `reset [new root]` before `pick`ing the commit that wants to become a root commit. Example: reset [new root] pick 012345 a commit that is about to become a root commit pick 234567 this commit will have the previous one as parent This does not conflict with other uses of the `reset` command because `[new root]` is not (part of) a valid ref name: both the opening bracket as well as the space are illegal in ref names. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | rebase -i --root: let the sequencer handle even the initial partJohannes Schindelin2018-05-063-10/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In this developer's earlier attempt to accelerate interactive rebases by converting large parts from Unix shell script into portable, performant C, the --root handling was specifically excluded (to simplify the task a little bit; it still took over a year to get that reduced set of patches into Git proper). This patch ties up that loose end: now only --preserve-merges uses the slow Unix shell script implementation to perform the interactive rebase. As the rebase--helper reports progress to stderr (unlike the scripted interactive rebase, which reports it to stdout, of all places), we have to adjust a couple of tests that did not expect that for `git rebase -i --root`. This patch fixes -- at long last! -- the really old bug reported in 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with --root *always* rewrote the root commit, even if there were no changes. The bug still persists in --preserve-merges mode, of course, but that mode will be deprecated as soon as the new --rebase-merges mode stabilizes, anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | sequencer: learn about the special "fake root commit" handlingJohannes Schindelin2018-05-062-3/+125
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an interactive rebase wants to recreate a root commit, it - first creates a new, empty root commit, - checks it out, - converts the next `pick` command so that it amends the empty root commit Introduce support in the sequencer to handle such an empty root commit, by looking for the file <GIT_DIR>/rebase-merge/squash-onto; if it exists and contains a commit name, the sequencer will compare the HEAD to said root commit, and if identical, a new root commit will be created. While converting scripted code into proper, portable C, we also do away with the old "amend with an empty commit message, then cherry-pick without committing, then amend again" dance and replace it with code that uses the internal API properly to do exactly what we want: create a new root commit. To keep the implementation simple, we always spawn `git commit` to create new root commits. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | sequencer: extract helper to update active_cache_treeJohannes Schindelin2018-05-061-9/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch extracts the code from is_index_unchanged() to initialize or update the index' cache tree (i.e. a tree object reflecting the current index' top-level tree). The new helper will be used in the upcoming code to support `git rebase -i --root` via the sequencer. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | Merge branch 'dd/send-email-reedit'Junio C Hamano2018-05-301-7/+31
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git send-email" can sometimes offer confirmation dialog "Send this email?" with choices 'Yes', 'No', 'Quit', and 'All'. A new action 'Edit' has been added to this dialog's choice. * dd/send-email-reedit: git-send-email: allow re-editing of message
| * | | | | | | | | git-send-email: allow re-editing of messageDrew DeVault2018-05-061-7/+31
| | |_|_|_|_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When shown the email summary, an opportunity is presented for the user to edit the email as if they had specified --annotate. This also permits them to edit it multiple times. Signed-off-by: Drew DeVault <sir@cmpwn.com> Reviewed-by: Simon Ser <contact@emersion.fr> Helped-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | Sync with Git 2.17.1Junio C Hamano2018-05-2928-72/+794
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * maint: (25 commits) Git 2.17.1 Git 2.16.4 Git 2.15.2 Git 2.14.4 Git 2.13.7 fsck: complain when .gitmodules is a symlink index-pack: check .gitmodules files with --strict unpack-objects: call fsck_finish() after fscking objects fsck: call fsck_finish() after fscking objects fsck: check .gitmodules content fsck: handle promisor objects in .gitmodules check fsck: detect gitmodules files fsck: actually fsck blob data fsck: simplify ".git" check index-pack: make fsck error message more specific verify_path: disallow symlinks in .gitmodules update-index: stat updated files earlier verify_dotfile: mention case-insensitivity in comment verify_path: drop clever fallthrough skip_prefix: add case-insensitive variant ...
| * | | | | | | | | Git 2.17.1v2.17.1Junio C Hamano2018-05-223-2/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | Merge branch 'jk/submodule-fsck-loose' into maintJunio C Hamano2018-05-228-30/+271
| |\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * jk/submodule-fsck-loose: fsck: complain when .gitmodules is a symlink index-pack: check .gitmodules files with --strict unpack-objects: call fsck_finish() after fscking objects fsck: call fsck_finish() after fscking objects fsck: check .gitmodules content fsck: handle promisor objects in .gitmodules check fsck: detect gitmodules files fsck: actually fsck blob data fsck: simplify ".git" check index-pack: make fsck error message more specific
| | * | | | | | | | | fsck: complain when .gitmodules is a symlinkJeff King2018-05-212-2/+38
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We've recently forbidden .gitmodules to be a symlink in verify_path(). And it's an easy way to circumvent our fsck checks for .gitmodules content. So let's complain when we see it. Signed-off-by: Jeff King <peff@peff.net>