Reimplement the `check_term_format` shell function in C and add
a `--check-term-format` subcommand to `git bisect--helper` to call it
from git-bisect.sh
Using `--check-term-format` subcommand is a temporary measure to port
shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and its
implementation will be called by some other method/subcommand. For
eg. In conversion of write_terms() of git-bisect.sh, the subcommand will
be removed and instead check_term_format() will be called in its C
implementation while a new subcommand will be introduced for write_terms().
Helped-by: Johannes Schindelein <Johannes.Schindelein@gmx.de>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`--next-all` is meant to be used as a subcommand to support multiple
"operation mode" though the current implementation does not contain any
other subcommand along side with `--next-all` but further commits will
include some more subcommands.
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a caller of sha1_object_info_extended() sets the
"contentp" field in object_info, we call unpack_sha1_rest()
but do not check whether it signaled an error.
This causes two problems:
1. We pass back NULL to the caller via the contentp field,
but the function returns "0" for success. A caller
might reasonably expect after a successful return that
it can access contentp without a NULL check and
segfault.
As it happens, this is impossible to trigger in the
current code. There is exactly one caller which uses
contentp, read_object(). And the only thing it does
after a successful call is to return the content
pointer to its caller, using NULL as a sentinel for
errors. So in effect it converts the success code from
sha1_object_info_extended() back into an error!
But this is still worth addressing avoid problems for
future users of "contentp".
2. Callers of unpack_sha1_rest() are expected to close the
zlib stream themselves on error. Which means that we're
leaking the stream.
The problem in (1) comes from from c84a1f3ed4 (sha1_file:
refactor read_object, 2017-06-21), which added the contentp
field. Before that, we called unpack_sha1_rest() via
unpack_sha1_file(), which directly used the NULL to signal
an error.
But note that the leak in (2) is actually older than that.
The original unpack_sha1_file() directly returned the result
of unpack_sha1_rest() to its caller, when it should have
been closing the zlib stream itself on error.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reported-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
lookup_blob() and lookup_tree() can return NULL if they find an object
of an unexpected type. Accessing the object member is undefined in that
case. Cast the result to a struct object pointer instead; we can do
that because object is the first member of all object types. This trick
is already used in other places in the code.
An error message is already shown by object_as_type(), which is called
by the lookup functions. The walk callback functions are expected to
handle NULL object pointers passed to them, but put_object_name() needs
a valid object, so avoid calling it without one.
Suggested-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The next patches will tweak the behavior of this function. Document it
in order to establish a basis for those patches.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to a previous patch, we do not need to use `newfd` to signal
that we have a lockfile to clean up. We can just unconditionally call
`rollback_lock_file`. If we do not hold the lock, it will be a no-op.
Where we check `newfd` to decide whether we need to take the lock, we
can instead use `is_lock_file_locked()`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have two users of `struct apply_state` and the related functionality
in apply.c. Each user sets up its `apply_state` by handing over a
pointer to its static `lock_file`. (Before 076aa2cbd (tempfile:
auto-allocate tempfiles on heap, 2017-09-05), we could never free
lockfiles, so making them static was a reasonable approach.)
Other than that, they never directly access their `lock_file`s, which
are instead handled by the functionality in apply.c.
To make life easier for the caller and to make it less tempting for a
future caller to mess with the lock, make apply.c fully responsible for
setting up the `lock_file`. As mentioned above, it is now safe to free a
`lock_file`, so we can make the `struct apply_state` contain an actual
`struct lock_file` instead of a pointer to one.
The user in builtin/apply.c is rather simple. For builtin/am.c, we might
worry that the lock state is actually meant to be inherited across
calls. But the lock is only taken as `apply_all_patches()` executes, and
code inspection shows that it will always be released.
Alternatively, we can observe that the lock itself is never queried
directly. When we decide whether we should lock, we check a related
variable `newfd`. That variable is not inherited, so from the point of
view of apply.c, the state machine really is reset with each call to
`init_apply_state()`. (It would be a bug if `newfd` and the lock status
were not in sync. The duplication of information in `newfd` and the lock
will be addressed in the next patch.)
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After we have taken the lock using `LOCK_DIE_ON_ERROR`, we know that
`newfd` is non-negative. So when we check for exactly that property
before calling `write_locked_index()`, the outcome is guaranteed.
If we write and commit successfully, we set `newfd = -1`, so that we can
later avoid calling `rollback_lock_file` on an already-committed lock.
But we might just as well unconditionally call `rollback_lock_file()` --
it will be a no-op if we have already committed.
All in all, we use `newfd` as a bool and the only benefit we get from it
is that we can avoid calling a no-op. Remove `newfd` so that we have one
variable less to reason about.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`newfd` starts out negative. If we then take the lock, `newfd` will
become non-negative. We later check for exactly that property before
calling `write_locked_index()`. That is, we are simply using `newfd` as
a boolean to keep track of whether we took the lock or not. (We always
use `newfd` and `lock_file` together, so they really are mirroring each
other.)
Drop `newfd` and check with `is_lock_file_locked()` instead. While at
it, move the `static struct lock_file` into `cmd_checkout_index()` and
make it non-static. It is only used in this function, and after
076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we
can have lockfiles on the stack.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function has always been documented as returning 0 or -1. It is in
fact `void`. Correct that. As part of the rearrangements we lose the
mention that `delete_tempfile()` might set `errno`. Because there is
no return value, the user can't really know whether it did anyway.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 83a3069a3 (lockfile: do not rollback lock on failed close,
2017-09-05) forgot to update the documentation by the function definition
to reflect that the lock is not rolled back in case closing fails.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no longer any need to allocate and leak a `struct lock_file`.
The previous patch addressed an instance where we needed a minor tweak
alongside the trivial changes.
Deal with the remaining instances where we allocate and leak a struct
within a single function. Change them to have the `struct lock_file` on
the stack instead.
These instances were identified by running `git grep "^\s*struct
lock_file\s*\*"`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no longer any need to allocate and leak a `struct lock_file`.
Initialize it on the stack instead.
Before this patch, we set `lock = NULL` to signal that we have already
rolled back, and that we should not do any more work. We need to take
another approach now that we cannot assign NULL. We could, e.g., use
`is_lock_file_locked()`. But we already have another variable that we
could use instead, `found`. Its scope is only too small.
Bump `found` to the scope of the whole function and rearrange the "roll
back or write?"-checks to a straightforward if-else on `found`. This
also future-proves the code by making it obvious that we intend to take
exactly one of these paths.
Improved-by: Jeff King <peff@peff.net>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 2841e8f ("convert: add "status=delayed" to filter process protocol",
2017-06-30) we taught the filter process protocol to delay responses.
That means an external filter might answer in the first write_entry()
call on a file that requires filtering "I got your request, but I
can't answer right now. Ask again later!". As Git got no answer, we do
not write anything to the filesystem. Consequently, the lstat() call in
the finish block of the function writes garbage to the cache entry.
The garbage is eventually overwritten when the filter answers with
the final file content in a subsequent write_entry() call.
Fix the brief time window of garbage in the cache entry by adding a
special finish block that does nothing for delayed responses. The cache
entry is written properly in a subsequent write_entry() call where
the filter responds with the final file content.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Spell the name of our system as "Git" in the output from
request-pull script.
* ar/request-pull-phrasofix:
request-pull: capitalise "Git" to make it a proper noun
Add comment to clarify that the style file is meant to be used with
clang-5 and the rules are still work in progress.
* sb/git-clang-format:
clang-format: add a comment about the meaning/status of the
Code clean-up.
* rs/cleanup-strbuf-users:
graph: use strbuf_addchars() to add spaces
use strbuf_addstr() for adding strings to strbufs
path: use strbuf_add_real_path()
Code clean-up.
* rs/resolve-ref-optional-result:
refs: pass NULL to resolve_refdup() if hash is not needed
refs: pass NULL to refs_resolve_refdup() if hash is not needed
The checkpoint command "git fast-import" did not flush updates to
refs and marks unless at least one object was created since the
last checkpoint, which has been corrected, as these things can
happen without any new object getting created.
* er/fast-import-dump-refs-on-checkpoint:
fast-import: checkpoint: dump branches/tags/marks even if object_count==0
Peff points out that different atom parsers handle the empty
"sub-argument" list differently. An example of this is the format
"%(refname:)".
Since callers often use `string_list_split` (which splits the empty
string with any delimiter as a 1-ary string_list containing the empty
string), this makes handling empty sub-argument strings non-ergonomic.
Let's fix this by declaring that atom parser implementations must
not care about distinguishing between the empty string "%(refname:)"
and no sub-arguments "%(refname)". Current code aborts, either with
"unrecognised arg" (e.g. "refname:") or "does not take args"
(e.g. "body:") as an error message.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Reviewed-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update the test fsmonitor-watchman integration script to properly
preserve utf8 filenames when outputting the .git/watchman-output.out log
file.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In Perl, setting $/ sets the string that is used as the "record
separator," which sets the boundary that the `<>` construct reads to.
Setting `local $/ = 0666;` evaluates the octal, getting 438, and
stringifies it. Thus, the later read from `<CHLD_OUT>` stops as soon
as it encounters the string "438" in the watchman output, yielding
invalid JSON; repositories containing filenames with SHA1 hashes are
able to trip this easily.
Set `$/` to undefined, thus slurping all output from watchman. Also
close STDIN which is provided to watchman, to better guarantee that we
cannot deadlock with watchman while both attempting to read.
Signed-off-by: Alex Vandiver <alexmv@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
strbuf_release leaves the strbuf in a valid, initialized state, so
there is no need to call strbuf_init after it.
Moreover, this is not likely to change in the future: strbuf_release
leaving the strbuf in a valid state has been easy to maintain and has
been very helpful for Git's robustness and simplicity (e.g.,
preventing use-after-free vulnerabilities).
Document the semantics so the next generation of Git developers can
become familiar with them without reading the implementation. It is
still not advisable to call strbuf_release too often because it is
wasteful, so add a note pointing to strbuf_reset for that.
The same semantics apply to strbuf_detach. Add a similar note to its
docstring to make that clear.
Improved-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our documentation advises to not re-use a strbuf, after strbuf_release
has been called on it. Use the proper reset instead.
Currently 'strbuf_release' releases and re-initializes the strbuf, so it
is safe, but slow. 'strbuf_reset' only resets the internal length variable,
such that this could also be accounted for as a micro-optimization.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently the argv is only allocated on the stack, and then assigned to
process->argv. When the start_subprocess function goes out of scope,
the local argv variable is eliminated from the stack, but the pointer is
still kept around in process->argv.
Much later when we try to access the same process->argv in
finish_command, this leads us to access a memory location that no longer
contains what we want. As argv0 is only used for printing errors, this
is not easily noticed in normal git operations. However when running
t0021-conversion.sh through valgrind, valgrind rightfully complains:
==21024== Invalid read of size 8
==21024== at 0x2ACF64: finish_command (run-command.c:869)
==21024== by 0x2D6B18: subprocess_exit_handler (sub-process.c:72)
==21024== by 0x2AB41E: cleanup_children (run-command.c:45)
==21024== by 0x2AB526: cleanup_children_on_exit (run-command.c:81)
==21024== by 0x54AD487: __run_exit_handlers (in /usr/lib/libc-2.26.so)
==21024== by 0x54AD4D9: exit (in /usr/lib/libc-2.26.so)
==21024== by 0x11A9EF: handle_builtin (git.c:550)
==21024== by 0x11ABCC: run_argv (git.c:602)
==21024== by 0x11AD8E: cmd_main (git.c:679)
==21024== by 0x1BF125: main (common-main.c:43)
==21024== Address 0x1ffeffec00 is on thread 1's stack
==21024== 1504 bytes below stack pointer
==21024==
These days, the child_process structure has its own args array, and
the standard way to set up its argv[] is to use that one, instead of
assigning to process->argv to point at an array that is outside.
Use that facility automatically fixes this issue.
Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The get_oid_hex_from_objpath takes care of creating a oid from a
pathname. It does this by memcpy'ing the first two bytes of the path to
the "hex" string, then skipping the '/', and then copying the rest of the
path to the "hex" string. Currently it fails to increase the pointer to
the hex string, so the second memcpy invocation just mashes over what
was copied in the first one, and leaves the last two bytes in the string
uninitialized.
This breaks valgrind in t5540, although the test passes without
valgrind:
==5490== Use of uninitialised value of size 8
==5490== at 0x13C6B5: hexval (cache.h:1238)
==5490== by 0x13C6DB: hex2chr (cache.h:1247)
==5490== by 0x13C734: get_sha1_hex (hex.c:42)
==5490== by 0x13C78E: get_oid_hex (hex.c:53)
==5490== by 0x118BDA: get_oid_hex_from_objpath (http-push.c:1023)
==5490== by 0x118C92: process_ls_object (http-push.c:1038)
==5490== by 0x118E5B: handle_remote_ls_ctx (http-push.c:1077)
==5490== by 0x118227: xml_end_tag (http-push.c:815)
==5490== by 0x50C1448: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== by 0x50C221B: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== by 0x50BFBF2: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== by 0x50C0B24: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== Uninitialised value was created by a stack allocation
==5490== at 0x118B63: get_oid_hex_from_objpath (http-push.c:1012)
==5490==
Fix this by correctly incrementing the pointer to the "hex" variable, so
the first two bytes are left untouched by the memcpy call, and the last
two bytes are correctly initialized.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In cleanup_path we're passing in a char array, run a memcmp on it, and
run through it without ever checking if something is in the array in the
first place. This can lead us to access uninitialized memory, for
example in t5541-http-push-smart.sh test 7, when run under valgrind:
==4423== Conditional jump or move depends on uninitialised value(s)
==4423== at 0x242FA9: cleanup_path (path.c:35)
==4423== by 0x242FA9: mkpath (path.c:456)
==4423== by 0x256CC7: refname_match (refs.c:364)
==4423== by 0x26C181: count_refspec_match (remote.c:1015)
==4423== by 0x26C181: match_explicit_lhs (remote.c:1126)
==4423== by 0x26C181: check_push_refs (remote.c:1409)
==4423== by 0x2ABB4D: transport_push (transport.c:870)
==4423== by 0x186703: push_with_options (push.c:332)
==4423== by 0x18746D: do_push (push.c:409)
==4423== by 0x18746D: cmd_push (push.c:566)
==4423== by 0x1183E0: run_builtin (git.c:352)
==4423== by 0x11973E: handle_builtin (git.c:539)
==4423== by 0x11973E: run_argv (git.c:593)
==4423== by 0x11973E: main (git.c:698)
==4423== Uninitialised value was created by a heap allocation
==4423== at 0x4C2CD8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4423== by 0x4C2F195: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4423== by 0x2C196B: xrealloc (wrapper.c:137)
==4423== by 0x29A30B: strbuf_grow (strbuf.c:66)
==4423== by 0x29A30B: strbuf_vaddf (strbuf.c:277)
==4423== by 0x242F9F: mkpath (path.c:454)
==4423== by 0x256CC7: refname_match (refs.c:364)
==4423== by 0x26C181: count_refspec_match (remote.c:1015)
==4423== by 0x26C181: match_explicit_lhs (remote.c:1126)
==4423== by 0x26C181: check_push_refs (remote.c:1409)
==4423== by 0x2ABB4D: transport_push (transport.c:870)
==4423== by 0x186703: push_with_options (push.c:332)
==4423== by 0x18746D: do_push (push.c:409)
==4423== by 0x18746D: cmd_push (push.c:566)
==4423== by 0x1183E0: run_builtin (git.c:352)
==4423== by 0x11973E: handle_builtin (git.c:539)
==4423== by 0x11973E: run_argv (git.c:593)
==4423== by 0x11973E: main (git.c:698)
==4423==
Avoid this by using skip_prefix(), which knows not to go beyond the
end of the string.
Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Check if the strbuf containing data to sort is empty before attempting
to trim a trailing newline character.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error message shown when a flag is found when expecting a
filename wasn't clear as it didn't communicate what was wrong
using the 'suitable' words in *all* cases.
$ git ls-files
README.md
test-file
Correct case,
$ git rev-parse README.md --flags
README.md
--flags
fatal: bad flag '--flags' used after filename
Incorrect case,
$ git grep "some random regex" -n
fatal: bad flag '-n' used after filename
The above case is incorrect as "some random regex" isn't a filename
in this case.
Change the error message to be general and communicative. This results
in the following output,
$ git rev-parse README.md --flags
README.md
--flags
fatal: option '--flags' must come before non-option arguments
$ git grep "some random regex" -n
fatal: option '-n' must come before non-option arguments
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error messages shown when the branch command is misused
by supplying it wrong number of parameters wasn't meaningful.
That's because it used the the phrase "too many branches"
assuming all parameters to be "valid" branch names. It's not
always the case as exemplified below,
$ git branch
foo
* master
$ git branch -m foo foo old
fatal: too many branches for a rename operation
Change the messages to be more general thus making no assumptions
about the "parameters".
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jk/ui-color-always-to-auto-maint:
color: make "always" the same as "auto" in config
provide --color option for all ref-filter users
t3205: use --color instead of color.branch=always
t3203: drop "always" color test
t6006: drop "always" color config tests
t7502: use diff.noprefix for --verbose test
t7508: use test_terminal for color output
t3701: use test-terminal to collect color output
t4015: prefer --color to -c color.diff=always
test-terminal: set TERM=vt100
This test wants to confirm that "clean -i" shows color
output. Using test_terminal gives us a more realistic
environment than "color.ui=always", and prepares us for the
behavior of "always" changing in a future patch.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tests for --color-moved write their output to a file,
but doing so suppresses color output under "auto". Right now
this is solved by running the whole script under
"color.diff=always". In preparation for the behavior of
"always" changing, let's explicitly enable color.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It can be handy to use `--color=always` (or it's synonym
`--color`) on the command-line to convince a command to
produce color even if it's stdout isn't going to the
terminal or a pager.
What's less clear is whether it makes sense to set config
variables like color.ui to `always`. For a one-shot like:
git -c color.ui=always ...
it's potentially useful (especially if the command doesn't
directly support the `--color` option). But setting `always`
in your on-disk config is much muddier, as you may be
surprised when piped commands generate colors (and send them
to whatever is consuming the pipe downstream).
Some people have done this anyway, because:
1. The documentation for color.ui makes it sound like
using `always` is a good idea, when you almost
certainly want `auto`.
2. Traditionally not every command (and especially not
plumbing) respected color.ui in the first place. So
the confusion came up less frequently than it might
have.
The situation changed in 136c8c8b8f (color: check color.ui
in git_default_config(), 2017-07-13), which negated point
(2): now scripts using only plumbing commands (like
add-interactive) are broken by this setting.
That commit was fixing real issues (e.g., by making
`color.ui=never` work, since `auto` is the default), so we
don't want to just revert it. We could turn `always` into a
noop in plumbing commands, but that creates a hard-to-explain
inconsistency between the plumbing and other commands.
Instead, let's just turn `always` into `auto` for all config.
This does break the "one-shot" config shown above, but again,
we're probably better to have simple and consistent rules than
to try to special-case command-line config.
There is one place where `always` should retain its meaning:
on the command line, `--color=always` should continue to be
the same as `--color`, overriding any isatty checks. Since the
command-line parser also depends on git_config_colorbool(), we
can use the existence of the "var" string to deterine whether
we are serving the command-line or the config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When ref-filter learned about want_color() in 11b087adfd
(ref-filter: consult want_color() before emitting colors,
2017-07-13), it became useful to be able to turn colors off
and on for specific commands. For git-branch, you can do so
with --color/--no-color.
But for git-for-each-ref and git-tag, the other users of
ref-filter, you have no option except to tweak the
"color.ui" config setting. Let's give both of these commands
the usual color command-line options.
This is a bit more obvious as a method for overriding the
config. And it also prepares us for the behavior of "always"
changing (so that we are still left with a way of forcing
color when our output goes to a non-terminal).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To test the color output, we must convince "git branch" to
write colors to a non-terminal. We do that now by setting
the color config to "always". In preparation for the
behavior of "always" changing, let's switch to using the
"--color" command-line option, which is more direct.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for the behavior of "always" changing to
match "auto", we can simply drop this test. We already check
other forms (like "--color") independently.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We test the %C() format placeholders with a variety of
color-inducing options, including "--color" and
"-c color.ui=always". In preparation for the behavior of
"always" changing, we need to do something with those
"always" tests.
We can drop ones that expect "always" to turn on color even
to a file, as that will become a synonym for "auto", which
is already tested.
For the "--no-color" test, we need to make sure that color
would otherwise be shown. To do this, we can use
test_terminal, which enables colors in the default setup.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To check that "status -v" respects diff config, we set
"color.diff" and look at the output of "status". We could
equally well use any diff config. Since color output depends
on a lot of other factors (like whether stdout is a tty, and
how we interpret "always"), let's use a more mundane option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This script tests the output of status with various formats
when color is enabled. It uses the "always" setting so that
the output is valid even though we capture it in a file.
Using test_terminal gives us a more realistic environment,
and prepares us for the behavior of "always" changing.
Arguably we are testing less than before, since "auto" is
already the default, and we can no longer tell if the config
is actually doing anything.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When testing whether "add -p" can generate colors, we set
color.ui to "always". This isn't a very good test, as in the
real-world a user typically has "auto" coupled with stdout
going to a terminal (and it's plausible that this could mask
a real bug in add--interactive if we depend on plumbing's
isatty check).
Let's switch to test_terminal, which gives us a more
realistic environment. This also prepare us for future
changes to the "always" color option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t4015 contains many color-related tests which need to
override the "is stdout a tty" check. They do so by setting
the color.diff config, but we can accomplish the same with
the --color option. Besides being shorter to type, switching
will prepare us for upcoming changes to "always" when see it
in config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of the test-terminal script is to simulate in the
test scripts an environment where output is going to a real
terminal.
But since test-lib.sh also sets TERM=dumb, the simulation
isn't very realistic. The color code will skip auto-coloring
for TERM=dumb, leading to us liberally sprinkling
test_terminal env TERM=vt100 git ...
through the test suite to convince the tests to actually
generate colors. Let's set TERM for programs run under
test_terminal, which is one less thing for test-writers to
remember.
In most cases the callers can be simplified, but note there
is one interesting case in t4202. It uses test_terminal to
check the auto-enabling of --decorate, but the expected
output _doesn't_ contain colors (because TERM=dumb
suppresses them). Using TERM=vt100 is closer to what the
real world looks like; adjust the expected output to match.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of just taking $ENV{'PWD'}, use the same logic that converts
PWD to $git_work_tree on MSYS_NT in the watchman integration hook
script also on MINGW.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
