summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* diff-highlight: split code into modulejk/diff-highlight-moduleJeff King2017-06-155-19/+82
| | | | | | | | | | | | | | | | | | | | | | The diff-so-fancy project is also written in perl, and most of its users pipe diffs through both diff-highlight and diff-so-fancy. It would be nice if this could be done in a single script. So let's pull most of diff-highlight's code into its own module which can be used by diff-so-fancy. In addition, we'll abstract a few basic items like reading from stdio so that a script using the module can do more processing before or after diff-highlight handles the lines. See the README update for more details. One small downside is that the diff-highlight script must now be built using the Makefile. There are ways around this, but it quickly gets into perl arcana. Let's go with the simple solution. As a bonus, our Makefile now respects the PERL_PATH variable if it is set. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Prepare for 2.13.2cc/subprocess-comment-fixJunio C Hamano2017-06-132-1/+38
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ad/pull-remote-doc' into maintJunio C Hamano2017-06-131-3/+3
|\ | | | | | | | | | | | | Docfix. * ad/pull-remote-doc: docs: fix formatting and grammar
| * docs: fix formatting and grammarad/pull-remote-docAdam Dinwoodie2017-06-021-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When compiling the documentation, asciidoc thinks a backtick surrounded by whitespace shouldn't be interpreted as marking the start or end of a literal. In most cases, that's useful behaviour, but in the git-pull documentation it means asciidoc is failing to correctly detect which text should be monospaced and which shouldn't. To avoid this, remove the extraneous spaces from the text to be monospaced. It would also be possible to fix the formatting by switching to asciidoc's ++ monospace format markers and still have the space characters included in the monospace text, but the spaces aren't necessary and not having them keeps the markup simpler. Also include a minor grammar fix suggested by Jeff while we're changing these lines. Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/drop-free-refspecs' into maintJunio C Hamano2017-06-131-22/+6
|\ \ | | | | | | | | | | | | | | | | | | Code clean-up. * jk/drop-free-refspecs: remote: drop free_refspecs() function
| * | remote: drop free_refspecs() functionjk/drop-free-refspecsJeff King2017-06-011-22/+6
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We already have free_refspec(), a public function which does the same thing as the static free_refspecs(). Let's just keep one. There are two minor differences between the functions: 1. free_refspecs() is a noop when the refspec argument is NULL. This probably doesn't matter in practice. The nr_refspec parameter would presumably be 0 in that case, skipping the loop. And free(NULL) is explicitly OK. But it doesn't hurt for us to port this extra safety to free_refspec(), as one of the callers passes a funny "i+1" count. 2. The order of arguments is reversed between the two functions. This patch uses the already-public order of free_refspec(), as it matches the argument order on the parsing side. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/connect-symref-info-leak-fix' into maintJunio C Hamano2017-06-131-1/+1
|\ \ | | | | | | | | | | | | | | | | | | Leakfix. * jk/connect-symref-info-leak-fix: connect.c: fix leak in parse_one_symref_info()
| * | connect.c: fix leak in parse_one_symref_info()jk/connect-symref-info-leak-fixJeff King2017-05-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we successfully parse a symref value like "HEAD:refs/heads/master", we add the result to a string list. But because the string list is marked STRING_LIST_INIT_DUP, the string list code will make a copy of the string and add the copy. This patch fixes it by adding the entry with string_list_append_nodup(), which lets the string list take ownership of our newly allocated string. There are two alternatives that seem like they would work, but aren't the right solution. The first is to initialize the list with the "NODUP" initializer. That would avoid the copy, but then the string list would not realize that it owns the strings. When we eventually call string_list_clear(), it would not free the strings, causing a leak. The second option would be to use the normal string_list_append(), but free the local copy in our function. We can't do this because the local copy actually contains _two_ strings; the symref name and its target. We point to the target pointer via the "util" field, and its memory must last as long as the string list does. You may also wonder whether it's safe to ever free the local copy, since the target points into it. The answer is yes, because we duplicate it in annotaate_refs_with_symref_info before clearing the string list. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'rf/completion-config-commit' into maintJunio C Hamano2017-06-131-0/+3
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Completion update. * rf/completion-config-commit: completion: add completions for git config commit
| * | | completion: add completions for git config commitrf/completion-config-commitRikard Falkeborn2017-05-301-0/+3
| | |/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | Add missing completions for git config: * commit.cleanup * commit.gpgSign * commit.verbose Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'ab/t3070-test-dedup' into maintJunio C Hamano2017-06-131-1/+0
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Test cleanup. * ab/t3070-test-dedup: wildmatch test: remove redundant duplicate test
| * | | wildmatch test: remove redundant duplicate testab/t3070-test-dedupÆvar Arnfjörð Bjarmason2017-05-291-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove a test line that's exactly the same as the preceding line. This was brought in in commit feabcc173b ("Integrate wildmatch to git", 2012-10-15), these tests are originally copied from rsync.git, but the duplicate line was never present there, so must have just snuck in during integration with git by accident. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jh/memihash-opt' into maintJunio C Hamano2017-06-131-5/+42
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | perf-test update. * jh/memihash-opt: p0004: don't error out if test repo is too small p0004: don't abort if multi-threaded is too slow p0004: use test_perf p0004: avoid using pipes p0004: simplify calls of test-lazy-init-name-hash
| * | | | p0004: don't error out if test repo is too smallRené Scharfe2017-05-161-5/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Repositories with less than 4000 entries are always handled using a single thread, causing test-lazy-init-name-hash --multi to error out. Don't abort the whole test script in that case, but simply skip the multi-threaded performance check. We can still use it to compare the single-threaded speed of different versions in that case. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff Hostetler <git@jeffhostetler.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | p0004: don't abort if multi-threaded is too slowRené Scharfe2017-05-161-4/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the single-threaded variant beats the multi-threaded one then we may have a performance bug, but that doesn't justify aborting the test. Drop that check; we can compare the results for --single and --multi using the actual performance tests. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff Hostetler <git@jeffhostetler.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | p0004: use test_perfRené Scharfe2017-05-161-0/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The perf test suite (more specifically: t/perf/aggregate.perl) requires each test script to write test results into a file, otherwise it aborts when aggregating. Add actual performance tests with test_perf to allow p0004 to be run together with other perf scripts. Calibrate the value for the parameter --count based on the size of the test repository, in order to get meaningful results with smaller repos yet still be able to finish the script against huge ones without having to wait for hours. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff Hostetler <git@jeffhostetler.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | p0004: avoid using pipesRené Scharfe2017-05-161-3/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The return code of commands on the producing end of a pipe is ignored. Evaluate the outcome of test-lazy-init-name-hash by calling sort separately. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff Hostetler <git@jeffhostetler.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | p0004: simplify calls of test-lazy-init-name-hashRené Scharfe2017-05-161-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test library puts helpers into $PATH, so we can simply call them without specifying their location. The suffix $X is also not necessary because .exe files on Windows can be started without specifying their extension, and on other platforms it's empty anyway. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff Hostetler <git@jeffhostetler.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'tb/pull-ff-rebase-autostash' into maintJunio C Hamano2017-06-132-11/+32
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git pull --rebase --autostash" didn't auto-stash when the local history fast-forwards to the upstream. * tb/pull-ff-rebase-autostash: pull: ff --rebase --autostash works in dirty repo
| * | | | | pull: ff --rebase --autostash works in dirty repotb/pull-ff-rebase-autostashTyler Brazier2017-06-012-11/+32
| | |_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When `git pull --rebase --autostash` in a dirty repository resulted in a fast-forward, nothing was being autostashed and the pull failed. This was due to a shortcut to avoid running rebase when we can fast-forward, but autostash is ignored on that codepath. Now we will only take the shortcut if autostash is not in effect. Based on a few tests against the git.git repo, the shortcut does not seem to give us significant performance benefits, on Linux at least. Regardless, it is more important to be correct than to be fast. Signed-off-by: Tyler Brazier <tyler@tylerbrazier.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'jh/close-index-before-stat' into maintJunio C Hamano2017-06-131-4/+9
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The timestamp of the index file is now taken after the file is closed, to help Windows, on which a stale timestamp is reported by fstat() on a file that is opened for writing and data was written but not yet closed. * jh/close-index-before-stat: read-cache: close index.lock in do_write_index
| * | | | | read-cache: close index.lock in do_write_indexjh/close-index-before-statJeff Hostetler2017-04-281-4/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach do_write_index() to close the index.lock file before getting the mtime and updating the istate.timestamp fields. On Windows, a file's mtime is not updated until the file is closed. On Linux, the mtime is set after the last flush. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'sl/clean-d-ignored-fix' into maintJunio C Hamano2017-06-136-5/+109
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git clean -d" used to clean directories that has ignored files, even though the command should not lose ignored ones without "-x". "git status --ignored" did not list ignored and untracked files without "-uall". These have been corrected. * sl/clean-d-ignored-fix: clean: teach clean -d to preserve ignored paths dir: expose cmp_name() and check_contains() dir: hide untracked contents of untracked dirs dir: recurse into untracked dirs for ignored files t7061: status --ignored should search untracked dirs t7300: clean -d should skip dirs with ignored files
| * | | | | | clean: teach clean -d to preserve ignored pathssl/clean-d-ignored-fixSamuel Lijin2017-05-242-1/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There is an implicit assumption that a directory containing only untracked and ignored paths should itself be considered untracked. This makes sense in use cases where we're asking if a directory should be added to the git database, but not when we're asking if a directory can be safely removed from the working tree; as a result, clean -d would assume that an "untracked" directory containing ignored paths could be deleted, even though doing so would also remove the ignored paths. To get around this, we teach clean -d to collect ignored paths and skip an untracked directory if it contained an ignored path, instead just removing the untracked contents thereof. To achieve this, cmd_clean() has to collect all untracked contents of untracked directories, in addition to all ignored paths, to determine which untracked dirs must be skipped (because they contain ignored paths) and which ones should *not* be skipped. For this purpose, correct_untracked_entries() is introduced to prune a given dir_struct of untracked entries containing ignored paths and those untracked entries encompassed by the untracked entries which are not pruned away. A memory leak is also fixed in cmd_clean(). This also fixes the known breakage in t7300, since clean -d now skips untracked directories containing ignored paths. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | dir: expose cmp_name() and check_contains()Samuel Lijin2017-05-222-5/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We want to use cmp_name() and check_contains() (which both compare `struct dir_entry`s, the former in terms of the sort order, the latter in terms of whether one lexically contains another) outside of dir.c, so we have to (1) change their linkage and (2) rename them as appropriate for the global namespace. The second is achieved by renaming cmp_name() to cmp_dir_entry() and check_contains() to check_dir_entry_contains(). Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | dir: hide untracked contents of untracked dirsSamuel Lijin2017-05-224-3/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we taught read_directory_recursive() to recurse into untracked directories in search of ignored files given DIR_SHOW_IGNORED_TOO, that had the side effect of teaching it to collect the untracked contents of untracked directories. It doesn't always make sense to return these, though (we do need them for `clean -d`), so we introduce a flag (DIR_KEEP_UNTRACKED_CONTENTS) to control whether or not read_directory() strips dir->entries of the untracked contents of untracked dirs. We also introduce check_contains() to check if one dir_entry corresponds to a path which contains the path corresponding to another dir_entry. This also fixes known breakages in t7061, since status --ignored now searches untracked directories for ignored files. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | dir: recurse into untracked dirs for ignored filesSamuel Lijin2017-05-221-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We consider directories containing only untracked and ignored files to be themselves untracked, which in the usual case means we don't have to search these directories. This is problematic when we want to collect ignored files with DIR_SHOW_IGNORED_TOO, though, so we teach read_directory_recursive() to recurse into untracked directories to find the ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This has the side effect of also collecting all untracked files in untracked directories as well. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | t7061: status --ignored should search untracked dirsSamuel Lijin2017-05-221-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Per eb8c5b87, `status --ignored` by design does not list ignored files if they are in a directory which contains only ignored and untracked files (which is itself considered to be untracked) without `-uall`. This does not make sense for `--ignored`, which claims to "Show ignored files as well." Thus we revisit eb8c5b87 and decide that for such directories, `status --ignored` will list the directory as untracked *and* list all ignored files within said directory even without `-uall`. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | t7300: clean -d should skip dirs with ignored filesSamuel Lijin2017-05-221-0/+16
| | |_|_|_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If git sees a directory which contains only untracked and ignored files, clean -d should not remove that directory. It was recently discovered that this is *not* true of git clean -d, and it's possible that this has never worked correctly; this test and its accompanying patch series aims to fix that. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'dk/send-email-avoid-net-smtp-ssl-when-able' into maintJunio C Hamano2017-06-131-19/+35
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A hotfix to a topic in 'master'. * dk/send-email-avoid-net-smtp-ssl-when-able: send-email: Net::SMTP::starttls was introduced in v2.34 send-email: Net::SMTP::SSL is obsolete, use only when necessary
| * | | | | | send-email: Net::SMTP::starttls was introduced in v2.34dk/send-email-avoid-net-smtp-ssl-when-ableJonathan Nieder2017-06-011-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We cannot rely on the starttls method being present in Net::SMTP until c274b798e6881a941d941808c6d89966975cb8c8 (Merge branch 'ipv6_ssl' of https://github.com/noxxi/perl-libnet into noxxi-ipv6_ssl, 2014-06-02), which set the module version to 2.34. This version was first shipped as part of perl in v5.21.5~169 (Update libnet to CPAN version 3.01, 2014-10-10). Noticed on an Ubuntu system with perl 5.18.2-2ubuntu1.1, which provides Net::SMTP version 2.31. The error message is Can't locate object method "starttls" via package "Net::SMTP" at /usr/lib/git-core/git-send-email line 1410. Reported-by: Brandon Williams <bmwill@google.com> Reported-and-tested-by: Eric Biggers <ebiggers3@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | send-email: Net::SMTP::SSL is obsolete, use only when necessaryDennis Kaarsemaker2017-05-201-19/+35
| |/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28 isn't that old yet, keep the old code in place and use it when necessary. While we're in the area, mark some messages for translation that were not yet marked as such. Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'jc/skip-test-in-the-middle' into maintJunio C Hamano2017-06-132-50/+56
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A recent update to t5545-push-options.sh started skipping all the tests in the script when a web server testing is disabled or unavailable, not just the ones that require a web server. Non HTTP tests have been salvaged to always run in this script. * jc/skip-test-in-the-middle: t5545: enhance test coverage when no http server is installed test: allow skipping the remainder
| * | | | | | t5545: enhance test coverage when no http server is installedjc/skip-test-in-the-middleStefan Beller2017-05-181-42/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit 438fc68462 ("push options: pass push options to the transport helper", 08-02-2017), the test coverage was reduced to run no tests at all if you lack a http server. Move the http initialization to the end, such that only http tests are skipped when a http server is missing. The test in between that tests submodule propagation is safe to run before the http tests as it makes its own test directories `parent` and `parent_upstream`. Noticed-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | test: allow skipping the remainderJunio C Hamano2017-05-181-8/+13
| | |_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because TAP output does not like to see the remainder of the test getting skipped after running one or more tests, bf4b7219 ("test-lib.sh: Add check for invalid use of 'skip_all' facility", 2012-09-01) made sure that test_done errors out when this happens. Instead, loosen the check so that we only pretend that the rest of the test script did not exist in such a case. We'd lose a bit of information (i.e. TAP does not notice that we are skipping some tests), but not very much (i.e. TAP wasn't told how many tests are skipped anyway). This will allow inclusion of lib-httpd.sh in the middle of a test, which will skip the remainder of the test scripts when tests that involve web server are declined with GIT_TEST_HTTPD=false, for example. Acked-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'bw/forking-and-threading' into maintJunio C Hamano2017-06-139-134/+449
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "run-command" API implementation has been made more robust against dead-locking in a threaded environment. * bw/forking-and-threading: usage.c: drop set_error_handle() run-command: restrict PATH search to executable files run-command: expose is_executable function run-command: block signals between fork and execve run-command: add note about forking and threading run-command: handle dup2 and close errors in child run-command: eliminate calls to error handling functions in child run-command: don't die in child when duping /dev/null run-command: prepare child environment before forking string-list: add string_list_remove function run-command: use the async-signal-safe execv instead of execvp run-command: prepare command before forking t0061: run_command executes scripts without a #! line t5550: use write_script to generate post-update hook
| * | | | | | usage.c: drop set_error_handle()bw/forking-and-threadingJeff King2017-05-152-10/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The set_error_handle() function was introduced by 3b331e926 (vreportf: report to arbitrary filehandles, 2015-08-11) so that run-command could send post-fork, pre-exec errors to the parent's original stderr. That use went away in 79319b194 (run-command: eliminate calls to error handling functions in child, 2017-04-19), which pushes all of the error reporting to the parent. This leaves no callers of set_error_handle(). As we're not likely to add any new ones, let's drop it. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Brandon Williams <bmwill@google.com> Reviewed-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: restrict PATH search to executable filesBrandon Williams2017-04-252-1/+48
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In some situations run-command will incorrectly try (and fail) to execute a directory instead of an executable file. This was observed by having a directory called "ssh" in $PATH before the real ssh and trying to use ssh protoccol, reslting in the following: $ git ls-remote ssh://url fatal: cannot exec 'ssh': Permission denied It ends up being worse and run-command will even try to execute a non-executable file if it preceeds the executable version of a file on the PATH. For example, if PATH=~/bin1:~/bin2:~/bin3 and there exists a directory 'git-hello' in 'bin1', a non-executable file 'git-hello' in bin2 and an executable file 'git-hello' (which prints "Hello World!") in bin3 the following will occur: $ git hello fatal: cannot exec 'git-hello': Permission denied This is due to only checking 'access()' when locating an executable in PATH, which doesn't distinguish between files and directories. Instead use 'is_executable()' which check that the path is to a regular, executable file. Now run-command won't try to execute the directory or non-executable file 'git-hello': $ git hello Hello World! which matches what execvp(3) would have done when asked to execute git-hello with such a $PATH. Reported-by: Brian Hatfield <bhatfield@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: expose is_executable functionBrandon Williams2017-04-253-42/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the logic for 'is_executable()' from help.c to run_command.c and expose it so that callers from outside help.c can access the function. This is to enable run-command to be able to query if a file is executable in a future patch. Signed-off-by: Brandon Williams <bmwill@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: block signals between fork and execveEric Wong2017-04-201-0/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signal handlers of the parent firing in the forked child may have unintended side effects. Rather than auditing every signal handler we have and will ever have, block signals while forking and restore default signal handlers in the child before execve. Restoring default signal handlers is required because execve does not unblock signals, it only restores default signal handlers. So we must restore them with sigprocmask before execve, leaving a window when signal handlers we control can fire in the child. Continue ignoring ignored signals, but reset the rest to defaults. Similarly, disable pthread cancellation to future-proof our code in case we start using cancellation; as cancellation is implemented with signals in glibc. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: add note about forking and threadingBrandon Williams2017-04-201-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All non-Async-Signal-Safe functions (e.g. malloc and die) were removed between 'fork' and 'exec' in start_command in order to avoid potential deadlocking when forking while multiple threads are running. This deadlocking is possible when a thread (other than the one forking) has acquired a lock and didn't get around to releasing it before the fork. This leaves the lock in a locked state in the resulting process with no hope of it ever being released. Add a note describing this potential pitfall before the call to 'fork()' so people working in this section of the code know to only use Async-Signal-Safe functions in the child process. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: handle dup2 and close errors in childBrandon Williams2017-04-201-16/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: eliminate calls to error handling functions in childBrandon Williams2017-04-201-32/+89
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All of our standard error handling paths have the potential to call malloc or take stdio locks; so we must avoid them inside the forked child. Instead, the child only writes an 8 byte struct atomically to the parent through the notification pipe to propagate an error. All user-visible error reporting happens from the parent; even avoiding functions like atexit(3) and exit(3). Helped-by: Eric Wong <e@80x24.org> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: don't die in child when duping /dev/nullBrandon Williams2017-04-201-15/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: prepare child environment before forkingBrandon Williams2017-04-201-10/+56
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In order to avoid allocation between 'fork()' and 'exec()' prepare the environment to be used in the child process prior to forking. Switch to using 'execve()' so that the construct child environment can used in the exec'd process. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | string-list: add string_list_remove functionBrandon Williams2017-04-202-0/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach string-list to be able to remove a string from a sorted 'struct string_list'. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: use the async-signal-safe execv instead of execvpBrandon Williams2017-04-201-1/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert the function used to exec from 'execvp()' to 'execv()' as the (p) variant of exec isn't async-signal-safe and has the potential to call malloc during the path resolution it performs. Instead we simply do the path resolution ourselves during the preparation stage prior to forking. There also don't exist any portable (p) variants which also take in an environment to use in the exec'd process. This allows easy migration to using 'execve()' in a future patch. Also, as noted in [1], in the event of an ENOEXEC the (p) variants of exec will attempt to execute the command by interpreting it with the 'sh' utility. To maintain this functionality, if 'execv()' fails with ENOEXEC, start_command will atempt to execute the command by interpreting it with 'sh'. [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/exec.html Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | run-command: prepare command before forkingBrandon Williams2017-04-201-20/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | According to [1] we need to only call async-signal-safe operations between fork and exec. Using malloc to build the argv array isn't async-signal-safe. In order to avoid allocation between 'fork()' and 'exec()' prepare the argv array used in the exec call prior to forking the process. [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/fork.html Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | t0061: run_command executes scripts without a #! lineBrandon Williams2017-04-201-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a test to 't0061-run-command.sh' to ensure that run_command can continue to execute scripts which don't include a '#!' line. As shell scripts are not natively executable on Windows, we use a workaround to check "#!" when running scripts from Git. As this test requires the platform (not with Git's help) to run scripts without "#!", skipt it on Windows. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | t5550: use write_script to generate post-update hookBrandon Williams2017-04-181-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The post-update hooks created in t5550-http-fetch-dumb.sh is missing the "!#/bin/sh" line which can cause issues with portability. Instead create the hook using the 'write_script' function which includes the proper "#!" line. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>