summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2018-07-24Merge branch 'wc/find-commit-with-pattern-on-detached-head'Libravatar Junio C Hamano1-0/+26
"git rev-parse ':/substring'" did not consider the history leading only to HEAD when looking for a commit with the given substring, when the HEAD is detached. This has been fixed. * wc/find-commit-with-pattern-on-detached-head: sha1-name.c: for ":/", find detached HEAD commits
2018-07-24Merge branch 'jc/t3404-one-shot-export-fix'Libravatar Junio C Hamano1-1/+8
Correct a broken use of "VAR=VAL shell_func" in a test. * jc/t3404-one-shot-export-fix: t3404: fix use of "VAR=VAL cmd" with a shell function
2018-07-24Merge branch 'mk/merge-in-sparse-checkout'Libravatar Junio C Hamano1-0/+59
"git reset --merge" (hence "git merge ---abort") and "git reset --hard" had trouble working correctly in a sparsely checked out working tree after a conflict, which has been corrected. * mk/merge-in-sparse-checkout: unpack-trees: do not fail reset because of unmerged skipped entry
2018-07-24Merge branch 'jk/empty-pick-fix'Libravatar Junio C Hamano1-1/+6
Handling of an empty range by "git cherry-pick" was inconsistent depending on how the range ended up to be empty, which has been corrected. * jk/empty-pick-fix: sequencer: don't say BUG on bogus input sequencer: handle empty-set cases consistently
2018-07-24Merge branch 'jt/partial-clone-fsck-connectivity'Libravatar Junio C Hamano2-0/+80
Partial clone support of "git clone" has been updated to correctly validate the objects it receives from the other side. The server side has been corrected to send objects that are directly requested, even if they may match the filtering criteria (e.g. when doing a "lazy blob" partial clone). * jt/partial-clone-fsck-connectivity: clone: check connectivity even if clone is partial upload-pack: send refs' objects despite "filter"
2018-07-24Merge branch 'bc/send-email-auto-cte'Libravatar Junio C Hamano1-0/+57
The content-transfer-encoding of the message "git send-email" sends out by default was 8bit, which can cause trouble when there is an overlong line to bust RFC 5322/2822 limit. A new option 'auto' to automatically switch to quoted-printable when there is such a line in the payload has been introduced and is made the default. * bc/send-email-auto-cte: docs: correct RFC specifying email line length send-email: automatically determine transfer-encoding send-email: accept long lines with suitable transfer encoding send-email: add an auto option for transfer encoding
2018-07-24Merge branch 'kn/userdiff-php'Libravatar Junio C Hamano7-0/+34
The userdiff pattern for .php has been updated. * kn/userdiff-php: userdiff: support new keywords in PHP hunk header t4018: add missing test cases for PHP
2018-07-24Merge branch 'jk/fetch-all-peeled-fix'Libravatar Junio C Hamano1-6/+13
Test modernization. * jk/fetch-all-peeled-fix: t5500: prettify non-commit tag tests
2018-07-24Merge branch 'jt/connectivity-check-after-unshallow'Libravatar Junio C Hamano6-0/+484
"git fetch" failed to correctly validate the set of objects it received when making a shallow history deeper, which has been corrected. * jt/connectivity-check-after-unshallow: fetch-pack: write shallow, then check connectivity fetch-pack: implement ref-in-want fetch-pack: put shallow info in output parameter fetch: refactor to make function args narrower fetch: refactor fetch_refs into two functions fetch: refactor the population of peer ref OIDs upload-pack: test negotiation with changing repository upload-pack: implement ref-in-want test-pkt-line: add unpack-sideband subcommand
2018-07-24Merge branch 'jk/for-each-ref-icase'Libravatar Junio C Hamano1-0/+11
The "--ignore-case" option of "git for-each-ref" (and its friends) did not work correctly, which has been fixed. * jk/for-each-ref-icase: ref-filter: avoid backend filtering with --ignore-case for-each-ref: consistently pass WM_IGNORECASE flag t6300: add a test for --ignore-case
2018-07-24Merge branch 'en/t5407-rebase-m-fix'Libravatar Junio C Hamano1-1/+1
* en/t5407-rebase-m-fix: t5407: fix test to cover intended arguments
2018-07-24Merge branch 'en/rebase-consistency'Libravatar Junio C Hamano4-11/+200
"git rebase" behaved slightly differently depending on which one of the three backends gets used; this has been documented and an effort to make them more uniform has begun. * en/rebase-consistency: git-rebase: make --allow-empty-message the default t3401: add directory rename testcases for rebase and am git-rebase.txt: document behavioral differences between modes directory-rename-detection.txt: technical docs on abilities and limitations git-rebase.txt: address confusion between --no-ff vs --force-rebase git-rebase: error out when incompatible options passed t3422: new testcases for checking when incompatible options passed git-rebase.sh: update help messages a bit git-rebase.txt: document incompatible options
2018-07-24Merge branch 'sb/submodule-move-head-error-msg'Libravatar Junio C Hamano1-1/+2
"git checkout --recurse-submodules another-branch" did not report in which submodule it failed to update the working tree, which resulted in an unhelpful error message. * sb/submodule-move-head-error-msg: submodule.c: report the submodule that an error occurs in
2018-07-18Merge branch 'en/rebase-i-microfixes'Libravatar Junio C Hamano1-0/+32
* en/rebase-i-microfixes: git-rebase--merge: modernize "git-$cmd" to "git $cmd" Fix use of strategy options with interactive rebases t3418: add testcase showing problems with rebase -i and strategy options
2018-07-18Merge branch 'mb/filter-branch-optim'Libravatar Junio C Hamano1-0/+15
"git filter-branch" when used with the "--state-branch" option still attempted to rewrite the commits whose filtered result is known from the previous attempt (which is recorded on the state branch); the command has been corrected not to waste cycles doing so. * mb/filter-branch-optim: filter-branch: skip commits present on --state-branch
2018-07-18Merge branch 'jk/branch-l-0-deprecation'Libravatar Junio C Hamano2-19/+19
The "-l" option in "git branch -l" is an unfortunate short-hand for "--create-reflog", but many users, both old and new, somehow expect it to be something else, perhaps "--list". This step warns when "-l" is used as a short-hand for "--create-reflog" and warns about the future repurposing of the it when it is used. * jk/branch-l-0-deprecation: branch: deprecate "-l" option t: switch "branch -l" to "branch --create-reflog" t3200: unset core.logallrefupdates when testing reflog creation
2018-07-18Merge branch 'tb/grep-column'Libravatar Junio C Hamano1-0/+95
"git grep" learned the "--column" option that gives not just the line number but the column number of the hit. * tb/grep-column: contrib/git-jump/git-jump: jump to exact location grep.c: add configuration variables to show matched option builtin/grep.c: add '--column' option to 'git-grep(1)' grep.c: display column number of first match grep.[ch]: extend grep_opt to allow showing matched column grep.c: expose {,inverted} match column in match_line() Documentation/config.txt: camel-case lineNumber for consistency
2018-07-18Merge branch 'xy/format-patch-prereq-patch-id-fix'Libravatar Junio C Hamano1-2/+4
Recently added "--base" option to "git format-patch" command did not correctly generate prereq patch ids. * xy/format-patch-prereq-patch-id-fix: format-patch: clear UNINTERESTING flag before prepare_bases
2018-07-18Merge branch 'pw/rebase-i-keep-reword-after-conflict'Libravatar Junio C Hamano2-0/+76
Bugfix for "rebase -i" corner case regression. * pw/rebase-i-keep-reword-after-conflict: sequencer: do not squash 'reword' commits when we hit conflicts
2018-07-18Merge branch 'sb/submodule-core-worktree'Libravatar Junio C Hamano2-2/+8
"git submodule" did not correctly adjust core.worktree setting that indicates whether/where a submodule repository has its associated working tree across various state transitions, which has been corrected. * sb/submodule-core-worktree: submodule deinit: unset core.worktree submodule: ensure core.worktree is set after update submodule: unset core.worktree if no working tree is present
2018-07-12t3404: fix use of "VAR=VAL cmd" with a shell functionLibravatar Junio C Hamano1-1/+8
Bash may take it happily but running test with dash reveals a breakage. This was not discovered for a long time as no tests after this test depended on GIT_AUTHOR_NAME to be reverted correctly back to the original value after this step is done. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-12sha1-name.c: for ":/", find detached HEAD commitsLibravatar William Chargin1-0/+26
This patch broadens the set of commits matched by ":/<pattern>" to include commits reachable from HEAD but not any named ref. This avoids surprising behavior when working with a detached HEAD and trying to refer to a commit that was recently created and only exists within the detached state. If multiple worktrees exist, only the current worktree's HEAD is considered reachable. This is consistent with the existing behavior for other per-worktree refs: e.g., bisect refs are considered reachable, but only within the relevant worktree. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: William Chargin <wchargin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-11unpack-trees: do not fail reset because of unmerged skipped entryLibravatar Max Kirillov1-0/+59
After modify/delete merge conflict happens in a file skipped by sparse checkout, "git reset --merge", which implements the "--abort" actions, and "git reset --hard" fail with message "Entry * not uptodate. Cannot update sparse checkout." As explained in [1], the up-to-date checker mistakenly treats conflicted entry which does not exist in HEAD as still skipped by sparse checkout. Use the fix suggested in [1]. Also, add test case which verifies the issue is fixed. [1] https://public-inbox.org/git/20180616051444.GA29754@duynguyen.home/ Signed-off-by: Duy Nguyen <pclouds@gmail.com> Signed-off-by: Max Kirillov <max@max630.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-11sequencer: handle empty-set cases consistentlyLibravatar Jeff King1-1/+6
If the user gives us a set that prepare_revision_walk() takes to be empty, like: git cherry-pick base..base then we report an error. It's nonsense, and there's nothing to pick. But if they use revision options that later cull the list, like: git cherry-pick --author=nobody base~2..base then we quietly create an empty todo list and return success. Arguably either behavior is acceptable, but we should definitely be consistent about it. Reporting an error seems to match the original intent, which dates all the way back to 7e2bfd3f99 (revert: allow cherry-picking more than one commit, 2010-06-02). That in turn was trying to match the single-commit case that existed before then (and which continues to issue an error). Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-09clone: check connectivity even if clone is partialLibravatar Jonathan Tan1-0/+48
The commit that introduced the partial clone feature - 548719fbdc ("clone: partial clone", 2017-12-08) - excluded connectivity checks for partial clones, but this also meant that it is possible for a clone to succeed, yet not have all objects either present or promised. Specifically, if cloning with --filter=blob:none from a repository that has a tag pointing to a blob, and the blob is not sent in the packfile, the clone will pass, even if the blob is not referenced by any tree in the packfile. Turn on connectivity checks for partial clone. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-09upload-pack: send refs' objects despite "filter"Libravatar Jonathan Tan2-0/+32
A filter line in a request to upload-pack filters out objects regardless of whether they are directly referenced by a "want" line or not. This means that cloning with "--filter=blob:none" (or another filter that excludes blobs) from a repository with at least one ref pointing to a blob (for example, the Git repository itself) results in output like the following: error: missing object referenced by 'refs/tags/junio-gpg-pub' and if that particular blob is not referenced by a fetched tree, the resulting clone fails fsck because there is no object from the remote to vouch that the missing object is a promisor object. Update both the protocol and the upload-pack implementation to include all explicitly specified "want" objects in the packfile regardless of the filter specification. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-09send-email: automatically determine transfer-encodingLibravatar brian m. carlson1-0/+21
git send-email, when invoked without a --transfer-encoding option, sends 8bit data without a MIME version or a transfer encoding. This has several downsides. First, unless the transfer encoding is specified, it defaults to 7bit, meaning that non-ASCII data isn't allowed. Second, if lines longer than 998 bytes are used, we will send an message that is invalid according to RFC 5322. The --validate option, which is the default, catches this issue, but it isn't clear to many people how to resolve this. To solve these issues, default the transfer encoding to "auto", so that we explicitly specify 8bit encoding when lines don't exceed 998 bytes and quoted-printable otherwise. This means that we now always emit Content-Transfer-Encoding and MIME-Version headers, so remove the conditionals from this portion of the code. It is unlikely that the unconditional inclusion of these two headers will affect the deliverability of messages in anything but a positive way, since MIME is already widespread and well understood by most email programs. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-09send-email: accept long lines with suitable transfer encodingLibravatar brian m. carlson1-0/+13
With --validate (which is the default), we warn about lines exceeding 998 characters due to the limits specified in RFC 5322. However, if we're using a suitable transfer encoding (quoted-printable or base64), we're guaranteed not to have lines exceeding 76 characters, so there's no need to fail in this case. The auto transfer encoding handles this specific case, so accept it as well. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-09send-email: add an auto option for transfer encodingLibravatar brian m. carlson1-0/+23
For most patches, using a transfer encoding of 8bit provides good compatibility with most servers and makes it as easy as possible to view patches. However, there are some patches for which 8bit is not a valid encoding: RFC 5322 specifies that a message must not have lines exceeding 998 octets. Add a transfer encoding value, auto, which indicates that a patch should use 8bit where allowed and quoted-printable otherwise. Choose quoted-printable instead of base64, since base64-encoded plain text is treated as suspicious by some spam filters. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-06userdiff: support new keywords in PHP hunk headerLibravatar Kana Natsuno4-0/+19
Recent version of PHP supports interface, trait, abstract class and final class. This patch fixes the PHP hunk header regexp to support all of these keywords. Signed-off-by: Kana Natsuno <dev@whileimautomaton.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-06t4018: add missing test cases for PHPLibravatar Kana Natsuno3-0/+15
A later patch changes the built-in PHP pattern. These test cases demonstrate aspects of the pattern that we do not want to change. Signed-off-by: Kana Natsuno <dev@whileimautomaton.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-06t5500: prettify non-commit tag testsLibravatar Jeff King1-6/+13
We don't need to use backslash continuation, as the "&&" already provides continuation (and happily soaks up empty lines between commands). We can also expand the multi-line printf into a here-document, which lets us use line breaks more naturally (and avoids another continuation that required us to break the natural indentation). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03fetch-pack: write shallow, then check connectivityLibravatar Jonathan Tan1-0/+43
When fetching, connectivity is checked after the shallow file is updated. There are 2 issues with this: (1) the connectivity check is only performed up to ancestors of existing refs (which is not thorough enough if we were deepening an existing ref in the first place), and (2) there is no rollback of the shallow file if the connectivity check fails. To solve (1), update the connectivity check to check the ancestry chain completely in the case of a deepening fetch by refraining from passing "--not --all" when invoking rev-list in connected.c. To solve (2), have fetch_pack() perform its own connectivity check before updating the shallow file. To support existing use cases in which "git fetch-pack" is used to download objects without much regard as to the connectivity of the resulting objects with respect to the existing repository, the connectivity check is only done if necessary (that is, the fetch is not a clone, and the fetch involves shallow/deepen functionality). "git fetch" still performs its own connectivity check, preserving correctness but sometimes performing redundant work. This redundancy is mitigated by the fact that fetch_pack() reports if it has performed a connectivity check itself, and if the transport supports connect or stateless-connect, it will bubble up that report so that "git fetch" knows not to perform the connectivity check in such a case. This was noticed when a user tried to deepen an existing repository by fetching with --no-shallow from a server that did not send all necessary objects - the connectivity check as run by "git fetch" succeeded, but a subsequent "git fsck" failed. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03ref-filter: avoid backend filtering with --ignore-caseLibravatar Jeff King1-1/+1
When for-each-ref is used with --ignore-case, we expect match_name_as_path() to do a case-insensitive match. But there's an extra layer of filtering that happens before we even get there. Since commit cfe004a5a9 (ref-filter: limit traversal to prefix, 2017-05-22), we feed the prefix to the ref backend so that it can optimize the ref iteration. There's no mechanism for us to tell the backend we're matching case-insensitively. Nor is there likely to be one anytime soon, since the packed backend relies on binary-searching the sorted list of refs. Let's just punt on this case. The extra filtering is an optimization that we simply can't do. We'll still give the correct answer via the filtering in match_name_as_path(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t6300: add a test for --ignore-caseLibravatar Jeff King1-0/+11
The --ignore-case option was added by 3bb16a8bf2 (tag, branch, for-each-ref: add --ignore-case for sorting and filtering, 2016-12-04), but it was never tested. And indeed, it does not work due to multiple bugs (which will be fixed in subsequent patches). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28t5407: fix test to cover intended argumentsLibravatar Elijah Newren1-1/+1
Test 8 in t5407 appears to be an accidental exact duplicate of of test 5; the testcode is identical and has identical repo state, but the test description is different and suggests that rebase -m followed by rebase --skip was what was actually supposed to be tested. Modify the test to include the -m option. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28Merge branch 'sb/fix-fetching-moved-submodules'Libravatar Junio C Hamano1-5/+1
The code to try seeing if a fetch is necessary in a submodule during a fetch with --recurse-submodules got confused when the path to the submodule was changed in the range of commits in the superproject, sometimes showing "(null)". This has been corrected. * sb/fix-fetching-moved-submodules: t5526: test recursive submodules when fetching moved submodules submodule: fix NULL correctness in renamed broken submodules
2018-06-28Merge branch 'jc/clean-after-sanity-tests'Libravatar Junio C Hamano5-9/+6
test cleanup. * jc/clean-after-sanity-tests: tests: clean after SANITY tests
2018-06-28Merge branch 'nd/completion-negation'Libravatar Junio C Hamano1-2/+39
Continuing with the idea to programmatically enumerate various pieces of data required for command line completion, the codebase has been taught to enumerate options prefixed with "--no-" to negate them. * nd/completion-negation: completion: collapse extra --no-.. options completion: suppress some -no- options parse-options: option to let --git-completion-helper show negative form
2018-06-28Merge branch 'pw/add-p-recount'Libravatar Junio C Hamano1-0/+43
When user edits the patch in "git add -p" and the user's editor is set to strip trailing whitespaces indiscriminately, an empty line that is unchanged in the patch would become completely empty (instead of a line with a sole SP on it). The code introduced in Git 2.17 timeframe failed to parse such a patch, but now it learned to notice the situation and cope with it. * pw/add-p-recount: add -p: fix counting empty context lines in edited patches
2018-06-28Merge branch 'jk/fetch-all-peeled-fix'Libravatar Junio C Hamano1-0/+41
"git fetch-pack --all" used to unnecessarily fail upon seeing an annotated tag that points at an object other than a commit. * jk/fetch-all-peeled-fix: fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits fetch-pack: don't try to fetch peel values with --all
2018-06-28Merge branch 'jh/partial-clone'Libravatar Junio C Hamano1-0/+8
The recent addition of "partial clone" experimental feature kicked in when it shouldn't, namely, when there is no partial-clone filter defined even if extensions.partialclone is set. * jh/partial-clone: list-objects: check if filter is NULL before using
2018-06-28Merge branch 'sg/gpg-tests-fix'Libravatar Junio C Hamano4-7/+6
Some flaky tests have been fixed. * sg/gpg-tests-fix: tests: make forging GPG signed commits and tags more robust t7510-signed-commit: use 'test_must_fail'
2018-06-28Merge branch 'as/safecrlf-quiet-fix'Libravatar Junio C Hamano1-0/+10
Fix for 2.17-era regression around `core.safecrlf`. * as/safecrlf-quiet-fix: config.c: fix regression for core.safecrlf false
2018-06-28fetch-pack: implement ref-in-wantLibravatar Brandon Williams1-0/+149
Implement ref-in-want on the client side so that when a server supports the "ref-in-want" feature, a client will send "want-ref" lines for each reference the client wants to fetch. This feature allows clients to tolerate inconsistencies that exist when a remote repository's refs change during the course of negotiation. This allows a client to request to request a particular ref without specifying the OID of the ref. This means that instead of hitting an error when a ref no longer points at the OID it did at the beginning of negotiation, negotiation can continue and the value of that ref will be sent at the termination of negotiation, just before a packfile is sent. More information on the ref-in-want feature can be found in Documentation/technical/protocol-v2.txt. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28upload-pack: test negotiation with changing repositoryLibravatar Brandon Williams4-0/+99
Add tests to check the behavior of fetching from a repository which changes between rounds of negotiation (for example, when different servers in a load-balancing agreement participate in the same stateless RPC negotiation). This forms a baseline of comparison to the ref-in-want functionality (which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28upload-pack: implement ref-in-wantLibravatar Brandon Williams1-0/+160
Currently, while performing packfile negotiation, clients are only allowed to specify their desired objects using object ids. This causes a vulnerability to failure when an object turns non-existent during negotiation, which may happen if, for example, the desired repository is provided by multiple Git servers in a load-balancing arrangement and there exists replication delay. In order to eliminate this vulnerability, implement the ref-in-want feature for the 'fetch' command in protocol version 2. This feature enables the 'fetch' command to support requests in the form of ref names through a new "want-ref <ref>" parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-27Fix use of strategy options with interactive rebasesLibravatar Elijah Newren1-1/+1
git-rebase.sh wrote strategy options to .git/rebase/merge/strategy_opts in the following format: '--ours' '--renormalize' Note the double spaces. git-rebase--interactive uses sequencer.c to parse that file, and sequencer.c used split_cmdline() to get the individual strategy options. After splitting, sequencer.c prefixed each "option" with a double dash, so, concatenating all its options would result in: -- --ours -- --renormalize So, when it ended up calling try_merge_strategy(), that in turn would run git merge-$strategy -- --ours -- --renormalize $merge_base -- $head $remote instead of the expected/desired git merge-$strategy --ours --renormalize $merge_base -- $head $remote Remove the extra spaces so that when it goes through split_cmdline() we end up with the desired command line. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-27t3418: add testcase showing problems with rebase -i and strategy optionsLibravatar Elijah Newren1-0/+32
We are not passing the same args to merge strategies when we are doing an --interactive rebase as we do with a --merge rebase. The merge strategy should not need to be aware of which type of rebase is in effect. Add a testcase which checks for the appropriate args. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-27git-rebase: make --allow-empty-message the defaultLibravatar Elijah Newren2-11/+7
rebase backends currently behave differently with empty commit messages, largely as a side-effect of the different underlying commands on which they are based. am-based rebases apply commits with an empty commit message without stopping or requiring the user to specify an extra flag. (It is interesting to note that am-based rebases are the default rebase type, and no one has ever requested a --no-allow-empty-message flag to change this behavior.) merge-based and interactive-based rebases (which are ultimately based on git-commit), will currently halt on any such commits and require the user to manually specify what to do with the commit and continue. One possible rationale for the difference in behavior is that the purpose of an "am" based rebase is solely to transplant an existing history, while an "interactive" rebase is one whose purpose is to polish a series before making it publishable. Thus, stopping and asking for confirmation for a possible problem is more appropriate in the latter case. However, there are two problems with this rationale: 1) merge-based rebases are also non-interactive and there are multiple types of rebases that use the interactive machinery but are not explicitly interactive (e.g. when either --rebase-merges or --keep-empty are specified without --interactive). These rebases are also used solely to transplant an existing history, and thus also should default to --allow-empty-message. 2) this rationale only says that the user is more accepting of stopping in the case of an explicitly interactive rebase, not that stopping for this particular reason actually makes sense. Exploring whether it makes sense, requires backing up and analyzing the underlying commands... If git-commit did not error out on empty commits by default, accidental creation of commits with empty messages would be a very common occurrence (this check has caught me many times). Further, nearly all such empty commit messages would be considered an accidental error (as evidenced by a huge amount of documentation across version control systems and in various blog posts explaining how important commit messages are). A simple check for what would otherwise be a common error thus made a lot of sense, and git-commit gained an --allow-empty-message flag for special case overrides. This has made commits with empty messages very rare. There are two sources for commits with empty messages for rebase (and cherry-pick): (a) commits created in git where the user previously specified --allow-empty-message to git-commit, and (b) commits imported into git from other version control systems. In case (a), the user has already explicitly specified that there is something special about this commit that makes them not want to specify a commit message; forcing them to re-specify with every cherry-pick or rebase seems more likely to be infuriating than helpful. In case (b), the commit is highly unlikely to have been authored by the person who has imported the history and is doing the rebase or cherry-pick, and thus the user is unlikely to be the appropriate person to write a commit message for it. Stopping and expecting the user to modify the commit before proceeding thus seems counter-productive. Further, note that while empty commit messages was a common error case for git-commit to deal with, it is a rare case for rebase (or cherry-pick). The fact that it is rare raises the question of why it would be worth checking and stopping on this particular condition and not others. For example, why doesn't an interactive rebase automatically stop if the commit message's first line is 2000 columns long, or is missing a blank line after the first line, or has every line indented with five spaces, or any number of other myriad problems? Finally, note that if a user doing an interactive rebase does have the necessary knowledge to add a message for any such commit and wants to do so, it is rather simple for them to change the appropriate line from 'pick' to 'reword'. The fact that the subject is empty in the todo list that the user edits should even serve as a way to notify them. As far as I can tell, the fact that merge-based and interactive-based rebases stop on commits with empty commit messages is solely a by-product of having been based on git-commit. It went without notice for a long time precisely because such cases are rare. The rareness of this situation made it difficult to reason about, so when folks did eventually notice this behavior, they assumed it was there for a good reason and just added an --allow-empty-message flag. In my opinion, stopping on such messages not desirable in any of these cases, even the (explicitly) interactive case. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>