Start using selected c99 constructs in small, stable and
essentialpart of the system to catch people who care about
older compilers that do not grok them.
* jk/c99:
clean.c: use designated initializer
strbuf: use designated initializers in STRBUF_INIT
"%C(color name)" in the pretty print format always produced ANSI
color escape codes, which was an early design mistake. They now
honor the configuration (e.g. "color.ui = never") and also tty-ness
of the output medium.
* jk/ref-filter-colors:
ref-filter: consult want_color() before emitting colors
pretty: respect color settings for %C placeholders
rev-list: pass diffopt->use_colors through to pretty-print
for-each-ref: load config earlier
color: check color.ui in git_default_config()
ref-filter: pass ref_format struct to atom parsers
ref-filter: factor out the parsing of sorting atoms
ref-filter: make parse_ref_filter_atom a private function
ref-filter: provide a function for parsing sort options
ref-filter: move need_color_reset_at_eol into ref_format
ref-filter: abstract ref format into its own struct
ref-filter: simplify automatic color reset
t: use test_decode_color rather than literal ANSI codes
docs/for-each-ref: update pointer to color syntax
check return value of verify_ref_format()
The advice message given when "git rebase" stops for conflicting
changes has been improved.
* wd/rebase-conflict-guide:
rebase: make resolve message clearer for inexperienced users
Code clean-up.
* rs/move-array:
ls-files: don't try to prune an empty index
apply: use COPY_ARRAY and MOVE_ARRAY in update_image()
use MOVE_ARRAY
add MOVE_ARRAY
Code clean-up.
* rs/bswap-ubsan-fix:
bswap: convert get_be16, get_be32 and put_be32 to inline functions
bswap: convert to unsigned before shifting in get_be32
Remove an example that is now obsolete from a sample hook,
and improve an old example in it that added a sign-off manually
to use the interpret-trailers command.
* ks/prepare-commit-msg-sample:
hook: add a simple first example
hook: add sign-off using "interpret-trailers"
hook: name the positional variables
hook: cleanup script
Conversion from uchar[20] to struct object_id continues.
* bc/object-id:
sha1_name: convert uses of 40 to GIT_SHA1_HEXSZ
sha1_name: convert GET_SHA1* flags to GET_OID*
sha1_name: convert get_sha1* to get_oid*
Convert remaining callers of get_sha1 to get_oid.
builtin/unpack-file: convert to struct object_id
bisect: convert bisect_checkout to struct object_id
builtin/update_ref: convert to struct object_id
sequencer: convert to struct object_id
remote: convert struct push_cas to struct object_id
submodule: convert submodule config lookup to use object_id
builtin/merge-tree: convert remaining caller of get_sha1 to object_id
builtin/fsck: convert remaining caller of get_sha1 to object_id
Conversion from uchar[20] to struct object_id continues.
* sb/object-id:
tag: convert gpg_verify_tag to use struct object_id
commit: convert lookup_commit_graft to struct object_id
When unpack_entry() encounters a broken packed object, it returns early.
It adjusts the reference count of the pack window, but leaks the buffer
for a big delta stack in case the small automatic one was not enough.
Jump to the cleanup code at end instead, which takes care of that.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the code for releasing tree buffers and commit buffers in
fsck_obj() to the end of the function and make sure it's executed no
matter of an error is encountered or not.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
getdelim(3) returns -1 at the end of the file and if it encounters an
error, but sets errno only in the latter case. Set errno to zero before
calling it to avoid misdiagnosing an out-of-memory condition due to a
left-over value from some other function call.
Reported-by: Yaroslav Halchenko <yoh@onerussian.com>
Suggested-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>
If realloc() fails then the original buffer is still valid. Free it
before exiting the function.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Get rid of a magic string length constant by using skip_prefix() instead
of starts_with().
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
v1.2.0~121 (New tutorial, 2006-01-22) rewrote the tutorial such that the
original intent of 2ae6c70674 (Adapt tutorial to cygwin and add test case,
2005-10-13) to test the examples from the tutorial doesn't hold any more.
There are dedicated tests for the commands used, even "git whatchanged",
such that removing these tests doesn't seem like a reduction in test
coverage.
Signed-off-by: Stefan Beller <sbeller@google.com>
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Long ago in 628522ec14 (sha1-lookup: more memory efficient
search in sorted list of SHA-1, 2007-12-29) we added
sha1_entry_pos(), a binary search that uses the uniform
distribution of sha1s to scale the selection of mid-points.
As this was a performance experiment, we tied it to the
GIT_USE_LOOKUP environment variable and never enabled it by
default.
This code was successful in reducing the number of steps in
each search. But the overhead of the scaling ends up making
it slower when the cache is warm. Here are best-of-five
timings for running rev-list on linux.git, which will have
to look up every object:
$ time git rev-list --objects --all >/dev/null
real 0m35.357s
user 0m35.016s
sys 0m0.340s
$ time GIT_USE_LOOKUP=1 git rev-list --objects --all >/dev/null
real 0m37.364s
user 0m37.045s
sys 0m0.316s
The USE_LOOKUP version might have more benefit on a cold
cache, as the time to fault in each page would dominate. But
that would be for a single lookup. In practice, most
operations tend to look up many objects, and the whole pack
.idx will end up warm.
It's possible that the code could be better optimized to
compete with a naive binary search for the warm-cache case,
and we could have the best of both worlds. But over the
years nobody has done so, and this is largely dead code that
is rarely run outside of the test suite. Let's drop it in
the name of simplicity.
This lets us remove sha1_entry_pos() entirely, as the .idx
lookup code was the only caller. Note that sha1-lookup.c
still contains sha1_pos(), which differs from
sha1_entry_pos() in two ways:
- it has a different interface; it uses a function pointer
to access sha1 entries rather than a size/offset pair
describing the table's memory layout
- it only scales the initial selection of "mi", rather
than each iteration of the search
We can't get rid of this function, as it's called from
several places. It may be that we could replace it with a
simple binary search, but that's out of scope for this patch
(and would need benchmarking).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 1a812f3a70 (hashcmp(): inline memcmp() by hand to
optimize, 2011-04-28), it was reported that an open-coded
loop outperformed memcmp() for comparing sha1s.
Discussion[1] a few years later in 2013 showed that this
depends on your libc's version of memcmp(). In particular,
glibc 2.13 optimized their memcmp around 2011. Here are
current timings with glibc 2.24 (best-of-five, on
linux.git):
[before this patch, open-coded]
$ time git rev-list --objects --all
real 0m35.357s
user 0m35.016s
sys 0m0.340s
[after this patch, memcmp]
real 0m32.930s
user 0m32.630s
sys 0m0.300s
Now that we've had 6 years for that version of glibc to
make its way onto people's machines, it's worth revisiting
our benchmarks and switching to memcmp().
It may be that there are other non-glibc systems where
memcmp() isn't as well optimized. But since our single data
point in favor of open-coding was on a now-ancient glibc, we
should probably assume the system memcmp is good unless
proven otherwise. We may end up with a SLOW_MEMCMP Makefile
knob, but we can hold off on that until we actually find
such a system in practice.
[1] https://public-inbox.org/git/20130318073229.GA5551@sigill.intra.peff.net/
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use a NULL-and-NUL check to see if we have a prefix and consistently use
C string functions on it instead of storing its length in a member of
struct apply_state. This avoids strlen() calls and simplifies the code.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In addition to adding the missing newline, add the x-ecutable bit
'mode change' character to the error message. This message now has
the same form as similar messages output by 'update-index'.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
find_pack_entry_one() uses the fan-out table of pack indexes to find out
which entries match the first byte of the searched hash and does a
binary search on this subset of the main index table.
If there are no matching entries then lo and hi will have the same
value. The binary search still starts and compares the hash of the
following entry (which has a non-matching first byte, so won't cause any
trouble), or whatever comes after the sorted list of entries.
The probability of that stray comparison matching by mistake is low, but
let's not take any chances and check when entering the binary search
loop if we're actually done already.
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>
OpenBSD's regex library has a repetition limit (RE_DUP_MAX) of 255.
That's the minimum acceptable value according to POSIX. In t4062 we use
4096 repetitions in the test "-G matches", though, causing it to fail.
Combine two repetition operators, both less than 256, to arrive at 4096
zeros instead of using a single one, to fix the test on OpenBSD.
Original-patch-by: David Coppa <dcoppa@openbsd.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
76e368c378 (t3700: fix broken test under !SANITY) explains that the test
'git add --chmod=[+-]x changes index with already added file' can fail
if xfoo3 is still present as a symlink from a previous test and deletes
it with rm(1). That still leaves it present in the index, which causes
the test to fail if POSIXPERM is not defined. Get rid of it by calling
"git reset --hard" as well, as 76e368c378 already mentioned in passing.
Helped-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If there was no 'Signed-off-by:' trailer but another trailer such as
'Reported-by:' then 'git am --signoff' would add a blank line between
the existing trailers and the added 'Signed-off-by:' line. e.g.
Rebase accepts '--rerere-autoupdate' as an option but only honors
it if '-m' is also given. Fix it for a non-interactive rebase by
passing on the option to 'git am' and 'git cherry-pick'.
Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Fix by using the code provided for this purpose in sequencer.c.
Change the tests so that they check the formatting of the
'Signed-off-by:' lines rather than just grepping for them.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In parse_svn_date() prepend the correct UTC offset to the timestamp
returned. This is the offset in effect at the commit time instead of
the offset in effect at calling time.
Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>
Reviewed-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The only difference between these is that the former takes an argument
`name` which it ignores completely. Still, the callers are quite careful
to provide reasonable values for it.
Once in-flight topics have landed, we should be able to remove
git_config_maybe_bool. In the meantime, document it as deprecated in the
technical documentation. While at it, document git_parse_maybe_bool.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Both of these act on a string `value` which they parse as a boolean. The
"parse"-variant was introduced as a replacement for the "config"-variant
which for historical reasons takes an unused argument `name`. That it
was intended as a replacement is not obvious from commit 9a549d43
("config.c: rename git_config_maybe_bool_text and export it as
git_parse_maybe_bool", 2015-08-19), but that is what the background on
the mailing list suggests [1].
However, these two functions do not parse `value` in exactly the same
way. In particular, git_config_maybe_bool accepts integers (0 for false,
non-0 for true). This means there are two slightly different definitions
of "maybe_bool" in the code-base, and that every time a call to
git_config_maybe_bool is changed to use git_parse_maybe_bool, it risks
breaking someone's workflow.
Move the implementation of "config" into "parse" and make the latter a
trivial wrapper.
This also fixes the only user of git_parse_maybe_bool, `git push
--signed=..`.
[1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 9a549d43 ("config.c: rename git_config_maybe_bool_text and export
it as git_parse_maybe_bool", 2015-08-19) intended git_parse_maybe_bool
to be a replacement for git_config_maybe_bool, which could then be
retired. That is not obvious from the commit message, but that is what
the background on the mailing list suggests [1].
However, git_{config,parse}_maybe_bool do not handle all input the same.
Before the rename, that was by design and there is a caller in config.c
which requires git_parse_maybe_bool to behave exactly as it does.
Prepare for the next patch by renaming git_parse_maybe_bool to ..._text
and reimplementing the first one as a simple call to the second one. Let
the existing users in config.c use ..._text, since it does what they
need.
[1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When accepting booleans as command-line or config options throughout
Git, there are several documented synonyms for true and false.
However, one particular user is slightly broken: `git push --signed=..`
does not understand the integer synonyms for true and false.
This is hardly wanted. The --signed option has a different notion of
boolean than all other arguments and config options, including the
config option corresponding to it, push.gpgSign.
Add a test documenting the failure to handle --signed=1.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since we're about to touch the behavior of --signed=, do this as a
preparatory step.
The documentation mentions --sign=, and it works. But that's just
because it's an unambiguous abbreviation of --signed, which is how it is
actually implemented. This was added in commit 30261094 ("push: support
signing pushes iff the server supports it", 2015-08-19). Back when that
series was developed [1] [2], there were suggestions about both --sign=
and --signed=. The final implementation settled on --signed=, but some
of the documentation and commit messages ended up using --sign=.
The option is referred to as --signed= in Documentation/config.txt
(under push.gpgSign).
One could argue that we have promised --sign for two years now, so we
should implement it as an alias for --signed. (Then we might also
deprecate the latter, something which was considered already then.) That
would be a slightly more intrusive change.
This minor issue would only be a problem once we want to implement some
other option --signfoo, but the earlier we do this step, the better.
[1] v1-thread:
https://public-inbox.org/git/1439492451-11233-1-git-send-email-dborowitz@google.com/T/#u
[2] v2-thread:
https://public-inbox.org/git/1439998007-28719-1-git-send-email-dborowitz@google.com/T/#m6533a6c4707a30b0d81e86169ff8559460cbf6eb
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We want to make sure that people who copy & paste code would see
fewer instances of "git-foo". The use of these dashed forms have
been discouraged since v1.6.0 days.
Signed-off-by: Michael Forney <mforney@mforney.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parameter to basename(3) and dirname(3) traditionally had the type
"char *", but on OpenBSD it's been "const char *" for years. That
causes (at least) Clang to throw an incompatible-pointer-types warning
for test-path-utils, where we try to pass around pointers to these
functions.
Avoid this warning (which is fatal in DEVELOPER mode) by ignoring the
promise of OpenBSD's implementations to keep input strings unmodified
and enclosing them in POSIX-compatible wrappers.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The sub-test "init in long base path" in t0001 checks the ability to
handle long base paths with restrictive permissions (--x). On OpenBSD
getcwd(3) fails in that case even for short paths. Check the two
aspects separately by trying to use a long base path both with and
without execute-only permissions. Only attempt the former if we know
that getcwd(3) doesn't care.
Original-patch-by: David Coppa <dcoppa@openbsd.org>
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change an argument to test_line_count (which'll ultimately be turned
into a "test" expression) to use "-gt" instead of ">" for an
arithmetic test.
This broken on e.g. OpenBSD as of v2.13.0 with my commit
ac3f5a3468 ("ref-filter: add --no-contains option to
tag/branch/for-each-ref", 2017-03-24).
Downstream just worked around it by patching git and didn't tell us
about it, I discovered this when reading various Git packaging
implementations: https://github.com/openbsd/ports/commit/7e48bf88a20
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach 'git clone --recurse-submodules' to respect the '-q' option by
passing down the quiet flag to the process which handles cloning of
submodules.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When running, e.g., `git -c alias.bar=foo bar`, we expand the alias and
execute `git-foo` as a dashed external. This is true even if git foo is
a builtin. That is on purpose, and is motivated in a comment which was
added in commit 441981bc ("git: simplify environment save/restore
logic", 2016-01-26).
Shortly before we launch a dashed external, and unless we have already
found out whether we should use a pager, we check `pager.foo`. This was
added in commit 92058e4d ("support pager.* for external commands",
2011-08-18). If the dashed external is a builtin, this does not match
that commit's intention and is arguably wrong, since it would be cleaner
if we let the "dashed external builtin" handle `pager.foo`.
This has not mattered in practice, but a recent patch taught `git-tag`
to ignore `pager.tag` under certain circumstances. But, when started
using an alias, it doesn't get the chance to do so, as outlined above.
That recent patch added a test to document this breakage.
Do not check `pager.foo` before launching a builtin as a dashed
external, i.e., if we recognize the name of the external as a builtin.
Change the test to use `test_expect_success`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous patch taught `git tag` to only respect `pager.tag` in
list-mode. That patch left the default value of `pager.tag` at "off".
After that patch, it makes sense to let the default value be "on"
instead, since it will help with listing many tags, but will not hurt
users of `git tag -a` as it would have before. Make that change. Update
documentation and tests.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Using, e.g., `git -c pager.tag tag -a new-tag` results in errors such as
"Vim: Warning: Output is not to a terminal" and a garbled terminal.
Someone who makes use of both `git tag -a` and `git tag -l` will
probably not set `pager.tag`, so that `git tag -a` will actually work,
at the cost of not paging output of `git tag -l`.
Use the mechanisms introduced in two earlier patches to ignore
`pager.tag` in git.c and let the `git tag` builtin handle it on its own.
Only respect `pager.tag` when running in list-mode.
There is a window between where the pager is started before and after
this patch. This means that early errors can behave slightly different
before and after this patch. Since operation-parsing has to happen
inside this window, this can be seen with `git -c pager.tag="echo pager
is used" tag -l --unknown-option`. This change in paging-behavior should
be acceptable since it only affects erroneous usages.
Update the documentation and update tests.
If an alias is used to run `git tag -a`, then `pager.tag` will still be
respected. Document this known breakage. It will be fixed in a later
commit. Add a similar test for `-l`, which works.
Noticed-by: Anatoly Borodin <anatoly.borodin@gmail.com>
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Using, e.g., `git -c pager.tag tag -a new-tag` results in errors such as
"Vim: Warning: Output is not to a terminal" and a garbled terminal.
Someone who makes use of both `git tag -a` and `git tag -l` will
probably not set `pager.tag`, so that `git tag -a` will actually work,
at the cost of not paging output of `git tag -l`.
Since we're about to change how `git tag` respects `pager.tag`, add tests
around this, including how the configuration is ignored if --no-pager or
--paginate are used.
Construct tests with a few different subcommands. First, use -l. Second,
use "no arguments" and --contains, since those imply -l. (There are
more arguments which imply -l, but using these two should be enough.)
Third, use -a as a representative for "not -l". Actually, the tests use
`git tag -am` so no editor is launched, but that is irrelevant, since we
just want to see whether the pager is used or not. Make one of the tests
demonstrate the broken behavior mentioned above, where `git tag -a`
respects `pager.tag`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous patch introduced a way for builtins to declare that they
will take responsibility for handling the `pager.foo`-config item. (See
the commit message of that patch for why that could be useful.)
Provide setup_auto_pager(), which builtins can call in order to handle
`pager.<cmd>`, including possibly starting the pager. Make this function
don't do anything if a pager has already been started, as indicated by
use_pager or pager_in_use().
Whenever this function is called from a builtin, git.c will already have
called commit_pager_choice(). Since commit_pager_choice() treats the
special value -1 as "punt" or "not yet decided", it is not a problem
that we might end up calling commit_pager_choice() once in git.c and
once (or more) in the builtin. Make the new function use -1 in the same
way and document it as "punt".
Don't add any users of setup_auto_pager just yet, one will follow in
a later patch.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before launching a builtin git foo and unless mechanisms with precedence
are in use, we check for and handle the `pager.foo` config. This is done
without considering exactly how git foo is being used, and indeed, git.c
cannot (and should not) know what the arguments to git foo are supposed
to achieve.
In practice this means that, e.g., `git -c pager.tag tag -a new-tag`
results in errors such as "Vim: Warning: Output is not to a terminal"
and a garbled terminal. Someone who makes use of both `git tag -a` and
`git tag -l` will probably not set `pager.tag`, so that `git tag -a`
will actually work, at the cost of not paging output of `git tag -l`.
To allow individual builtins to make more informed decisions about when
to respect `pager.foo`, introduce a flag DELAY_PAGER_CONFIG. If the flag
is set, do not check `pager.foo`.
Do not check for DELAY_PAGER_CONFIG in `execv_dashed_external()`. That
call site is arguably wrong, although in a way that is not yet visible,
and will be changed in a slightly different direction in a later patch.
Don't add any users of DELAY_PAGER_CONFIG just yet, one will follow in a
later patch.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>