When the option spec contains no switches or only hidden switches,
parse_options will emit an extra blank line at the end of help output so
that the help text will end in two blank lines instead of one.
When parse_options produces internal help output after an error has
occurred it will emit blank lines within the usage string to stdout
instead of stderr.
Update t/helper/test-parse-options.c to have a description body in the
usage string to exercise this second bug and mark tests as failing in
t0040.
Add tests to t1502 to demonstrate both of these problems.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge-strategies: avoid implying that "-s theirs" exists
The description of `-Xours` merge option has a parenthetical note
that tells the readers that it is very different from `-s ours`,
which is correct, but the description of `-Xtheirs` that follows it
carelessly says "this is the opposite of `ours`", giving a false
impression that the readers also need to be warned that it is very
different from `-s theirs`, which in reality does not even exist.
Documentation/config: clarify the meaning of submodule.<name>.update
With more commands (that potentially change a submodule) paying attention
to submodules as well as the recent discussion[1] on
submodule.<name>.update, let's spell out that submodule.<name>.update
is strictly to be used for configuring the "submodule update" command
and not to be obeyed by other commands.
These other commands usually have a strict meaning of what they should
do (i.e. checkout, reset, rebase, merge) as well as have their name
overlapping with the modes possible for submodule.<name>.update.
[1] https://public-inbox.org/git/4283F0B0-BC1C-4ED1-8126-7E512D84484B@gmail.com/
submodule.<name>.update was set to "none", triggering unexpected
behavior as the submodule was thought to never be touched.
However a newer version of Git taught 'git pull --rebase' to also
populate and rebase submodules if they were active.
The newer options such as submodule.active and command specific
flags would not have triggered unexpected behavior.
Reported-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Decode =XY in quoted-printable segments only if X and Y are hexadecimal
digits, otherwise just copy them. That's at least better than
interpreting negative results from hexval() as a character.
Reported-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs: make sha1 output parameter of refs_resolve_ref_unsafe() optional
Allow callers of refs_resolve_ref_unsafe() to pass NULL if they don't
need the resolved hash value. We already allow the same for the flags
parameter. This new leniency is inherited by the various wrappers like
resolve_ref_unsafe().
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The git-read-tree manpage has a table that is meant to
be shown with its spacing exactly as it is in the source. We
mark it as a "literal paragraph" by indenting each line by
at least one space. This renders OK with asciidoc for both
the HTML and manpage versions.
But there are two problems when we render it with
asciidoctor.
The first is that some lines mix tabs and spaces. Even if
asciidoctor is correctly configured for 8-space tabs, it
seems to handle this case differently, soaking up some of
the initial literal-paragraph spaces and mis-aligning the
table text.
The second problem is that the table uses blank lines to
group rows. But as blank lines separate paragraphs in
asciidoc, this actually means that each chunk of the table
is rendered in its own pre-formatted <div> block. This
happens even with vanilla asciidoc, but there's no visible
result because the literal paragraphs aren't styled in any
special way. But with asciidoctor (or at least the styles
used on git-scm.com), literal paragraphs are styled with a
different background. This breaks the table into a visually
distracting sequence of chunks.
We can fix both by adding a literal-paragraph block
delimiter. That turns the whole table into a single block
(for both implementations) and causes asciidoctor to render
the indentation as it is in the source.
Reported-at: https://github.com/git/git-scm.com/issues/1023 Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap[-write]: use `object_array_clear()`, don't leak
Instead of setting the fields of rev->pending to 0/NULL, thereby leaking
memory, call `object_array_clear(&rev->pending)`.
In pack-bitmap.c, we make copies of those fields as `pending_nr` and
`pending_e`. We never update the aliases and the original fields never
change, so the aliases are not really needed and just make it harder
than necessary to understand the code. While we're here, remove the
aliases to make the code easier to follow.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a couple of places, we pop objects off an object array `foo` by
decreasing `foo.nr`. We access `foo.nr` in many places, but most if not
all other times we do so read-only, e.g., as we iterate over the array.
But when we change `foo.nr` behind the array's back, it feels a bit
nasty and looks like it might leak memory.
Leaks happen if the popped element has an allocated `name` or `path`.
At the moment, that is not the case. Still, 1) the object array might
gain more fields that want to be freed, 2) a code path where we pop
might start using names or paths, 3) one of these code paths might be
copied to somewhere where we do, and 4) using a dedicated function for
popping is conceptually cleaner.
Introduce and use `object_array_pop()` instead. Release memory in the
new function. Document that popping an object leaves the associated
elements in limbo.
The converted places were identified by grepping for "\.nr\>" and
looking for "--".
Make the new function return NULL on an empty array. This is consistent
with `pop_commit()` and allows the following:
while ((o = object_array_pop(&foo)) != NULL) {
// do something
}
But as noted above, we don't need to go out of our way to avoid reading
`foo.nr`. This is probably more readable:
while (foo.nr) {
... o = object_array_pop(&foo);
// do something
}
The name of `object_array_pop()` does not quite align with
`add_object_array()`. That is unfortunate. On the other hand, it matches
`object_array_clear()`. Arguably it's `add_...` that is the odd one out,
since it reads like it's used to "add" an "object array". For that
reason, side with `object_array_clear()`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
object_array: use `object_array_clear()`, not `free()`
Instead of freeing `foo.objects` for an object array `foo` (sometimes
conditionally), call `object_array_clear(&foo)`. This means we don't
poke as much into the implementation, which is already a good thing, but
also that we release the individual entries as well, thereby fixing at
least one memory-leak (in diff-lib.c).
If someone is holding on to a pointer to an element's `name` or `path`,
that is now a dangling pointer, i.e., we'd be turning an unpleasant
situation into an outright bug. To the best of my understanding no such
long-term pointers are being taken.
The way we handle `study` in builting/reflog.c still looks like it might
leak. That will be addressed in the next commit.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
leak_pending: use `object_array_clear()`, not `free()`
Setting `leak_pending = 1` tells `prepare_revision_walk()` not to
release the `pending` array, and makes that the caller's responsibility.
See 4a43d374f (revision: add leak_pending flag, 2011-10-01) and 353f5657a (bisect: use leak_pending flag, 2011-10-01).
Commit 1da1e07c8 (clean up name allocation in prepare_revision_walk,
2014-10-15) fixed a memory leak in `prepare_revision_walk()` by
switching from `free()` to `object_array_clear()`. However, where we use
the `leak_pending`-mechanism, we're still only calling `free()`.
Use `object_array_clear()` instead. Copy some helpful comments from 353f5657a to the other callers that we update to clarify the memory
responsibilities, and to highlight that the commits are not affected
when we clear the array -- it is indeed correct to both tidy up the
commit flags and clear the object array.
Document `leak_pending` in revision.h to help future users get this
right.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/commit: fix memory leak in `prepare_index()`
Release `pathspec` and the string list `partial`.
When we clear the string list, make sure we do not free the `util`
pointers. That would result in double-freeing, since we set them up as
`item->util = item` in `list_paths()`.
Initialize the string list early, so that we can always release it. That
introduces some unnecessary overhead in various code paths, but means
there is one and only one way out of the function. If we ever accumulate
more things we need to free, it should be straightforward to do so.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating a new branch B by copying the branch A that happens to
be the current branch, it also updates HEAD to point at the new
branch. It probably was made this way because "git branch -c A B"
piggybacked its implementation on "git branch -m A B",
This does not match the usual expectation. If I were sitting on a
blue chair, and somebody comes and repaints it to red, I would
accept ending up sitting on a chair that is now red (I am also OK to
stand, instead, as there no longer is my favourite blue chair). But
if somebody creates a new red chair, modelling it after the blue
chair I am sitting on, I do not expect to be booted off of the blue
chair and ending up on sitting on the new red one.
Let's fix this before it hits 'next'. Those who want to create a
new branch and switch to it can do "git checkout B" after doing a
"git branch -c B", and if that operation is so useful and deserves a
short-hand way to do so, perhaps extend "git checkout -b B" to copy
configurations while creating the new branch B.
This allows us to recreate even historical tags which would now be consider
invalid, such as v2.6.12-rc2..v2.6.13-rc3 in the Linux kernel source tree which
lack the `tagger` header.
With "--state-branch=<branchname>" option, the mapping from old object names
and filtered ones in ./map/ directory is stashed away in the object database,
and the one from the previous run is read to populate the ./map/ directory,
allowing for incremental updates of large trees.
Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is pure code motion to enable a subsequent patch to add code which needs
to happen with the reset $GIT_* but before the temporary directory has been
cleaned up.
Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
consistently use "fallthrough" comments in switches
Gcc 7 adds -Wimplicit-fallthrough, which can warn when a
switch case falls through to the next case. The general idea
is that the compiler can't tell if this was intentional or
not, so you should annotate any intentional fall-throughs as
such, leaving it to complain about any unannotated ones.
There's a GNU __attribute__ which can be used for
annotation, but of course we'd have to #ifdef it away on
non-gcc compilers. Gcc will also recognize
specially-formatted comments, which matches our current
practice. Let's extend that practice to all of the
unannotated sites (which I did look over and verify that
they were behaving as intended).
Ideally in each case we'd actually give some reasons in the
comment about why we're falling through, or what we're
falling through to. And gcc does support that with
-Wimplicit-fallthrough=2, which relaxes the comment pattern
matching to anything that contains "fallthrough" (or a
variety of spelling variants). However, this isn't the
default for -Wimplicit-fallthrough, nor for -Wextra. In the
name of simplicity, it's probably better for us to support
the default level, which requires "fallthrough" to be the
only thing in the comment (modulo some window dressing like
"else" and some punctuation; see the gcc manual for the
complete set of patterns).
This patch suppresses all warnings due to
-Wimplicit-fallthrough. We might eventually want to add that
to the DEVELOPER Makefile knob, but we should probably wait
until gcc 7 is more widely adopted (since earlier versions
will complain about the unknown warning type).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our trace handler is called by curl with a curl_infotype
variable to interpret its data field. For most types we
print the data and then break out of the switch. But for
CURLINFO_TEXT, we print data and then fall through to the
"default" case, which does the exact same thing (nothing!)
that breaking out of the switch would.
This is probably a leftover from an early iteration of the
patch where the code after the switch _did_ do something
interesting that was unique to the non-text case arms.
But in its current form, this fallthrough is merely
confusing (and causes gcc's -Wimplicit-fallthrough to
complain).
Let's make CURLINFO_TEXT like the other case arms, and push
the default arm to the end where it's more obviously a
catch-all.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The handle_command() function matches an incoming command
string with a sequence of starts_with() checks. But it also
surrounds these with a switch on the first character of the
command, which lets us jump to the right block of
starts_with() without going linearly through the list.
However, each case arm of the switch falls through to the
one below it. This is pointless (we know that a command
starting with 'b' does not need to check any of the commands
in the 'c' block), and it makes gcc's -Wimplicit-fallthrough
complain.
We could solve this by adding a break at the end of each
block. However, this optimization isn't helping anything.
Even if it does make matching faster (which is debatable),
this is code that is run only in the test suite, and each
run receives at most two of these "commands". We should
favor simplicity and readability over micro-optimizing.
Instead, let's drop the switch statement completely and
replace it with an if/else cascade.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit dc944b65f1 (get_sha1_with_context: dynamically
allocate oc->path, 2017-05-19) changed the rules that
callers must follow for seeing if we parsed a path in the
object name. The rules switched from "check if the oc.path
buffer is empty" to "check if the oc.path pointer is NULL".
But that commit forgot to update some sites in
cat_one_file(), meaning we might dereference a NULL pointer.
You can see this by making a path-aware request like
--textconv without specifying --path, and giving an object
name that doesn't have a path in it. Like:
git cat-file --textconv HEAD
which will reliably segfault.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/githooks: mention merge in commit-msg hook
The commit-msg hook is invoked by both commit and merge now.
Reported-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
09f5e97 ("travis-ci: skip a branch build if equal tag is present",
2017-09-17) introduced the "skip_branch_tip_with_tag" function with
a broken string comparison. Fix it!
Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pathspec doc: parse_pathspec does not maintain references to args
The command line arguments passed to main() are valid for the life of
a program, but the same is not true for all other argv-style arrays
(e.g. when a caller creates an argv_array). Clarify that
parse_pathspec does not rely on the argv passed to it to remain valid.
This makes it easier to tell that callers like "git rev-list --stdin"
are safe and ensures that that is more likely to remain true as the
implementation of parse_pathspec evolves.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many of our programs consider that it is OK to release dynamic
storage that is used throughout the life of the program by simply
exiting, but this makes it harder to leak detection tools to avoid
reporting false positives. Plug many existing leaks and introduce
a mechanism for developers to mark that the region of memory
pointed by a pointer is not lost/leaking to help these tools.
* jk/leak-checkers:
git-compat-util: make UNLEAK less error-prone
When run with the "-C" option, fast-export writes 'C' commands in its
output whenever the internal diff mechanism detects a file copy,
indicating that fast-import should copy the given existing file to the
given new filename. However, the diff mechanism works against the
prior version of the file, whereas fast-import uses whatever is current.
This causes issues when a commit both modifies a file and uses it as the
source for a copy.
Therefore, teach fast-export to refrain from writing 'C' when it has
already written a modification command for a file.
An existing test in t9350-fast-export is also fixed in this patch. The
existing line "C file6 file7" copies the wrong version of file6, but it
has coincidentally worked because file7 was subsequently overridden.
Reported-by: Juraj Oršulić <juraj.orsulic@fer.hr> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
revision: replace "struct cmdline_pathspec" with argv_array
We assemble an array of strings in a custom struct,
NULL-terminate the result, and then pass it to
parse_pathspec().
But then we never free the array or the individual strings
(nor can we do the latter, as they are heap-allocated when
they come from stdin but not when they come from the
passed-in argv).
Let's swap this out for an argv_array. It does the same
thing with fewer lines of code, and it's safe to call
argv_array_clear() at the end to avoid a memory leak.
Reported-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some implementations of `echo` support the '-e' option to enable
backslash interpretation of the following string.
As an addition, they support '-E' to turn it off.
However, none of these are portable, POSIX doesn't even mention them,
and many implementations don't support them.
A check for '-n' is already done in check-non-portable-shell.pl,
extend it to cover '-n', '-e' or '-E'.
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 0e5bba5 ("add UNLEAK annotation for reducing leak false
positives", 2017-09-08) introduced an UNLEAK macro to be used as
"UNLEAK(var);", but its existing definitions leave semicolons that act
as empty statements, which will lead to syntax errors, e.g.
if (condition)
UNLEAK(var);
else
something_else(var);
would be broken with two statements between if (condition) and else.
Lose the excess semicolon from the end of the macro replacement text.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Even though this probably works almost everywhere, it is undefined
behavior, and it could plausibly cause highly-optimizing compilers to
misbehave. C99 section 6.5.6 paragraph 8 explains:
If both the pointer operand and the result point to elements
of the same array object, or one past the last element of the
array object, the evaluation shall not produce an overflow;
otherwise, the behavior is undefined.
and (6.3.2.3.3) a null pointer does not point to anything.
Guard the loop with a NULL check to make the intent crystal clear to
even the most pedantic compiler. A suitably clever compiler could let
the NULL check only run in the first iteration, but regardless, this
overhead is likely to be dwarfed by the work to be done on each item.
This problem was noticed by Coverity.
[jn: using a NULL check instead of a placeholder empty list;
fleshed out the commit message based on mailing list discussion]
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
describe: teach --match to handle branches and remotes
When `git describe` uses `--match`, it matches only tags, basically
ignoring the `--all` argument even when it is specified.
Fix it by also matching branch name and $remote_name/$remote_branch_name,
for remote-tracking references, with the specified patterns. Update
documentation accordingly and add tests.
Signed-off-by: Max Kirillov <max@max630.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we fail to open $GIT_DIR/info/alternates, we silently
assume there are no alternates. This is the right thing to
do for ENOENT, but not for other errors.
A hard error is probably overkill here. If we fail to read
an alternates file then either we'll complete our operation
anyway, or we'll fail to find some needed object. Either
way, a warning is good idea. And we already have a helper
function to handle this pattern; let's just call
warn_on_fopen_error().
Note that technically the errno from strbuf_read_file()
might be from a read() error, not open(). But since read()
would never return ENOENT or ENOTDIR, and since it produces
a generic "unable to access" error, it's suitable for
handling errors from either.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch fixes a regression in v2.11.1 where we might read
past the end of an mmap'd buffer. It was introduced in cf3c635210.
The link_alt_odb_entries() function has always taken a
ptr/len pair as input. Until cf3c635210 (alternates: accept
double-quoted paths, 2016-12-12), we made a copy of those
bytes in a string. But after that commit, we switched to
parsing the input left-to-right, and we ignore "len"
totally, instead reading until we hit a NUL.
This has mostly gone unnoticed for a few reasons:
1. All but one caller passes a NUL-terminated string, with
"len" pointing to the NUL.
2. The remaining caller, read_info_alternates(), passes in
an mmap'd file. Unless the file is an exact multiple of
the page size, it will generally be followed by NUL
padding to the end of the page, which just works.
The easiest way to demonstrate the problem is to build with:
make SANITIZE=address NO_MMAP=Nope test
Any test which involves $GIT_DIR/info/alternates will fail,
as the mmap emulation (correctly) does not add an extra NUL,
and ASAN complains about reading past the end of the buffer.
One solution would be to teach link_alt_odb_entries() to
respect "len". But it's actually a bit tricky, since we
depend on unquote_c_style() under the hood, and it has no
ptr/len variant.
We could also just make a NUL-terminated copy of the input
bytes and operate on that. But since all but one caller
already is passing a string, instead let's just fix that
caller to provide NUL-terminated input in the first place,
by swapping out mmap for strbuf_read_file().
There's no advantage to using mmap on the alternates file.
It's not expected to be large (and anyway, we're copying its
contents into an in-memory linked list). Nor is using
git_open() buying us anything here, since we don't keep the
descriptor open for a long period of time.
Let's also drop the "len" parameter entirely from
link_alt_odb_entries(), since it's completely ignored. That
will avoid any new callers re-introducing a similar bug.
Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t9010-*.sh: skip all tests if the PIPE prereq is missing
Every test in this file, except one, is marked with the PIPE prereq.
However, that lone test ('set up svn repo'), only performs some setup
work and checks whether the following test should be executed (by
setting an additional SVNREPO prerequisite). Since the following test
also requires the PIPE prerequisite, performing the setup test, when the
PIPE preequisite is missing, is simply wasted effort. Use the skip-all
test facility to skip all tests when the PIPE prerequisite is missing.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
test-lib: use more compact expression in PIPE prerequisite
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Improve the performance of the directory listing logic when it wants to list
non-empty ignored directories. In order to show non-empty ignored directories,
the existing logic will recursively iterate through all contents of an ignored
directory. This change introduces the optimization to stop iterating through
the contents once it finds the first file. This can have a significant
improvement in 'git status --ignored' performance in repositories with a large
number of files in ignored directories.
For an example of the performance difference on an example repository with
196,000 files in 400 ignored directories:
| Command | Time (s) |
| -------------------------- | --------- |
| git status | 1.2 |
| git status --ignored (old) | 3.9 |
| git status --ignored (new) | 1.4 |
Signed-off-by: Jameson Miller <jamill@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running `git fetch --unshallow` on a repo that is not in fact shallow
produces a fatal error message. Add a helper to rev-parse that scripters
can use to determine whether a repo is shallow or not.
Signed-off-by: Øystein Walle <oystwa@gmail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-rebase: don't ignore unexpected command line arguments
Currently, git-rebase will silently ignore any unexpected command-line
switches and arguments (the command-line produced by git rev-parse).
This allowed the rev-parse bug, fixed in the preceding commits, to go
unnoticed. Let's make sure that doesn't happen again. We shouldn't be
ignoring unexpected arguments. Let's not.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rev-parse parseopt: interpret any whitespace as start of help text
Currently, rev-parse only interprets a space ' ' character as the
delimiter between the option spec and the help text. So if a tab
character is placed between the option spec and the help text, it will
be interpreted as part of the long option name or as part of the arg
hint. If it is interpreted as part of the long option name, then
rev-parse will produce what will be interpreted as multiple arguments
on the command line.
For example, the following option spec (note: there is a <tab> between
"frotz" and "enable"):
frotz enable frotzing
will produce the following set expression when --frotz is used:
set -- --frotz --
instead of this:
set -- --frotz enable --
Mark t1502.2 as fixed.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rev-parse parseopt: do not search help text for flag chars
When searching for flag characters in the option spec, we should ensure
the search stays within the bounds of the option spec and does not enter
the help text portion of the spec. So when we find the boundary white
space marking the start of the help text, let's mark it with a nul
character. Then when we search for flag characters starting from the
beginning of the string we'll stop at the nul and won't enter the help
text.
Now, the following option spec:
exclame this does something!
will produce this 'set' expression when --exclame is specified:
set -- --exclame --
instead of this one:
set -- --exclame this does something --
Mark t1502.4 and t1502.5 as fixed.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since commit 2d893df rev-parse will scan forward from the beginning of
the option string looking for a flag character. If there are no flag
characters then the scan will spill over into the help text and will
interpret the characters preceding the "flag" as part of the option-spec
i.e. the long option name.
For example, the following option spec:
exclame this does something!
will produce this 'set' expression when --exclame is specified:
set -- --exclame this does something --
which will be interpreted as four separate parameters by the shell. And
will produce a help string that looks like:
--exclame this does something
this does something!
git-rebase.sh has such an option (--autosquash), and so will add extra
parameters to the 'set' expression when --autosquash is used.
git-rebase continues to work correctly though because when it parses the
arguments, it ignores ones that it does not recognize.
Also, rev-parse --parseopt does not currently interpret a tab character
as a delimiter between the option spec and the help text. If a tab is
used at the end of the option spec, before the help text, and before a
space has been specified, then rev-parse will interpret the tab as part
of the preceding component (either the long name or the arg hint).
For example, the following option spec (note: there is a <tab> between
"frotz" and "enable"):
frotz enable frotzing
will produce this 'set' expression when --frotz is specified:
set -- --frotz enable --
which will be interpreted as 2 separate arguments by the shell.
git-rebase.sh has one of these too (--keep-empty). In this case the tab
is immediately followed by spaces so there are no additional parameters
produced on the command line. The only side-effect is misalignment in
the help text.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Unlike "git commit-tree < file", "git commit-tree -F file" did not
pass the contents of the file verbatim and instead completed an
incomplete line at the end, if exists. The latter has been updated
to match the behaviour of the former.
* rk/commit-tree-make-F-verbatim:
commit-tree: do not complete line in -F input
* rs/strbuf-leakfix: (34 commits)
wt-status: release strbuf after use in wt_longstatus_print_tracking()
wt-status: release strbuf after use in read_rebase_todolist()
vcs-svn: release strbuf after use in end_revision()
utf8: release strbuf on error return in strbuf_utf8_replace()
userdiff: release strbuf after use in userdiff_get_textconv()
transport-helper: release strbuf after use in process_connect_service()
sequencer: release strbuf after use in save_head()
shortlog: release strbuf after use in insert_one_record()
sha1_file: release strbuf on error return in index_path()
send-pack: release strbuf on error return in send_pack()
remote: release strbuf after use in set_url()
remote: release strbuf after use in migrate_file()
remote: release strbuf after use in read_remote_branches()
refs: release strbuf on error return in write_pseudoref()
notes: release strbuf after use in notes_copy_from_stdin()
merge: release strbuf after use in write_merge_heads()
merge: release strbuf after use in save_state()
mailinfo: release strbuf on error return in handle_boundary()
mailinfo: release strbuf after use in handle_from()
help: release strbuf on error return in exec_woman_emacs()
...
Implement transactional update to the packed-ref representation of
references.
* mh/packed-ref-transactions:
files_transaction_finish(): delete reflogs before references
packed-backend: rip out some now-unused code
files_ref_store: use a transaction to update packed refs
t1404: demonstrate two problems with reference transactions
files_initial_transaction_commit(): use a transaction for packed refs
prune_refs(): also free the linked list
files_pack_refs(): use a reference transaction to write packed refs
packed_delete_refs(): implement method
packed_ref_store: implement reference transactions
struct ref_transaction: add a place for backends to store data
packed-backend: don't adjust the reference count on lock/unlock
* kw/merge-recursive-cleanup:
merge-recursive: change current file dir string_lists to hashmap
merge-recursive: remove return value from get_files_dirs
merge-recursive: fix memory leak
As "git commit" to conclude a conflicted "git merge" honors the
commit-msg hook, "git merge" that recoreds a merge commit that
cleanly auto-merges should, but it didn't.
* sb/merge-commit-msg-hook:
builtin/merge: honor commit-msg hook for merges
Many of our programs consider that it is OK to release dynamic
storage that is used throughout the life of the program by simply
exiting, but this makes it harder to leak detection tools to avoid
reporting false positives. Plug many existing leaks and introduce
a mechanism for developers to mark that the region of memory
pointed by a pointer is not lost/leaking to help these tools.
* jk/leak-checkers:
add UNLEAK annotation for reducing leak false positives
set_git_dir: handle feeding gitdir to itself
repository: free fields before overwriting them
reset: free allocated tree buffers
reset: make tree counting less confusing
config: plug user_config leak
update-index: fix cache entry leak in add_one_file()
add: free leaked pathspec after add_files_to_cache()
test-lib: set LSAN_OPTIONS to abort by default
test-lib: --valgrind should not override --verbose-log
Our hashmap implementation in hashmap.[ch] is not thread-safe when
adding a new item needs to expand the hashtable by rehashing; add
an API to disable the automatic rehashing to work it around.
* jh/hashmap-disable-counting:
hashmap: add API to disable item counting when threaded
The long-standing rule that an in-core lockfile instance, once it
is used, must not be freed, has been lifted and the lockfile and
tempfile APIs have been updated to reduce the chance of programming
errors.
* jk/incore-lockfile-removal:
stop leaking lock structs in some simple cases
ref_lock: stop leaking lock_files
lockfile: update lifetime requirements in documentation
tempfile: auto-allocate tempfiles on heap
tempfile: remove deactivated list entries
tempfile: use list.h for linked list
tempfile: release deactivated strbufs instead of resetting
tempfile: robustify cleanup handler
tempfile: factor out deactivation
tempfile: factor out activation
tempfile: replace die("BUG") with BUG()
tempfile: handle NULL tempfile pointers gracefully
tempfile: prefer is_tempfile_active to bare access
lockfile: do not rollback lock on failed close
tempfile: do not delete tempfile on failed close
always check return value of close_tempfile
verify_signed_buffer: prefer close_tempfile() to close()
setup_temporary_shallow: move tempfile struct into function
setup_temporary_shallow: avoid using inactive tempfile
write_index_as_tree: cleanup tempfile on error
"git gc" and friends when multiple worktrees are used off of a
single repository did not consider the index and per-worktree refs
of other worktrees as the root for reachability traversal, making
objects that are in use only in other worktrees to be subject to
garbage collection.
* nd/prune-in-worktree:
refs.c: reindent get_submodule_ref_store()
refs.c: remove fallback-to-main-store code get_submodule_ref_store()
rev-list: expose and document --single-worktree
revision.c: --reflog add HEAD reflog from all worktrees
files-backend: make reflog iterator go through per-worktree reflog
revision.c: --all adds HEAD from all worktrees
refs: remove dead for_each_*_submodule()
refs.c: move for_each_remote_ref_submodule() to submodule.c
revision.c: use refs_for_each*() instead of for_each_*_submodule()
refs: add refs_head_ref()
refs: move submodule slash stripping code to get_submodule_ref_store
refs.c: refactor get_submodule_ref_store(), share common free block
revision.c: --indexed-objects add objects from all worktrees
revision.c: refactor add_index_objects_to_pending()
refs.c: use is_dir_sep() in resolve_gitlink_ref()
revision.h: new flag in struct rev_info wrt. worktree-related refs
* ma/split-symref-update-fix:
refs/files-backend: add `refname`, not "HEAD", to list
refs/files-backend: correct return value in lock_ref_for_update
refs/files-backend: fix memory leak in lock_ref_for_update
refs/files-backend: add longer-scoped copy of string to list
* mh/notes-cleanup:
load_subtree(): check that `prefix_len` is in the expected range
load_subtree(): declare some variables to be `size_t`
hex_to_bytes(): simpler replacement for `get_oid_hex_segment()`
get_oid_hex_segment(): don't pad the rest of `oid`
load_subtree(): combine some common code
get_oid_hex_segment(): return 0 on success
load_subtree(): only consider blobs to be potential notes
load_subtree(): check earlier whether an internal node is a tree entry
load_subtree(): separate logic for internal vs. terminal entries
load_subtree(): fix incorrect comment
load_subtree(): reduce the scope of some local variables
load_subtree(): remove unnecessary conditional
notes: make GET_NIBBLE macro more robust
gc: call fscanf() with %<len>s, not %<len>c, when reading hostname
Earlier in this codepath, we (ab)used "%<len>c" to read the hostname
recorded in the lockfile into locking_host[HOST_NAME_MAX + 1] while
substituting <len> with the actual value of HOST_NAME_MAX.
This turns out to be incorrect, as it is an instruction to read
exactly the specified number of bytes. Because we are trying to
read at most that many bytes, we should be using "%<len>s" instead.
Helped-by: A. Wilcox <awilfox@adelielinux.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
add test for bug in git-mv for recursive submodules
When using git-mv with a submodule it will detect that and update the
paths for its configurations (.gitmodules, worktree and gitfile). This
does not work for recursive submodules where a user renames the root
submodule.
We discovered this fact when working on on-demand fetch for renamed
submodules. Lets add a test to document.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Set curl as the runtime default when it is available.
When linked against older curl versions (< 7_34_0) or without curl,
use the legacy imap implementation.
The goal is to validate feature parity between the legacy and
the curl implementation, deprecate the legacy implementation
later on and in the long term, hopefully drop it altogether.
Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
imap_send: setup_curl: retreive credentials if not set in config file
Up to this point, the curl mode only supported getting the username
and password from the gitconfig file while the legacy mode could also
fetch them using the credential API.
Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
curl_append_msgs_to_imap always returned 0, whether curl failed or not.
Return a proper status so git imap-send will exit with an error code
if something wrong happened.
Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit-template: change a message to be more intuitive
It's not good to use the phrase 'do not touch' to convey the
information that the cut-line should not be modified or removed as
it could possibly be mis-interpreted by a person who doesn't know
that the word 'touch' has the meaning of 'tamper with'. Further, it
could make translations a little difficult as it might not have the
intended meaning in a few languages when translated as such.
So, use more intuitive terms in the sentence.
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
test-lib: don't use ulimit in test prerequisites on cygwin
On cygwin (and MinGW), the 'ulimit' built-in bash command does not have
the desired effect of limiting the resources of new processes, at least
for the stack and file descriptors. However, it always returns success
and leads to several test prerequisites being erroneously set to true.
Add a check for cygwin and MinGW to the prerequisite expressions, using
a 'test_have_prereq !MINGW,!CYGWIN' clause, to guard against using ulimit.
This affects the prerequisite expressions for the ULIMIT_STACK_SIZE,
CMDLINE_LIMIT and ULIMIT_FILE_DESCRIPTORS prerequisites.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
read_packed_refs(): make parsing of the header line more robust
The old code parsed the traits in the `packed-refs` header by looking
for the string " trait " (i.e., the name of the trait with a space on
either side) in the header line. This is fragile, because if any other
implementation of Git forgets to write the trailing space, the last
trait would silently be ignored (and the error might never be
noticed).
So instead, use `string_list_split_in_place()` to split the traits
into tokens then use `unsorted_string_list_has_string()` to look for
the tokens we are interested in. This means that we can read the
traits correctly even if the header line is missing a trailing
space (or indeed, if it is missing the space after the colon, or if it
has multiple spaces somewhere).
However, older Git clients (and perhaps other Git implementations)
still require the surrounding spaces, so we still have to output the
header with a trailing space.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>