This trick was performed by rebasing the builtin-stash-rebase branch
thicket via `git rebase -kir v2.19.0-rc1`, replacing all branches that
made it into `pu` by their current versions (and also the builtin-stash
by the newest iteration as of ungps/git), and then calling these
commands:
# save current tip
tip=$(git rev-parse HEAD)
# revert previous merge
git reset --hard git-for-windows/master^0
git revert -n -m 1 HEAD
git commit --squash HEAD -s -m "Let's drop this"
# now perform the 3-way merge with v2.19.0-rc1 as base
git merge-recursive v2.19.0-rc1 -- HEAD $tip
git merge --ff-only \
$(git commit-tree -p HEAD -p $tip -m "Merge" \
$(git write-tree))
git commit -c HEAD^^ --amend -s
The merge-recursive dance is necessary because of the merging-rebases:
the fake merges with which these start are mistaken by `git merge` to
mean that the branches were already merged, when the fake merges undid
the corresponding changes.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Let's drop this branch thicket during the next merging rebase, in
preparation for merging a newer branch thicket with builtin-stash-v8 and
builtin-rebase-i-v7.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This branch adds back the scripted versions, then adds the option to
use the builtin versions of `stash` and `rebase` by setting
`stash.useBuiltin=true` and `rebase.useBuiltin=true`, respectively,
(the latter already worked for the top-level `git rebase` command and
the `--am` backend, and now it also works for the interactive backend).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
A couple of fixes that should be squashed during the next merging
rebase of Git for Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The upcoming Git for Windows v2.19.0 wants to ship with the builtin
versions of stash, rebase and rebase -i. The reason: these are just *so
much faster*: t3400 and t3404 run about 60-70 percent faster, and t3903
even more than 80% faster.
However, these are still all pretty fresh, still being reviewed and
iterated on the Git mailing list.
So let's try to give users a way to test these (or to boldly use them
for their mission-critical tasks, as this here developer plans on
doing), but stay with the safe option by default: use the scripted
versions (which might be slow, but they are well tested).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We recently converted the `git stash` command from Unix shell scripts
to builtins.
Just like we have `rebase.useBuiltin` to fall back to the scripted
rebase, to give end users a way out when they discover a bug in the
builtin command, this commit adds support for `stash.useBuiltin`.
This is necessary because Git for Windows wants to ship the builtin
stash earlier than core Git: Git for Windows v2.19.0 will come with
the option of a drastically faster (if a lot less battle-tested)
`git stash`.
As the file name `git-stash` is already in use, let's rename the
scripted backend to `git-legacy-stash`.
To make the test suite pass with `stash.useBuiltin=false`, this commit
also backports rudimentary support for `-q` (but only *just* enough
to appease the test suite), and adds a super-ugly hack to force exit
code 129 for `git stash -h`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This simply copies the version as of v2.19.0-rc0 verbatim. As of now,
it is not hooked up.
The next commit will change the builtin `stash` to hand off to the
scripted `git stash` when `stash.useBuiltin=false`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We recently converted both the `git rebase` and the `git rebase -i`
command from Unix shell scripts to builtins.
The former has a safety valve allowing to fall back to the scripted
`rebase`, just in case that there is a bug in the builtin `rebase`:
setting the config variable `rebase.useBuiltin` to `false` will
fall back to using the scripted version.
The latter did not have such a safety hatch.
Let's reinstate the scripted interactive rebase backend so that `rebase.useBuiltin=false` will not use the builtin interactive rebase,
just in case that an end user runs into a bug with the builtin version
and needs to get out of the fix really quickly.
This is necessary because Git for Windows wants to ship the builtin
rebase/interactive rebase earlier than core Git: Git for Windows
v2.19.0 will come with the option of a drastically faster (if a lot
less battle-tested) `git rebase`/`git rebase -i`.
As the file name `git-rebase--interactive` is already in use, let's
rename the scripted backend to `git-legacy-rebase--interactive`.
A couple of additional touch-ups are needed (such as teaching the
builtin `rebase--interactive`, which assumed the role of the
`rebase--helper`, to perform the two tricks to skip the unnecessary
picks and to generate a new todo list) to make things work again.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This patch teaches the builtin rebase to avoid the scripted --am backend
and call `git format-patch` and `git am` directly.
Meaning: apart from the --merge and the --preserve-merges backends, `git
rebase` is now implemented in pure C, with no need to ask the Unix shell
interpreter for help.
This brings us really close to a fully builtin `git rebase`: the
--preserve-merges mode is about to be deprecated (as soon as the
--rebase-merges mode has proven stable and robust enough), and there are
plans to scrap the `git-rebase--merge` backend in favor of teaching the
interactive rebase enough tricks to run the --merge mode, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This *would* be a fixup commit, except that we want to avoid rewriting
commits that we merged from upstream's `pu` branch. Instead, we want to
send a new iteration, and then re-merge the new iteration once it made
it into the `pu` branch.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This simply copies the version as of v2.19.0-rc0 verbatim. As of now,
it is not hooked up (because it needs a couple more changes to work);
The next commit will use the scripted interactive rebase backend from
`git rebase` again when `rebase.useBuiltin=false`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This final patch flips the switch and makes the builtin rebase the
default. The old, Unix shell scripted version can still be called via
git -c rebase.useBuiltin=false rebase [...]
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This branch first merges the builtin interactive rebase, and then
teaches the builtin rebase to hand off interactive rebases to the
builtin backend correctly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This fifth batch of builtin rebase patches concludes the conversion: the
builtin rebase is now feature-complete.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This wave of built rebase patches implements the remaining rebase
options in the builtin rebase.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This set of patches implements the actions (such as --continue, --skip,
etc) in the builtin rebase.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is the first batch of the patches that turn `git rebase` into
a builtin.
This not only helps performance on Windows, but *especially* makes
things more robust, as no MSYS2 Bash will be required to run this
command any longer.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This merges the builtin stash.
Upstream Git did not integrate it into any stable integration branch
yet, but the performance improvements are substantial enough,
especially on Windows, that we really, really, really want to have it
early.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
While the scripted `git rebase` still has to rely on the
`git-rebase--am.sh` script to implement the glue between the `rebase`
and the `am` commands, we can go a more direct route in the builtin
rebase and avoid using a shell script altogether.
This reduces the chances of Git for Windows running into trouble due to
problems with the POSIX emulation layer (known as "MSYS2 runtime",
itself a derivative of the Cygwin runtime): when no shell script is
called, the POSIX emulation layer is avoided altogether.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Change "fetch" to treat "+" in refspecs (aka --force) to mean we
should clobber a local tag of the same name.
This changes the long-standing behavior of "fetch" added in
853a3697dc ("[PATCH] Multi-head fetch.", 2005-08-20). Before this
change, all tag fetches effectively had --force enabled. See the
git-fetch-script code in fast_forward_local() with the comment:
> Tags need not be pointing at commits so there is no way to
> guarantee "fast-forward" anyway.
That commit and the rest of the history of "fetch" shows that the
"+" (--force) part of refpecs was only conceived for branch updates,
while tags have accepted any changes from upstream unconditionally and
clobbered the local tag object. Changing this behavior has been
discussed as early as 2011[1].
The current behavior doesn't make sense to me, it easily results in
local tags accidentally being clobbered. We could namespace our tags
per-remote and not locally populate refs/tags/*, but as with my
97716d217c ("fetch: add a --prune-tags option and fetch.pruneTags
config", 2018-02-09) it's easier to work around the current
implementation than to fix the root cause.
So this change implements suggestion #1 from Jeff's 2011 E-Mail[1],
"fetch" now only clobbers the tag if either "+" is provided as part of
the refspec, or if "--force" is provided on the command-line.
This also makes it nicely symmetrical with how "tag" itself works when
creating tags. I.e. we refuse to clobber any existing tags unless
"--force" is supplied. Now we can refuse all such clobbering, whether
it would happen by clobbering a local tag with "tag", or by fetching
it from the remote with "fetch".
Ref updates outside refs/{tags,heads/* are still still not symmetrical
with how "git push" works, as discussed in the recently changed
pull-fetch-param.txt documentation. This change brings the two
divergent behaviors more into line with one another. I don't think
there's any reason "fetch" couldn't fully converge with the behavior
used by "push", but that's a topic for another change.
One of the tests added in 31b808a032 ("clone --single: limit the fetch
refspec to fetched branch", 2012-09-20) is being changed to use
--force where a clone would clobber a tag. This changes nothing about
the existing behavior of the test.
1. https://public-inbox.org/git/20111123221658.GA22313@sigill.intra.peff.net/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refer to the new git-push(1) documentation about when ref updates are
and aren't allowed with and without --force, noting how "git-fetch"
differs from the behavior of "git-push".
Perhaps it would be better to split this all out into a new
gitrefspecs(7) man page, or present this information using tables.
In lieu of that, this is accurate, and fixes a big omission in the
existing refspec docs.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's complex rules governing whether a push is allowed to take
place depending on whether we're pushing to refs/heads/*, refs/tags/*
or refs/not-that/*. See is_branch() in refs.c, and the various
assertions in refs/files-backend.c. (e.g. "trying to write non-commit
object %s to branch '%s'").
This documentation has never been quite correct, but went downhill
after dbfeddb12e ("push: require force for refs under refs/tags/",
2012-11-29) when we started claiming that <dst> couldn't be a tag
object, which is incorrect. After some of the logic in that patch was
changed in 256b9d70a4 ("push: fix "refs/tags/ hierarchy cannot be
updated without --force"", 2013-01-16) the docs weren't updated, and
we've had some version of documentation that confused whether <src>
was a tag or not with whether <dst> would accept either an annotated
tag object or the commit it points to.
This makes the intro somewhat more verbose & complex, perhaps we
should have a shorter description here and split the full complexity
into a dedicated section. Very few users will find themselves needing
to e.g. push blobs or trees to refs/custom-namespace/* (or blobs or
trees at all), and that could be covered separately as an advanced
topic.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This change will be followed-up with a subsequent change where I'll
change both sides of this mention of "tag <tag>" to be something
that's best read without interruption.
To make that change smaller, let's move this mention of "tag <tag>" to
the end of the "<refspec>..." section, it's now somewhere in the
middle.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Saying that "git push <remote> <src>:<dst>" won't push a merger of
<src> and <dst> to <dst> is clear from the rest of the context here,
so mentioning it is redundant, furthermore the mention of "EXAMPLES
below" isn't specific or useful.
This phrase was originally added in 149f6ddfb3 ("Docs: Expand
explanation of the use of + in git push refspecs.", 2009-02-19), as
can be seen in that change the point of the example being cited was to
show that force pushing can leave unreferenced commits on the
remote. It's enough that we explain that in its own section, it
doesn't need to be mentioned here.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test suite only incidentally (and unintentionally) tested for the
current behavior of eager tag clobbering on "fetch". This is a
followup to 380efb65df ("push tests: assert re-pushing annotated
tags", 2018-07-31) which tests for it explicitly.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The quoted -m'msg' option would mean the same as -mmsg when passed
through the test_force_push_tag helper. Let's instead use a string
with spaces in it, to have a working example in case we need to pass
other whitespace-delimited arguments to git-tag.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix up a logic error in 380efb65df ("push tests: assert re-pushing
annotated tags", 2018-07-31), where the $tag_type_description variable
was assigned to but never used, unlike in the subsequently added
companion test for fetches in 2d216a7ef6 ("fetch tests: add a test for
clobbering tag behavior", 2018-04-29).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The -h output has been referring to the --force command as forcing the
overwriting of local branches, but since "fetch" more generally
fetches all sorts of references in all refs/ namespaces, let's talk
about forcing the update of a a "reference" instead.
This wording was initially introduced in 8320199873 ("Rewrite
builtin-fetch option parsing to use parse_options().", 2007-12-04).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
OLD_ICONV has long been needed by FreeBSD so config.mak.uname defines
it unconditionally. However, recent versions do not need it, and its
presence results in compilation warnings. Resolve this issue by defining
OLD_ICONV only for older FreeBSD versions.
Specifically, revision r281550[1], which is part of FreeBSD 11, removed
the need for OLD_ICONV, and r282275[2] back-ported that change to 10.2.
Versions prior to 10.2 do need it.
[1] b0813ee288
[2] b709ec868a
[es: commit message; tweak version check to distinguish 10.x versions]
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
doc-diff creates a temporary working tree (git-worktree) and generates a
bunch of temporary files which it does not remove since they act as a
cache to speed up subsequent runs. Although doc-diff's working tree and
generated files are not strictly build products of the Makefile (which,
itself, never runs doc-diff), as a convenience, update "make clean" to
clean up doc-diff's working tree and generated files along with other
development detritus normally removed by "make clean".
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As part of its operation, doc-diff creates a bunch of temporary
working files and holds onto them in order to speed up subsequent
invocations. These files are never deleted. Moreover, it creates a
temporary working tree (via git-wortkree) which likewise never gets
removed.
Without knowing the implementation details of the tool, a user may not
know how to clean up manually afterward. Worse, the user may find it
surprising and alarming to discover a working tree which s/he did not
create explicitly.
To address these issues, add a --clean mode which removes the
temporary working tree and deletes all generated files.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
doc-diff invokes 'man' with the -l option to force "local" mode,
however, neither MacOS nor FreeBSD recognize this option. On those
platforms, if the argument to 'man' contains a slash, it is
automatically interpreted as a file specification, so a "local"-like
mode is not needed. And, it turns out, 'man' which does support -l
falls back to enabling -l automatically if it can't otherwise find a
manual entry corresponding to the argument. Since doc-diff always
passes an absolute path of the nroff source file to 'man', the -l
option kicks in anyhow, despite not being specified explicitly.
Therefore, make the invocation portable to the various platforms by
simply dropping -l.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commits introduces a optimization by avoiding calling the
same functions again. For example, `git stash push -u`
would call at some points the following functions:
* `check_changes()` (inside `do_push_stash()`)
* `do_create_stash()`, which calls: `check_changes()` and
`get_untracked_files()`
Note that `check_changes()` also calls `get_untracked_files()`.
So, `check_changes()` is called 2 times and `get_untracked_files()`
3 times.
`get_untracked_files()` has now only two parameters and it will
fill a global strbuf called `untracked_files`.
The old function `check_changes()` now consists of two functions:
`get_untracked_files()` and `check_changes_tracked_files()`.
These are the call chains for `push` and `create`:
* `push_stash()` -> `do_push_stash()` -> `do_create_stash()`
* `create_stash()` -> `do_create_stash()`
To prevent calling the same functions over and over again,
`check_changes()` inside `do_create_stash()` is now placed
in the caller functions (`create_stash()` and `do_push_stash()`).
This way `check_changes()` and `get_untracked files()` are called
only one time.
https://public-inbox.org/git/20180818223329.GJ11326@hank.intra.tgummerer.com/
Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
The old shell script `git-stash.sh` was removed and replaced
entirely by `builtin/stash.c`. In order to do that, `create` and
`push` were adapted to work without `stash.sh`. For example, before
this commit, `git stash create` called `git stash--helper create
--message "$*"`. If it called `git stash--helper create "$@"`, then
some of these changes wouldn't have been necessary.
This commit also removes the word `helper` since now stash is
called directly and not by a shell script.
Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
The previous commit switched "-l" to meaning "--list", but a
few vestiges of its prior meaning as "--create-reflog"
remained:
- the synopsis mentioned "-l" when creating a new branch;
we can drop this entirely, as it has been the default
for years
- the --list command mentions the unfortunate "-l"
confusion, but we've now fixed that
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The exact byte count of the delta base file is important.
The test-delta helper will feed it to patch_delta(), which
will barf if it doesn't match the size byte given in the
delta. Using "echo" may end up with unexpected line endings
on some platforms (e.g,. "\r\n" instead of just "\n").
This actually wouldn't cause the test to fail (since we
already expect test-delta to complain about these bogus
deltas), but would mean that we're not exercising the code
we think we are.
Let's use printf instead (which we already trust to give us
byte-perfect output when we generate the deltas).
While we're here, let's tighten the 5-byte result size used
in the "truncated copy parameters" test. This just needs to
have enough room to attempt to parse the bogus copy command,
meaning 2 is sufficient. Using 5 was arbitrary and just
copied from the base size; since those no longer match, it's
simply confusing. Let's use a more meaningful number.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 3afc679b "commit: use generations in paint_down_to_common()",
the queue in paint_down_to_common() was changed to use a priority
order based on generation number before commit date. This served
two purposes:
1. When generation numbers are present, the walk guarantees
correct topological relationships, regardless of clock skew in
commit dates.
2. It enables short-circuiting the walk when the min_generation
parameter is added in d7c1ec3e "commit: add short-circuit to
paint_down_to_common()". This short-circuit helps commands
like 'git branch --contains' from needing to walk to a merge
base when we know the result is false.
The commit message for 3afc679b includes the following sentence:
This change does not affect the number of commits that are
walked during the execution of paint_down_to_common(), only
the order that those commits are inspected.
This statement is incorrect. Because it changes the order in which
the commits are inspected, it changes the order they are added to
the queue, and hence can change the number of loops before the
queue_has_nonstale() method returns true.
This change makes a concrete difference depending on the topology
of the commit graph. For instance, computing the merge-base between
consecutive versions of the Linux kernel has no effect for versions
after v4.9, but 'git merge-base v4.8 v4.9' presents a performance
regression:
v2.18.0: 0.122s
v2.19.0-rc1: 0.547s
HEAD: 0.127s
To determine that this was simply an ordering issue, I inserted
a counter within the while loop of paint_down_to_common() and
found that the loop runs 167,468 times in v2.18.0 and 635,579
times in v2.19.0-rc1.
The topology of this case can be described in a simplified way
here:
v4.9
| \
| \
v4.8 \
| \ \
| \ |
... A B
| / /
| / /
|/__/
C
Here, the "..." means "a very long line of commits". By generation
number, A and B have generation one more than C. However, A and B
have commit date higher than most of the commits reachable from
v4.8. When the walk reaches v4.8, we realize that it has PARENT1
and PARENT2 flags, so everything it can reach is marked as STALE,
including A. B has only the PARENT1 flag, so is not STALE.
When paint_down_to_common() is run using
compare_commits_by_commit_date, A and B are removed from the queue
early and C is inserted into the queue. At this point, C and the
rest of the queue entries are marked as STALE. The loop then
terminates.
When paint_down_to_common() is run using
compare_commits_by_gen_then_commit_date, B is removed from the
queue only after the many commits reachable from v4.8 are explored.
This causes the loop to run longer. The reason for this regression
is simple: the queue order is intended to not explore a commit
until everything that _could_ reach that commit is explored. From
the information gathered by the original ordering, we have no
guarantee that there is not a commit D reachable from v4.8 that
can also reach B. We gained absolute correctness in exchange for
a performance regression.
The performance regression is probably the worse option, since
these incorrect results in paint_down_to_common() are rare. The
topology required for the performance regression are less rare,
but still require multiple merge commits where the parents differ
greatly in generation number. In our example above, the commit A
is as important as the commit B to demonstrate the problem, since
otherwise the commit C will sit in the queue as non-stale just as
long in both orders.
The solution provided uses the min_generation parameter to decide
if we should use generation numbers in our ordering. When
min_generation is equal to zero, it means that the caller has no
known cutoff for the walk, so we should rely on our commit-date
heuristic as before; this is the case with merge_bases_many().
When min_generation is non-zero, then the caller knows a valuable
cutoff for the short-circuit mechanism; this is the case with
remove_redundant() and in_merge_bases_many().
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we see a delta command instructing us to copy bytes
from the base, we have to read the offset and size from the
delta stream. We do this without checking whether we're at
the end of the stream, meaning we may read past the end of
the buffer.
In practice this isn't exploitable in any interesting way
because:
1. Deltas are always in packfiles, so we have at least a
20-byte trailer that we'll end up reading.
2. The worst case is that we try to perform a nonsense
copy from the base object into the result, based on
whatever was in the pack stream next. In most cases
this will simply fail due to our bounds-checks against
the base or the result.
But even if you carefully constructed a pack stream for
which it succeeds, it wouldn't perform any delta
operation that you couldn't have simply included in a
non-broken form.
But obviously it's poor form to read past the end of the
buffer we've been given. Unfortunately there's no easy way
to do a single length check, since the number of bytes we
need depends on the number of bits set in the initial
command byte. So we'll just check each byte as we parse. We
can hide the complexity in a macro; it's ugly, but not as
ugly as writing out each individual conditional.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When applying a delta, if we see an opcode that cannot be
fulfilled (e.g., asking to write more bytes than the
destination has left), we break out of our parsing loop but
don't signal an explicit error. We rely on the sanity check
after the loop to see if we have leftover delta bytes or
didn't fill our result buffer.
This can silently ignore corruption when the delta buffer
ends with a bogus command and the destination buffer is
already full. Instead, let's jump into the error handler
directly when we see this case.
Note that the tests also cover the "bad opcode" case, which
already handles this correctly.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If `cmd` is in the range [0x01,0x7f] and `cmd > top-data`, the
`memcpy(out, data, cmd)` can copy out-of-bounds data from after `delta_buf`
into `dst_buf`.
This is not an exploitable bug because triggering the bug increments the
`data` pointer beyond `top`, causing the `data != top` sanity check after
the loop to trigger and discard the destination buffer - which means that
the result of the out-of-bounds read is never used for anything.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't have any tests that specifically check boundary
cases in patch_delta(). It obviously gets exercised by tests
which read from packfiles, but it's hard to create packfiles
with bogus deltas.
So let's cover some obvious boundary cases:
1. commands that overflow the result buffer
a. literal content from the delta
b. copies from a base
2. commands where the source isn't large enough
a. literal content from a truncated delta
b. copies that need more bytes than the base has
3. copy commands who parameters are truncated
And indeed, we have problems with both 2a and 3. I've marked
these both as expect_failure, though note that because they
involve reading past the end of a buffer, they will
typically only be caught when run under valgrind or ASan.
There's one more test here, too, which just applies a basic
delta. Since all of the other tests expect failure and we
don't otherwise use "test-tool delta" in the test suite,
this gives a sanity check that the tool works at all.
These are based on an earlier patch by Jann Horn
<jannh@google.com>.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We currently read the input to test-delta by mmap()-ing it.
However, memory-checking tools like valgrind and ASan are
less able to detect reads/writes past the end of an mmap'd
buffer, because the OS is likely to give us extra bytes to
pad out the final page size. So instead, let's read into a
heap buffer.
As a bonus, this also makes it possible to write tests with
empty bases, as mmap() will complain about a zero-length
map.
This is based on a patch by Jann Horn <jannh@google.com>
which actually aligned the data at the end of a page, and
followed it with another page marked with mprotect(). That
would detect problems even without a tool like ASan, but it
was significantly more complex and may have introduced
portability problems. By comparison, this approach pushes
the complexity onto existing memory-checking tools.
Note that this could be done even more simply by using
strbuf_read_file(), but that would defeat the purpose:
strbufs generally overallocate (and at the very least
include a trailing NUL which we do not care about), which
would defeat most memory checkers.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The doc-diff script immediately resolves its two endpoints
to actual object ids, so that we can reuse cached results
even if they appear under a different name. But we still use
the original name the user fed us when running "git
checkout" in our temporary worktree. This can lead to
confusing results:
- the namespace inside the worktree is different than the
one outside. In particular, "./doc-diff origin HEAD"
will resolve HEAD inside the worktree, whose detached
HEAD will be pointing at origin! As a result, such a
diff would always be empty.
- worse, we will store this result under the oid we got by
resolving HEAD in the main worktree, thus polluting our
cache
- we didn't pass --detach, which meant that using a branch
name would cause us to actually check out that branch,
making it unavailable to other worktrees.
We can solve this by feeding the already-resolved object id
to git-checkout. That naturally forces a detached HEAD, but
just to make clear our expectation, let's explicitly pass
--detach.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add stash save to the helper and delete functions which are no
longer needed (`show_help()`, `save_stash()`, `push_stash()`,
`create_stash()`, `clear_stash()`, `untracked_files()` and
`no_changes()`).
The `-m` option is no longer supported as it might not make
sense to have two ways of passing a message. Even if this is
a change in behaviour, the documentation remains the same
because the `-m` parameter was omitted before.
Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
There is a change in behaviour with this commit. When there was
no initial commit, the shell version of stash would still display
a message. This commit makes `push` to not display any message if
`--quiet` or `-q` is specified.
Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
For cleanliness, "git worktree prune" deletes the .git/worktrees
directory if it is empty after pruning is complete.
For consistency, make "git worktree remove <path>" likewise delete
.git/worktrees if it is empty after the removal.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For consistency with "add -f -f" and "move -f -f" which override
the lock on a worktree, allow "remove -f -f" to do so, as well, as a
convenience.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
