summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)Author
2021-09-10t5562: use alarm() to interrupt timed child-waitJeff King
The t5562 script occasionally takes 60 extra seconds to complete due to a race condition in the invoke-with-content-length.pl helper. The way it's supposed to work is this: - we set up a SIGCLD handler - we kick off http-backend and write to it with a set content-length, but _don't_ close the pipe - we sleep for 60 seconds, assuming that SIGCLD from http-backend finishing will interrupt us - after the sleep finishes (whetherby 60 seconds or because it was interrupted by the signal), we check a flag to see if our SIGCLD handler was called. If not, then we complain. This usually completes immediately, because the signal interrupts our sleep. But very occasionally the child process dies _before_ we hit the sleep, so we don't realize it. The test still completes successfully (because our $exited flag is set), but it takes an extra 60 seconds. There's no way to check the flag and sleep atomically. So the best we can do with this approach is to sleep in smaller chunks (say, 1 second) and check the flag incrementally. Then we waste a maximum of 1 second if we lose the race. This was proposed in: https://lore.kernel.org/git/20190218205028.32486-1-max@max630.net/ and it does work. But we can do better. Instead of blocking on sleep and waiting for the child signal to interrupt us, we can block on the child exiting and set an alarm signal to trigger the timeout. This lets us exit the script immediately when the child behaves (with no race possible), and wait a maximum of 60 seconds when it doesn't. Note one small subtlety: perl is very willing to restart the waitpid() call after the alarm is delivered, even if we've thrown an exception via die. "perldoc -f alarm" claims you can get around this with an eval/die combo (and even has some example code), but it doesn't seem to work for me with waitpid(); instead, we continue waiting until the child exits. So instead, we'll instruct the child process to exit in the alarm handler itself. In the original code this was done by calling close($out). That would continue to work, since our child is always http-backend, which should exit when its stdin closes. But we can be even more robust against a hung or confused child by sending a KILL signal, which should terminate it immediately. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-07send-email: fix a "first config key wins" regression in v2.33.0Ævar Arnfjörð Bjarmason
Fix a regression in my c95e3a3f0b8 (send-email: move trivial config handling to Perl, 2021-05-28) where we'd pick the first config key out of multiple defined ones, instead of using the normal "last key wins" semantics of "git config --get". This broke e.g. cases where a .git/config would have a different sendemail.smtpServer than ~/.gitconfig. We'd pick the ~/.gitconfig over .git/config, instead of preferring the repository-local version. The same would go for /etc/gitconfig etc. The full list of impacted config keys (the %config_settings values which are references to scalars, not arrays) is: sendemail.smtpencryption sendemail.smtpserver sendemail.smtpserverport sendemail.smtpuser sendemail.smtppass sendemail.smtpdomain sendemail.smtpauth sendemail.smtpbatchsize sendemail.smtprelogindelay sendemail.tocmd sendemail.cccmd sendemail.aliasfiletype sendemail.envelopesender sendemail.confirm sendemail.from sendemail.assume8bitencoding sendemail.composeencoding sendemail.transferencoding sendemail.sendmailcmd I.e. having any of these set in say ~/.gitconfig and in-repo .git/config regressed in v2.33.0 to prefer the --global one over the --local. To test this add a test of config priority to one of these config variables, most don't have tests at all, but there was an existing one for sendemail.8bitEncoding. The "git config" (instead of "test_config") is somewhat of an anti-pattern, but follows established conventions in t9001-send-email.sh, likewise with any other pattern or idiom in this test. The populating of home/.gitconfig and setting of HOME= is copied from a test in t0017-env-helper.sh added in 1ff750b128e (tests: make GIT_TEST_GETTEXT_POISON a boolean, 2019-06-21). This test fails without this bugfix, but now it works. Reported-by: Eli Schwartz <eschwartz@archlinux.org> Tested-by: Eli Schwartz <eschwartz@archlinux.org> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-05apply: resolve trivial merge without hitting ll-merge with "--3way"Junio C Hamano
The ll_binary_merge() function assumes that the ancestor blob is different from either side of the new versions, and always fails the merge in conflict, unless -Xours or -Xtheirs is in effect. The normal "merge" machineries all resolve the trivial cases (e.g. if our side changed while their side did not, the result is ours) without triggering the file-level merge drivers, so the assumption is warranted. The code path in "git apply --3way", however, does not check for the trivial three-way merge situation and always calls the file-level merge drivers. This used to be perfectly OK back when we always first attempted a straight patch application and used the three-way code path only as a fallback. Any binary patch that can be applied as a trivial three-way merge (e.g. the patch is based exactly on the version we happen to have) would always cleanly apply, so the ll_binary_merge() that is not prepared to see the trivial case would not have to handle such a case. This no longer is true after we made "--3way" to mean "first try three-way and then fall back to straight application", and made "git apply -3" on a binary patch that is based on the current version no longer apply. Teach "git apply -3" to first check for the trivial merge cases and resolve them without hitting the file-level merge drivers. Signed-off-by: Jerry Zhang <jerry@skydio.com> [jc: stolen tests from Jerry's patch] Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-03update-ref: fix streaming of status updatesPatrick Steinhardt
When executing git-update-ref(1) with the `--stdin` flag, then the user can queue updates and, since e48cf33b61 (update-ref: implement interactive transaction handling, 2020-04-02), interactively drive the transaction's state via a set of transactional verbs. This interactivity is somewhat broken though: while the caller can use these verbs to drive the transaction's state, the status messages which confirm that a verb has been processed is not flushed. The caller may thus be left hanging waiting for the acknowledgement. Fix the bug by flushing stdout after writing the status update. Add a test which catches this bug. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01upload-pack.c: treat want-ref relative to namespaceKim Altintop
When 'upload-pack' runs within the context of a git namespace, treat any 'want-ref' lines the client sends as relative to that namespace. Also check if the wanted ref is hidden via 'hideRefs'. If it is hidden, respond with an error as if the ref didn't exist. Helped-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Kim Altintop <kim@eagain.st> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01t5730: introduce fetch command helperKim Altintop
Assembling a "raw" fetch command to be fed directly to "test-tool serve-v2" is extracted into a test helper. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kim Altintop <kim@eagain.st> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-31fast-export: fix anonymized tag using original lengthTal Kelrich
Commit 7f4075949686 (fast-export: tighten anonymize_mem() interface to handle only strings, 2020-06-23) changed the interface used in anonymizing strings, but failed to update the size of annotated tag messages to match the new anonymized string. As a result, exporting tags having messages longer than 13 characters would create output that couldn't be parsed by fast-import, as the data length indicated was larger than the data output. Reset the message size when anonymizing, and add a tag with a "long" message to the test. Signed-off-by: Tal Kelrich <hasturkun@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-30send-email: avoid incorrect header propagationMarvin Häuser
If multiple independent patches are sent with send-email, even if the "In-Reply-To" and "References" headers are not managed by --thread or --in-reply-to, their values may be propagated from prior patches to subsequent patches with no such headers defined. To mitigate this and potential future issues, make sure all global patch-specific variables are always either handled by command-specific code (e.g. threading), or are reset to their default values for every iteration. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-30test-lib: set GIT_CEILING_DIRECTORIES to protect the surrounding repositorySZEDER Gábor
Every once in a while a test somehow manages to escape from its trash directory and modifies the surrounding repository, whether because of a bug in git itself, a bug in a test [1], or e.g. when trying to run tests with a shell that is, in general, unable to run our tests [2]. Set GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/.." as an additional safety measure to protect the surrounding repository at least from modifications by git commands executed in the tests (assuming that handling of ceiling directories during repository discovery is not broken, and, of course, it won't save us from regular shell commands, e.g. 'cd .. && rm -f ...'). [1] e.g. https://public-inbox.org/git/20210423051255.GD2947267@szeder.dev [2] $ git symbolic-ref HEAD refs/heads/master $ ksh ./t2011-checkout-invalid-head.sh [... a lot of "not ok" ...] $ git symbolic-ref HEAD refs/heads/other (In short: 'ksh' doesn't support the 'local' builtin command, which is used by 'test_oid', causing it to return with error whenever it's called, leaving ZERO_OID set to empty, so when the test 'checkout main from invalid HEAD' runs 'echo $ZERO_OID >.git/HEAD' it writes a corrupt (not invalid) HEAD, and subsequent git commands don't recognize the repository in the trash directory anymore, but operate on the surrounding repo.) Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-27branch: allow deleting dangling branches with --forceRené Scharfe
git branch only allows deleting branches that point to valid commits. Skip that check if --force is given, as the caller is indicating with it that they know what they are doing and accept the consequences. This allows deleting dangling branches, which previously had to be reset to a valid start-point using --force first. Reported-by: Ulrich Windl <Ulrich.Windl@rz.uni-regensburg.de> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-27logmsg_reencode(): warn when iconv() failsJeff King
If the user asks for a pretty-printed commit to be converted (either explicitly with --encoding=foo, or implicitly because the commit is non-utf8 and we want to convert it), we pass it through iconv(). If that fails, we fall back to showing the input verbatim, but don't tell the user that the output may be bogus. Let's add a warning to do so, along with a mention in the documentation for --encoding. Two things to note about the implementation: - we could produce the warning closer to the call to iconv() in reencode_string_len(), which would let us relay the value of errno. But this is not actually very helpful. reencode_string_len() does not know we are operating on a commit, and indeed does not know that the caller won't produce an error of its own. And the errno values from iconv() are seldom helpful (iconv_open() only ever produces EINVAL; perhaps EILSEQ from iconv() might be illuminating, but it can also return EINVAL for incomplete sequences). - if the reason for the failure is that the output charset is not supported, then the user will see this warning for every commit we try to display. That might be ugly and overwhelming, but on the other hand it is making it clear that every one of them has not been converted (and the likely outcome anyway is to re-try the command with a supported output encoding). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-27checkout: make delayed checkout respect --quiet and --no-progressMatheus Tavares
The 'Filtering contents...' progress report from delayed checkout is displayed even when checkout and clone are invoked with --quiet or --no-progress. Furthermore, it is displayed unconditionally, without first checking whether stdout is a tty. Let's fix these issues and also add some regression tests for the two code paths that currently use delayed checkout: unpack_trees.c:check_updates() and builtin/checkout.c:checkout_worktree(). Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-26column: fix parsing of the '--nl' optionSZEDER Gábor
'git column's '--nl' option can be used to specify a "string to be printed at the end of each line" (quoting the man page), but this option and its mandatory argument has been parsed as OPT_INTEGER since the introduction of the command in 7e29b8254f (Add column layout skeleton and git-column, 2012-04-21). Consequently, any non-number argument is rejected by parse-options, and any number other than 0 leads to segfault: $ printf "%s\n" one two |git column --mode=plain --nl=foo error: option `nl' expects a numerical value $ printf "%s\n" one two |git column --mode=plain --nl=42 Segmentation fault (core dumped) $ printf "%s\n" one two |git column --mode=plain --nl=0 one two Parse this option as OPT_STRING. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-25diff-lib: ignore paths that are outside $cwd if --relative askedĐoàn Trần Công Danh
For diff family commands, we can tell them to exclude changes outside of some directories if --relative is requested. In diff_unmerge(), NULL will be returned if the requested path is outside of the interesting directories, thus we'll run into NULL pointer dereference in run_diff_files when trying to dereference its return value. Checking for return value of diff_unmerge before dereferencing is not sufficient, though. Since, diff engine will try to work on such pathspec later. Let's not run diff on those unintesting entries, instead. As a side effect, by skipping like that, we can save some CPU cycles. Reported-by: Thomas De Zeeuw <thomas@slight.dev> Tested-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-25t6300: check for cat-file exit status codeĐoàn Trần Công Danh
In test_atom(), we're piping the output of cat-file to tail(1), thus, losing its exit status. Let's use a temporary file to preserve git exit status code. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-25t6300: don't run cat-file on non-existent objectĐoàn Trần Công Danh
In t6300, some tests are guarded behind some prerequisites. Thus, objects created by those tests ain't available if those prerequisites are unsatistified. Attempting to run "cat-file" on those objects will run into failure. In fact, running t6300 in an environment without gpg(1), we'll see those warnings: fatal: Not a valid object name refs/tags/signed-empty fatal: Not a valid object name refs/tags/signed-short fatal: Not a valid object name refs/tags/signed-long Let's put those commands into the real tests, in order to: * skip their execution if prerequisites aren't satistified. * check their exit status code The expected value for objects with type: commit needs to be computed outside the test because we can't rely on "$3" there. Furthermore, to prevent the accidental usage of that computed expected value, BUG out on unknown object's type. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-25t5323: drop mentions of "master"Jeff King
Commit 0696232390 (pack-redundant: fix crash when one packfile in repo, 2020-12-16) added one some new tests to t5323. At the time, the sub-repo we used was called "master". But in a parallel branch, this was switched to "main". When the latter branch was merged in 27d7c8599b (Merge branch 'js/default-branch-name-tests-final-stretch', 2021-01-25), some of those spots caused textual conflicts, but some (for tests that were far enough away from other changed code) were just semantic. The merge resolution fixed up most spots, but missed this one. Even though this did impact actual code, it turned out not to fail the tests. Running 'cd "$master_repo"' ended up staying in the same directory, running the test in the main trash repo instead of the sub-repo. But because the point of the test is checking behavior when there are no packfiles, it worked in either repo (since both are empty at this point in the script). Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-24ls-remote: set packet_trace_identity(<name>)Ævar Arnfjörð Bjarmason
Set packet_trace_identity() for ls-remote. This replaces the generic "git" identity in GIT_TRACE_PACKET=<file> traces to "ls-remote", e.g.: [...] packet: upload-pack> version 2 [...] packet: upload-pack> agent=git/2.32.0-dev [...] packet: ls-remote< version 2 [...] packet: ls-remote< agent=git/2.32.0-dev Where in an "git ls-remote file://<path>" dialog ">" is the sender (or "to the server") and "<" is the recipient (or "received by the client"). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-24maintenance: skip bootout/bootstrap when plist is registeredDerrick Stolee
On macOS, we use launchctl to manage the background maintenance schedule. This uses a set of .plist files to describe the schedule, but these files are also registered with 'launchctl bootstrap'. If multiple 'git maintenance start' commands run concurrently, then they can collide replacing these schedule files and registering them with launchctl. To avoid extra launchctl commands, do a check for the .plist files on disk and check if they are registered using 'launchctl list <name>'. This command will return with exit code 0 if it exists, or exit code 113 if it does not. We can test this behavior using the GIT_TEST_MAINT_SCHEDULER environment variable. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-24t9001: PATH must not use Windows-style pathsJohannes Sixt
On Windows, $(pwd) returns a drive-letter style path C:/foo, while $PWD contains a POSIX style /c/foo path. When we want to interpolate the current directory in the PATH variable, we must not use the C:/foo style, because the meaning of the colon is ambiguous. Use the POSIX style. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-24t5582: remove spurious 'cd "$D"' lineMickey Endito
The variable D is never defined in test t5582, more severely the test fails if D is defined by something outside the test suite, so remove this spurious line. Signed-off-by: Mickey Endito <mickey.endito.2323@protonmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-23rebase -r: fix merge -c with a merge strategyPhillip Wood
If a rebase is started with a --strategy option other than "ort" or "recursive" then "merge -c" does not allow the user to reword the commit message. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-23rebase -r: don't write .git/MERGE_MSG when fast-forwardingPhillip Wood
When fast-forwarding we do not create a new commit so .git/MERGE_MSG is not removed and can end up seeding the message of a commit made after the rebase has finished. Avoid writing .git/MERGE_MSG when we are fast-forwarding by writing the file after the fast-forward checks. Note that there are no changes to the fast-forward code, it is simply moved. Note that the way this change is implemented means we no longer write the author script when fast-forwarding either. I believe this is safe for the reasons below but it is a departure from what we do when fast-forwarding a non-merge commit. If we reword the merge then 'git commit --amend' will keep the authorship of the commit we're rewording as it ignores GIT_AUTHOR_* unless --reset-author is passed. It will also export the correct GIT_AUTHOR_* variables to any hooks and we already test the authorship of the reworded commit. If we are not rewording then we no longer call spilt_ident() which means we are no longer checking the commit author header looks sane. However this is what we already do when fast-forwarding non-merge commits in skip_unnecessary_picks() so I don't think we're breaking any promises by not checking the author here. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-23rebase -i: add another reword testPhillip Wood
None of the existing reword tests check that there are no uncommitted changes when the editor is opened. Reuse the editor script from the last commit to fix this omission. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-20rebase -r: make 'merge -c' behave like rewordPhillip Wood
If the user runs git log while rewording a commit it is confusing if sometimes we're amending the commit that's being reworded and at other times we're creating a new commit depending on whether we could fast-forward or not[1]. For this reason the reword command ensures that there are no uncommitted changes when rewording. The reword command also allows the user to edit the todo list while the rebase is paused. As 'merge -c' also rewords commits make it behave like reword and add a test. [1] https://lore.kernel.org/git/xmqqlfvu4be3.fsf@gitster-ct.c.googlers.com/T/#m133009cb91cf0917bcf667300f061178be56680a Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-18completion: bash: fix for suboptions with valueFelipe Contreras
We need to ignore options that don't start with -- as well. Depending on the value of COMP_WORDBREAKS the last word could be duplicated otherwise. Can be tested with: git merge -X diff-algorithm=<tab> Tested-by: David Aguilar <davvid@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-15commit: restore --edit when combined with --fixupJoel Klinghed
Recent changes to --fixup, adding amend suboption, caused the --edit flag to be ignored as use_editor was always set to zero. Restore edit_flag having higher priority than fixup_message when deciding the value of use_editor by moving the edit flag condition later in the method. Signed-off-by: Joel Klinghed <the_jk@spawned.biz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-13rebase --continue: remove .git/MERGE_MSGPhillip Wood
If the user skips the final commit by removing all the changes from the index and worktree with 'git restore' (or read-tree) and then runs 'git rebase --continue' .git/MERGE_MSG is left behind. This will seed the commit message the next time the user commits which is not what we want to happen. Reported-by: Victor Gambier <vgambier@excilys.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-13rebase --apply: restore some testsPhillip Wood
980b482d28 ("rebase tests: mark tests specific to the am-backend with --am", 2020-02-15) sought to prepare tests testing the "apply" backend in preparation for 2ac0d6273f ("rebase: change the default backend from "am" to "merge"", 2020-02-15). However some tests seem to have been missed leading to us testing the "merge" backend twice. This patch fixes some cases that I noticed while adding tests to these files, I have not audited all the other rebase test files. I've reworded a couple of the test descriptions to make it clear which backend they are testing. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-13t3403: fix commit authorshipPhillip Wood
Setting GIT_AUTHOR_* when committing with --amend will only change the author if we also pass --reset-author. This commit is used in some tests that ensure the author ident does not change when rebasing. Creating this commit without changing the authorship meant that the test would not catch regressions that caused rebase to discard the original authorship information. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-11Merge branch 'jn/log-m-does-not-imply-p'Junio C Hamano
Earlier "git log -m" was changed to always produce patch output, which would break existing scripts, which has been reverted. * jn/log-m-does-not-imply-p: Revert 'diff-merges: let "-m" imply "-p"'
2021-08-10connect, protocol: log negotiated protocol versionJosh Steadmon
It is useful for performance monitoring and debugging purposes to know the wire protocol used for remote operations. This may differ from the version set in local configuration due to differences in version and/or configuration between the server and the client. Therefore, log the negotiated wire protocol version via trace2, for both clients and servers. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-10apply: keep buffer/size pair in sync when parsing binary hunksJeff King
We parse through binary hunks by looping through the buffer with code like: llen = linelen(buffer, size); ...do something with the line... buffer += llen; size -= llen; However, before we enter the loop, there is one call that increments "buffer" but forgets to decrement "size". As a result, our "size" is off by the length of that line, and subsequent calls to linelen() may look past the end of the buffer for a newline. The fix is easy: we just need to decrement size as we do elsewhere. This bug goes all the way back to 0660626caf (binary diff: further updates., 2006-05-05). Presumably nobody noticed because it only triggers if the patch is corrupted, and even then we are often "saved" by luck. We use a strbuf to store the incoming patch, so we overallocate there, plus we add a 16-byte run of NULs as slop for memory comparisons. So if this happened accidentally, the common case is that we'd just read a few uninitialized bytes from the end of the strbuf before producing the expected "this patch is corrupted" error complaint. However, it is possible to carefully construct a case which reads off the end of the buffer. The included test does so. It will pass both before and after this patch when run normally, but using a tool like ASan shows that we get an out-of-bounds read before this patch, but not after. Reported-by: Xingman Chen <xichixingman@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-09Revert 'diff-merges: let "-m" imply "-p"'Jonathan Nieder
This reverts commit f5bfcc823ba242a46e20fb6f71c9fbf7ebb222fe, which made "git log -m" imply "--patch" by default. The logic was that "-m", which makes diff generation for merges perform a diff against each parent, has no use unless I am viewing the diff, so we could save the user some typing by turning on display of the resulting diff automatically. That wasn't expected to adversely affect scripts because scripts would either be using a command like "git diff-tree" that already emits diffs by default or would be combining -m with a diff generation option such as --name-status. By saving typing for interactive use without adversely affecting scripts in the wild, it would be a pure improvement. The problem is that although diff generation options are only relevant for the displayed diff, a script author can imagine them affecting path limiting. For example, I might run git log -w --format=%H -- README hoping to list commits that edited README, excluding whitespace-only changes. In fact, a whitespace-only change is not TREESAME so the use of -w here has no effect (since we don't apply these diff generation flags to the diff_options struct rev_info::pruning used for this purpose), but the documentation suggests that it should work Suppose you specified foo as the <paths>. We shall call commits that modify foo !TREESAME, and the rest TREESAME. (In a diff filtered for foo, they look different and equal, respectively.) and a script author who has not tested whitespace-only changes wouldn't notice. Similarly, a script author could include git log -m --first-parent --format=%H -- README to filter the first-parent history for commits that modified README. The -m is a no-op but it reflects the script author's intent. For example, until 1e20a407fe2 (stash list: stop passing "-m" to "git log", 2021-05-21), "git stash list" did this. As a result, we can't safely change "-m" to imply "-p" without fear of breaking such scripts. Restore the previous behavior. Noticed because Rust's src/bootstrap/bootstrap.py made use of this same construct: https://github.com/rust-lang/rust/pull/87513. That script has been updated to omit the unnecessary "-m" option, but we can expect other scripts in the wild to have similar expectations. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-06Merge branch 'cb/t7508-regexp-fix'Junio C Hamano
* cb/t7508-regexp-fix: t7508: avoid non POSIX BRE
2021-08-06Merge branch 'fc/disable-checkwinsize'Junio C Hamano
* fc/disable-checkwinsize: test: fix for COLUMNS and bash 5
2021-08-06test: fix for COLUMNS and bash 5Felipe Contreras
Since c49a177bec (test-lib.sh: set COLUMNS=80 for --verbose repeatability, 2021-06-29) multiple tests have been failing when using bash 5 because checkwinsize is enabled by default, therefore COLUMNS is reset using TIOCGWINSZ even for non-interactive shells. It's debatable whether or not bash should even be doing that, but for now we can avoid this undesirable behavior by disabling this option. Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> [jc: with SZEDER Gábor's suggestion to do this before setting COLUMNS] Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-04Merge branch 'tb/mingw-rmdir-symlink-to-directory'Junio C Hamano
Windows rmdir() equivalent behaves differently from POSIX ones in that when used on a symbolic link that points at a directory, the target directory gets removed, which has been corrected. * tb/mingw-rmdir-symlink-to-directory: mingw: align symlinks-related rmdir() behavior with Linux
2021-08-04Merge branch 'ab/getcwd-test'Junio C Hamano
Portability test update. * ab/getcwd-test: t0001: fix broken not-quite getcwd(3) test in bed67874e2
2021-08-04Merge branch 'pb/merge-autostash-more'Junio C Hamano
The local changes stashed by "git merge --autostash" were lost when the merge failed in certain ways, which has been corrected. * pb/merge-autostash-more: merge: apply autostash if merge strategy fails merge: apply autostash if fast-forward fails Documentation: define 'MERGE_AUTOSTASH' merge: add missing word "strategy" to a message
2021-08-04Merge branch 'en/ort-perf-batch-14'Junio C Hamano
Further optimization on "merge -sort" backend. * en/ort-perf-batch-14: merge-ort: restart merge with cached renames to reduce process entry cost merge-ort: avoid recursing into directories when we don't need to merge-ort: defer recursing into directories when merge base is matched merge-ort: add a handle_deferred_entries() helper function merge-ort: add data structures for allowable trivial directory resolves merge-ort: add some more explanations in collect_merge_info_callback() merge-ort: resolve paths early when we have sufficient information
2021-08-04Merge branch 'ds/commit-and-checkout-with-sparse-index'Junio C Hamano
"git checkout" and "git commit" learn to work without unnecessarily expanding sparse indexes. * ds/commit-and-checkout-with-sparse-index: unpack-trees: resolve sparse-directory/file conflicts t1092: document bad 'git checkout' behavior checkout: stop expanding sparse indexes sparse-index: recompute cache-tree commit: integrate with sparse-index p2000: compress repo names p2000: add 'git checkout -' test and decrease depth
2021-08-04Merge branch 'ar/submodule-add'Junio C Hamano
Rewrite of "git submodule" in C continues. * ar/submodule-add: submodule: drop unused sm_name parameter from show_fetch_remotes() submodule--helper: introduce add-clone subcommand submodule--helper: refactor module_clone() submodule: prefix die messages with 'fatal' t7400: test failure to add submodule in tracked path
2021-08-02mingw: align symlinks-related rmdir() behavior with LinuxThomas Bétous
When performing a rebase, rmdir() is called on the folder .git/logs. On Unix rmdir() exits without deleting anything in case .git/logs is a symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir and RemoveDirectoryW) do not behave the same and remove the folder if it is symlinked even if it is not empty. This creates issues when folders in .git/ are symlinks which is especially the case when git-repo[1] is used: It replaces `.git/logs/` with a symlink. One such issue is that the _target_ of that symlink is removed e.g. during a `git rebase`, where `delete_reflog("REBASE_HEAD")` will not only try to remove `.git/logs/REBASE_HEAD` but then recursively try to remove the parent directories until an error occurs, a technique that obviously relies on `rmdir()` refusing to remove a symlink. This was reported in https://github.com/git-for-windows/git/issues/2967. This commit updates mingw_rmdir() so that its behavior is the same as Linux rmdir() in case of symbolic links. To verify that Git does not regress on the reported issue, this patch adds a regression test for the `git rebase` symptom, even if the same `rmdir()` behavior is quite likely to cause potential problems in other Git commands as well. [1]: git-repo is a python tool built on top of Git which helps manage many Git repositories. It stores all the .git/ folders in a central place by taking advantage of symbolic links. More information: https://gerrit.googlesource.com/git-repo/ Signed-off-by: Thomas Bétous <tomspycell@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-02t7508: avoid non POSIX BRECarlo Marcelo Arenas Belón
24c30e0b6 (wt-status: tolerate dangling marks, 2020-09-01) adds a test that uses a BRE which breaks at least with OpenBSD's grep. switch to an ERE as it is done for similar checks and while at it, remove the now obsolete test_i18ngrep call. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-02Merge branch 'jt/bulk-prefetch'Junio C Hamano
"git read-tree" had a codepath where blobs are fetched one-by-one from the promisor remote, which has been corrected to fetch in bulk. * jt/bulk-prefetch: cache-tree: prefetch in partial clone read-tree unpack-trees: refactor prefetching code
2021-08-02Merge branch 'ab/bundle-tests'Junio C Hamano
"git bundle" gained more test coverage. * ab/bundle-tests: bundle tests: use test_cmp instead of grep bundle tests: use ">file" not ": >file"
2021-08-02Merge branch 'ps/perf-with-separate-output-directory'Junio C Hamano
Test update. * ps/perf-with-separate-output-directory: perf: fix when running with TEST_OUTPUT_DIRECTORY
2021-07-30t0001: fix broken not-quite getcwd(3) test in bed67874e2Ævar Arnfjörð Bjarmason
With a54e938e5b (strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD, 2017-03-26) we had t0001 break on systems like OpenBSD and AIX whose getcwd(3) has standard (but not like glibc et al) behavior. This was partially fixed in bed67874e2 (t0001: skip test with restrictive permissions if getpwd(3) respects them, 2017-08-07). The problem with that fix is that while its analysis of the problem is correct, it doesn't actually call getcwd(3), instead it invokes "pwd -P". There is no guarantee that "pwd -P" is going to call getcwd(3), as opposed to e.g. being a shell built-in. On AIX under both bash and ksh this test breaks because "pwd -P" will happily display the current working directory, but getcwd(3) called by the "git init" we're testing here will fail to get it. I checked whether clobbering the $PWD environment variable would affect it, and it didn't. Presumably these shells keep track of their working directory internally. There's possible follow-up work here in teaching strbuf_getcwd() to get the working directory with whatever method "pwd" uses on these platforms. See [1] for a discussion of that, but let's take the easy way out here and just skip these tests by fixing the GETCWD_IGNORES_PERMS prerequisite to match the limitations of strbuf_getcwd(). 1. https://lore.kernel.org/git/b650bef5-d739-d98d-e9f1-fa292b6ce982@web.de/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-28Merge branch 'ps/t0000-output-directory-fix'Junio C Hamano
"TEST_OUTPUT_DIRECTORY=there make test" failed to work, which has been corrected. * ps/t0000-output-directory-fix: t0000: fix test if run with TEST_OUTPUT_DIRECTORY