When formatting a patch series over `origin..HEAD`, one would expect
that range to be used as the current-series-range when computing a
range-diff between the previous and current versions of a patch series.
However, infer_range_diff_ranges() ignores `origin..HEAD` when
--range-diff=<prev> specifies a single revision rather than a range, and
instead unexpectedly computes the current-series-range based upon
<prev>. Address this anomaly by unconditionally using `origin..HEAD` as
the current-series-range regardless of <prev> as long as `origin` is
known, and only fall back to basing current-series-range on <prev> when
`origin` is not known.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To compute and show an interdiff, show_interdiff() needs only the two
OID's to compare and a diffopts, yet it expects callers to supply an
entire rev_info. The demand for rev_info is not only overkill, but also
places unnecessary burden on potential future callers which might not
otherwise have a rev_info at hand. Address this by tightening its
signature to require only the items it needs instead of a full rev_info.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
show_interdiff() is a relatively small function and not likely to grow
larger or more complicated. Rather than dedicating an entire source file
to it, relocate it to diff-lib.c which houses other "take two things and
compare them" functions meant to be re-used but not so low-level as to
reside in the core diff implementation.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have had parallel implementations of "add -i/-p" since 2.25 and
have been using them from various codepaths since 2.26 days, but
never made the built-in version the default.
We have found and fixed a handful of corner case bugs in the
built-in version, and it may be a good time to start switching over
the user base from the scripted version to the built-in version.
Let's enable the built-in version for those who opt into the
feature.experimental guinea-pig program to give wider exposure.
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
asan reports that the C version of `add -p` is not freeing all the
memory it allocates. Fix this by introducing a function to clear
`struct add_p_state` and use it instead of freeing individual members.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
expat.lib -> libexpat.lib (libexpatd.lib for debug build).
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our color tests of "git add -p" do something a bit different from how a
normal user would behave: we pretend there's a pager in use, so that Git
thinks it's OK to write color to a non-tty stdout. This comes from
8539b46534 (t3701: avoid depending on the TTY prerequisite, 2019-12-06),
which allows us to avoid a lot of complicated mock-tty code.
However, those environment variables also make their way down to
sub-processes of add--interactive, including the "diff-files" we run to
generate the patches. As a result, it thinks it should output color,
too. So in t3701.50, for example, the machine-readable version of the
diff we get unexpectedly has color in it. We fail to parse it as a diff
and think there are zero hunks.
The test does still pass, though, because even with zero hunks we'll
dump the diff header (and we consider those unparseable bits to be part
of the header!), and so the output still has the expected color codes in
it. We don't notice that the command was totally broken and failed to
apply anything.
And in fact we're not really testing what we think we are about the
color, either. While add--interactive does correctly show the version we
got from running "diff-files --color", we'd also pass the test if we had
accidentally shown the machine-readable version, too, since it
(erroneously) has color codes in it.
One could argue that the test isn't very realistic; it's setting up this
"pretend there's a pager" situation to get around the tty restrictions
of the test environment. So one option would be to move back towards
using a real tty. But the behavior of add--interactive really is
user-visible here. If a user, for whatever reason, did run "git
--paginate add --patch" (perhaps because their pager is really a filter
or something), the command would totally fail to do anything useful.
Since we know that we don't want color in this output, let's just make
add--interactive more defensive, and say "--no-color" explicitly. It
doesn't hurt anything in the common case, but it fixes this odd case and
lets our test function properly again.
Note that the C builtin run_add_p() already passes --no-color, so it
doesn't need a similar fix. That will eventually replace this perl code
anyway, but the test change here will be valuable for ensuring that.
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After applying hunks to a file with "add -p", the C patch_update_file()
function tries to refresh the index (just like the perl version does).
We can only refresh the index if we're able to read it in, so we first
check the return value of repo_read_index(). But unlike many functions,
where "0" is success, that function is documented to return the number
of entries in the index. Hence we should be checking for success with a
non-negative return value.
Neither the tests nor any users seem to have noticed this, probably due
to a combination of:
- this affects only the C version, which is not yet the default
- following it up with any porcelain command like "git diff" or "git
commit" would refresh the index automatically.
But you can see the problem by running the plumbing "git diff-files"
immediately after "add -p" stages all hunks. Running the new test with
GIT_TEST_ADD_I_USE_BUILTIN=1 fails without the matching code change.
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
By default, `git worktree add` creates a new worktree associated with a
particular branch (which may have been created automatically if not
specified explicitly on the command-line). It is also convenient to
create throwaway worktrees not associated with any branch, which can be
handy when making experimental changes or doing testing. However, the
latter use-case may not be obvious to newcomers since the high-level
description of worktrees talks only about checking out "more than one
branch at a time". Therefore, enhance the description to to discuss both
use-cases.
A secondary goal of highlighting the distinction between branch-based
and throwaway worktrees is to help newcomers understand that the
simplest form `git worktree add <path>` automatically creates a new
branch. Stating this early in the description, may help newcomers avoid
creating branches without realizing they are doing so, and later
wondering why `git branch --list` shows branches the user did not
intentionally create.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Like `git switch` and `git checkout`, `git worktree add` can check out a
branch or set up a detached HEAD. However, unlike those other commands,
`git worktree add` does not understand -d as shorthand for --detach,
which may confound users accustomed to using -d for this purpose.
Address this shortcoming by teaching `add` to recognize -d for --detach,
thus bringing it in line with the other commands.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`git checkout` learned -d as short option for --detach in 163e3b2975
(switch: add short option for --detach, 2019-03-29) but the
documentation was never updated to reflect the change.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The argv argument of collect_changed_submodules() contains only object ids
(the objects references of all the refs).
Notify setup_revisions() that the input is not filenames by passing
assume_dashdash, so it can avoid redundant stat for each ref.
Also suppress refname_ambiguity flag to avoid filesystem lookups for
each object. Similar logic can be found in cat-file, pack-objects and more.
This change reduces the time for git fetch in my repo from 25s to 6s.
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call hashwrite_be32() instead of open-coding it. This is shorter and
easier to read.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Emit byte-sized values using hashwrite_u8() instead of buffering them
locally first. The hashwrite functions already do their own buffering,
so this double-buffering does not reduce the number of system calls.
Getting rid of it shortens and simplifies the code a bit.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call write_pack_header() to hash and write a pack header instead of
open-coding this function. This gets rid of duplicate code and of the
magic version number 2 -- which has been used here since c90be46abd
(Changed fast-import's pack header creation to use pack.h, 2006-08-16)
and in pack.h (again) since 29f049a0c2 (Revert "move pack creation to
version 3", 2006-10-14).
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a function for building a refspec using printf-style formatting. It
frees callers from managing their own buffer. Use it throughout the
tree to shorten and simplify its callers.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
map_refspec() either returns the passed in ref string or a detached
strbuf. This makes it hard for callers to release the possibly
allocated memory, and set_refspecs() consequently leaks it.
Let map_refspec() append any refspecs directly and release its own
strbufs after use. Rename it to refspec_append_mapped() and don't
return anything to reflect its increased responsibility.
set_refspecs() also leaks its strbufs. Do the same here and directly
call refspec_append() in each if branch instead of holding onto a
detached strbuf, then dispose of the allocated memory after use. We
need to add an else branch for the final call because all the other
conditional branches already add their formatted refspec now.
setup_push_upstream() and setup_push_current() forgot to release their
strbufs as well; plug these leaks, too, while at it.
None of these leaks were likely to impact users, because the number
and sizes of refspecs are usually small and the allocations are only
done once per program run. Clean them up nevertheless, as another
step on the long road towards zero memory leaks.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
44c7e1a7e0 (mem-pool: use more standard initialization and finalization,
2020-08-15) moved the allocation of the mem-pool structure to callers.
It also added an allocation to load_cache_entries_threaded(), but for an
unrelated mem-pool. Fix that by allocating the correct one instead --
the one that is initialized two lines later.
Reported-by: Sandor Bodo-Merle <sbodomerle@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Tools based on LibClang [1] can make use of a 'JSON Compilation
Database' [2] that keeps track of the exact options used to compile a set
of source files.
For example, clangd [3], which is a C language server protocol
implementation, can use a JSON compilation database to determine the
flags needed to compile a file so it can provide proper editor
integration. As a result, editors supporting the language server
protocol (such as VS Code, Emacs, or Vim, with suitable plugins) can
provide better searching, integration, and refactoring tools.
The Clang compiler can generate JSON fragments when compiling [4],
using the `-MJ` flag. These JSON fragments (one per compiled source
file) can then be concatenated to create the compilation database,
commonly called 'compile_commands.json'.
Add support to the Makefile for generating these JSON fragments as well
as the compilation database itself, if the environment variable
'GENERATE_COMPILATION_DATABASE' is set.
If this variable is set, check that $(CC) indeed supports the `-MJ`
flag, following what is done for automatic dependencies.
All JSON fragments are placed in the 'compile_commands/' directory, and
the compilation database 'compile_commands.json' is generated as a
dependency of the 'all' target using a `sed` invocation.
[1] https://clang.llvm.org/docs/Tooling.html
[2] https://clang.llvm.org/docs/JSONCompilationDatabase.html
[3] https://clangd.llvm.org/
[4] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mj-arg
Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Get rid of needless check of 'parents' for NULL. The NULL case
is already handled right above, and 'parents' is dereferenced
without check below anyway.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Handle first_parent_only by breaking from generic loop early
rather than by duplicating (part of) the loop body.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When fetching recursively with submodules, for each ref in the
superproject, we call check_for_new_submodule_commits() which collects all
the objects that have to be checked for submodule changes on
calculate_changed_submodule_paths(). On the first call, it also collects all
the existing refs for excluding them from the scan.
calculate_changed_submodule_paths() creates an argument array with all the
collected new objects, followed by --not and all the old objects. This argv
is passed to setup_revisions, which parses each argument, converts it back
to an oid and resolves the object. The parsing itself also does redundant
work, because it is treated like user input, while in fact it is a full
oid. So it needlessly attempts to look it up as ref (checks if it has ^, ~
etc.), checks if it is a file name etc.
For a repository with many refs, all of this is expensive. But if the fetch
in the superproject did not update the ref (i.e. the objects that are
required to exist in the submodule did not change), there is no need to
include it in the list.
Before commit be76c212 (fetch: ensure submodule objects fetched,
2018-12-06), submodule reference changes were only detected for refs that
were changed, but not for new refs. This commit covered also this case, but
what it did was to just include every ref.
This change should reduce the number of scanned refs by about half (except
the case of a no-op fetch, which will not scan any ref), because all the
existing refs will still be listed after --not.
The regression was reported here:
https://public-inbox.org/git/CAGHpTBKSUJzFSWc=uznSu2zB33qCSmKXM-
iAjxRCpqNK5bnhRg@mail.gmail.com/
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It was possible for xrealloc() to send a non-NULL pointer that has
been freed, which has been fixed.
* jk/xrealloc-avoid-use-after-free:
xrealloc: do not reuse pointer freed by zero-length realloc()
"git diff --stat -w" showed 0-line changes for paths whose changes
were only whitespaces, which was not intuitive. We now omit such
paths from the stat output.
* mr/diff-hide-stat-wo-textual-change:
diff: teach --stat to ignore uninteresting modifications
Updates to on-demand fetching code in lazily cloned repositories.
* jt/lazy-fetch:
fetch: no FETCH_HEAD display if --no-write-fetch-head
fetch-pack: remove no_dependents code
promisor-remote: lazy-fetch objects in subprocess
fetch-pack: do not lazy-fetch during ref iteration
fetch: only populate existing_refs if needed
fetch: avoid reading submodule config until needed
fetch: allow refspecs specified through stdin
negotiator/noop: add noop fetch negotiator
A handful of places in in-tree code still relied on being able to
execute the git subcommands, especially built-ins, in "git-foo"
form, which have been corrected.
* jc/undash-in-tree-git-callers:
credential-cache: use child_process.args
cvsexportcommit: do not run git programs in dashed form
transport-helper: do not run git-remote-ext etc. in dashed form
Trim an unused binary and turn a bunch of commands into built-in.
* jk/slimmed-down:
drop vcs-svn experiment
make git-fast-import a builtin
make git-bugreport a builtin
make credential helpers builtins
Makefile: drop builtins from MSVC pdb list
Bugfix for "git fetch" when the packfile URI capability is in use.
* jt/fetch-pack-loosen-validation-with-packfile-uri:
fetch-pack: make packfile URIs work with transfer.fsckobjects
fetch-pack: document only_packfile in get_pack()
(various): document from_promisor parameter
Test clean-up.
* ss/t7401-modernize:
t7401: add a NEEDSWORK
t7401: change indentation for enhanced readability
t7401: change syntax of test_i18ncmp calls for clarity
t7401: use 'short' instead of 'verify' and cut in rev-parse calls
t7401: modernize style
"git rebase -i" learns a bit more options.
* pw/rebase-i-more-options:
t3436: do not run git-merge-recursive in dashed form
rebase: add --reset-author-date
rebase -i: support --ignore-date
rebase -i: support --committer-date-is-author-date
am: stop exporting GIT_COMMITTER_DATE
rebase -i: add --ignore-whitespace flag
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a user checks out the upstream branch of HEAD, the upstream branch
not being a local branch, and then runs "git status", like this:
git clone $URL client
cd client
git checkout @{u}
git status
no status is printed, but instead an error message:
fatal: HEAD does not point to a branch
(This error message when running "git branch" persists even after
checking out other things - it only stops after checking out a branch.)
This is because "git status" reads the reflog when determining the "HEAD
detached" message, and thus attempts to DWIM "@{u}", but that doesn't
work because HEAD no longer points to a branch.
Therefore, when calculating the status of a worktree, tolerate dangling
marks. This is done by adding an additional parameter to
dwim_ref() and repo_dwim_ref().
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes it clear that dwim_ref() is just repo_dwim_ref() without the
first parameter.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for a future patch adding a boolean parameter to
repo_interpret_branch_name(), which might be easily confused with an
existing unsigned int parameter, refactor repo_interpret_branch_name()
to take an option struct instead of the unsigned int parameter.
The static function interpret_branch_mark() is also updated to take the
option struct in preparation for that future patch, since it will also
make use of the to-be-introduced boolean parameter.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
887952b8c6 ("fetch: optionally allow disabling FETCH_HEAD update",
2020-08-18) introduced the ability to disable writing to FETCH_HEAD
during fetch, but did not suppress the "<source> -> FETCH_HEAD" message
when this ability is used. This message is misleading in this case,
because FETCH_HEAD is not written. Also, because "fetch" is used to
lazy-fetch missing objects in a partial clone, this significantly
clutters up the output in that case since the objects to be fetched are
potentially numerous.
Therefore, suppress this message when --no-write-fetch-head is passed
(but not when --dry-run is set).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the common case where users have _not_ pushed a `ci-config` branch to
configure which branches should be included in the GitHub workflow runs,
there is a big fat ugly annotation about a failure in the run's log:
X Check failure on line 1 in .github
@github-actions github-actions / ci-config
.github#L1
Process completed with exit code 128.
The reason is that the `ci-config` job tries to clone that `ci-config`
branch, and even if it is configured to continue on error, the
annotation is displayed, and it is distracting.
Let's just handle this on the shell script level, so that the job's step
is not marked as a failure.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The section added in e76eec3554 (ci: allow per-branch config for
GitHub Actions, 2020-05-07) contains a `&&`-chain that connects several
commands. The first command is actually so long that it stretches over
multiple lines, and as per usual, the continuation lines are indented one
more level than the first.
However, the subsequent commands in the `&&`-chain were also indented
one more level than the first command, which was almost certainly
unintended.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch fixes a bug where xrealloc(ptr, 0) can double-free and
corrupt the heap on some platforms (including at least glibc).
The C99 standard says of malloc (section 7.20.3):
If the size of the space requested is zero, the behavior is
implementation-defined: either a null pointer is returned, or the
behavior is as if the size were some nonzero value, except that the
returned pointer shall not be used to access an object.
So we might get NULL back, or we might get an actual pointer (but we're
not allowed to look at its contents). To simplify our code, our
xmalloc() handles a NULL return by converting it into a single-byte
allocation. That way callers get consistent behavior. This was done way
back in 4e7a2eccc2 (?alloc: do not return NULL when asked for zero
bytes, 2005-12-29).
We also gave xcalloc() and xrealloc() the same treatment. And according
to C99, that is fine; the text above is in a paragraph that applies to
all three. But what happens to the memory we passed to realloc() in such
a case? I.e., if we do:
ret = realloc(ptr, 0);
and "ptr" is non-NULL, but we get NULL back, is "ptr" still valid? C99
doesn't cover this case specifically, but says (section 7.20.3.4):
The realloc function deallocates the old object pointed to by ptr and
returns a pointer to a new object that has the size specified by size.
So "ptr" is now deallocated, and we must only look at "ret". And since
"ret" is NULL, that means we have no allocated object at all. But that's
not quite the whole story. It also says:
If memory for the new object cannot be allocated, the old object is
not deallocated and its value is unchanged.
[...]
The realloc function returns a pointer to the new object (which may
have the same value as a pointer to the old object), or a null pointer
if the new object could not be allocated.
So if we see a NULL return with a non-zero size, we can expect that the
original object _is_ still valid. But with a non-zero size, it's
ambiguous. The NULL return might mean a failure (in which case the
object is valid), or it might mean that we successfully allocated
nothing, and used NULL to represent that.
The glibc manpage for realloc() explicitly says:
[...]if size is equal to zero, and ptr is not NULL, then the call is
equivalent to free(ptr).
Likewise, this StackOverflow answer:
https://stackoverflow.com/a/2135302
claims that C89 gave similar guidance (but I don't have a copy to verify
it). A comment on this answer:
https://stackoverflow.com/a/2022410
claims that Microsoft's CRT behaves the same.
But our current "retry with 1 byte" code passes the original pointer
again. So on glibc, we effectively free() the pointer and then try to
realloc() it again, which is undefined behavior.
The simplest fix here is to just pass "ret" (which we know to be NULL)
to the follow-up realloc(). But that means that a system which _doesn't_
free the original pointer would leak it. It's not clear if any such
systems exist, and that interpretation of the standard seems unlikely
(I'd expect a system that doesn't deallocate to simply return the
original pointer in this case). But it's easy enough to err on the safe
side, and just never pass a zero size to realloc() at all.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In fde97d8ac6 (Update documentation to remove incorrect GIT_DIFF_OPTS
example., 2006-11-27), the description of the 'GIT_EXTERNAL_DIFF'
variable was moved from 'diff-format.txt' to 'git.txt', and the
documentation was updated to remove a 'diff(1)' invocation since Git did
not use an external diff program anymore by default.
However, the description of 'GIT_EXTERNAL_DIFF' still mentions "instead
of the diff invocation described above", which is confusing.
Correct that outdated sentence.
Also, link to git(1) in 'diff-generate-patch.txt' when GIT_DIFF_OPTS and
GIT_EXTERNAL_DIFF are mentioned, so that users can easily know what
these variables are about.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
