aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Merge branch 'jk/ui-color-always-to-auto-maint' into jk/ui-color-always-to-autoJunio C Hamano2017-10-04
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | * jk/ui-color-always-to-auto-maint: color: make "always" the same as "auto" in config provide --color option for all ref-filter users t3205: use --color instead of color.branch=always t3203: drop "always" color test t6006: drop "always" color config tests t7502: use diff.noprefix for --verbose test t7508: use test_terminal for color output t3701: use test-terminal to collect color output t4015: prefer --color to -c color.diff=always test-terminal: set TERM=vt100
| * color: make "always" the same as "auto" in configJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It can be handy to use `--color=always` (or it's synonym `--color`) on the command-line to convince a command to produce color even if it's stdout isn't going to the terminal or a pager. What's less clear is whether it makes sense to set config variables like color.ui to `always`. For a one-shot like: git -c color.ui=always ... it's potentially useful (especially if the command doesn't directly support the `--color` option). But setting `always` in your on-disk config is much muddier, as you may be surprised when piped commands generate colors (and send them to whatever is consuming the pipe downstream). Some people have done this anyway, because: 1. The documentation for color.ui makes it sound like using `always` is a good idea, when you almost certainly want `auto`. 2. Traditionally not every command (and especially not plumbing) respected color.ui in the first place. So the confusion came up less frequently than it might have. The situation changed in 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13), which negated point (2): now scripts using only plumbing commands (like add-interactive) are broken by this setting. That commit was fixing real issues (e.g., by making `color.ui=never` work, since `auto` is the default), so we don't want to just revert it. We could turn `always` into a noop in plumbing commands, but that creates a hard-to-explain inconsistency between the plumbing and other commands. Instead, let's just turn `always` into `auto` for all config. This does break the "one-shot" config shown above, but again, we're probably better to have simple and consistent rules than to try to special-case command-line config. There is one place where `always` should retain its meaning: on the command line, `--color=always` should continue to be the same as `--color`, overriding any isatty checks. Since the command-line parser also depends on git_config_colorbool(), we can use the existence of the "var" string to deterine whether we are serving the command-line or the config. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * provide --color option for all ref-filter usersJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When ref-filter learned about want_color() in 11b087adfd (ref-filter: consult want_color() before emitting colors, 2017-07-13), it became useful to be able to turn colors off and on for specific commands. For git-branch, you can do so with --color/--no-color. But for git-for-each-ref and git-tag, the other users of ref-filter, you have no option except to tweak the "color.ui" config setting. Let's give both of these commands the usual color command-line options. This is a bit more obvious as a method for overriding the config. And it also prepares us for the behavior of "always" changing (so that we are still left with a way of forcing color when our output goes to a non-terminal). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t3205: use --color instead of color.branch=alwaysJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | To test the color output, we must convince "git branch" to write colors to a non-terminal. We do that now by setting the color config to "always". In preparation for the behavior of "always" changing, let's switch to using the "--color" command-line option, which is more direct. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t3203: drop "always" color testJeff King2017-10-04
| | | | | | | | | | | | | | | | | | In preparation for the behavior of "always" changing to match "auto", we can simply drop this test. We already check other forms (like "--color") independently. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t6006: drop "always" color config testsJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We test the %C() format placeholders with a variety of color-inducing options, including "--color" and "-c color.ui=always". In preparation for the behavior of "always" changing, we need to do something with those "always" tests. We can drop ones that expect "always" to turn on color even to a file, as that will become a synonym for "auto", which is already tested. For the "--no-color" test, we need to make sure that color would otherwise be shown. To do this, we can use test_terminal, which enables colors in the default setup. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t7502: use diff.noprefix for --verbose testJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | To check that "status -v" respects diff config, we set "color.diff" and look at the output of "status". We could equally well use any diff config. Since color output depends on a lot of other factors (like whether stdout is a tty, and how we interpret "always"), let's use a more mundane option. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t7508: use test_terminal for color outputJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This script tests the output of status with various formats when color is enabled. It uses the "always" setting so that the output is valid even though we capture it in a file. Using test_terminal gives us a more realistic environment, and prepares us for the behavior of "always" changing. Arguably we are testing less than before, since "auto" is already the default, and we can no longer tell if the config is actually doing anything. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t3701: use test-terminal to collect color outputJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When testing whether "add -p" can generate colors, we set color.ui to "always". This isn't a very good test, as in the real-world a user typically has "auto" coupled with stdout going to a terminal (and it's plausible that this could mask a real bug in add--interactive if we depend on plumbing's isatty check). Let's switch to test_terminal, which gives us a more realistic environment. This also prepare us for future changes to the "always" color option. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t4015: prefer --color to -c color.diff=alwaysJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | t4015 contains many color-related tests which need to override the "is stdout a tty" check. They do so by setting the color.diff config, but we can accomplish the same with the --color option. Besides being shorter to type, switching will prepare us for upcoming changes to "always" when see it in config. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * test-terminal: set TERM=vt100Jeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The point of the test-terminal script is to simulate in the test scripts an environment where output is going to a real terminal. But since test-lib.sh also sets TERM=dumb, the simulation isn't very realistic. The color code will skip auto-coloring for TERM=dumb, leading to us liberally sprinkling test_terminal env TERM=vt100 git ... through the test suite to convince the tests to actually generate colors. Let's set TERM for programs run under test_terminal, which is one less thing for test-writers to remember. In most cases the callers can be simplified, but note there is one interesting case in t4202. It uses test_terminal to check the auto-enabling of --decorate, but the expected output _doesn't_ contain colors (because TERM=dumb suppresses them). Using TERM=vt100 is closer to what the real world looks like; adjust the expected output to match. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | t7301: use test_terminal to check colorJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | This test wants to confirm that "clean -i" shows color output. Using test_terminal gives us a more realistic environment than "color.ui=always", and prepares us for the behavior of "always" changing in a future patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | t4015: use --color with --color-movedJeff King2017-10-04
| | | | | | | | | | | | | | | | | | | | | | The tests for --color-moved write their output to a file, but doing so suppresses color output under "auto". Right now this is solved by running the whole script under "color.diff=always". In preparation for the behavior of "always" changing, let's explicitly enable color. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | The twelfth batch for 2.15Junio C Hamano2017-10-03
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'bw/git-clang-format'Junio C Hamano2017-10-03
|\ \ | | | | | | | | | | | | | | | | | | Adjust clang-format penalty parameters. * bw/git-clang-format: clang-format: adjust line break penalties
| * | clang-format: adjust line break penaltiesJohannes Schindelin2017-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We really, really, really want to limit the columns to 80 per line: One of the few consistent style comments on the Git mailing list is that the lines should not have more than 80 columns/line (even if 79 columns/line would make more sense, given that the code is frequently viewed as diff, and diffs adding an extra character). The penalty of 5 for excess characters is way too low to guarantee that, though, as pointed out by Brandon Williams. From the existing clang-format examples and documentation, it appears that 100 is a penalty deemed appropriate for Stuff You Really Don't Want, so let's assign that as the penalty for "excess characters", i.e. overly long lines. While at it, adjust the penalties further: we are actually not that keen on preventing new line breaks within comments or string literals, so the penalty of 100 seems awfully high. Likewise, we are not all that adamant about keeping line breaks away from assignment operators (a lot of Git's code breaks immediately after the `=` character just to keep that 80 columns/line limit). We do frown a little bit more about functions' return types being on their own line than the penalty 0 would suggest, so this was adjusted, too. Finally, we do not particularly fancy breaking before the first parameter in a call, but if it keeps the line shorter than 80 columns/line, that's what we do, so lower the penalty for breaking before a call's first parameter, but not quite as much as introducing new line breaks to comments. 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>
* | | Merge branch 'ad/doc-markup-fix'Junio C Hamano2017-10-03
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Docfix. * ad/doc-markup-fix: doc: correct command formatting
| * | | doc: correct command formattingAdam Dinwoodie2017-09-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Leaving spaces around the `-delimeters for commands means asciidoc fails to parse them as the start of a literal string. Remove an extraneous space that is causing a literal to not be formatted as such. Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> Acked-by: Andreas Heiduk <asheiduk@gmail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'mh/mmap-packed-refs'Junio C Hamano2017-10-03
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Operations that do not touch (majority of) packed refs have been optimized by making accesses to packed-refs file lazy; we no longer pre-parse everything, and an access to a single ref in the packed-refs does not touch majority of irrelevant refs, either. * mh/mmap-packed-refs: (21 commits) packed-backend.c: rename a bunch of things and update comments mmapped_ref_iterator: inline into `packed_ref_iterator` ref_cache: remove support for storing peeled values packed_ref_store: get rid of the `ref_cache` entirely ref_store: implement `refs_peel_ref()` generically packed_read_raw_ref(): read the reference from the mmapped buffer packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator` read_packed_refs(): ensure that references are ordered when read packed_ref_cache: keep the `packed-refs` file mmapped if possible packed-backend.c: reorder some definitions mmapped_ref_iterator_advance(): no peeled value for broken refs mmapped_ref_iterator: add iterator over a packed-refs file packed_ref_cache: remember the file-wide peeling state read_packed_refs(): read references with minimal copying read_packed_refs(): make parsing of the header line more robust read_packed_refs(): only check for a header at the top of the file read_packed_refs(): use mmap to read the `packed-refs` file die_unterminated_line(), die_invalid_line(): new functions packed_ref_cache: add a backlink to the associated `packed_ref_store` prefix_ref_iterator: break when we leave the prefix ...
| * | | | packed-backend.c: rename a bunch of things and update commentsMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We've made huge changes to this file, and some of the old names and comments are no longer very fitting. So rename a bunch of things: * `struct packed_ref_cache` → `struct snapshot` * `acquire_packed_ref_cache()` → `acquire_snapshot()` * `release_packed_ref_buffer()` → `clear_snapshot_buffer()` * `release_packed_ref_cache()` → `release_snapshot()` * `clear_packed_ref_cache()` → `clear_snapshot()` * `struct packed_ref_entry` → `struct snapshot_record` * `cmp_packed_ref_entries()` → `cmp_packed_ref_records()` * `cmp_entry_to_refname()` → `cmp_record_to_refname()` * `sort_packed_refs()` → `sort_snapshot()` * `read_packed_refs()` → `create_snapshot()` * `validate_packed_ref_cache()` → `validate_snapshot()` * `get_packed_ref_cache()` → `get_snapshot()` * Renamed local variables and struct members accordingly. Also update a bunch of comments to reflect the renaming and the accumulated changes that the code has undergone. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | mmapped_ref_iterator: inline into `packed_ref_iterator`Michael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since `packed_ref_iterator` is now delegating to `mmapped_ref_iterator` rather than `cache_ref_iterator` to do the heavy lifting, there is no need to keep the two iterators separate. So "inline" `mmapped_ref_iterator` into `packed_ref_iterator`. This removes a bunch of boilerplate. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ref_cache: remove support for storing peeled valuesMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the `packed-refs` backend doesn't use `ref_cache`, there is nobody left who might want to store peeled values of references in `ref_cache`. So remove that feature. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed_ref_store: get rid of the `ref_cache` entirelyMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that everything has been changed to read what it needs directly out of the `packed-refs` file, `packed_ref_store` doesn't need to maintain a `ref_cache` at all. So get rid of it. First of all, this will save a lot of memory and lots of little allocations. Instead of needing to store complicated parsed data structures in memory, we just mmap the file (potentially sharing memory with other processes) and parse only what we need. Moreover, since the mmapped access to the file reads only the parts of the file that it needs, this might save reading all of the data from disk at all (at least if the file starts out sorted). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ref_store: implement `refs_peel_ref()` genericallyMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We're about to stop storing packed refs in a `ref_cache`. That means that the only way we have left to optimize `peel_ref()` is by checking whether the reference being peeled is the one currently being iterated over (in `current_ref_iter`), and if so, using `ref_iterator_peel()`. But this can be done generically; it doesn't have to be implemented per-backend. So implement `refs_peel_ref()` in `refs.c` and remove the `peel_ref()` method from the refs API. This removes the last callers of a couple of functions, so delete them. More cleanup to come... Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed_read_raw_ref(): read the reference from the mmapped bufferMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of reading the reference from the `ref_cache`, read it directly from the mmapped buffer. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator`Michael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that we have an efficient way to iterate, in order, over the mmapped contents of the `packed-refs` file, we can use that directly to implement reference iteration for the `packed_ref_store`, rather than iterating over the `ref_cache`. This is the next step towards getting rid of the `ref_cache` entirely. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | read_packed_refs(): ensure that references are ordered when readMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It doesn't actually matter now, because the references are only iterated over to fill the associated `ref_cache`, which itself puts them in the correct order. But we want to get rid of the `ref_cache`, so we want to be able to iterate directly over the `packed-refs` buffer, and then the iteration will need to be ordered correctly. In fact, we already write the `packed-refs` file sorted, but it is possible that other Git clients don't get it right. So let's not assume that a `packed-refs` file is sorted unless it is explicitly declared to be so via a `sorted` trait in its header line. If it is *not* declared to be sorted, then scan quickly through the file to check. If it is found to be out of order, then sort the records into a new memory-only copy. This checking and sorting is done quickly, without parsing the full file contents. However, it needs a little bit of care to avoid reading past the end of the buffer even if the `packed-refs` file is corrupt. Since *we* always write the file correctly sorted, include that trait when we write or rewrite a `packed-refs` file. This means that the scan described in the previous paragraph should only have to be done for `packed-refs` files that were written by older versions of the Git command-line client, or by other clients that haven't yet learned to write the `sorted` trait. If `packed-refs` was already sorted, then (if the system allows it) we can use the mmapped file contents directly. But if the system doesn't allow a file that is currently mmapped to be replaced using `rename()`, then it would be bad for us to keep the file mmapped for any longer than necessary. So, on such systems, always make a copy of the file contents, either as part of the sorting process, or afterwards. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed_ref_cache: keep the `packed-refs` file mmapped if possibleMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Keep a copy of the `packed-refs` file contents in memory for as long as a `packed_ref_cache` object is in use: * If the system allows it, keep the `packed-refs` file mmapped. * If not (either because the system doesn't support `mmap()` at all, or because a file that is currently mmapped cannot be replaced via `rename()`), then make a copy of the file's contents in heap-allocated space, and keep that around instead. We base the choice of behavior on a new build-time switch, `MMAP_PREVENTS_DELETE`. By default, this switch is set for Windows variants. After this commit, `MMAP_NONE` and `MMAP_TEMPORARY` are still handled identically. But the next commit will introduce a difference. This whole change is still pointless, because we only read the `packed-refs` file contents immediately after instantiating the `packed_ref_cache`. But that will soon change. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed-backend.c: reorder some definitionsMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | No code has been changed. This will make subsequent patches more self-contained. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | mmapped_ref_iterator_advance(): no peeled value for broken refsMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a reference is broken, suppress its peeled value. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | mmapped_ref_iterator: add iterator over a packed-refs fileMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new `mmapped_ref_iterator`, which can iterate over the references in an mmapped `packed-refs` file directly. Use this iterator from `read_packed_refs()` to fill the packed refs cache. Note that we are not yet willing to promise that the new iterator generates its output in order. That doesn't matter for now, because the packed refs cache doesn't care what order it is filled. This change adds a lot of boilerplate without providing any obvious benefits. The benefits will come soon, when we get rid of the `ref_cache` for packed references altogether. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed_ref_cache: remember the file-wide peeling stateMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rather than store the peeling state (i.e., the one defined by traits in the `packed-refs` file header line) in a local variable in `read_packed_refs()`, store it permanently in `packed_ref_cache`. This will be needed when we stop reading all packed refs at once. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | read_packed_refs(): read references with minimal copyingMichael Haggerty2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of copying data from the `packed-refs` file one line at time and then processing it, process the data in place as much as possible. Also, instead of processing one line per iteration of the main loop, process a reference line plus its corresponding peeled line (if present) together. Note that this change slightly tightens up the parsing of the `packed-refs` file. Previously, the parser would have accepted multiple "peeled" lines for a single reference (ignoring all but the last one). Now it would reject that. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | read_packed_refs(): make parsing of the header line more robustMichael Haggerty2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The old code parsed the traits in the `packed-refs` header by looking for the string " trait " (i.e., the name of the trait with a space on either side) in the header line. This is fragile, because if any other implementation of Git forgets to write the trailing space, the last trait would silently be ignored (and the error might never be noticed). So instead, use `string_list_split_in_place()` to split the traits into tokens then use `unsorted_string_list_has_string()` to look for the tokens we are interested in. This means that we can read the traits correctly even if the header line is missing a trailing space (or indeed, if it is missing the space after the colon, or if it has multiple spaces somewhere). However, older Git clients (and perhaps other Git implementations) still require the surrounding spaces, so we still have to output the header with a trailing space. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | read_packed_refs(): only check for a header at the top of the fileMichael Haggerty2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This tightens up the parsing a bit; previously, stray header-looking lines would have been processed. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | read_packed_refs(): use mmap to read the `packed-refs` fileMichael Haggerty2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's still done in a pretty stupid way, involving more data copying than necessary. That will improve in future commits. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | die_unterminated_line(), die_invalid_line(): new functionsMichael Haggerty2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Extract some helper functions for reporting errors. While we're at it, prevent them from spewing unlimited output to the terminal. These functions will soon have more callers. These functions accept the problematic line as a `(ptr, len)` pair rather than a NUL-terminated string, and `die_invalid_line()` checks for an EOL itself, because these calling conventions will be convenient for future callers. (Efficiency is not a concern here because these functions are only ever called if the `packed-refs` file is corrupt.) Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | packed_ref_cache: add a backlink to the associated `packed_ref_store`Michael Haggerty2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It will prove convenient in upcoming patches. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | prefix_ref_iterator: break when we leave the prefixJeff King2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the underlying iterator is ordered, then `prefix_ref_iterator` can stop as soon as it sees a refname that comes after the prefix. This will rarely make a big difference now, because `ref_cache_iterator` only iterates over the directory containing the prefix (and usually the prefix will span a whole directory anyway). But if *hint, hint* a future reference backend doesn't itself know where to stop the iteration, then this optimization will be a big win. Note that there is no guarantee that the underlying iterator doesn't include output preceding the prefix, so we have to skip over any unwanted references before we get to the ones that we want. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ref_iterator: keep track of whether the iterator output is orderedMichael Haggerty2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | References are iterated over in order by refname, but reflogs are not. Some consumers of reference iteration care about the difference. Teach each `ref_iterator` to keep track of whether its output is ordered. `overlay_ref_iterator` is one of the picky consumers. Add a sanity check in `overlay_ref_iterator_begin()` to verify that its inputs are ordered. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'mr/doc-negative-pathspec'Junio C Hamano2017-10-03
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Doc updates. * mr/doc-negative-pathspec: docs: improve discoverability of exclude pathspec
| * | | | | docs: improve discoverability of exclude pathspecManav Rathi2017-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The ability to exclude paths with a negative pathspec is not mentioned in the man pages for git grep and other commands where it might be useful. Add an example and a pointer to the pathspec glossary entry in the man page for git grep to help the user to discover this ability. Add similar pointers from the git-add and git-status man pages. Additionally, - Add a test for the behaviour when multiple exclusions are present. - Add a test for the ^ alias. - Improve name of existing test. - Improve grammar in glossary description of the exclude pathspec. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Manav Rathi <mnvrth@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'sb/submodule-diff-header-fix'Junio C Hamano2017-10-03
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Error message tweak. * sb/submodule-diff-header-fix: submodule: correct error message for missing commits
| * | | | | | submodule: correct error message for missing commitsStefan Beller2017-09-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a submodule diff should be displayed we currently just add the submodule objects to the main object store and then e.g. walk the revision graph and create a summary for that submodule. It is possible that we are missing the submodule either completely or partially, which we currently differentiate with different error messages depending on whether (1) the whole submodule object store is missing or (2) just the needed for this particular diff. (1) is reported as "not initialized", and (2) is reported as "commits not present". If a submodule is deinit'ed its repository data is still around inside the superproject, such that the diff can still be produced. In that way the error message (1) is misleading as we can have a diff despite the submodule being not initialized. Downgrade the error message (1) to be the same as (2) and just say the commits are not present, as that is the true reason why the diff cannot be shown. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'sb/diff-color-move'Junio C Hamano2017-10-03
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The output from "git diff --summary" was broken in a recent topic that has been merged to 'master' and lost a LF after reporting of mode change. This has been fixed. * sb/diff-color-move: diff: correct newline in summary for renamed files
| * | | | | | | diff: correct newline in summary for renamed filesStefan Beller2017-09-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY, 2017-06-29), the conversion from direct printing to the symbol emission dropped the new line character for renamed, copied and rewritten files. Add the emission of a newline, add a test for this case. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'sb/test-submodule-update-config'Junio C Hamano2017-10-03
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * sb/test-submodule-update-config: t7406: submodule.<name>.update command must not be run from .gitmodules
| * | | | | | | | t7406: submodule.<name>.update command must not be run from .gitmodulesStefan Beller2017-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | submodule.<name>.update can be assigned an arbitrary command via setting it to "!command". When this command is found in the regular config, Git ought to just run that command instead of other update mechanisms. However if that command is just found in the .gitmodules file, it is potentially untrusted, which is why we do not run it. Add a test confirming the behavior. Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | Merge branch 'jk/validate-headref-fix'Junio C Hamano2017-10-03
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up. * jk/validate-headref-fix: validate_headref: use get_oid_hex for detached HEADs validate_headref: use skip_prefix for symref parsing validate_headref: NUL-terminate HEAD buffer
| * | | | | | | | | validate_headref: use get_oid_hex for detached HEADsJeff King2017-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a candidate HEAD isn't a symref, we check that it contains a viable sha1. But in a post-sha1 world, we should be checking whether it has any plausible object-id. We can do that by switching to get_oid_hex(). Note that both before and after this patch, we only check for a plausible object id at the start of the file, and then call that good enough. We ignore any content _after_ the hex, so a string like: 0123456789012345678901234567890123456789 foo is accepted. Though we do put extra bytes like this into some pseudorefs (e.g., FETCH_HEAD), we don't typically do so with HEAD. We could tighten this up by using parse_oid_hex(), like: if (!parse_oid_hex(buffer, &oid, &end) && *end++ == '\n' && *end == '\0') return 0; But we're probably better to remain on the loose side. We're just checking here for a plausible-looking repository directory, so heuristics are acceptable (if we really want to be meticulous, we should use the actual ref code to parse HEAD). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>