aboutsummaryrefslogtreecommitdiff
path: root/builtin
Commit message (Collapse)AuthorAge
* pack-objects: add list-objects filteringJeff Hostetler2017-11-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | Teach pack-objects to use the filtering provided by the traverse_commit_list_filtered() interface to omit unwanted objects from the resulting packfile. Filtering requires the use of the "--stdout" option. Add t5317 test. In the future, we will introduce a "partial clone" mechanism wherein an object in a repo, obtained from a remote, may reference a missing object that can be dynamically fetched from that remote once needed. This "partial clone" mechanism will have a way, sometimes slow, of determining if a missing link is one of the links expected to be produced by this mechanism. This patch introduces handling of missing objects to help debugging and development of the "partial clone" mechanism, and once the mechanism is implemented, for a power user to perform operations that are missing-object aware without incurring the cost of checking if a missing link is expected. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* rev-list: add list-objects filtering supportJeff Hostetler2017-11-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach rev-list to use the filtering provided by the traverse_commit_list_filtered() interface to omit unwanted objects from the result. Object filtering is only allowed when one of the "--objects*" options are used. When the "--filter-print-omitted" option is used, the omitted objects are printed at the end. These are marked with a "~". This option can be combined with "--quiet" to get a list of just the omitted objects. Add t6112 test. In the future, we will introduce a "partial clone" mechanism wherein an object in a repo, obtained from a remote, may reference a missing object that can be dynamically fetched from that remote once needed. This "partial clone" mechanism will have a way, sometimes slow, of determining if a missing link is one of the links expected to be produced by this mechanism. This patch introduces handling of missing objects to help debugging and development of the "partial clone" mechanism, and once the mechanism is implemented, for a power user to perform operations that are missing-object aware without incurring the cost of checking if a missing link is expected. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jk/ref-filter-colors-fix'Junio C Hamano2017-10-18
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | This is the "theoretically more correct" approach of simply stepping back to the state before plumbing commands started paying attention to "color.ui" configuration variable. Let's run with this one. * jk/ref-filter-colors-fix: tag: respect color.ui config Revert "color: check color.ui in git_default_config()" Revert "t6006: drop "always" color config tests" Revert "color: make "always" the same as "auto" in config"
| * tag: respect color.ui configJeff King2017-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 11b087adfd (ref-filter: consult want_color() before emitting colors, 2017-07-13), we expect that setting "color.ui" to "always" will enable color tag formats even without a tty. As that commit was built on top of 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13) from the same series, we didn't need to touch tag's config parsing at all. However, since we reverted 136c8c8b8f, we now need to explicitly call git_color_default_config() to make this work. Let's do so, and also restore the test dropped in 0c88bf5050 (provide --color option for all ref-filter users, 2017-10-03). That commit swapped out our "color.ui=always" test for "--color" in preparation for "always" going away. But since it is here to stay, we should test both cases. Note that for-each-ref also lost its color.ui support as part of reverting 136c8c8b8f. But as a plumbing command, it should _not_ respect the color.ui config. Since it also gained a --color option in 0c88bf5050, that's the correct way to ask it for color. We'll continue to test that, and confirm that "color.ui" is not respected. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * Revert "color: check color.ui in git_default_config()"Jeff King2017-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 136c8c8b8fa39f1315713248473dececf20f8fe7. That commit was trying to address a bug caused by 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10), in which plumbing like diff-tree defaulted to "auto" color, but did not respect a "color.ui" directive to disable it. But it also meant that we started respecting "color.ui" set to "always". This was a known problem, but 4c7f1819b3 argued that nobody ought to be doing that. However, that turned out to be wrong, and we got a number of bug reports related to "add -p" regressing in v2.14.2. Let's revert 136c8c8b8, fixing the regression to "add -p". This leaves the problem from 4c7f1819b3 unfixed, but: 1. It's a pretty obscure problem in the first place. I only noticed it while working on the color code, and we haven't got a single bug report or complaint about it. 2. We can make a more moderate fix on top by respecting "never" but not "always" for plumbing commands. This is just the minimal fix to go back to the working state we had before v2.14.2. Note that this isn't a pure revert. We now have a test in t3701 which shows off the "add -p" regression. This can be flipped to success. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * Merge branch 'jk/ui-color-always-to-auto-maint' (early part) into ↵Junio C Hamano2017-10-17
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | jk/ref-filter-colors-fix-maint * 'jk/ui-color-always-to-auto-maint' (early part): 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
* | \ Merge branch 'ds/avoid-overflow-in-midpoint-computation'Junio C Hamano2017-10-11
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up. * ds/avoid-overflow-in-midpoint-computation: cleanup: fix possible overflow errors in binary search
| * | | cleanup: fix possible overflow errors in binary searchDerrick Stolee2017-10-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A common mistake when writing binary search is to allow possible integer overflow by using the simple average: mid = (min + max) / 2; Instead, use the overflow-safe version: mid = min + (max - min) / 2; This translation is safe since the operation occurs inside a loop conditioned on "min < max". The included changes were found using the following git grep: git grep '/ *2;' '*.c' Making this cleanup will prevent future review friction when a new binary search is contructed based on existing code. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ks/branch-tweak-error-message-for-extra-args'Junio C Hamano2017-10-07
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Error message tweak. * ks/branch-tweak-error-message-for-extra-args: branch: change the error messages to be more meaningful
| * | | | branch: change the error messages to be more meaningfulKaartic Sivaraam2017-10-04
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The error messages shown when the branch command is misused by supplying it wrong number of parameters wasn't meaningful. That's because it used the the phrase "too many branches" assuming all parameters to be "valid" branch names. It's not always the case as exemplified below, $ git branch foo * master $ git branch -m foo foo old fatal: too many branches for a rename operation Change the messages to be more general thus making no assumptions about the "parameters". Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jk/ui-color-always-to-auto'Junio C Hamano2017-10-07
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix regression of "git add -p" for users with "color.ui = always" in their configuration, by merging the topic below and adjusting it for the 'master' front. * jk/ui-color-always-to-auto: t7301: use test_terminal to check color t4015: use --color with --color-moved 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
| * \ \ \ 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
| | * | | 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>
* | | | | Merge branch 'ma/builtin-unleak'Junio C Hamano2017-10-07
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many variables that points at a region of memory that will live throughout the life of the program have been marked with UNLEAK marker to help the leak checkers concentrate on real leaks.. * ma/builtin-unleak: builtin/: add UNLEAKs
| * | | | | builtin/: add UNLEAKsMartin Ågren2017-10-02
| | |_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the variables in the same order as we've declared them. While addressing `msg` in builtin/tag.c, convert the existing `strbuf_release()` calls as well. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'sb/branch-avoid-repeated-strbuf-release'Junio C Hamano2017-10-07
|\ \ \ \ \ | |_|_|_|/ |/| | | | | | | | | | | | | | * sb/branch-avoid-repeated-strbuf-release: branch: reset instead of release a strbuf
| * | | | branch: reset instead of release a strbufStefan Beller2017-10-04
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Our documentation advises to not re-use a strbuf, after strbuf_release has been called on it. Use the proper reset instead. Currently 'strbuf_release' releases and re-initializes the strbuf, so it is safe, but slow. 'strbuf_reset' only resets the internal length variable, such that this could also be accounted for as a micro-optimization. Reviewed-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 'rs/cleanup-strbuf-users'Junio C Hamano2017-10-05
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up. * rs/cleanup-strbuf-users: graph: use strbuf_addchars() to add spaces use strbuf_addstr() for adding strings to strbufs path: use strbuf_add_real_path()
| * | | | use strbuf_addstr() for adding strings to strbufsRené Scharfe2017-10-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use strbuf_addstr() instead of strbuf_addf() for adding strings. That's simpler and makes the intent clearer. Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci; adjusted indentation in refs/packed-backend.c manually. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'rs/resolve-ref-optional-result'Junio C Hamano2017-10-05
|\ \ \ \ \ | |_|_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up. * rs/resolve-ref-optional-result: refs: pass NULL to resolve_refdup() if hash is not needed refs: pass NULL to refs_resolve_refdup() if hash is not needed
| * | | | refs: pass NULL to resolve_refdup() if hash is not neededRené Scharfe2017-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows us to get rid of several write-only variables. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'jk/read-in-full'Junio C Hamano2017-10-03
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up to prevent future mistakes by copying and pasting code that checks the result of read_in_full() function. * jk/read-in-full: worktree: check the result of read_in_full() worktree: use xsize_t to access file size distinguish error versus short read from read_in_full() avoid looking at errno for short read_in_full() returns prefer "!=" when checking read_in_full() result notes-merge: drop dead zero-write code files-backend: prefer "0" for write_in_full() error check
| * | | | | worktree: check the result of read_in_full()Jeff King2017-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We try to read "len" bytes into a buffer and just assume that it happened correctly. In practice this should usually be the case, since we just stat'd the file to get the length. But we could be fooled by transient errors or by other processes racily truncating the file. Let's be more careful. There's a slim chance this could catch a real error, but it also prevents people and tools from getting worried while reading the code. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | worktree: use xsize_t to access file sizeJeff King2017-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To read the "gitdir" file into memory, we stat the file and allocate a buffer. But we store the size in an "int", which may be truncated. We should use a size_t and xsize_t(), which will detect truncation. An overflow is unlikely for a "gitdir" file, but it's a good practice to model. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | distinguish error versus short read from read_in_full()Jeff King2017-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many callers of read_in_full() expect to see the exact number of bytes requested, but their error handling lumps together true read errors and short reads due to unexpected EOF. We can give more specific error messages by separating these cases (showing errno when appropriate, and otherwise describing the short read). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | prefer "!=" when checking read_in_full() resultJeff King2017-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Comparing the result of read_in_full() using less-than is potentially dangerous, as a negative return value may be converted to an unsigned type and be considered a success. This is discussed further in 561598cfcf (read_pack_header: handle signed/unsigned comparison in read result, 2017-09-13). Each of these instances is actually fine in practice: - in get-tar-commit-id, the HEADERSIZE macro expands to a signed integer. If it were switched to an unsigned type (e.g., a size_t), then it would be a bug. - the other two callers check for a short read only after handling a negative return separately. This is a fine practice, but we'd prefer to model "!=" as a general rule. So all of these cases can be considered cleanups and not actual bugfixes. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'jk/no-optional-locks'Junio C Hamano2017-10-03
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some commands (most notably "git status") makes an opportunistic update when performing a read-only operation to help optimize later operations in the same repository. The new "--no-optional-locks" option can be passed to Git to disable them. * jk/no-optional-locks: git: add --no-optional-locks option
| * | | | | | git: add --no-optional-locks optionJeff King2017-09-27
| | |_|_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some tools like IDEs or fancy editors may periodically run commands like "git status" in the background to keep track of the state of the repository. Some of these commands may refresh the index and write out the result in an opportunistic way: if they can get the index lock, then they update the on-disk index with any updates they find. And if not, then their in-core refresh is lost and just has to be recomputed by the next caller. But taking the index lock may conflict with other operations in the repository. Especially ones that the user is doing themselves, which _aren't_ opportunistic. In other words, "git status" knows how to back off when somebody else is holding the lock, but other commands don't know that status would be happy to drop the lock if somebody else wanted it. There are a couple possible solutions: 1. Have some kind of "pseudo-lock" that allows other commands to tell status that they want the lock. This is likely to be complicated and error-prone to implement (and maybe even impossible with just dotlocks to work from, as it requires some inter-process communication). 2. Avoid background runs of commands like "git status" that want to do opportunistic updates, preferring instead plumbing like diff-files, etc. This is awkward for a couple of reasons. One is that "status --porcelain" reports a lot more about the repository state than is available from individual plumbing commands. And two is that we actually _do_ want to see the refreshed index. We just don't want to take a lock or write out the result. Whereas commands like diff-files expect us to refresh the index separately and write it to disk so that they can depend on the result. But that write is exactly what we're trying to avoid. 3. Ask "status" not to lock or write the index. This is easy to implement. The big downside is that any work done in refreshing the index for such a call is lost when the process exits. So a background process may end up re-hashing a changed file multiple times until the user runs a command that does an index refresh themselves. This patch implements the option 3. The idea (and the test) is largely stolen from a Git for Windows patch by Johannes Schindelin, 67e5ce7f63 (status: offer *not* to lock the index and update it, 2016-08-12). The twist here is that instead of making this an option to "git status", it becomes a "git" option and matching environment variable. The reason there is two-fold: 1. An environment variable is carried through to sub-processes. And whether an invocation is a background process or not should apply to the whole process tree. So you could do "git --no-optional-locks foo", and if "foo" is a script or alias that calls "status", you'll still get the effect. 2. There may be other programs that want the same treatment. I've punted here on finding more callers to convert, since "status" is the obvious one to call as a repeated background job. But "git diff"'s opportunistic refresh of the index may be a good candidate. The test is taken from 67e5ce7f63, and it's worth repeating Johannes's explanation: Note that the regression test added in this commit does not *really* verify that no index.lock file was written; that test is not possible in a portable way. Instead, we verify that .git/index is rewritten *only* when `git status` is run without `--no-optional-locks`. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'sd/branch-copy'Junio C Hamano2017-10-03
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git branch" learned "-c/-C" to create a new branch by copying an existing one. * sd/branch-copy: branch: fix "copy" to never touch HEAD branch: add a --copy (-c) option to go with --move (-m) branch: add test for -m renaming multiple config sections config: create a function to format section headers
| * | | | | | branch: fix "copy" to never touch HEADJunio C Hamano2017-09-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When creating a new branch B by copying the branch A that happens to be the current branch, it also updates HEAD to point at the new branch. It probably was made this way because "git branch -c A B" piggybacked its implementation on "git branch -m A B", This does not match the usual expectation. If I were sitting on a blue chair, and somebody comes and repaints it to red, I would accept ending up sitting on a chair that is now red (I am also OK to stand, instead, as there no longer is my favourite blue chair). But if somebody creates a new red chair, modelling it after the blue chair I am sitting on, I do not expect to be booted off of the blue chair and ending up on sitting on the new red one. Let's fix this before it hits 'next'. Those who want to create a new branch and switch to it can do "git checkout B" after doing a "git branch -c B", and if that operation is so useful and deserves a short-hand way to do so, perhaps extend "git checkout -b B" to copy configurations while creating the new branch B. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | branch: add a --copy (-c) option to go with --move (-m)Sahil Dua2017-06-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add the ability to --copy a branch and its reflog and configuration, this uses the same underlying machinery as the --move (-m) option except the reflog and configuration is copied instead of being moved. This is useful for e.g. copying a topic branch to a new version, e.g. work to work-2 after submitting the work topic to the list, while preserving all the tracking info and other configuration that goes with the branch, and unlike --move keeping the other already-submitted branch around for reference. Like --move, when the source branch is the currently checked out branch the HEAD is moved to the destination branch. In the case of --move we don't really have a choice (other than remaining on a detached HEAD) and in order to keep the functionality consistent, we are doing it in similar way for --copy too. The most common usage of this feature is expected to be moving to a new topic branch which is a copy of the current one, in that case moving to the target branch is what the user wants, and doesn't unexpectedly behave differently than --move would. One outstanding caveat of this implementation is that: git checkout maint && git checkout master && git branch -c topic && git checkout - Will check out 'maint' instead of 'master'. This is because the @{-N} feature (or its -1 shorthand "-") relies on HEAD reflogs created by the checkout command, so in this case we'll checkout maint instead of master, as the user might expect. What to do about that is left to a future change. Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Sahil Dua <sahildua2305@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'bc/rev-parse-parseopt-fix'Junio C Hamano2017-10-03
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Recent versions of "git rev-parse --parseopt" did not parse the option specification that does not have the optional flags (*=?!) correctly, which has been corrected. * bc/rev-parse-parseopt-fix: parse-options: only insert newline in help text if needed parse-options: write blank line to correct output stream t0040,t1502: Demonstrate parse_options bugs git-rebase: don't ignore unexpected command line arguments rev-parse parseopt: interpret any whitespace as start of help text rev-parse parseopt: do not search help text for flag chars t1502: demonstrate rev-parse --parseopt option mis-parsing
| * | | | | | | rev-parse parseopt: interpret any whitespace as start of help textBrandon Casey2017-09-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, rev-parse only interprets a space ' ' character as the delimiter between the option spec and the help text. So if a tab character is placed between the option spec and the help text, it will be interpreted as part of the long option name or as part of the arg hint. If it is interpreted as part of the long option name, then rev-parse will produce what will be interpreted as multiple arguments on the command line. For example, the following option spec (note: there is a <tab> between "frotz" and "enable"): frotz enable frotzing will produce the following set expression when --frotz is used: set -- --frotz -- instead of this: set -- --frotz enable -- Mark t1502.2 as fixed. Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | rev-parse parseopt: do not search help text for flag charsBrandon Casey2017-09-19
| | |/ / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When searching for flag characters in the option spec, we should ensure the search stays within the bounds of the option spec and does not enter the help text portion of the spec. So when we find the boundary white space marking the start of the help text, let's mark it with a nul character. Then when we search for flag characters starting from the beginning of the string we'll stop at the nul and won't enter the help text. Now, the following option spec: exclame this does something! will produce this 'set' expression when --exclame is specified: set -- --exclame -- instead of this one: set -- --exclame this does something -- Mark t1502.4 and t1502.5 as fixed. Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'js/rebase-i-final'Junio C Hamano2017-10-03
|\ \ \ \ \ \ \ | |_|_|_|_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The final batch to "git rebase -i" updates to move more code from the shell script to C. * js/rebase-i-final: rebase -i: rearrange fixup/squash lines using the rebase--helper t3415: test fixup with wrapped oneline rebase -i: skip unnecessary picks using the rebase--helper rebase -i: check for missing commits in the rebase--helper t3404: relax rebase.missingCommitsCheck tests rebase -i: also expand/collapse the SHA-1s via the rebase--helper rebase -i: do not invent onelines when expanding/collapsing SHA-1s rebase -i: remove useless indentation rebase -i: generate the script via rebase--helper t3415: verify that an empty instructionFormat is handled as before
| * | | | | | rebase -i: rearrange fixup/squash lines using the rebase--helperJohannes Schindelin2017-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This operation has quadratic complexity, which is especially painful on Windows, where shell scripts are *already* slow (mainly due to the overhead of the POSIX emulation layer). Let's reimplement this with linear complexity (using a hash map to match the commits' subject lines) for the common case; Sadly, the fixup/squash feature's design neglected performance considerations, allowing arbitrary prefixes (read: `fixup! hell` will match the commit subject `hello world`), which means that we are stuck with quadratic performance in the worst case. The reimplemented logic also happens to fix a bug where commented-out lines (representing empty patches) were dropped by the previous code. While at it, clarify how the fixup/squash feature works in `git rebase -i`'s man page. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | rebase -i: skip unnecessary picks using the rebase--helperJohannes Schindelin2017-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In particular on Windows, where shell scripts are even more expensive than on MacOSX or Linux, it makes sense to move a loop that forks Git at least once for every line in the todo list into a builtin. Note: The original code did not try to skip unnecessary picks of root commits but punts instead (probably --root was not considered common enough of a use case to bother optimizing). We do the same, for now. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | rebase -i: check for missing commits in the rebase--helperJohannes Schindelin2017-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In particular on Windows, where shell scripts are even more expensive than on MacOSX or Linux, it makes sense to move a loop that forks Git at least once for every line in the todo list into a builtin. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | rebase -i: also expand/collapse the SHA-1s via the rebase--helperJohannes Schindelin2017-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is crucial to improve performance on Windows, as the speed is now mostly dominated by the SHA-1 transformation (because it spawns a new rev-parse process for *every* line, and spawning processes is pretty slow from Git for Windows' MSYS2 Bash). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | rebase -i: generate the script via rebase--helperJohannes Schindelin2017-07-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The first step of an interactive rebase is to generate the so-called "todo script", to be stored in the state directory as "git-rebase-todo" and to be edited by the user. Originally, we adjusted the output of `git log <options>` using a simple sed script. Over the course of the years, the code became more complicated. We now use shell scripting to edit the output of `git log` conditionally, depending whether to keep "empty" commits (i.e. commits that do not change any files). On platforms where shell scripting is not native, this can be a serious drag. And it opens the door for incompatibilities between platforms when it comes to shell scripting or to Unix-y commands. Let's just re-implement the todo script generation in plain C, using the revision machinery directly. This is substantially faster, improving the speed relative to the shell script version of the interactive rebase from 2x to 3x on Windows. Note that the rearrange_squash() function in git-rebase--interactive relied on the fact that we set the "format" variable to the config setting rebase.instructionFormat. Relying on a side effect like this is no good, hence we explicitly perform that assignment (possibly again) in rearrange_squash(). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ma/leakplugs'Junio C Hamano2017-09-29
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Memory leaks in various codepaths have been plugged. * ma/leakplugs: pack-bitmap[-write]: use `object_array_clear()`, don't leak object_array: add and use `object_array_pop()` object_array: use `object_array_clear()`, not `free()` leak_pending: use `object_array_clear()`, not `free()` commit: fix memory leak in `reduce_heads()` builtin/commit: fix memory leak in `prepare_index()`
| * | | | | | | object_array: add and use `object_array_pop()`Martin Ågren2017-09-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In a couple of places, we pop objects off an object array `foo` by decreasing `foo.nr`. We access `foo.nr` in many places, but most if not all other times we do so read-only, e.g., as we iterate over the array. But when we change `foo.nr` behind the array's back, it feels a bit nasty and looks like it might leak memory. Leaks happen if the popped element has an allocated `name` or `path`. At the moment, that is not the case. Still, 1) the object array might gain more fields that want to be freed, 2) a code path where we pop might start using names or paths, 3) one of these code paths might be copied to somewhere where we do, and 4) using a dedicated function for popping is conceptually cleaner. Introduce and use `object_array_pop()` instead. Release memory in the new function. Document that popping an object leaves the associated elements in limbo. The converted places were identified by grepping for "\.nr\>" and looking for "--". Make the new function return NULL on an empty array. This is consistent with `pop_commit()` and allows the following: while ((o = object_array_pop(&foo)) != NULL) { // do something } But as noted above, we don't need to go out of our way to avoid reading `foo.nr`. This is probably more readable: while (foo.nr) { ... o = object_array_pop(&foo); // do something } The name of `object_array_pop()` does not quite align with `add_object_array()`. That is unfortunate. On the other hand, it matches `object_array_clear()`. Arguably it's `add_...` that is the odd one out, since it reads like it's used to "add" an "object array". For that reason, side with `object_array_clear()`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | object_array: use `object_array_clear()`, not `free()`Martin Ågren2017-09-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of freeing `foo.objects` for an object array `foo` (sometimes conditionally), call `object_array_clear(&foo)`. This means we don't poke as much into the implementation, which is already a good thing, but also that we release the individual entries as well, thereby fixing at least one memory-leak (in diff-lib.c). If someone is holding on to a pointer to an element's `name` or `path`, that is now a dangling pointer, i.e., we'd be turning an unpleasant situation into an outright bug. To the best of my understanding no such long-term pointers are being taken. The way we handle `study` in builting/reflog.c still looks like it might leak. That will be addressed in the next commit. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | leak_pending: use `object_array_clear()`, not `free()`Martin Ågren2017-09-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Setting `leak_pending = 1` tells `prepare_revision_walk()` not to release the `pending` array, and makes that the caller's responsibility. See 4a43d374f (revision: add leak_pending flag, 2011-10-01) and 353f5657a (bisect: use leak_pending flag, 2011-10-01). Commit 1da1e07c8 (clean up name allocation in prepare_revision_walk, 2014-10-15) fixed a memory leak in `prepare_revision_walk()` by switching from `free()` to `object_array_clear()`. However, where we use the `leak_pending`-mechanism, we're still only calling `free()`. Use `object_array_clear()` instead. Copy some helpful comments from 353f5657a to the other callers that we update to clarify the memory responsibilities, and to highlight that the commits are not affected when we clear the array -- it is indeed correct to both tidy up the commit flags and clear the object array. Document `leak_pending` in revision.h to help future users get this right. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | builtin/commit: fix memory leak in `prepare_index()`Martin Ågren2017-09-24
| | |/ / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Release `pathspec` and the string list `partial`. When we clear the string list, make sure we do not free the `util` pointers. That would result in double-freeing, since we set them up as `item->util = item` in `list_paths()`. Initialize the string list early, so that we can always release it. That introduces some unnecessary overhead in various code paths, but means there is one and only one way out of the function. If we ever accumulate more things we need to free, it should be straightforward to do so. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'rj/no-sign-compare'Junio C Hamano2017-09-29
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many codepaths have been updated to squelch -Wsign-compare warnings. * rj/no-sign-compare: ALLOC_GROW: avoid -Wsign-compare warnings cache.h: hex2chr() - avoid -Wsign-compare warnings commit-slab.h: avoid -Wsign-compare warnings git-compat-util.h: xsize_t() - avoid -Wsign-compare warnings
| * | | | | | | ALLOC_GROW: avoid -Wsign-compare warningsRamsay Jones2017-09-22
| |/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'jt/fast-export-copy-modify-fix'Junio C Hamano2017-09-29
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git fast-export" with -M/-C option issued "copy" instruction on a path that is simultaneously modified, which was incorrect. * jt/fast-export-copy-modify-fix: fast-export: do not copy from modified file
| * | | | | | | fast-export: do not copy from modified fileJonathan Tan2017-09-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When run with the "-C" option, fast-export writes 'C' commands in its output whenever the internal diff mechanism detects a file copy, indicating that fast-import should copy the given existing file to the given new filename. However, the diff mechanism works against the prior version of the file, whereas fast-import uses whatever is current. This causes issues when a commit both modifies a file and uses it as the source for a copy. Therefore, teach fast-export to refrain from writing 'C' when it has already written a modification command for a file. An existing test in t9350-fast-export is also fixed in this patch. The existing line "C file6 file7" copies the wrong version of file6, but it has coincidentally worked because file7 was subsequently overridden. Reported-by: Juraj Oršulić <juraj.orsulic@fer.hr> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'mk/describe-match-with-all'Junio C Hamano2017-09-29
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git describe --match <pattern>" has been taught to play well with the "--all" option. * mk/describe-match-with-all: describe: teach --match to handle branches and remotes