summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2020-01-16built-in add -i: do not try to `patch`/`diff` an empty list of filesLibravatar Johannes Schindelin1-2/+2
When the user does not select any files to `patch` or `diff`, there is no need to call `run_add_p()` on them. Even worse: we _have_ to avoid calling `parse_pathspec()` with an empty list because that would trigger this error: BUG: pathspec.c:557: PATHSPEC_PREFER_CWD requires arguments So let's avoid doing any work on a list of files that is empty anyway. This fixes https://github.com/git-for-windows/git/issues/2466. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: offer the `quit` commandLibravatar Johannes Schindelin1-5/+11
We do not really want to `exit()` here, of course, as this is safely libified code. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: re-implement the `diff` commandLibravatar Johannes Schindelin1-0/+42
It is not only laziness that we simply spawn `git diff -p --cached` here: this command needs to use the pager, and the pager needs to exit when the diff is done. Currently we do not have any way to make that happen if we run the diff in-process. So let's just spawn. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: implement the `patch` commandLibravatar Johannes Schindelin1-7/+84
Well, it is not a full implementation yet. In the interest of making this easy to review (and easy to keep bugs out), we still hand off to the Perl script to do the actual work. The `patch` functionality actually makes up for more than half of the 1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl to C incrementally, later. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: re-implement `add-untracked` in CLibravatar Johannes Schindelin1-0/+91
This is yet another command, ported to C. It builds nicely on the support functions introduced for other commands, with the notable difference that only names are displayed for untracked files, no file type or diff summary. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: re-implement `revert` in CLibravatar Johannes Schindelin1-0/+109
This is a relatively straight-forward port from the Perl version, with the notable exception that we imitate `git reset -- <paths>` in the C version rather than the convoluted `git ls-tree HEAD -- <paths> | git update-index --index-info` followed by `git update-index --force-remove -- <paths>` for the missed ones. While at it, we fix the pretty obvious bug where the `revert` command offers to unstage files that do not have staged changes. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: implement the `update` commandLibravatar Johannes Schindelin1-20/+110
After `status` and `help`, it is now time to port the `update` command to C, the second command that is shown in the main loop menu of `git add -i`. This `git add -i` command is the first one which lets the user choose a subset of a list of files, and as such, this patch lays the groundwork for the other commands of that category: - It teaches the `print_file_item()` function to show a unique prefix if we found any (the code to find it had been added already in the previous patch where we colored the unique prefixes of the main loop commands, but that patch uses the `print_command_item()` function to display the menu items). - This patch also adds the help text that is shown when the user input to select items from the shown list could not be parsed. - As `get_modified_files()` clears the list of files, it now has to take care of clearing the _full_ `prefix_item_list` lest the `sorted` and `selected` fields go stale and inconsistent. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: prepare for multi-selection commandsLibravatar Johannes Schindelin1-25/+89
The `update`, `revert` and `add-untracked` commands allow selecting multiple entries. Let's extend the `list_and_choose()` function to accommodate those use cases. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01built-in add -i: allow filtering the modified files listLibravatar Johannes Schindelin1-7/+26
In the `update` command of `git add -i`, we are primarily interested in the list of modified files that have worktree (i.e. unstaged) changes. At the same time, we need to determine _also_ the staged changes, to be able to produce the full added/deleted information. The Perl script version of `git add -i` has a parameter of the `list_modified()` function for that matter. In C, we can be a lot more precise, using an `enum`. The C implementation of the filter also has an easier time to avoid unnecessary work, simply by using an adaptive order of the `diff-index` and `diff-files` phases, and then skipping files in the second phase when they have not been seen in the first phase. Seeing as we change the meaning of the `phase` field, we rename it to `mode` to reflect that the order depends on the exact invocation of the `git add -i` command. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01add-interactive: make sure to release `rev.prune_data`Libravatar Johannes Schindelin1-0/+3
During a review, Junio Hamano pointed out that the `rev.prune_data` was copied from another pathspec but never cleaned up. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18built-in add -i: implement the `help` commandLibravatar Slavica Đukić2-0/+46
This imitates the code to show the help text from the Perl script `git-add--interactive.perl` in the built-in version. To make sure that it renders exactly like the Perl version of `git add -i`, we also add a test case for that to `t3701-add-interactive.sh`. Signed-off-by: Slavica Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18built-in add -i: use color in the main loopLibravatar Slavica Đukić1-6/+29
The error messages as well as the unique prefixes are colored in `git add -i` by default; We need to do the same in the built-in version. Signed-off-by: Slavica Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18built-in add -i: support `?` (prompt help)Libravatar Johannes Schindelin1-1/+21
With this change, we print out the same colored help text that the Perl-based `git add -i` prints in the main loop when question mark is entered. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18built-in add -i: show unique prefixes of the commandsLibravatar Johannes Schindelin1-11/+177
Just like in the Perl script `git-add--interactive.perl`, for each command a unique prefix is determined (if there exists any within the given parameters), and shown in the list, and accepted as a shortcut for the command. To determine the unique prefixes, as well as to look up the command in question, we use a copy of the list and sort it. While this might seem like overkill for a single command, it will make much more sense when all the commands are implemented, and when we reuse the same logic to present a list of files to edit, with convenient unique prefixes. At the start of the development of this patch series, a dedicated data structure was introduced that imitated the Trie that the Perl version implements. However, this was deemed overkill, and we now simply sort the list before determining the length of the unique prefixes by looking at each item's neighbor. As a bonus, we now use the same sorted list to perform a binary search using the user-provided prefix as search key. Original-patch-by: Slavica Đukić <slawica92@hotmail.com> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18built-in add -i: implement the main loopLibravatar Johannes Schindelin1-2/+135
The reason why we did not start with the main loop to begin with is that it is the first user of `list_and_choose()`, which uses the `list()` function that we conveniently introduced for use by the `status` command. In contrast to the Perl version, in the built-in interactive `add`, we will keep the `list()` function (which only displays items) and the `list_and_choose()` function (which uses `list()` to display the items, and only takes care of the "and choose" part) separate. The `list_and_choose()` function, as implemented in `git-add--interactive.perl` knows a few more tricks than the function we introduce in this patch: - There is a flag to let the user select multiple items. - In multi-select mode, the list of items is prefixed with a marker indicating what items have been selected. - Initially, for each item a unique prefix is determined (if there exists any within the given parameters), and shown in the list, and accepted as a shortcut for the selection. These features will be implemented in the C version later. This patch does not add any new main loop command, of course, the built-in `git add -i` still only supports the `status` command. The remaining commands to follow over the course of the next commits. To accommodate for listing the commands in columns, preparing for the commands that will be implemented over the course of the next patches/patch series, we teach the `list()` function to do precisely that. Note that we only have a prompt ending in a single ">" at this stage; later commits will add commands that display a double ">>" to indicate that the user is in a different loop than the main one. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-14built-in add -i: color the header in the `status` commandLibravatar Slavica Đukić1-4/+37
For simplicity, we only implemented the `status` command without colors. This patch starts adding color, matching what the Perl script `git-add--interactive.perl` does. Original-Patch-By: Daniel Ferreira <bnmvco@gmail.com> Signed-off-by: Slavica Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-14built-in add -i: implement the `status` commandLibravatar Daniel Ferreira1-1/+250
This implements the `status` command of `git add -i`. The data structures introduced in this commit will be extended later, as needed. At this point, we re-implement only part of the `list_and_choose()` function of the Perl script `git-add--interactive.perl` and call it `list()`. It does not yet color anything, or do columns, or allow user input. Over the course of the next commits, we will introduce a `list_and_choose()` function that uses `list()` to display the list of options and let the user choose one or more of the displayed items. This will be used to implement the main loop of the built-in `git add -i`, at which point the new `status` command can actually be used. Signed-off-by: Daniel Ferreira <bnmvco@gmail.com> Signed-off-by: Slavica Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-14diff: export diffstat interfaceLibravatar Daniel Ferreira2-23/+36
Make the diffstat interface (namely, the diffstat_t struct and compute_diffstat) no longer be internal to diff.c and allow it to be used by other parts of git. This is helpful for code that may want to easily extract information from files using the diff machinery, while flushing it differently from how the show_* functions used by diff_flush() do it. One example is the builtin implementation of git-add--interactive's status. Signed-off-by: Daniel Ferreira <bnmvco@gmail.com> Signed-off-by: Slavica Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-14Start to implement a built-in version of `git add --interactive`Libravatar Johannes Schindelin6-0/+37
Unlike previous conversions to C, where we started with a built-in helper, we start this conversion by adding an interception in the `run_add_interactive()` function when the new opt-in `add.interactive.useBuiltin` config knob is turned on (or the corresponding environment variable `GIT_TEST_ADD_I_USE_BUILTIN`), and calling the new internal API function `run_add_i()` that is implemented directly in libgit.a. At this point, the built-in version of `git add -i` only states that it cannot do anything yet. In subsequent patches/patch series, the `run_add_i()` function will gain more and more functionality, until it is feature complete. The whole arc of the conversion can be found in the PRs #170-175 at https://github.com/gitgitgadget/git. The "--helper approach" can unfortunately not be used here: on Windows we face the very specific problem that a `system()` call in Perl seems to close `stdin` in the parent process when the spawned process consumes even one character from `stdin`. Which prevents us from implementing the main loop in C and still trying to hand off to the Perl script. The very real downside of the approach we have to take here is that the test suite won't pass with `GIT_TEST_ADD_I_USE_BUILTIN=true` until the conversion is complete (the `--helper` approach would have let it pass, even at each of the incremental conversion steps). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-04Git 2.24Libravatar Junio C Hamano1-1/+1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-04Merge branch 'bc/doc-use-docbook-5'Libravatar Junio C Hamano1-1/+2
Finishing touches to the recent update to the build procedure for the documentation. * bc/doc-use-docbook-5: manpage-bold-literal.xsl: match for namespaced "d:literal" in template
2019-11-04Merge branch 'ds/commit-graph-on-fetch'Libravatar Junio C Hamano4-6/+25
Regression fix. * ds/commit-graph-on-fetch: commit-graph: fix writing first commit-graph during fetch t5510-fetch.sh: demonstrate fetch.writeCommitGraph bug
2019-11-04Merge branch 'jt/delay-fetch-if-missing'Libravatar Junio C Hamano1-2/+2
Work-around a lazy fetch glitch. * jt/delay-fetch-if-missing: fetch: delay fetch_if_missing=0 until after config
2019-11-04Merge https://github.com/prati0100/git-guiLibravatar Junio C Hamano4-21/+210
* https://github.com/prati0100/git-gui: git-gui: improve Japanese translation git-gui: add a readme git-gui: support for diff3 conflict style git-gui: use existing interface to query a path's attribute git-gui (Windows): use git-bash.exe if it is available treewide: correct several "up-to-date" to "up to date" Fix build with core.autocrlf=true
2019-11-04Merge tag 'l10n-2.24.0-rnd2' of https://github.com/git-l10n/git-poLibravatar Junio C Hamano10-22221/+23300
l10n-2.24.0-rnd2 * tag 'l10n-2.24.0-rnd2' of https://github.com/git-l10n/git-po: l10n: zh_CN: for git v2.24.0 l10n round 1~2 l10n: de.po: Update German translation l10n: sv.po: Update Swedish translation (4695t0f0u) l10n: bg.po: Updated Bulgarian translation (4694) l10n: vi(4694t): Updated translation for v2.24.0 l10n: es: 2.24.0 round 2 l10n: it.po: update the Italian translation for Git 2.24.0 round #2 l10n: fr v2.24.0 rnd2 l10n: git.pot: v2.24.0 round 2 (1 new) l10n: it.po: update the Italian translation for Git 2.24.0 l10n: fr 2.24.0 rnd 1 l10n: git.pot: v2.24.0 round 1 (35 new, 16 removed) l10n: bg.po: Updated Bulgarian translation (4693) l10n: sv.po: Update Swedish translation (4674t0f0u) l10n: Update Catalan translation
2019-11-02l10n: zh_CN: for git v2.24.0 l10n round 1~2Libravatar Jiang Xin1-2439/+2539
Translate 36 new messages (4694t0f0u) for git 2.24.0. Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2019-11-02RelNotes/2.24.0: fix self-contradictory noteLibravatar Elijah Newren1-4/+3
As per Wikipedia, "In current technical usage, for one to state that a feature is deprecated is merely a recommendation against using it." It is thus contradictory to claim that something is not "officially deprecated" and then to immediately state that we are both discouraging its use and pointing people elsewhere. Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02manpage-bold-literal.xsl: match for namespaced "d:literal" in templateLibravatar Martin Ågren1-1/+2
We recently regressed our rendering with Asciidoctor of "literal" elements in our manpages, i.e, stuff we have placed within `backticks` in order to render as monospace. In particular, we lost the bold rendering of such literal text. The culprit is f6461b82b9 ("Documentation: fix build with Asciidoctor 2", 2019-09-15), where we switched from DocBook 4.5 to DocBook 5 with Asciidoctor. As part of the switch, we started using the namespaced DocBook XSLT stylesheets rather than the non-namespaced ones. (See f6461b82b9 for more details on why we changed to the namespaced ones.) The bold literals are implemented as an XSLT snippet <xsl:template match="literal">...</xsl:template>. Now that we use namespaces, this doesn't pick up our literals like it used to. Match for "d:literal" in addition to just "literal", after defining the d namespace. ("d" is what http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl uses.) Note that we need to keep matching without the namespace for AsciiDoc. This boldness was introduced by 5121a6d993 ("Documentation: option to render literal text as bold for manpages", 2009-03-27) and made the default in 5945717009 ("Documentation: bold literals in man", 2016-05-31). One reason this was not caught in review is that our doc-diff tool diffs without any boldness, i.e., it "only" compares text. As pointed out by Peff in review of this patch, one can use `MAN_KEEP_FORMATTING=1 ./doc-diff <...>` This has been optically tested with AsciiDoc 8.6.10, Asciidoctor 1.5.5 and Asciidoctor 2.0.10. I've also verified that doc-diff produces the empty output for all three programs, as expected, and that with the MAN_KEEP_FORMATTING trick, AsciiDoc yields no diff, whereas with Asciidoctor, we get bold literals, just like we want. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Acked-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02RelNotes/2.24.0: typofixLibravatar Elijah Newren1-1/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-31l10n: de.po: Update German translationLibravatar Matthias Rüster1-2454/+2590
Signed-off-by: Matthias Rüster <matthias.ruester@gmail.com> Reviewed-by: Ralf Thielow <ralf.thielow@gmail.com> Reviewed-by: Phillip Szelat <phillip.szelat@gmail.com>
2019-10-30l10n: sv.po: Update Swedish translation (4695t0f0u)Libravatar Peter Krefting1-2455/+2581
Signed-off-by: Peter Krefting <peter@softwolves.pp.se>
2019-10-30Git 2.24-rc2Libravatar Junio C Hamano1-0/+2
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-30Merge branch 'wb/fsmonitor-bitmap-fix'Libravatar Junio C Hamano1-3/+5
Comment update. * wb/fsmonitor-bitmap-fix: t7519-status-fsmonitor: improve comments
2019-10-30Merge branch 'rl/gitweb-blame-prev-fix'Libravatar Junio C Hamano1-1/+1
Fix a rather old bug in gitweb---incremental blame output in javascript actions mode never worked. * rl/gitweb-blame-prev-fix: gitweb: correctly store previous rev in javascript-actions mode
2019-10-30Merge branch 'js/mingw-needs-hiding-fix'Libravatar Junio C Hamano1-0/+2
Fix for a (rather old) buffer-overrun bug. * js/mingw-needs-hiding-fix: mingw: avoid a buffer overrun in `needs_hiding()`
2019-10-30Merge branch 'master' of github.com:vnwildman/gitLibravatar Jiang Xin1-2454/+2586
* 'master' of github.com:vnwildman/git: l10n: vi(4694t): Updated translation for v2.24.0
2019-10-30Merge branch 'next' of github.com:ChrisADR/git-poLibravatar Jiang Xin1-2541/+2679
* 'next' of github.com:ChrisADR/git-po: l10n: es: 2.24.0 round 2
2019-10-30t7519-status-fsmonitor: improve commentsLibravatar William Baker1-3/+5
The comments for the staging/unstaging test did not accurately describe the scenario being tested. It is not essential that the test files being staged/unstaged appear at the end of the index. All that is required is that the test files are not flagged with CE_FSMONITOR_VALID and have a position in the index greater than the number of entries in the index after unstaging. The comment for this test has been updated to be more accurate with respect to the scenario that's being tested. Signed-off-by: William Baker <William.Baker@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-29l10n: bg.po: Updated Bulgarian translation (4694)Libravatar Alexander Shopov1-87/+51
Signed-off-by: Alexander Shopov <ash@kambanaria.org>
2019-10-29l10n: vi(4694t): Updated translation for v2.24.0Libravatar Tran Ngoc Quan1-2454/+2586
Signed-off-by: Tran Ngoc Quan <vnwildman@gmail.com>
2019-10-28l10n: es: 2.24.0 round 2Libravatar Christopher Diaz Riveros1-2541/+2679
Signed-off-by: Christopher Diaz Riveros <christopher.diaz.riv@gmail.com>
2019-10-29Merge branch 'l10n/it/update-italian-translation'Libravatar Jiang Xin1-61/+67
* 'update-italian-translation' of github.com:AlessandroMenti/git-po: l10n: it.po: update the Italian translation for Git 2.24.0 round #2
2019-10-28l10n: it.po: update the Italian translation for Git 2.24.0 round #2Libravatar Alessandro Menti1-61/+67
Signed-off-by: Alessandro Menti <alessandro.menti@alessandromenti.it>
2019-10-28l10n: fr v2.24.0 rnd2Libravatar Jean-Noël Avila1-55/+75
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
2019-10-28l10n: git.pot: v2.24.0 round 2 (1 new)Libravatar Jiang Xin1-45/+50
Generate po/git.pot from v2.24.0-rc1 for git v2.24.0 l10n round 2. Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2019-10-28Merge tag 'v2.24.0-rc1' of github.com:git/git into masterLibravatar Jiang Xin27-29/+247
Git 2.24-rc1 * tag 'v2.24.0-rc1' of github.com:git/git: Git 2.24-rc1 repo-settings: read an int for index.version ci: fix GCC install in the Travis CI GCC OSX job Eleventh batch ci(osx): use new location of the `perforce` cask t7419: change test_must_fail to ! for grep t4014: make output-directory tests self-contained ci(visual-studio): actually run the tests in parallel ci(visual-studio): use strict compile flags, and optimization userdiff: fix some corner cases in dts regex test-progress: fix test failures on big-endian systems completion: clarify installation instruction for zsh grep: avoid leak of chartables in PCRE2 grep: make PCRE2 aware of custom allocator grep: make PCRE1 aware of custom allocator remote-curl: pass on atomic capability to remote side diff-highlight: fix a whitespace nit fsmonitor: don't fill bitmap with entries to be removed
2019-10-28mingw: avoid a buffer overrun in `needs_hiding()`Libravatar Johannes Schindelin1-0/+2
When this function is passed a path with a trailing slash, it runs right over the end of that path. Let's fix this. Co-authored-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-28gitweb: correctly store previous rev in javascript-actions modeLibravatar Robert Luberda1-1/+1
Without this change, the setting $feature{'javascript-actions'}{'default'} = [1]; in gitweb.conf breaks gitweb's blame page: clicking on line numbers displayed in the second column on the page has no effect. For comparison, with javascript-actions disabled, clicking on line numbers loads the previous version of the line. Addresses https://bugs.debian.org/741883. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Robert Luberda <robert@debian.org> Acked-by: Jakub Narębski <jnareb@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-25commit-graph: fix writing first commit-graph during fetchLibravatar Derrick Stolee4-7/+10
The previous commit includes a failing test for an issue around fetch.writeCommitGraph and fetching in a repo with a submodule. Here, we fix that bug and set the test to "test_expect_success". The problem arises with this set of commands when the remote repo at <url> has a submodule. Note that --recurse-submodules is not needed to demonstrate the bug. $ git clone <url> test $ cd test $ git -c fetch.writeCommitGraph=true fetch origin Computing commit graph generation numbers: 100% (12/12), done. BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2> Aborted (core dumped) As an initial fix, I converted the code in builtin/fetch.c that calls write_commit_graph_reachable() to instead launch a "git commit-graph write --reachable --split" process. That code worked, but is not how we want the feature to work long-term. That test did demonstrate that the issue must be something to do with internal state of the 'git fetch' process. The write_commit_graph() method in commit-graph.c ensures the commits we plan to write are "closed under reachability" using close_reachable(). This method walks from the input commits, and uses the UNINTERESTING flag to mark which commits have already been visited. This allows the walk to take O(N) time, where N is the number of commits, instead of O(P) time, where P is the number of paths. (The number of paths can be exponential in the number of commits.) However, the UNINTERESTING flag is used in lots of places in the codebase. This flag usually means some barrier to stop a commit walk, such as in revision-walking to compare histories. It is not often cleared after the walk completes because the starting points of those walks do not have the UNINTERESTING flag, and clear_commit_marks() would stop immediately. This is happening during a 'git fetch' call with a remote. The fetch negotiation is comparing the remote refs with the local refs and marking some commits as UNINTERESTING. I tested running clear_commit_marks_many() to clear the UNINTERESTING flag inside close_reachable(), but the tips did not have the flag, so that did nothing. It turns out that the calculate_changed_submodule_paths() method is at fault. Thanks, Peff, for pointing out this detail! More specifically, for each submodule, the collect_changed_submodules() runs a revision walk to essentially do file-history on the list of submodules. That revision walk marks commits UNININTERESTING if they are simplified away by not changing the submodule. Instead, I finally arrived on the conclusion that I should use a flag that is not used in any other part of the code. In commit-reach.c, a number of flags were defined for commit walk algorithms. The REACHABLE flag seemed like it made the most sense, and it seems it was not actually used in the file. The REACHABLE flag was used in early versions of commit-reach.c, but was removed by 4fbcca4 (commit-reach: make can_all_from_reach... linear, 2018-07-20). Add the REACHABLE flag to commit-graph.c and use it instead of UNINTERESTING in close_reachable(). This fixes the bug in manual testing. Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Jeff King <peff@peff.net> Helped-by: Szeder Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-25t5510-fetch.sh: demonstrate fetch.writeCommitGraph bugLibravatar Derrick Stolee1-0/+16
While dogfooding, Johannes found a bug in the fetch.writeCommitGraph config behavior. His example initially happened during a clone with --recurse-submodules, we found that this happens with the first fetch after cloning a repository that contains a submodule: $ git clone <url> test $ cd test $ git -c fetch.writeCommitGraph=true fetch origin Computing commit graph generation numbers: 100% (12/12), done. BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2> Aborted (core dumped) In the repo I had cloned, there were really 60 commits to scan, but only 12 were in the list to write when calling compute_generation_numbers(). A commit in the list expects to see a parent, but that parent is not in the list. A follow-up will fix the bug, but first we create a test that demonstrates the problem. This test must be careful about an existing commit-graph file, since GIT_TEST_COMMIT_GRAPH=1 will cause the repo we are cloning to already have one. This then prevents the incremtnal commit-graph write during the first 'git fetch'. Helped-by: Jeff King <peff@peff.net> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Szeder Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>