diff-delta: set size out-parameter to 0 for NULL delta
When we cannot generate a delta, we return NULL but leave delta_size
untouched. This is generally OK, as callers rely on NULL to decide if
the output is usable or not. But it can confuse compilers; in
particular, gcc 9.2.1 with "-flto -O3" complains in fast-import's
store_object() that delta_len may be used uninitialized.
Let's change the diff-delta code to set the size explicitly to 0 for a
NULL return. That silences the compiler and makes it easier to reason
about the result.
Reported-by: Stephan Beyer <s-beyer@gmx.net> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We declare a "struct hashfile_checkpoint" but only sometimes actually
call hashfile_checkpoint() on it. That makes it not immediately obvious
that it's valid when we later access its members.
In fact, the code is fine: we fill it in unconditionally in the while(1)
loop as long as "idx" is non-NULL. And then if "idx" is NULL, we exit
early from the function (because we're just computing the hash, not
actually writing), before we look at the struct.
However, this does seem to confuse gcc 9.2.1's -Wmaybe-uninitialized
when compiled with "-flto -O2" (probably because with LTO it can now
realize that our call to hashfile_truncate() does not set the members
either). Let's zero-initialize the struct to tell the compiler, as well
as any readers of the code, that all is well.
Reported-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The only caller of packlist_alloc() already has a "struct object_id",
and we immediately copy the hash they pass us into our own object_id.
Let's avoid the unnecessary round-trip to a raw sha1 pointer.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-am: handle missing "author" when parsing commit
We try to parse the "author" line out of a commit buffer. We handle the
case that split_ident_line() doesn't work, but we don't do any error
checking that we found an "author" line in the first place! This would
cause us to segfault on such a corrupt object.
Let's put in an explicit NULL check (we can just die(), which is what a
bogus split would do, too). As a bonus, this silences a warning when
compiling with gcc 9.2.1 using "-flto -O3", which claims that ident_len
may be uninitialized (it would only be if we had a NULL here).
Reported-by: Stephan Beyer <s-beyer@gmx.net> 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 first line of verbose output from each test piece now carries
the test name and number to help scanning with eyeballs.
* sg/show-failed-test-names:
tests: show the test name and number at the start of verbose output
t0000-basic: use realistic test script names in the verbose tests
The code to write commit-graph over given commit object names has
been made a bit more robust.
* sg/commit-graph-validate:
commit-graph: error out on invalid commit oids in 'write --stdin-commits'
commit-graph: turn a group of write-related macro flags into an enum
t5318-commit-graph: use 'test_expect_code'
"git checkout" and "git restore" to re-populate the index from a
tree-ish (typically HEAD) did not work correctly for a path that
was removed and then added again with the intent-to-add bit, when
the corresponding working tree file was empty. This has been
corrected.
* vn/restore-empty-ita-corner-case-fix:
restore: add test for deleted ita files
checkout.c: unstage empty deleted ita files
* sg/do-not-skip-non-httpd-tests:
t: warn against adding non-httpd-specific tests after sourcing 'lib-httpd'
t5703: run all non-httpd-specific tests before sourcing 'lib-httpd.sh'
t5510-fetch: run non-httpd-specific test before sourcing 'lib-httpd.sh'
Codepaths to walk tree objects have been audited for integer
overflows and hardened.
* jk/tree-walk-overflow:
tree-walk: harden make_traverse_path() length computations
tree-walk: add a strbuf wrapper for make_traverse_path()
tree-walk: accept a raw length for traverse_path_len()
tree-walk: use size_t consistently
tree-walk: drop oid from traverse_info
setup_traverse_info(): stop copying oid
"git grep --recurse-submodules" that looks at the working tree
files looked at the contents in the index in submodules, instead of
files in the working tree.
* mt/grep-submodules-working-tree:
grep: fix worktree case in submodules
worktree remove: clarify error message on dirty worktree
To avoid data loss, 'git worktree remove' refuses to delete a worktree
if it's dirty or contains untracked files. However, the error message
only mentions that the worktree "is dirty", even if the worktree in
question is in fact clean, but contains untracked files:
$ git worktree add test-worktree
Preparing worktree (new branch 'test-worktree')
HEAD is now at aa53e60 Initial
$ >test-worktree/untracked-file
$ git worktree remove test-worktree/
fatal: 'test-worktree/' is dirty, use --force to delete it
$ git -C test-worktree/ diff
$ git -C test-worktree/ diff --cached
$ # Huh? Where are those dirty files?!
Clarify this error message to say that the worktree "contains modified
or untracked files".
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-fast-import.txt: clarify that multiple merge commits are allowed
The grammar for commits used a '?' rather than a '*' on the `merge`
directive line, despite the fact that the code allows multiple `merge`
directives in order to support n-way merges. In fact, elsewhere in
git-fast-import.txt there is an explicit declaration that "an unlimited
number of `merge` commands per commit are permitted by fast-import".
Fix the grammar to match the intent and implementation.
Reported-by: Joachim Klein <joachim.klein@automata.tools> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two perf scripts numbered p5600, but with otherwise different
names ("clone-reference" versus "partial-clone"). We store timing
results in files named after the whole script, so internally we don't
get confused between the two. But "aggregate.perl" just prints the test
number for each result, giving multiple entries for "5600.3". It also
makes it impossible to skip one test but not the other with
GIT_SKIP_TESTS.
Let's renumber the one that appeared later (by date -- the source of the
problem is that the two were developed on independent branches). For the
non-perf test suite, our test-lint rule would have complained about this
when the two were merged, but t/perf never learned that trick.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Inspired by 21416f0a07 ("restore: fix typo in docs", 2019-08-03), I ran
"git grep -E '(\b[a-zA-Z]+) \1\b' -- Documentation/" to find other cases
where words were duplicated, e.g. "the the", and in most cases removed
one of the repeated words.
There were many false positives by this grep command, including
deliberate repeated words like "really really" or valid uses of "that
that" which I left alone, of course.
I also did not correct any of the legitimate, accidentally repeated
words in old RelNotes.
Signed-off-by: Mark Rushakoff <mark.rushakoff@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
My IEE 'home for life' email service is being withdrawn on 30 Sept 2019.
Replace with my new email domain.
I also have a secondary (backup) 'home for life' through
<philipoakley@dunelm.org.uk>.
Signed-off-by: Philip Oakley <philipoakley@iee.email> Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* tag 'v2.23.0-rc2' of git://git.kernel.org/pub/scm/git/git: (63 commits)
Git 2.23-rc2
t0000: reword comments for "local" test
t: decrease nesting in test_oid_to_path
sha1-file: release strbuf after use
test-dir-iterator: use path argument directly
dir-iterator: release strbuf after use
commit-graph: release strbufs after use
l10n: reformat some localized strings for v2.23.0
merge-recursive: avoid directory rename detection in recursive case
commit-graph: fix bug around octopus merges
restore: fix typo in docs
doc: typo: s/can not/cannot/ and s/is does/does/
Git 2.23-rc1
log: really flip the --mailmap default
RelNotes/2.23.0: fix a few typos and other minor issues
RelNotes/2.21.1: typofix
log: flip the --mailmap default unconditionally
config: work around bug with includeif:onbranch and early config
A few more last-minute fixes
repack: simplify handling of auto-bitmaps and .keep files
...
Merge branch 'cb/xdiff-no-system-includes-in-dot-c' into maint
Compilation fix.
* cb/xdiff-no-system-includes-in-dot-c:
xdiff: remove duplicate headers from xpatience.c
xdiff: remove duplicate headers from xhistogram.c
xdiff: drop system includes in xutils.c
Commit 01d3a526ad (t0000: check whether the shell supports the "local"
keyword, 2017-10-26) added a test to gather data on whether people run
the test suite with shells that don't support "local".
After almost two years, nobody has complained, and several other uses
have cropped up in test-lib-functions.sh. Let's declare it acceptable to
use.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The stream of symbols that makes up this function is hard for humans
to follow, too. The complexity mostly comes from the repeated use of
the expression ${1#??} for the basename of the loose object. Use a
variable instead --- nowadays, the dialect of shell used by Git
permits local variables, so this is cheap.
An alternative way to work around [*] is to remove the double-quotes
around test_oid_to_path's return value. That makes the expression
easier for dash to read, but harder for humans. Let's prefer the
rephrasing that's helpful for humans, too.
Noticed by building on Ubuntu trusty, which uses dash 0.5.7.
[*] Fixed by v0.5.8~13 ("[EXPAND] Propagate EXP_QPAT in subevalvar, 2013-08-23).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
nedmalloc: avoid compiler warning about unused value
Cast the evaluated value of the macro INITIAL_LOCK to void to instruct
the compiler that we're not interested in said value nor the following
warning:
In file included from compat/nedmalloc/nedmalloc.c:63:
compat/nedmalloc/malloc.c.h: In function ‘init_user_mstate’:
compat/nedmalloc/malloc.c.h:1706:62: error: right-hand operand of comma expression has no effect [-Werror=unused-value]
1706 | #define INITIAL_LOCK(sl) (memset(sl, 0, sizeof(MLOCK_T)), 0)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
compat/nedmalloc/malloc.c.h:5020:3: note: in expansion of macro ‘INITIAL_LOCK’
5020 | INITIAL_LOCK(&m->mutex);
| ^~~~~~~~~~~~
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
nedmalloc: do assignments only after the declaration section
Avoid the following compiler warning:
In file included from compat/nedmalloc/nedmalloc.c:63:
compat/nedmalloc/malloc.c.h: In function ‘pthread_release_lock’:
compat/nedmalloc/malloc.c.h:1759:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
1759 | volatile unsigned int* lp = &sl->l;
| ^~~~~~~~
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge-recursive: avoid directory rename detection in recursive case
Ever since commit 8c8e5bd6eb33 ("merge-recursive: switch directory
rename detection default", 2019-04-05), the default handling with
directory rename detection was to report a conflict and leave unstaged
entries in the index. However, when creating a virtual merge base in
the recursive case, we absolutely need a tree, and the only way a tree
can be written is if we have no unstaged entries -- otherwise we hit a
BUG().
There are a few fixes possible here which at least fix the BUG(), but
none of them seem optimal for other reasons; see the comments with the
new testcase 13e in t6043 for details (which testcase triggered a BUG()
prior to this patch). As such, just opt for a very conservative and
simple choice that is still relatively reasonable: have the recursive
case treat 'conflict' as 'false' for opt->detect_directory_renames.
Reported-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
[master (root-commit) d1fbfbd] add file
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 file
ok 1 - commit works
i.e. first an "expecting success" (or "checking known breakage") line
followed by the commands to be executed, then the output of those
comamnds, and finally an "ok"/"not ok" line containing the test name.
Note that the test's name is only shown at the very end.
With '-x' tracing enabled and/or in longer tests the verbose output
might be several screenfulls long, making it harder than necessary to
find where the output of the test with a given name starts (especially
when the outputs to different file descriptors are racing, and the
"expecting success"/command block arrives earlier than the "ok" line
of the previous test).
Print the test name at the start of the test's verbose output, i.e. at
the end of the "expecting success" and "checking known breakage"
lines, to make the start of a particular test a bit easier to
recognize. Also print the test script and test case numbers, to help
those poor souls who regularly have to scan through the combined
verbose output of several test scripts.
t0000-basic: use realistic test script names in the verbose tests
Our test scripts are named something like 't1234-command.sh', but the
script names used in 't0000-basic.sh' don't follow this naming
convention. Normally this doesn't matter, because the test scripts
themselves don't care how they are called. However, the next patch
will start to include the test number in the test's verbose output, so
the test script's name will matter in the two tests checking the
verbose output.
Update the tests 'test --verbose' and 'test --verbose-only' to follow
out test script naming convention.
Leave the other tests in 't0000' unchanged: changing the names of
their test scripts would be only pointless code churn.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 1771be90 "commit-graph: merge commit-graph chains" (2019-06-18),
the method sort_and_scan_merged_commits() was added to merge the
commit lists of two commit-graph files in the incremental format.
Unfortunately, there was an off-by-one error in that method around
incrementing num_extra_edges, which leads to an incorrect offset
for the base graph chunk.
When we store an octopus merge in the commit-graph file, we store
the first parent in the normal place, but use the second parent
position to point into the "extra edges" chunk where the remaining
parents exist. This means we should be adding "num_parents - 1"
edges to this list, not "num_parents - 2". That is the basic error.
The reason this was not caught in the test suite is more subtle.
In 5324-split-commit-graph.sh, we test creating an octopus merge
and adding it to the tip of a commit-graph chain, then verify the
result. This _should_ have caught the problem, except that when
we load the commit-graph files we were overly careful to not fail
when the commit-graph chain does not match. This care was on
purpose to avoid race conditions as one process reads the chain
and another process modifies it. In such a case, the reading
process outputs the following message to stderr:
warning: commit-graph chain does not match
These warnings are output in the test suite, but ignored. By
checking the stderr of `git commit-graph verify` to include
the expected progress output, it will now catch this error.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit-graph: error out on invalid commit oids in 'write --stdin-commits'
While 'git commit-graph write --stdin-commits' expects commit object
ids as input, it accepts and silently skips over any invalid commit
object ids, and still exits with success:
# nonsense
$ echo not-a-commit-oid | git commit-graph write --stdin-commits
$ echo $?
0
# sometimes I forgot that refs are not good...
$ echo HEAD | git commit-graph write --stdin-commits
$ echo $?
0
# valid tree OID, but not a commit OID
$ git rev-parse HEAD^{tree} | git commit-graph write --stdin-commits
$ echo $?
0
$ ls -l .git/objects/info/commit-graph
ls: cannot access '.git/objects/info/commit-graph': No such file or directory
Check that all input records are indeed valid commit object ids and
return with error otherwise, the same way '--stdin-packs' handles
invalid input; see e103f7276f (commit-graph: return with errors during
write, 2019-06-12).
Note that it should only return with error when encountering an
invalid commit object id coming from standard input. However,
'--reachable' uses the same code path to process object ids pointed to
by all refs, and that includes tag object ids as well, which should
still be skipped over. Therefore add a new flag to 'enum
commit_graph_write_flags' and a corresponding field to 'struct
write_commit_graph_context', so we can differentiate between those two
cases.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"Can not" suggests one has the option to not do something, whereas
"cannot" more strongly suggests something is disallowed or impossible.
Noticed "can not", mistakenly used instead of "cannot" in git help
glossary, then ran git grep 'can not' and found many other instances.
Only files in the Documentation folder were modified.
'Can not' also occurs in some source code comments and some test
assertion messages, and there is an error message and translation "can
not move directory into itself" which I may fix and submit separately
from the documentation change.
Also noticed and fixed "is does" in git help fetch, but there are no
other occurrences of that typo according to git grep.
Signed-off-by: Mark Rushakoff <mark.rushakoff@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bits about .git/branches/* have been dropped from the series.
We may want to drop the support for it, but until that happens, the
tests should rely on the existence of the support to pass.
* js/visual-studio: (23 commits)
git: avoid calling aliased builtins via their dashed form
bin-wrappers: append `.exe` to target paths if necessary
.gitignore: ignore Visual Studio's temporary/generated files
.gitignore: touch up the entries regarding Visual Studio
vcxproj: also link-or-copy builtins
msvc: add a Makefile target to pre-generate the Visual Studio solution
contrib/buildsystems: add a backend for modern Visual Studio versions
contrib/buildsystems: handle options starting with a slash
contrib/buildsystems: also handle -lexpat
contrib/buildsystems: handle libiconv, too
contrib/buildsystems: handle the curl library option
contrib/buildsystems: error out on unknown option
contrib/buildsystems: optionally capture the dry-run in a file
contrib/buildsystems: redirect errors of the dry run into a log file
contrib/buildsystems: ignore gettext stuff
contrib/buildsystems: handle quoted spaces in filenames
contrib/buildsystems: fix misleading error message
contrib/buildsystems: ignore irrelevant files in Generators/
contrib/buildsystems: ignore invalidcontinue.obj
Vcproj.pm: urlencode '<' and '>' when generating VC projects
...
The recently added [includeif "onbranch:branch"] feature does not
work well with an early config mechanism, as it attempts to find
out what branch we are on before we even haven't located the git
repository. The inclusion during early config scan is ignored to
work around this issue.
* js/early-config-with-onbranch:
config: work around bug with includeif:onbranch and early config
`git restore --staged` uses the same machinery as `git checkout HEAD`,
so there should be a similar test case for "restore" as the existing
test case for "checkout" with deleted ita files.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: Varun Naik <vcnaik94@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is possible to delete a committed file from the index and then add it
as intent-to-add. After `git checkout HEAD <pathspec>`, the file should
be identical in the index and HEAD. The command already works correctly
if the file has contents in HEAD. This patch provides the desired
behavior even when the file is empty in HEAD.
`git checkout HEAD <pathspec>` calls tree.c:read_tree_1(), with fn
pointing to checkout.c:update_some(). update_some() creates a new cache
entry but discards it when its mode and oid match those of the old
entry. A cache entry for an ita file and a cache entry for an empty file
have the same oid. Therefore, an empty deleted ita file previously
passed both of these checks, and the new entry was discarded, so the
file remained unchanged in the index. After this fix, if the file is
marked as ita in the cache, then we avoid discarding the new entry and
add the new entry to the cache instead.
This change should not affect newly added ita files. For those, inside
tree.c:read_tree_1(), tree_entry_interesting() returns
entry_not_interesting, so fn is never called.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: Varun Naik <vcnaik94@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-refs: always refresh after taking the lock file
When a packed ref is deleted, the whole packed-refs file is
rewritten to omit the ref that no longer exists. However if another
gc command is running and calls `pack-refs --all` simultaneously,
there is a chance that a ref that was just updated lose the newly
created commits.
Through these steps, losing commits on newly updated refs can be
demonstrated:
# step 1: compile git without `USE_NSEC` option
Some kernel releases do enable it by default while some do
not. And if we compile git without `USE_NSEC`, it will be easier
demonstrated by the following steps.
# step 2: setup a repository and add the first commit
git init repo &&
(cd repo &&
git config core.logallrefupdates true &&
git commit --allow-empty -m foo)
# step 3: in one terminal, repack the refs repeatedly
cd repo &&
while true
do
git pack-refs --all
done
# step 4: in another terminal, simultaneously update the
# master with update-ref, and create and delete an
# unrelated ref also with update-ref
cd repo &&
while true
do
us=$(git commit-tree -m foo -p HEAD HEAD^{tree}) &&
git update-ref refs/heads/newbranch $us &&
git update-ref refs/heads/master $us &&
git update-ref -d refs/heads/newbranch &&
them=$(git rev-parse master) &&
if test "$them" != "$us"
then
echo >&2 "lost commit: $us"
exit 1
fi
# eye candy
printf .
done
Though we have the packed-refs lock file and loose refs lock
files to avoid updating conflicts, a ref will lost its newly
commits if racy stat-validity of `packed-refs` file happens
(which is quite same as the racy-git described in
`Documentation/technical/racy-git.txt`), the following
specific set of operations demonstrates the problem:
1. Call `pack-refs --all` to pack all the loose refs to
packed-refs, and let say the modify time of the
packed-refs is DATE_M.
2. Call `update-ref` to update a new commit to master while
it is already packed. the old value (let us call it
OID_A) remains in the packed-refs file and write the new
value (let us call it OID_B) to $GIT_DIR/refs/heads/master.
3. Call `update-ref -d` within the same DATE_M from the 1th
step to delete a different ref newbranch which is packed
in the packed-refs file. It check newbranch's oid from
packed-refs file without locking it.
Meanwhile it keeps a snapshot of the packed-refs file in
memory and record the file's attributes with the snapshot.
The oid of master in the packed-refs's snapshot is OID_A.
4. Call a new `pack-refs --all` to pack the loose refs, the
oid of master in packe-refs file is OID_B, and the loose
refs $GIT_DIR/refs/heads/master is removed. Let's say
the `pack-refs --all` is very quickly done and the new
packed-refs file's modify time is still DATE_M, and it
has the same file size, even the same inode.
5. 3th step now goes on after checking the newbranch, it
begin to rewrite the packed-refs file. After get the
lock file of packed-ref file, it checks it's on-disk
file attributes with the snapshot, suck as the timestamp,
the file size and the inode value. If they are both the
same values, and the snapshot is not refreshed.
Because the loose ref of master is removed by 4th step,
`update-ref -d` will updates the new packed-ref to disk
which contains master with the oid OID_A. So now the
newly commit OID_B of master is lost.
The best path forward is just always refreshing after take
the lock file of `packed-refs` file. Traditionally we avoided
that because refreshing it implied parsing the whole file.
But these days we mmap it, so it really is just an extra
open()/mmap() and a quick read of the header. That doesn't seem
like an outrageous cost to pay when we're already taking the lock.
Signed-off-by: Sun Chao <sunchao9@huawei.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Sun Chao <sunchao9@huawei.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t: warn against adding non-httpd-specific tests after sourcing 'lib-httpd'
We have a couple of test scripts that are not completely
httpd-specific, but do run a few httpd-specific tests at the end.
These test scripts source 'lib-httpd.sh' somewhere mid-script, which
then skips all the rest of the test script if the dependencies for
running httpd tests are not fulfilled.
As the previous two patches in this series show, already on two
occasions non-httpd-specific tests were appended at the end of such
test scripts, and, consequently, they were skipped as well when httpd
tests couldn't be run.
Add a comment at the end of these test scripts to warn against adding
non-httpd-specific tests at the end, in the hope that they will help
prevent similar issues in the future.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The make_traverse_path() function isn't very careful about checking its
output buffer boundaries. In fact, it doesn't even _know_ the size of
the buffer it's writing to, and just assumes that the caller used
traverse_path_len() correctly. And even then we assume that our
traverse_info.pathlen components are all correct, and just blindly write
into the buffer.
Let's improve this situation a bit:
- have the caller pass in their allocated buffer length, which we'll
check against our own computations
- check for integer underflow as we do our backwards-insertion of
pathnames into the buffer
- check that we do not run out items in our list to traverse before
we've filled the expected number of bytes
None of these should be triggerable in practice (especially since our
switch to size_t everywhere in a previous commit), but it doesn't hurt
to check our assumptions.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tree-walk: add a strbuf wrapper for make_traverse_path()
All but one of the callers of make_traverse_path() allocate a new heap
buffer to store the path. Let's give them an easy way to write to a
strbuf, which saves them from computing the length themselves (which is
especially tricky when they want to add to the path). It will also make
it easier for us to change the make_traverse_path() interface in a
future patch to improve its bounds-checking.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tree-walk: accept a raw length for traverse_path_len()
We take a "struct name_entry", but only care about the length of the
path name. Let's just take that length directly, making it easier to use
the function from callers that sometimes do not have a name_entry at
all.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We store and manipulate the cumulative traverse_info.pathlen as an
"int", which can overflow when we are fed ridiculously long pathnames
(e.g., ones at the edge of 2GB or 4GB, even if the individual tree entry
names are smaller than that). The results can be confusing, though
after some prodding I was not able to use this integer overflow to cause
an under-allocated buffer.
Let's consistently use size_t to generate and store these, and make
sure our addition doesn't overflow.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t5703: run all non-httpd-specific tests before sourcing 'lib-httpd.sh'
't5703-upload-pack-ref-in-want.sh' sources 'lib-httpd.sh' near the end
to run a couple of httpd-specific tests, but 'lib-httpd.sh' skips all
the rest of the test script if the dependencies for running httpd
tests are not fulfilled. However, the last six tests in 't5703' are
not httpd-specific, but they are skipped as well when httpd tests
can't be run.
Move these six tests earlier in the test script, before 'lib-httpd.sh'
is sourced, so they will be run even when httpd tests aren't. Note
that this is not merely a pure code movement, because the setup test
case for the httpd tests needed an additional 'rm -rf
"$LOCAL_PRISTINE"' to clean up a directory left behind by the moved
non-httpd-specific tests.
Also add a comment at the end of this test script to warn against
adding non-httpd-specific tests at the end, in the hope that it will
help prevent similar issues in the future.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t5510-fetch: run non-httpd-specific test before sourcing 'lib-httpd.sh'
't5510-fetch.sh' sources 'lib-httpd.sh' near the end to run a
httpd-specific test, but 'lib-httpd.sh' skips all the rest of the test
script if the dependencies for running httpd tests are not fulfilled.
Alas, recently cdbd70c437 (fetch: add --[no-]show-forced-updates
argument, 2019-06-18) appended a non-httpd-specific test at the end,
and this test is then skipped as well when httpd tests can't be run.
Move this new test earlier in the test script, before 'lib-httpd.sh'
is sourced, so it will be run even when httpd tests aren't.
Also add a comment at the end of this test script to warn against
adding non-httpd-specific tests at the end, in the hope that it will
help prevent similar issues in the future.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Squelch unneeded and misleading warnings from "repack" when the
command attempts to generate pack bitmaps without explicitly asked
for by the user.
* jk/repack-silence-auto-bitmap-warning:
repack: simplify handling of auto-bitmaps and .keep files
repack: silence warnings when auto-enabled bitmaps cannot be built
t7700: clean up .keep file in bitmap-writing test
Update to the tests to help SHA-256 transition continues.
* bc/hash-independent-tests-part-4:
t2203: avoid hard-coded object ID values
t1710: make hash independent
t1007: remove SHA1 prerequisites
t0090: make test pass with SHA-256
t0027: make hash size independent
t6030: make test work with SHA-256
t5000: make hash independent
t1450: make hash size independent
t1410: make hash size independent
t: add helper to convert object IDs to paths
RelNotes/2.23.0: fix a few typos and other minor issues
Fix the spelling of the new "--no-show-forced-updates" option that "git
fetch/pull" learned. Similarly, spell "--function-context" correctly and
fix a few typos, grammos and minor mistakes.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
It turns out that being cautious to warn against upcoming default
change was an unpopular behaviour, and such a care can easily be
defeated by distro packagers to render it ineffective anyway.
Just flip the default, with only a mention in the release notes.
config: work around bug with includeif:onbranch and early config
Since 07b2c0eacac (config: learn the "onbranch:" includeIf condition,
2019-06-05), there is a potential catch-22 in the early config path: if
the `include.onbranch:` feature is used, Git assumes that the Git
directory has been initialized already. However, in the early config
code path that is not true.
One way to trigger this is to call the following commands in any
repository:
git config includeif.onbranch:refs/heads/master.path broken
git help -a
The symptom triggered by the `git help -a` invocation reads like this:
BUG: refs.c:1851: attempting to get main_ref_store outside of repository
Let's work around this, simply by ignoring the `includeif.onbranch:`
setting when parsing the config when the ref store has not been
initialized (yet).
Technically, there is a way to solve this properly: teach the refs
machinery to initialize the ref_store from a given gitdir/commondir pair
(which we _do_ have in the early config code path), and then use that in
`include_by_branch()`. This, however, is a pretty involved project, and
we're already in the feature freeze for Git v2.23.0.
Note: when calling above-mentioned two commands _outside_ of any Git
worktree (passing the `--global` flag to `git config`, as there is
obviously no repository config available), at the point when
`include_by_branch()` is called, `the_repository` is `NULL`, therefore
we have to be extra careful not to dereference it in that case.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>