The previous patch resolved most of the test failures caused by
running our test suite with '-x' tracing and /bin/sh, and the
following patches in this series will resolve almost all of the
remaining failures. Unfortunately, not yet all.
Add means to disable '-x' tracing for individual test scripts by
setting the $test_untraceable variable to a non-empty value in the
test script before sourcing 'test-lib.sh'. However, since '-x'
tracing is not an issue with recent Bash versions supporting
BASH_XTRACEFD, i.e. v4.1 and later, don't disable tracing when the
test script is run with such a Bash version even when
$test_untraceable is set.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running a test script with '-x' turns on 'set -x' tracing, the output
of which is normally sent to stderr. This causes a lot of
test failures, because many tests redirect and verify the stderr
of shell functions, most frequently that of 'test_must_fail'.
These issues were worked around somewhat in d88785e424 (test-lib: set
BASH_XTRACEFD automatically, 2016-05-11), so at least we could
reliably run tests with '-x' tracing under a Bash version supporting
BASH_XTRACEFD, i.e. v4.1 and later.
Futhermore, redirecting the stderr of test helper functions like
'test_must_fail' or 'test_expect_code' is the cause of a different
issue as well. If these functions detect something unexpected, they
will write their error messages intended to the user to thier stderr.
However, if their stderr is redirected in order to save and verify the
stderr of the tested git command invoked in the function, then the
function's error messages will be redirected as well. Consequently,
those messages won't reach the user, making the test's verbose output
less useful.
This patch makes it safe to redirect and verify the stderr of those
test helper functions which are meant to run the tested command given
as argument, even when running tests with '-x' and /bin/sh. This is
achieved through a couple of file descriptor redirections:
- Duplicate stderr of the tested command executed in the test helper
function from the function's fd 7 (see next point), to ensure that
the tested command's error messages go to a different fd than the
'-x' trace of the commands executed in the function or the
function's error messages.
- Duplicate the test helper function's fd 7 from the function's
original stderr, meaning that, after taking a detour through fd 7,
the error messages of the tested command do end up on the
function's original stderr.
- Duplicate stderr of the test helper function from fd 4, i.e. the
fd connected to the test script's original stderr and the fd used
for BASH_XTRACEFD. This ensures that the '-x' trace of the
commands executed in the function
- doesn't go to the function's original stderr, so it won't mess
with callers who want to save and verify the tested command's
stderr.
- does go to the same fd independently from the shell running
the test script, be it /bin/sh, an older Bash without
BASH_XTRACEFD, or a more recent Bash already supporting
BASH_XTRACEFD.
Furthermore, this also makes sure that the function's error
messages go to this fd 4, meaning that the user will be able to
see them even if the function's stderr is redirected in the test.
- Specify the latter two redirections above in the test helper
function's definition, so they are performed every time the
function is invoked, without the need to modify the callsites of
the function.
Perform these redirections in those test helper functions which can be
expected to have their stderr redirected, i.e. in the functions
'test_must_fail', 'test_might_fail', 'test_expect_code', 'test_env',
'nongit', 'test_terminal' and 'perl'. Note that 'test_might_fail',
'test_env', and 'nongit' are not involved in any test failures when
running tests with '-x' and /bin/sh.
The other test helper functions are left unchanged, because they
either don't run commands specified as their arguments, or redirecting
their stderr wouldn't make sense, or both.
With this change the number of failures when running the test suite
with '-x' tracing and /bin/sh goes down from 340 failed tests in 43
test scripts to 22 failed tests in 6 scripts (or 23 in 7, if the
system (OSX) uses an older Bash version without BASH_XTRACEFD to run
't9903-bash-prompt.sh').
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test fixes.
* sg/test-i18ngrep:
t: make 'test_i18ngrep' more informative on failure
t: validate 'test_i18ngrep's parameters
t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'
t5536: let 'test_i18ngrep' read the file without redirection
t5510: consolidate 'grep' and 'test_i18ngrep' patterns
t4001: don't run 'git status' upstream of a pipe
t6022: don't run 'git merge' upstream of a pipe
t5812: add 'test_i18ngrep's missing filename parameter
t5541: add 'test_i18ngrep's missing filename parameter
"git format-patch" learned to give 72-cols to diffstat, which is
consistent with other line length limits the subcommand uses for
its output meant for e-mails.
* nd/format-patch-stat-width:
format-patch: reduce patch diffstat width to 72
format-patch: keep cover-letter diffstat wrapped in 72 columns
More tests for wildmatch functions.
* ab/wildmatch-tests:
wildmatch test: mark test as EXPENSIVE_ON_WINDOWS
test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite
wildmatch test: create & test files on disk in addition to in-memory
wildmatch test: perform all tests under all wildmatch() modes
wildmatch test: use test_must_fail, not ! for test-wildmatch
wildmatch test: remove dead fnmatch() test code
wildmatch test: use a paranoia pattern from nul_match()
wildmatch test: don't try to vertically align our output
wildmatch test: use more standard shell style
wildmatch test: indent with tabs, not spaces
* en/merge-recursive-fixes:
merge-recursive: add explanation for src_entry and dst_entry
merge-recursive: fix logic ordering issue
Tighten and correct a few testcases for merging and cherry-picking
The sequencer infrastructure is shared across "git cherry-pick",
"git rebase -i", etc., and has always spawned "git commit" when it
needs to create a commit. It has been taught to do so internally,
when able, by reusing the codepath "git commit" itself uses, which
gives performance boost for a few tens of percents in some sample
scenarios.
* pw/sequencer-in-process-commit:
sequencer: run 'prepare-commit-msg' hook
t7505: add tests for cherry-pick and rebase -i/-p
t7505: style fixes
sequencer: assign only free()able strings to gpg_sign
sequencer: improve config handling
t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1
sequencer: try to commit without forking 'git commit'
sequencer: load commit related config
sequencer: simplify adding Signed-off-by: trailer
commit: move print_commit_summary() to libgit
commit: move post-rewrite code to libgit
Add a function to update HEAD after creating a commit
commit: move empty message checks to libgit
t3404: check intermediate squash messages
Code clean-up.
* nd/shared-index-fix:
read-cache: don't write index twice if we can't write shared index
read-cache.c: move tempfile creation/cleanup out of write_shared_index
read-cache.c: change type of "temp" in write_shared_index()
The split-index mode had a few corner case bugs fixed.
* tg/split-index-fixes:
travis: run tests with GIT_TEST_SPLIT_INDEX
split-index: don't write cache tree with null oid entries
read-cache: fix reading the shared index for other repos
The http tracing code, often used to debug connection issues,
learned to redact potentially sensitive information from its output
so that it can be more safely sharable.
* jt/http-redact-cookies:
http: support omitting data from traces
http: support cookie redaction when tracing
The tracing machinery learned to report tweaking of environment
variables as well.
* nd/trace-with-env:
run-command.c: print new cwd in trace_run_command()
run-command.c: print env vars in trace_run_command()
run-command.c: print program 'git' when tracing git_cmd mode
run-command.c: introduce trace_run_command()
trace.c: move strbuf_release() out of print_trace_line()
trace: avoid unnecessary quoting
sq_quote_argv: drop maxlen parameter
The machinery to clone & fetch, which in turn involves packing and
unpacking objects, have been told how to omit certain objects using
the filtering mechanism introduced by the jh/object-filtering
topic, and also mark the resulting pack as a promisor pack to
tolerate missing objects, taking advantage of the mechanism
introduced by the jh/fsck-promisors topic.
* jh/partial-clone:
t5616: test bulk prefetch after partial fetch
fetch: inherit filter-spec from partial clone
t5616: end-to-end tests for partial clone
fetch-pack: restore save_commit_buffer after use
unpack-trees: batch fetching of missing blobs
clone: partial clone
partial-clone: define partial clone settings in config
fetch: support filters
fetch: refactor calculation of remote list
fetch-pack: test support excluding large blobs
fetch-pack: add --no-filter
fetch-pack, index-pack, transport: partial clone
upload-pack: add object filtering for partial clone
In preparation for implementing narrow/partial clone, the machinery
for checking object connectivity used by gc and fsck has been
taught that a missing object is OK when it is referenced by a
packfile specially marked as coming from trusted repository that
promises to make them available on-demand and lazily.
* jh/fsck-promisors:
gc: do not repack promisor packfiles
rev-list: support termination at promisor objects
sha1_file: support lazily fetching missing objects
introduce fetch-object: fetch one promisor object
index-pack: refactor writing of .keep files
fsck: support promisor objects as CLI argument
fsck: support referenced promisor objects
fsck: support refs pointing to promisor objects
fsck: introduce partialclone extension
extension.partialclone: introduce partial clone extension
The build procedure for perl/ part has been greatly simplified by
weaning ourselves off of MakeMaker.
* ab/simplify-perl-makefile:
perl: treat PERLLIB_EXTRA as an extra path again
perl: avoid *.pmc and fix Error.pm further
Makefile: replace perl/Makefile.PL with simple make rules
When 'test_i18ngrep' can't find the expected pattern, it exits
completely silently; when its negated form does find the pattern that
shouldn't be there, it prints the matching line(s) but otherwise exits
without any error message. This leaves the developer puzzled about
what could have gone wrong.
Make 'test_i18ngrep' more informative on failure by printing an error
message including the invoked 'grep' command and the contents of the
file it had to scan through.
Note that this "dump the scanned file" part is not quite perfect, as
it dumps only the file specified as the function's last positional
parameter, thus assuming that there is only a single file parameter.
I think that's a reasonable assumption to make, one that holds true in
the current code base. And even if someone were to scan multiple
files at once in the future, the worst thing that could happen is that
the verbose error message won't include the contents of all those
files, only the last one. Alas, we can't really do any better than
this, because checking whether the other positional parameters match a
filename can result in false positives: 't3400-rebase.sh' and
't3404-rebase-interactive.sh' contain one test each, where the
'test_i18ngrep's pattern verbatimly matches a file in the trash
directory.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some of the previous patches in this series fixed bogus
'test_i18ngrep' invocations:
- Two invocations where the tested git command's standard output is
directly piped into 'test_i18ngrep'. While convenient, this is an
antipattern, because the pipe hides the git command's exit code,
and the test could continue even if the command exited with error.
- Two invocations that had neither a filename parameter nor anything
piped into their standard input, yet both managed to remain
unnoticed for years. A third similarly bogus invocation is
currently lurking in 'pu' for a couple of weeks now.
Prevent similar mistakes in the future by validating 'test_i18ngrep's
parameters requiring that
- The last parameter names an existing file to be read, effectively
forbidding piping into 'test_i18ngrep'.
Note that this change will also forbid cases where 'test_i18ngrep'
would legitimately read its standard input, e.g. when its standard
input is redirected from a file, or when a git command's standard
output is first written to an intermediate file, which is then
preprocessed by a non-git command before the results are piped
into 'test_i18ngrep'. See two of the previous patches for the
only such cases we had in our test suite. However, reliably
preventing the piping antipattern is arguably more important than
supporting these cases, which can be easily worked around by
opening the file directly or using an intermediate file anyway.
- There are at least two parameters, not including the optional '!'
to negate the pattern. This ought to catch corner cases when
'test_i18ngrep' looks for the name of an existing file on its
standard input; the above check would miss this case becase the
filename as pattern would be the last parameter.
Note that this is not quite perfect, as it doesn't account for any
'grep --options' given as parameters. However, doing so would be
far too complicated, considering that patterns can start with
dashes as well, and in the majority of the cases we don't use any
such options anyway.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Both 'test_i18ncmp' and 'test_i18ngrep' helper functions are supposed
to be called from our test scripts, so they should be in
'test-lib-functions.sh'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Redirecting 'test_i18ngrep's standard input from a file will interfere
with the linting that will be added in a later patch.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the tests in 't5510-fetch.sh' checks the output of 'git fetch'
using 'test_i18ngrep', and while doing so it prefilters the output
with 'grep' before piping the result into 'test_i18ngrep'.
This prefiltering is unnecessary, with the appropriate pattern
'test_i18ngrep' can do it all by itself. Furthermore, piping data
into 'test_i18ngrep' will interfere with the linting that will be
added in a later patch.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The primary purpose of three tests in 't4001-diff-rename.sh' is to
check rename detection in 'git status', but all three do so by running
'git status' upstream of a pipe, hiding its exit code. Consequently,
the test could continue even if 'git status' exited with error.
Use an intermediate file between 'git status' and 'test_i18ngrep' to
catch a potential failure of the former.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The primary purpose of 't6022-merge-rename.sh' is to test 'git merge',
but one of the tests runs it upstream of a pipe, hiding its exit code.
Consequently, the test could continue even if 'git merge' exited with
error.
Use an intermediate file between 'git merge' and 'test_i18ngrep' to
catch a potential failure of the former.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The second 'test_i18ngrep' invocation in the test 'curl redirects
respect whitelist' is missing its filename parameter. This has
remained unnoticed since its introduction in f4113cac0 (http: limit
redirection to protocol-whitelist, 2015-09-22), because it would only
cause the test to fail if Git was built with a sufficiently old
libcurl version. The test's two ||-chained 'test_i18ngrep'
invocations are supposed to check that either one of the two patterns
is present in 'git clone's error message. As it happens, the first
invocation covers the error message from any reasonably up-to-date
libcurl, thus the second invocation, the one without the filename
parameter, isn't executed at all. Apparently no one has run the test
suite's httpd tests with such an old libcurl in the last 2+ years, or
at least they haven't bothered to notify us about the failed test.
Fix this by consolidating the two patterns into a single extended
regexp, eliminating the need for an ||-chained second 'test_i18ngrep'
invocation.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test 'push --no-progress silences progress but not status' runs
'test_i18ngrep' without specifying a filename parameter. This has
remained unnoticed since its introduction in e304aeba2 (t5541: test
more combinations of --progress, 2012-05-01), because that
'test_i18ngrep' is supposed to check that the given pattern is not
present in its input, and of course it won't find that pattern if its
input is empty (as it comes from /dev/null). This also means that
this test could miss a potential breakage of 'git push --no-progress'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This option allows commits with empty commit messages to be rebased,
matching the same option in git-commit and git-cherry-pick. While empty
log messages are frowned upon, sometimes one finds them in older
repositories (e.g. translated from another VCS [0]), or have other
reasons for desiring them. The option is available in git-commit and
git-cherry-pick, so it is natural to make other git tools play nicely
with them. Adding this as an option allows the default to be "give the
user a chance to fix", while not interrupting the user's workflow
otherwise [1].
[0]: https://stackoverflow.com/q/8542304
[1]: https://public-inbox.org/git/7vd33afqjh.fsf@alter.siamese.dyndns.org/
To implement this, add a new --allow-empty-message flag. Then propagate
it to all calls of 'git commit', 'git cherry-pick', and 'git rebase--helper'
within the rebase scripts.
Signed-off-by: Genki Sky <sky@genki.is>
Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is much easier to diff the output against a previous
one when the fields are sorted.
Helped-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes it easier to use the aggregate script
on the command line when one wants to get the
"environment" fields set in the codespeed output.
Previously setting GIT_REPO_NAME was needed
for this purpose.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes it easier to use the aggregate script
on the command line, to get results from
subsections.
Previously setting GIT_PERF_SUBSECTION was needed
for this purpose.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patches generated by format-patch are meant to be exchanged as emails,
most of the time. And since it's generally agreed that text in mails
should be wrapped around 70 columns or so, make sure these diffstat
follow the convention (especially when used with --cover-letter since we
already defaults to wrapping 72 columns). The default can still be
overriden with command line options.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add an EXPENSIVE_ON_WINDOWS prerequisite to mark those tests which are
very expensive to run on Windows, but cheap elsewhere.
Certain tests that heavily stress the filesystem or run a lot of shell
commands are disproportionately expensive on Windows, this
prerequisite will later be used by a tests that runs in 4-8 seconds on
a modern Linux system, but takes almost 10 minutes on Windows.
There's no reason to skip such tests by default on other platforms,
but Windows users shouldn't need to wait around while they finish.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There has never been any full roundtrip testing of what git-ls-files
and other commands that use wildmatch() actually do, rather we've been
satisfied with just testing the underlying C function.
Due to git-ls-files and friends having their own codepaths before they
call wildmatch() there's sometimes differences in the behavior between
the two. Even when we test for those (as with [1]), there was no one
place where you can review how these two modes differ.
Now there is. We now attempt to create a file called $haystack and
match $needle against it for each pair of $needle and $haystack that
we were passing to test-wildmatch.
If we can't create the file we skip the test. This ensures that we can
run this on all platforms and not maintain some infinitely growing
whitelist of e.g. platforms that don't support certain characters in
filenames.
A notable exception to this is Windows, where due to the reasons
explained in [2] the shellscript emulation layer might fake the
creation of a file such as "*", and "test -e" for it will succeed
since it just got created with some character that maps to "*", but
git ls-files won't be fooled by this.
Thus we need to skip creating certain filenames entirely on Windows,
the list here might be overly aggressive. I don't have access to a
Windows system to test this.
As a result of doing these tests we can now see the cases where these
two ways of testing wildmatch differ:
* Creating a file called 'a[]b' and running ls-files 'a[]b' will show
that file, but wildmatch("a[]b", "a[]b") will not match
* wildmatch() won't match a file called \ against \, but ls-files
will.
* `git --glob-pathspecs ls-files 'foo**'` will match a file
'foo/bba/arr', but wildmatch won't, however pathmatch will.
This seems like a bug to me, the two are otherwise equivalent as
these tests show.
This also reveals the case discussed in [1], since 2.16.0 '' is now an
error as far as ls-files is concerned, but wildmatch() itself happily
accepts it.
1. 9e4e8a64c2 ("pathspec: die on empty strings as pathspec",
2017-06-06)
2. nycvar.QRO.7.76.6.1801052133380.1337@wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet
(https://public-inbox.org/git/?q=nycvar.QRO.7.76.6.1801052133380.1337%40wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rewrite the wildmatch() test suite so that each test now tests all
combinations of the wildmatch() WM_CASEFOLD and WM_PATHNAME flags.
Before this change some test inputs were not tested on
e.g. WM_PATHNAME. Now the function is stress tested on all possible
inputs, and for each input we declare what the result should be if the
mode is case-insensitive, or pathname matching, or case-sensitive or
not matching pathnames.
Also before this change, nothing was testing case-insensitive
non-pathname matching, so I've added that to test-wildmatch.c and made
use of it.
This yields a rather scary patch, but there are no functional changes
here, just more test coverage. Some now-redundant tests were deleted
as a result of this change, since they were now duplicating an earlier
test.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use of ! should be reserved for non-git programs that are assumed not
to fail, see README. With this change only
t/t0110-urlmatch-normalization.sh is still using this anti-pattern.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the unused fnmatch() test parameter from the wildmatch
test. The code that used to test this was removed in 70a8fc999d ("stop
using fnmatch (either native or compat)", 2014-02-15).
As a --word-diff shows the only change to the body of the tests is the
removal of the second out of four parameters passed to match().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use a pattern from the nul_match() function in t7008-grep-binary.sh to
make sure that we don't just fall through to the "else" if there's an
unknown parameter.
This is something I added in commit 77f6f4406f ("grep: add a test
helper function for less verbose -f \0 tests", 2017-05-20) to grep
tests, which were modeled on these wildmatch tests, and I'm now
porting back to the original wildmatch tests.
I am not using the "say '...'; exit 1" pattern from t0000-basic.sh
because if I fail I want to run the rest of the tests (unless under
-i), and doing this makes sure we do that and don't exit right away
without fully reporting our errors.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't try to vertically align the test output, which is futile anyway
under the TAP output where we're going to be emitting a number for
each test without aligning the test count.
This makes subsequent changes of mine where I'm not going to be
aligning this output as I add new tests easier.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the wildmatch test to use more standard shell style, usually we
use "if test" not "if [".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Replace the 4-width mixed space & tab indentation in this file with
indentation with tabs as we do in most of the rest of our tests.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the declaration of struct sha1_stat. Adjust all usages of this
struct and replace hash{clr,cmp,cpy} with oid{clr,cmp,cpy} wherever
possible. Rename it to struct oid_stat.
Rename static function load_sha1_stat to load_oid_stat.
Remove macro EMPTY_BLOB_SHA1_BIN, as it's no longer used.
Signed-off-by: Patryk Obara <patryk.obara@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When git-daemon gets a pktline request, we strip off any
trailing newline, replacing it with a NUL. Clients prior to
5ad312bede (in git v1.4.0) would send:
git-upload-pack repo.git\n
and we need to strip it off to understand their request.
After 5ad312bede, we send the host attribute but no newline,
like:
git-upload-pack repo.git\0host=example.com\0
Both of these are parsed correctly by git-daemon. But if
some client were to combine the two:
git-upload-pack repo.git\n\0host=example.com\0
we don't parse it correctly. The problem is that we use the
"len" variable to record the position of the NUL separator,
but then decrement it when we strip the newline. So we start
with:
git-upload-pack repo.git\n\0host=example.com\0
^-- len
and end up with:
git-upload-pack repo.git\0\0host=example.com\0
^-- len
This is arguably correct, since "len" tells us the length of
the initial string, but we don't actually use it for that.
What we do use it for is finding the offset of the extended
attributes; they used to be at len+1, but are now at len+2.
We can solve that by just leaving "len" where it is. We
don't have to care about the length of the shortened string,
since we just treat it like a C string.
No version of Git ever produced such a string, but it seems
like the daemon code meant to handle this case (and it seems
like a reasonable thing for somebody to do in a 3rd-party
implementation).
Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All of our git-protocol tests rely on invoking the client
and having it make a request of a server. That gives a nice
real-world test of how the two behave together, but it
doesn't leave any room for testing how a server might react
to _other_ clients.
Let's add a few test helper functions which can be used to
manually conduct a git-protocol conversation with a remote
git-daemon:
1. To connect to a remote git-daemon, we need something
like "netcat". But not everybody will have netcat. And
even if they do, the behavior with respect to
half-duplex shutdowns is not portable (openbsd netcat
has "-N", with others you must rely on "-q 1", which is
racy).
Here we provide a "fake_nc" that is capable of doing
a client-side netcat, with sane half-duplex semantics.
It relies on perl's IO::Socket::INET. That's been in
the base distribution since 5.6.0, so it's probably
available everywhere. But just to be on the safe side,
we'll add a prereq.
2. To help tests speak and read pktline, this patch adds
packetize() and depacketize() functions.
I've put fake_nc() into lib-git-daemon.sh, since that's
really the only server where we'd need to use a network
socket. Whereas the pktline helpers may be of more general
use, so I've added them to test-lib-functions.sh. Programs
like upload-pack speak pktline, but can talk directly over
stdio without a network socket.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we receive a request with extended attributes after the
NUL, we try to write those attributes to the log. We do so
with a "%s" format specifier, which will only show
characters up to the first NUL.
That's enough for printing a "host=" specifier. But since
dfe422d04d (daemon: recognize hidden request arguments,
2017-10-16) we may have another NUL, followed by protocol
parameters, and those are not logged at all.
Let's cut out the attempt to show the whole string, and
instead log when we parse individual attributes. We could
leave the "extended attributes (%d bytes) exist" part of the
log, which in theory could alert us to attributes that fail
to parse. But anything we don't parse as a "host=" parameter
gets blindly added to the "protocol" attribute, so we'd see
it in that part of the log.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If receive a request like:
git-upload-pack /foo.git\0host=localhost
we mark the offset of the NUL byte as "len", and then log
the bytes after the NUL with a "%.*s" placeholder, using
"pktlen - len" as the length, and "line + len + 1" as the
start of the string.
This is off-by-one, since the start of the string skips past
the separating NUL byte, but the adjusted length includes
it. Fortunately this doesn't actually read past the end of
the buffer, since "%.*s" will stop when it hits a NUL. And
regardless of what is in the buffer, packet_read() will
always add an extra NUL terminator for safety.
As an aside, the git.git client sends an extra NUL after a
"host" field, too, so we'd generally hit that one first, not
the one added by packet_read(). You can see this in the test
output which reports 15 bytes, even though the string has
only 14 bytes of visible data. But the point is that even a
client sending unusual data could not get us to read past
the end of the buffer, so this is purely a cosmetic fix.
Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we start git-daemon for our tests, we send its stderr
log stream to a named pipe. We synchronously read the first
line to make sure that the daemon started, and then dump the
rest to descriptor 4. This is handy for debugging test
output with "--verbose", but the tests themselves can't
access the log data.
Let's dump the log into a file, as well, so that future
tests can check the log. There are a few subtleties worth
calling out here:
- we'll continue to send output to descriptor 4 for
viewing/debugging, which would imply swapping out "cat"
for "tee". But we want to ensure that there's no
buffering, and "tee" doesn't have a standard way to
ask for that. So we'll use a shell loop around "read"
and "printf" instead. That ensures that after a request
has been served, the matching log entries will have made
it to the file.
- the existing first-line shell loop used read/echo. We'll
switch to consistently using "read -r" and "printf" to
relay data as faithfully as possible.
- we open the logfile for append, rather than just output.
That makes it OK for tests to truncate the logfile
without restarting the daemon (the OS will atomically
seek to the end of the file when outputting each line).
That allows tests to look at the log without worrying
about pollution from earlier tests.
Helped-by: Lucas Werkmeister <mail@lucaswerkmeister.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't actually care about the clone operation here; we
just want to know if we were able to actually contact the
remote repository. Using ls-remote does that more
efficiently, and without us having to worry about managing
the tmp.git directory.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
