This gives us compile-time checking of our format strings,
which is a good thing.
I had also hoped it would help with confusing write_file()
and write_file_buf(), since the former's "..." can make it
match the signature of the latter. But given that the buffer
for write_file_buf() is generally not a string literal, the
compiler won't complain unless -Wformat-nonliteral is on,
and that creates a ton of false positives elsewhere in the
code base.
While we're there, let's also give the function a docstring,
which it never had.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are many callsites which could use write_file, but for
which it is a little awkward because they have a strbuf or
other pointer/len combo. Specifically:
1. write_file() takes a format string, so we have to use
"%s" or "%.*s", which are ugly.
2. Using any form of "%s" does not handle embedded NULs in
the output. That probably doesn't matter for our
call-sites, but it's nicer not to have to worry.
3. It's less efficient; we format into another strbuf
just to do the write. That's probably not measurably
slow for our uses, but it's simply inelegant.
We can fix this by providing a helper to write out the
formatted buffer, and just calling it from write_file().
Note that we don't do the usual "complete with a newline"
that write_file does. If the caller has their own buffer,
there's a reasonable chance they're doing something more
complicated than a single line, and they can call
strbuf_complete_line() themselves.
We could go even further and add strbuf_write_file(), but it
doesn't save much:
It would also be somewhat asymmetric with strbuf_read_file,
which actually returns errors rather than dying (and the
error handling is most of the benefit of write_file() in the
first place).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This simplifies the code a tiny bit, and provides consistent
error messages with other users of xopen().
While we're here, let's also switch to using O_WRONLY. We
know we're only going to open/write/close the file, so
there's no point in asking for O_RDWR.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are no callers left of write_file_gently(). Let's drop
it, as it doesn't seem likely for new callers to be added
(since its inception, the only callers who wanted the gentle
form generally just died immediately themselves, and have
since been converted).
While we're there, let's also drop the "int" return from
write_file, as it is never meaningful (in the non-gentle
form, we always either die or return 0).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
branch: use non-gentle write_file for branch description
We use write_file_gently() to do this job currently.
However, if we see an error, we simply complain via
error_errno() and then end up exiting with an error code.
By switching to the non-gentle form, the function will die
for us, with a better error. It is more specific about which
syscall caused the error, and that mentions the
actual filename we're trying to write.
Our exit code for the error case does switch from "1" to
"128", but that's OK; it wasn't a meaningful documented code
(and in fact it was odd that it was a different exit code
than most other error conditions).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
write_file() either returns 0 or dies, so there is no point in checking
its return value. The callers of the wrappers write_state_text(),
write_state_count() and write_state_bool() consequently already ignore
their return values. Stop pretending we care and make them void.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
config: fix bogus fd check when setting up default config
Since 9830534 (config --global --edit: create a template
file if needed, 2014-07-25), an edit of the global config
file will try to open() it with O_EXCL, and wants to handle
three cases:
1. We succeeded; the user has no config file, and we
should fill in the default template.
2. We got EEXIST; they have a file already, proceed as usual.
3. We got another error; we should complain.
However, the check for case 1 does "if (fd)", which will
generally _always_ be true (except for the oddball case that
somehow our stdin got closed and opening really did give us
a new descriptor 0).
So in the EEXIST case, we tried to write the default config
anyway! Fortunately, this turns out to be a noop, since we
just end up writing to and closing "-1", which does nothing.
But in case 3, we would fail to notice any other errors, and
just silently continue (given that we don't actually notice
write errors for the template either, it's probably not that
big a deal; we're about to spawn the editor, so it would
notice any problems. But the code is clearly _trying_ to hit
cover this case and failing).
We can fix it easily by using "fd >= 0" for case 1.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rebase -i: we allow extra spaces after fixup!/squash!
This new test case ensures that we handle commit messages that start
with fixup! or squash! followed by more than one space. While we do
not generate such messages when committing with --fixup/--squash, it
is perfectly legal for users to hand-craft their own fixup messages,
and we heed Postel's law by being lenient.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When rearranging the edit script, we happily mistake the comment
character for a command, and the command for a SHA-1. As a consequence,
when we move fixup! and squash! commits, our logic to skip lines with
already handled SHA-1s mistakenly skips anything but the first
commented-out pick line, too.
The upcoming rebase--helper patches will address this bug, therefore we
do not need to make the current autosquash code even more complex than
it already is, just to fix this bug.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
For the upcoming rebase--helper work (which will accelerate the
interactive rebase noticably), it is important to verify that the
--gpg-sign option is handled properly.
Please note that this patch does this on the cheap, by verifying that
the expected option is printed in the message of the 'edit' operation.
We really should test that the interactive rebase signs the commits
properly, iff GPG is available. This test is left for later.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
notes-merge: use O_EXCL to avoid overwriting existing files
Use the open(2) flag O_EXCL to ensure the file doesn't already exist
instead of (racily) calling stat(2) through file_exists(). While at it
switch to xopen() to reduce code duplication and get more consistent
error messages.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Makefile: add NEEDS_LIBRT to optionally link with librt
We unconditionally link with librt, when HAVE_CLOCK_GETTIME is defined.
But clock_gettime() has been available in most libc implementations for
some time now (e.g., for glibc since version 2.17) and no longer
requires linking with librt. Furthermore, commit a6c3c63 (configure.ac:
check for clock_gettime() and CLOCK_MONOTONIC) will automatically
determined which library (libc or librt) is required for linking when
checking for clock_gettime().
The assumption to unconditionally link with librt was OK, since either
almost every Unix-like system provides a version of librt for backwards
compatibility or other systems, namely Windows or OS X, never provided
clock_gettime(). However, in the latest release of OS X (macOS Sierra),
this function has been added to OS X libc version. As a result, when
running the configuration script, HAVE_CLOCK_GETTIME is set and since
librt is not present, it causes a linker error.
This patches requires those not building via the configuration scripts
to define NEEDS_LIBRT in addition to HAVE_CLOCK_GETTIME, if needed.
Signed-off-by: Ronald Wampler <rdwampler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Set the diff attribute for C source file to "cpp" in order to improve
git's ability to determine hunk headers. In particular it helps avoid
showing unindented labels in hunk headers. That in turn is useful for
git diff -W and git grep -W, which show whole functions now instead of
stopping at a label.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A bug, which caused "git p4" while running under verbose mode to
report paths that are omitted due to branch prefix incorrectly, has
been fixed; the command said "Ignoring file outside of prefix" for
paths that are _inside_.
One among four invocations of readlink(1) in our test suite has
been rewritten so that the test can run on systems without the
command (others are in valgrind test framework and t9802).
* ak/t7800-wo-readlink:
t7800: readlink may not be available
The internal code used to show local timezone offset is not
prepared to handle timestamps beyond year 2100, and gave a
bogus offset value to the caller. Use a more benign looking
+0000 instead and let "git log" going in such a case, instead
of aborting.
* jk/tzoffset-fix:
local_tzoffset: detect errors from tm_to_time_t
t0006: test various date formats
t0006: rename test-date's "show" to "relative"
Fix an unintended regression in v2.9 that breaks "clone --depth"
that recurses down to submodules by forcing the submodules to also
be cloned shallowly, which many server instances that host upstream
of the submodules are not prepared for.
* sb/clone-shallow-passthru:
clone: do not let --depth imply --shallow-submodules
A new run-command API function pipe_command() is introduced to
sanely feed data to the standard input while capturing data from
the standard output and the standard error of an external process,
which is cumbersome to hand-roll correctly without deadlocking.
The codepath to sign data in a prepared buffer with GPG has been
updated to use this API to read from the status-fd to check for
errors (instead of relying on GPG's exit status).
* jk/gpg-interface-cleanup:
gpg-interface: check gpg signature creation status
sign_buffer: use pipe_command
verify_signed_buffer: use pipe_command
run-command: add pipe_command helper
verify_signed_buffer: use tempfile object
verify_signed_buffer: drop pbuf variable
gpg-interface: use child_process.args
Formats of the various data (and how to validate them) where we use
GPG signature have been documented.
* mg/signature-doc:
Documentation/technical: signed merge tag format
Documentation/technical: signed commit format
Documentation/technical: signed tag format
Documentation/technical: describe signature formats
"log --graph --format=" learned that "%>|(N)" specifies the width
relative to the terminal's left edge, not relative to the area to
draw text that is to the right of the ancestry-graph section. It
also now accepts negative N that means the column limit is relative
to the right border.
* nd/graph-width-padded:
pretty.c: support <direction>|(<negative number>) forms
pretty: pass graph width to pretty formatting for use in '%>|(N)'
"git repack" learned the "--keep-unreachable" option, which sends
loose unreachable objects to a pack instead of leaving them loose.
This helps heuristics based on the number of loose objects
(e.g. "gc --auto").
Teach format-patch and mailsplit (hence "am") how a line that
happens to begin with "From " in the e-mail message is quoted with
">", so that these lines can be restored to their original shape.
* ew/mboxrd-format-am:
am: support --patch-format=mboxrd
mailsplit: support unescaping mboxrd messages
pretty: support "mboxrd" output format
"upload-pack" allows a custom "git pack-objects" replacement when
responding to "fetch/clone" via the uploadpack.packObjectsHook.
* jk/upload-pack-hook:
upload-pack: provide a hook for running pack-objects
t1308: do not get fooled by symbolic links to the source tree
config: add a notion of "scope"
config: return configset value for current_config_ functions
config: set up config_source for command-line config
git_config_parse_parameter: refactor cleanup code
git_config_with_options: drop "found" counting
Further preparatory clean-up for "worktree" feature continues.
* nd/worktree-cleanup-post-head-protection:
worktree: simplify prefixing paths
worktree: avoid 0{40}, too many zeroes, hard to read
worktree.c: use is_dot_or_dotdot()
git-worktree.txt: keep subcommand listing in alphabetical order
worktree.c: rewrite mark_current_worktree() to avoid strbuf
completion: support git-worktree
"git bisect" makes an internal call to "git diff-tree" when
bisection finds the culprit, but this call did not initialize the
data structure to pass to the diff-tree API correctly.
* jk/bisect-show-tree:
bisect: always call setup_revisions after init_revisions
Instead of taking advantage of a struct string_list that is
allocated with all NULs happens to be STRING_LIST_INIT_NODUP kind,
initialize them explicitly as such, to document their behaviour
better.
* jk/string-list-static-init:
use string_list initializer consistently
blame,shortlog: don't make local option variables static
interpret-trailers: don't duplicate option strings
parse_opt_string_list: stop allocating new strings
"git update-index --add --chmod=+x file" may be usable as an escape
hatch, but not a friendly thing to force for people who do need to
use it regularly. "git add --chmod=+x file" can be used instead.
The documentation set has been updated so that literal commands,
configuration variables and environment variables are consistently
typeset in fixed-width font and bold in manpages.
* tr/doc-tt:
doc: change configuration variables format
doc: more consistency in environment variables format
doc: change environment variables format
doc: clearer rule about formatting literals
mktemp is not available on all platforms, so the test
'temporary filenames are used with mergetool.writeToTemp'
fails there.
This patch does not replace mktemp but just disables
the test that otherwise would fail.
mergetool checks itself before executing mktemp and
reports an error.
Signed-off-by: Armin Kunaschik <megabreit@googlemail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before this change,
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes
would have the same effect as
$ echo "* text" >.gitattributes
$ git config core.eol crlf
Since the 'eol' attribute had higher priority than 'text=auto', this may
corrupt binary files and is not what most users expect to happen.
Make the 'eol' attribute to obey 'text=auto' and now
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes
behaves the same as
$ echo "* text=auto" >.gitattributes
$ git config core.eol crlf
In other words,
$ echo "* text=auto eol=crlf" >.gitattributes
has the same effect as
$ git config core.autocrlf true
and
$ echo "* text=auto eol=lf" >.gitattributes
has the same effect as
$ git config core.autocrlf input
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Makefile: drop extra dependencies for test helpers
A few test-helpers have Makefile dependencies on specific
object files. But since these files are part of libgit.a
(which all of the helpers link against), the inclusion is
simply redundant.
These were once necessary, but became redundant due to 5c5ba73 (Makefile: Use generic rule to build test programs,
2007-05-31), which added the $(GITLIBS) dependency (but
didn't prune the extra dependency lines). Later commits then
cargo-culted the practice (e.g., b4285c7).
Note that we _do_ need to leave the dependencies on the svn
library, as that is not part of the usual link command.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch: reduce duplicate in ref update status lines with placeholder
In the "remote -> local" line, if either ref is a substring of the
other, the common part in the other string is replaced with "*". For
example
abc -> origin/abc
refs/pull/123/head -> pull/123
become
abc -> origin/*
refs/*/head -> pull/123
Activated with fetch.output=compact.
For the record, this output is not perfect. A single giant ref can
push all refs very far to the right and likely be wrapped around. We
may have a few options:
- exclude these long lines smarter
- break the line after "->", exclude it from column width calculation
- implement a new format, { -> origin/}foo, which makes the problem
go away at the cost of a bit harder to read
- reverse all the arrows so we have "* <- looong-ref", again still
hard to read.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We do align "remote -> local" output by allocating 10 columns to
"remote". That produces aligned output only for short refs. An extra
pass is performed to find the longest remote ref name (that does not
produce a line longer than terminal width) to produce better aligned
output.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse_options: allocate a new array when concatenating
In exactly one callers (builtin/revert.c), we build up the
options list dynamically from multiple arrays. We do so by
manually inserting "filler" entries into one array, and then
copying the other array into the allocated space.
This is tedious and error-prone, as you have to adjust the
filler any time the second array is modified (although we do
at least check and die() when the counts do not match up).
Instead, let's just allocate a new array.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 84d32bf (sparse: Fix mingw_main() argument number/type errors,
2013-04-27), we addressed problems identified by the 'sparse' tool where
argv was declared inconsistently. The way we addressed it was by casting
from the non-const version to the const-version.
This patch is long overdue, fixing compat/mingw.h's declaration to
make the "argv" parameter const. This also allows us to lose the
"const" trickery introduced earlier to common-main.c:main().
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When git-daemon exits, we expect it to be with the SIGTERM
we just sent it. If we see anything else, we'll complain.
But our check against exit code "143" is not portable. For
example:
In 8bf4bec (add "ok=sigpipe" to test_must_fail and use it to
fix flaky tests, 2015-11-27), test_must_fail learned to
recognize "141" as a sigpipe failure. However, testing for
a signal is more complicated than that; we should use
test_match_signal to implement more portable checking.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The first test already uses this more portable construct
(that was where it was factored from initially), but the
later tests do a raw comparison against 141 to look for
SIGPIPE, which can fail on some shells and platforms.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In POSIX shells, a program which exits due to a signal
generally has an exit code of 128 plus the signal number.
However, ksh uses 256 plus the signal number. We've
accounted for that in t0005, but not in other tests. Let's
pull out the logic so we can use it elsewhere.
It would be nice for debugging if this additionally printed
errors to stderr, like our other test_* helpers. But we're
going to need to use it in other places besides the innards
of a test_expect block. So let's leave it as generic as
possible.
Note that we also leave the magic "3" for Windows out of the
generic helper. This is an artifact of the way we use
raise() to kill ourselves in test-sigchain.c, and will not
necessarily apply to all programs. So it's better to keep it
out of the helper, to reduce the chance of confusing it with
a real call to exit(3).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cmd_clone should detect a missing $url arg before using it otherwise
an uninitialized value error is emitted in even the simplest case of
'git svn clone' without arguments.
Signed-off-by: Christopher Layne <clayne@anodized.com> Signed-off-by: Eric Wong <e@80x24.org>
This should be part of every program, as otherwise users do
not get translated error messages. However, some external
commands forgot to do so (e.g., git-credential-store). This
fixes them, and eliminates the repeated code in programs
that did remember to use it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is another safety/sanity setup that should be in force
everywhere, but which we only applied in git.c. This did
catch most cases, since even external commands are typically
run via "git ..." (and the restoration applies to
sub-processes, too). But there were cases we missed, such as
somebody calling git-upload-pack directly via ssh, or
scripts which use dashed external commands directly.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is setup that should be done in every program for
safety, but we never got around to adding it everywhere (so
builtins benefited from the call in git.c, but any external
commands did not). Putting it in the common main() gives us
this safety everywhere.
Note that the case in daemon.c is a little funny. We wait
until we know whether we want to daemonize, and then either:
- call daemonize(), which will close stdio and reopen it to
/dev/null under the hood
- sanitize_stdfds(), to fix up any odd cases
But that is way too late; the point of sanitizing is to give
us reliable descriptors on 0/1/2, and we will already have
executed code, possibly called die(), etc. The sanitizing
should be the very first thing that happens.
With this patch, git-daemon will sanitize first, and can
remove the call in the non-daemonize case. It does mean that
daemonize() may just end up closing the descriptors we
opened, but that's not a big deal (it's not wrong to do so,
nor is it really less optimal than the case where our parent
process redirected us from /dev/null ahead of time).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Every program which links against libgit.a must call this
function, or risk hitting an assert() in system_path() that
checks whether we have configured argv0_path (though only
when RUNTIME_PREFIX is defined, so essentially only on
Windows).
Looking at the diff, you can see that putting it into the
common main() saves us having to do it individually in each
of the external commands. But what you can't see are the
cases where we _should_ have been doing so, but weren't
(e.g., git-credential-store, and all of the t/helper test
programs).
This has been an accident-waiting-to-happen for a long time,
but wasn't triggered until recently because it involves one
of those programs actually calling system_path(). That
happened with git-credential-store in v2.8.0 with ae5f677
(lazily load core.sharedrepository, 2016-03-11). The
program:
- takes a lock file, which...
- opens a tempfile, which...
- calls adjust_shared_perm to fix permissions, which...
- calls system_path() to find the location of
/etc/gitconfig
On systems with RUNTIME_PREFIX, this means credential-store
reliably hits that assert() and cannot be used.
We never noticed in the test suite, because we set
GIT_CONFIG_NOSYSTEM there, which skips the system_path()
lookup entirely. But if we were to tweak git_config() to
find /etc/gitconfig even when we aren't going to open it,
then the test suite shows multiple failures (for
credential-store, and for some other test helpers). I didn't
include that tweak here because it's way too specific to
this particular call to be worth carrying around what is
essentially dead code.
The implementation is fairly straightforward, with one
exception: there is exactly one caller (git.c) that actually
cares about the result of the function, and not the
side-effect of setting up argv0_path. We can accommodate
that by simply replacing the value of argv[0] in the array
we hand down to cmd_main().
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are certain startup tasks that we expect every git
process to do. In some cases this is just to improve the
quality of the program (e.g., setting up gettext()). In
others it is a requirement for using certain functions in
libgit.a (e.g., system_path() expects that you have called
git_extract_argv0_path()).
Most commands are builtins and are covered by the git.c
version of main(). However, there are still a few external
commands that use their own main(). Each of these has to
remember to include the correct startup sequence, and we are
not always consistent.
Rather than just fix the inconsistencies, let's make this
harder to get wrong by providing a common main() that can
run this standard startup.
We basically have two options to do this:
- the compat/mingw.h file already does something like this by
adding a #define that replaces the definition of main with a
wrapper that calls mingw_startup().
The upside is that the code in each program doesn't need
to be changed at all; it's rewritten on the fly by the
preprocessor.
The downside is that it may make debugging of the startup
sequence a bit more confusing, as the preprocessor is
quietly inserting new code.
- the builtin functions are all of the form cmd_foo(),
and git.c's main() calls them.
This is much more explicit, which may make things more
obvious to somebody reading the code. It's also more
flexible (because of course we have to figure out _which_
cmd_foo() to call).
The downside is that each of the builtins must define
cmd_foo(), instead of just main().
This patch chooses the latter option, preferring the more
explicit approach, even though it is more invasive. We
introduce a new file common-main.c, with the "real" main. It
expects to call cmd_main() from whatever other objects it is
linked against.
We link common-main.o against anything that links against
libgit.a, since we know that such programs will need to do
this setup. Note that common-main.o can't actually go inside
libgit.a, as the linker would not pick up its main()
function automatically (it has no callers).
The rest of the patch is just adjusting all of the various
external programs (mostly in t/helper) to use cmd_main().
I've provided a global declaration for cmd_main(), which
means that all of the programs also need to match its
signature. In particular, many functions need to switch to
"const char **" instead of "char **" for argv. This effect
ripples out to a few other variables and functions, as well.
This makes the patch even more invasive, but the end result
is much better. We should be treating argv strings as const
anyway, and now all programs conform to the same signature
(which also matches the way builtins are defined).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 4d5520053 (grep: make it clear i-t-a entries are
ignored, 2015-12-27) and adds an alternative fix to maintain the -L
--cached behavior.
4d5520053 caused 'git grep' to no longer find matches in new files in
the working tree where the corresponding index entry had the "intent to
add" bit set, despite the fact that these files are tracked.
The content in the index of a file for which the "intent to add" bit is
set is considered indeterminate and not empty. For most grep queries we
want these to behave the same, however for -L --cached (files without a
match) we don't want to respond positively for "intent to add" files as
their contents are indeterminate. This is in contrast to files with
empty contents in the index (no lines implies no matches for any grep
query expression) which should be reported in the output of a grep -L
--cached invocation.
Add tests to cover this case and a few related cases which previously
lacked coverage.
Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Charles Bailey <cbailey32@bloomberg.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We used character buffer manipulations to split messages from the
sideband at line breaks and insert "remote: " at the beginning of
each line, using the packet size to determine the end of a message.
However, since it is safe to assume that diagnostic messages from
the sideband never contain NUL characters, we can also NUL-terminate
the buffer, use strpbrk() for splitting lines and use format strings
to insert the prefix, to make the code easier to read and maintain.
A strbuf is used for accumulating the output which is then printed
using a single write(2) call to ensure the atomicity of the output.
See 9ac13ec (atomic write for sideband remote messages, 2006-10-11)
for details.
Helped-by: Jeff King <peff@peff.net> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Lukas Fleischer <lfleischer@lfos.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diffcore-pickaxe: support case insensitive match on non-ascii
Similar to the "grep -F -i" case, we can't use kws on icase search
outside ascii range, so we quote the string and pass it to regcomp as
a basic regexp and let regex engine deal with case sensitivity.
The new test is put in t7812 instead of t4209-log-pickaxe because
lib-gettext.sh might cause problems elsewhere, probably.
Noticed-by: Plamen Totev <plamen.totev@abv.bg> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's another regcomp code block coming in this function that needs
the same error handling. This function can help avoid duplicating
error handling code.
Helped-by: Jeff King <peff@peff.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the previous change in this function, we add locale support for
single-byte encodings only. It looks like pcre only supports utf-* as
multibyte encodings, the others are left in the cold (which is
fine).
We need to enable PCRE_UTF8 so pcre can find character boundary
correctly. It's needed for case folding (when --ignore-case is used)
or '*', '+' or similar syntax is used.
The "has_non_ascii()" check is to be on the conservative side. If
there's non-ascii in the pattern, the searched content could still be
in utf-8, but we can treat it just like a byte stream and everything
should work. If we force utf-8 based on locale only and pcre validates
utf-8 and the file content is in non-utf8 encoding, things break.
Noticed-by: Plamen Totev <plamen.totev@abv.bg> Helped-by: Plamen Totev <plamen.totev@abv.bg> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function returns true if git is running under an UTF-8
locale. pcre in the next patch will need this.
is_encoding_utf8() is used instead of strcmp() to catch both "utf-8"
and "utf8" suffixes.
When built with no gettext support, we peek in several env variables
to detect UTF-8. pcre library might support utf-8 even if libc is
built without locale support.. The peeking code is a copy from
compat/regex/regcomp.c
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep/pcre: prepare locale-dependent tables for icase matching
The default tables are usually built with C locale and only suitable
for LANG=C or similar. This should make case insensitive search work
correctly for all single-byte charsets.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep: rewrite an if/else condition to avoid duplicate expression
"!icase || ascii_only" is repeated twice in this if/else chain as this
series evolves. Rewrite it (and basically revert the first if
condition back to before the "grep: break down an "if" stmt..." commit).
Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to the previous commit, we can't use kws on icase search
outside ascii range. But we can't simply pass the pattern to
regcomp/pcre like the previous commit because it may contain regex
special characters, so we need to quote the regex first.
To avoid misquote traps that could lead to undefined behavior, we
always stick to basic regex engine in this case. We don't need fancy
features for grepping a literal string anyway.
basic_regex_quote_buf() assumes that if the pattern is in a multibyte
encoding, ascii chars must be unambiguously encoded as single
bytes. This is true at least for UTF-8. For others, let's wait until
people yell up. Chances are nobody uses multibyte, non utf-8 charsets
anymore.
Noticed-by: Plamen Totev <plamen.totev@abv.bg> Helped-by: René Scharfe <l.s.r@web.de> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
archive-tar: write extended headers for far-future mtime
The ustar format represents timestamps as seconds since the
epoch, but only has room to store 11 octal digits. To
express anything larger, we need to use an extended header.
This is exactly the same case we fixed for the size field in
the previous commit, and the solution here follows the same
pattern.
This is even mentioned as an issue in f2f0267 (archive-tar:
use xsnprintf for trivial formatting, 2015-09-24), but since
it only affected things far in the future, it wasn't deemed
worth dealing with. But note that my calculations claiming
thousands of years were off there; because our xsnprintf
produces a NUL byte, we only have until the year 2242 to fix
this.
Given that this is just around the corner (geologically
speaking, anyway), and because it's easy to fix, let's just
make it work. Unlike the previous fix for "size", where we
had to write an individual extended header for each file, we
can write one global header (since we have only one mtime
for the whole archive).
There's a slight bit of trickiness there. We may already be
writing a global header with a "comment" field for the
commit sha1. So we need to write our new field into the same
header. To do this, we push the decision of whether to write
such a header down into write_global_extended_header(),
which will now assemble the header as it sees fit, and will
return early if we have nothing to write (in practice, we'll
only have a large mtime if it comes from a commit, but this
makes it also work if you set your system clock ahead such
that time() returns a huge value).
Note that we don't (and never did) handle negative
timestamps (i.e., before 1970). This would probably not be
too hard to support in the same way, but since git does not
support negative timestamps at all, I didn't bother here.
After writing the extended header, we munge the timestamp in
the ustar headers to the maximum-allowable size. This is
wrong, but it's the least-wrong thing we can provide to a
tar implementation that doesn't understand pax headers (it's
also what GNU tar does).
Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
archive-tar: write extended headers for file sizes >= 8GB
The ustar format has a fixed-length field for the size of
each file entry which is supposed to contain up to 11 bytes
of octal-formatted data plus a NUL or space terminator.
These means that the largest size we can represent is 077777777777, or 1 byte short of 8GB. The correct solution
for a larger file, according to POSIX.1-2001, is to add an
extended pax header, similar to how we handle long
filenames. This patch does that, and writes zero for the
size field in the ustar header (the last bit is not
mentioned by POSIX, but it matches how GNU tar behaves with
--format=pax).
This should be a strict improvement over the current
behavior, which is to die in xsnprintf with a "BUG".
However, there's some interesting history here.
Prior to f2f0267 (archive-tar: use xsnprintf for trivial
formatting, 2015-09-24), we silently overflowed the "size"
field. The extra bytes ended up in the "mtime" field of the
header, which was then immediately written itself,
overwriting our extra bytes. What that means depends on how
many bytes we wrote.
If the size was 64GB or greater, then we actually overflowed
digits into the mtime field, meaning our value was
effectively right-shifted by those lost octal digits. And
this patch is again a strict improvement over that.
But if the size was between 8GB and 64GB, then our 12-byte
field held all of the actual digits, and only our NUL
terminator overflowed. According to POSIX, there should be a
NUL or space at the end of the field. However, GNU tar seems
to be lenient here, and will correctly parse a size up 64GB
(minus one) from the field. So sizes in this range might
have just worked, depending on the implementation reading
the tarfile.
This patch is mostly still an improvement there, as the 8GB
limit is specifically mentioned in POSIX as the correct
limit. But it's possible that it could be a regression
(versus the pre-f2f0267 state) if all of the following are
true:
1. You have a file between 8GB and 64GB.
2. Your tar implementation _doesn't_ know about pax
extended headers.
3. Your tar implementation _does_ parse 12-byte sizes from
the ustar header without a delimiter.
It's probably not worth worrying about such an obscure set
of conditions, but I'm documenting it here just in case.
Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ustar format only has room for 11 (or 12, depending on
some implementations) octal digits for the size and mtime of
each file. For values larger than this, we have to add pax
extended headers to specify the real data, and git does not
yet know how to do so.
Before fixing that, let's start off with some test
infrastructure, as designing portable and efficient tests
for this is non-trivial.
We want to use the system tar to check our output (because
what we really care about is interoperability), but we can't
rely on it:
1. being able to read pax headers
2. being able to handle huge sizes or mtimes
3. supporting a "t" format we can parse
So as a prerequisite, we can feed the system tar a reference
tarball to make sure it can handle these features. The
reference tar here was created with:
dd if=/dev/zero seek=64G bs=1 count=1 of=huge
touch -d @68719476737 huge
tar cf - --format=pax |
head -c 2048
using GNU tar. Note that this is not a complete tarfile, but
it's enough to contain the headers we want to examine.
Likewise, we need to convince git that it has a 64GB blob to
output. Running "git add" on that 64GB file takes many
minutes of CPU, and even compressed, the result is 64MB. So
again, I pre-generated that loose object, and then took only
the first 2k of it. That should be enough to generate 2MB of
data before hitting an inflate error, which is plenty for us
to generate the tar header (and then die of SIGPIPE while
streaming the rest out).
The tests are split so that we test as much as we can even
with an uncooperative system tar. This actually catches the
current breakage (which is that we die("BUG") trying to
write the ustar header) on every system, and then on systems
where we can, we go farther and actually verify the result.
Helped-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is sometimes useful to be able to read exactly N bytes from a
pipe. Doing this portably turns out to be surprisingly difficult
in shell scripts.
We want a solution that:
- is portable
- never reads more than N bytes due to buffering (which
would mean those bytes are not available to the next
program to read from the same pipe)
- handles partial reads by looping until N bytes are read
(or we see EOF)
- is resilient to stray signals giving us EINTR while
trying to read (even though we don't send them, things
like SIGWINCH could cause apparently-random failures)
Some possible solutions are:
- "head -c" is not portable, and implementations may
buffer (though GNU head does not)
- "read -N" is a bash-ism, and thus not portable
- "dd bs=$n count=1" does not handle partial reads. GNU dd
has iflags=fullblock, but that is not portable
- "dd bs=1 count=$n" fixes the partial read problem (all
reads are 1-byte, so there can be no partial response).
It does make a lot of write() calls, but for our tests
that's unlikely to matter. It's fairly portable. We
already use it in our tests, and it's unlikely that
implementations would screw up any of our criteria. The
most unknown one would be signal handling.
- perl can do a sysread() loop pretty easily. On my Linux
system, at least, it seems to restart the read() call
automatically. If that turns out not to be portable,
though, it would be easy for us to handle it.
That makes the perl solution the least bad (because we
conveniently omitted "length of code" as a criterion).
It's also what t9300 is currently using, so we can just pull
the implementation from there.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit -S: avoid invalid pointer with empty message
While it is not recommended, fsck.c says:
Not having a body is not a crime [...]
... which means that we cannot assume that the commit buffer
contains an empty line to separate header from body. A commit
object with only a header without any body, not even without
a blank line after the header, is valid.
So let's tread carefully here. strstr("\n\n") may find nothing
and return NULL.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
reset --hard: skip blank lines when reporting the commit subject
When there are blank lines at the beginning of a commit message, the
pretty printing machinery already skips them when showing a commit
subject (or the complete commit message). We shall henceforth do the
same when reporting the commit subject after the user called
git reset --hard <commit>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>