Merge branch 'jc/maint-protect-sh-from-ifs' into maint-1.7.11
When the user exports a non-default IFS without HT, scripts that
rely on being able to parse "ls-files -s | while read a b c..."
start to fail. Protect them from such a misconfiguration.
* jc/maint-protect-sh-from-ifs:
sh-setup: protect from exported IFS
Merge branch 'bc/receive-pack-stdout-protection' into maint-1.7.11
When "git push" triggered the automatic gc on the receiving end, a
message from "git prune" that said it was removing cruft leaked to
the standard output, breaking the communication protocol.
* bc/receive-pack-stdout-protection:
receive-pack: do not leak output from auto-gc to standard output
t/t5400: demonstrate breakage caused by informational message from prune
Merge branch 'jk/maint-null-in-trees' into maint-1.7.11
"git diff" had a confusion between taking data from a path in the
working tree and taking data from an object that happens to have
name 0{40} recorded in a tree.
* jk/maint-null-in-trees:
fsck: detect null sha1 in tree entries
do not write null sha1s to on-disk index
diff: do not use null sha1 as a sentinel value
Merge branch 'mm/die-with-dashdash-help' into maint-1.7.11
When the user gives an argument that can be taken as both a
revision name and a pathname without disambiguating with "--", we
used to give a help message "Use '--' to separate". The message
has been clarified to show where that '--' goes on the command
line.
* mm/die-with-dashdash-help:
setup: clarify error messages for file/revisions ambiguity
Merge branch 'jc/maint-abbrev-option-cli' into maint-1.7.11
We did not document that many commands take unique prefix
abbreviations of long options (e.g. "--option" may be the only flag
that the command accepts that begin with "--opt", in which case you
can give "--opt") anywhere easy to find for new people.
* jc/maint-abbrev-option-cli:
gitcli: describe abbreviation of long options
Merge branch 'jc/maint-rev-list-topo-doc' into maint-1.7.11
It was unclear what "--topo-order" was really about in the
documentation. It is not just about "children before parent", but
also about "don't mix lineages".
Merge branch 'hv/coding-guidelines' into maint-1.7.11
In earlier days, "imitate the style in the neibouring code" was
sufficient to keep the coherent style, but over time some parts of
the codebase have drifted enough to make it ineffective.
* hv/coding-guidelines:
Documentation/CodingGuidelines: spell out more shell guidelines
Our documentation used to assume having files in .git/refs/*
directories was the only to have branches and tags, but that is not
true for quite some time.
* jc/tag-doc:
Documentation: do not mention .git/refs/* directories
Make 'git submodule update --force' always check out submodules.
Currently, it will only do a checkout if the sha1 registered in the containing
repository doesn't match the HEAD of the submodule, regardless of whether the
submodule is dirty. As discussed on the mailing list, the '--force' flag is a
strong indicator that the state of the submodule is suspect, and should be reset
to HEAD.
Signed-off-by: Stefan Zager <szager@google.com> Acked-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
contrib/ciabot: Get ciabot configuration from git variables
These changes remove all need to modify the ciabot scripts for installation.
Instead, per-project configuration can be dome via variables in a [ciabot]
section of the config file.
Also, correct for the new server address.
Signed-off-by: Eric S. Raymond <esr@thyrsus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
One fictitious command "proxy-command" is enclosed inside a double
quote pair, while another fictitious command "default-proxy" is not
in the example, but the quoting does not change anything in the pair
of examples. Remove the quotes to avoid unnecessary confusion.
"stash apply" directly calls a backend merge function which does not
automatically invoke rerere. This confuses mergetool when leftover
rerere state is left behind from previous merges.
Invoke rerere explicitly when we encounter a conflict during stash
apply. This turns the test introduced by the previous commit to
succeed.
Signed-off-by: Phil Hord <hordp@cisco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a test to make sure that a conflicted "stash apply" invokes
rerere to record the conflicts and resolve the the files it can
(the current code doesn't, so the test is marked as failing).
Without correct state recorded for rerere, mergetool may be
confused, causing it to think no files have conflicts even though
they do. This condition is not verified by this test since a
subsequent commit will change the behavior to enable rerere for
stash conflicts.
Also, the next test expected us to finish up with a reset, which is
impossible to do if we fail (as we must) and it's an unreasonable
expectation anyway. Begin the next test with a reset of its own
instead.
Signed-off-by: Phil Hord <hordp@cisco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Letting the "--rebase" option squat on the short-and-sweet single
letter option "-r" was an unintended accident and was not even
documented, but the short option seems to be already used in the
wild. Let's document it so that other options that begin with "r"
would not be tempted to steal it.
Signed-off-by: Miklos Vajna <vmiklos@suse.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
It was unclear what "--topo-order" was really about in the
documentation. It is not just about "children before parent", but
also about "don't mix lineages".
Reword the description for both "--date-order" and "--topo-order",
and add an illustration to it.
gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO
When gitweb is used as a DirectoryIndex, it attempts to strip
PATH_INFO on its own, as $cgi->url() fails to do so.
However, it fails to account for the fact that PATH_INFO has
already been URL-decoded by the web server, but the value
returned by $cgi->url() has not been. This causes the stripping
to fail whenever the URL contains encoded characters.
To see this in action, setup gitweb as a DirectoryIndex and
then use it on a repository with a directory containing a
space in the name. Navigate to tree view, examine the gitweb
generated html and you'll see a link such as:
<a href="/test.git/tree/HEAD:/directory with spaces">directory with spaces</a>
When clicked on, the browser will URL-encode this link, giving
a $cgi->url() of the form:
/test.git/tree/HEAD:/directory%20with%20spaces
While PATH_INFO is:
/test.git/tree/HEAD:/directory with spaces
Fix this by calling unescape() on both $my_url and $my_uri before
stripping PATH_INFO from them.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com> Acked-by: Jakub Narebski <jnareb@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/CodingGuidelines: spell out more shell guidelines
In earlier days, "imitate the style in the neibouring code" was
sufficient to keep the coherent style, but over time some parts of
the codebase have drifted enough to make it ineffective.
Spell some of the guidelines out.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Let submodule command exit with error status if path does not exist
Various subcommands of the "git submodule" command exited with 0
status even though the path given by the user did not exist.
The reason behind that was that they all pipe the output of
module_list into the while loop which then does the action on the
paths specified by the commandline. Since the exit code of the
command on the upstream side of the pipe is ignored by the shell,
the status code of "ls-files --error-unmatch" nor "module_list" was
not propagated.
In case ls-files returns with an error code, we write a special
string that is not possible in non error situations, and no other
output, so that the downstream can detect the error and die with an
error code.
The error message that there is an unmatched pathspec comes through
stderr directly from ls-files. So the user still gets a hint whats going
on.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
'git rebase' uses the full onto sha1 for the reflog message whereas 'git
rebase -i' uses the short sha1. This is not only inconsistent, but can
lead to problems when the reflog is inspected at a later time at which
that abbreviation may have become ambiguous.
Make 'rebase -i' use the full onto sha1, as well.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add test cases for 'git rebase --keep-empty' with and without an
"empty" commit already in upstream. The empty commit that is about to
be rebased should be kept in both cases.
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many scripted Porcelains rely on being able to split words at the
default $IFS characters, i.e. SP, HT and LF. If the user exports a
non-default IFS to the environment, what they read from plumbing
commands such as ls-files that use HT to delimit fields may not be
split in the way we expect.
Protect outselves by resetting it, just like we do so against CDPATH
exported to the environment.
it marks it to be displayed in a monospace font. This works
fine when generating HTML output. However, when generating
docbook output, we override the expansion of a listingblock
to work around bugs in some versions of the docbook
toolchain. Our override did not mark the listingblock with
the "monospaced" class.
The main output that uses docbook as an intermediate format
is the manpages. We didn't notice any issue there because
the monospaced class seems to be ignored when generating
roff from the docbook manpages.
However, when generating texinfo to make info pages, docbook
does respect this class. The resulting texinfo output
properly uses "@example" blocks to display the listing in
this case. Besides possibly looking prettier in some texinfo
backends, one important effect is that the monospace font
suppresses texinfo's expansion of "--" and "---" into
en-dashes and em-dashes. With the current code, the example
above ends up looking like "git log -merge", which is
confusing and wrong.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
receive-pack: do not leak output from auto-gc to standard output
The standard output channel of receive-pack is a structured protocol
channel, and subprocesses must never be allowed to leak anything
into it by writing to their standard output.
Use RUN_COMMAND_STDOUT_TO_STDERR option to run_command_v_opt() just
like we do when running hooks to prevent output from "gc" leaking to
the standard output.
t/t5400: demonstrate breakage caused by informational message from prune
When receive-pack triggers 'git gc --auto' and 'git prune' is called to
remove a stale temporary object, 'git prune' prints an informational
message to stdout about the file that it will remove. Since this message
is written to stdout, it is sent back over the transport channel to the git
client which tries to interpret it as part of the pack protocol and then
promptly terminates with a complaint about a protocol error.
Introduce a test which exercises the auto-gc functionality of receive-pack
and demonstrates this breakage.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that git_terminal_prompt can cleanly interact with /dev/tty on
Solaris, enable HAVE_DEV_TTY so that this code path is used for
credential reading instead of relying on the crippled getpass().
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
terminal: seek when switching between reading and writing
When a stdio stream is opened in update mode (e.g., "w+"),
the C standard forbids switching between reading or writing
without an intervening positioning function. Many
implementations are lenient about this, but Solaris libc
will flush the recently-read contents to the output buffer.
In this instance, that meant writing the non-echoed password
that the user just typed to the terminal.
Fix it by inserting a no-op fseek between the read and
write.
The opposite direction (writing followed by reading) is also
disallowed, but our intervening fflush is an acceptable
positioning function for that alternative.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jn/block-sha1:
Makefile: BLK_SHA1 does not require fast htonl() and unaligned loads
block-sha1: put expanded macro parameters in parentheses
block-sha1: avoid pointer conversion that violates alignment constraints
* lm/git-blame-el:
git-blame.el: Do not use bare 0 to mean (point-min)
git-blame.el: Use with-current-buffer where appropriate
git-blame.el: Do not use goto-line in lisp code
Documentation: do not mention .git/refs/* directories
It is an implementation detail that a new tag is created by adding a
file in the .git/refs/tags directory. The only thing the user needs
to know is that a "git tag" creates a ref in the refs/tags namespace,
and without "-f", it does not overwrite an existing tag.
Inspired by a report from 乙酸鋰 <ch3cooli@gmail.com>; I think I
caught all the existing mention in Documentation/ directory in the
tip of 1.7.9.X maintenance track, but we may have added new ones
since then.
These assignments comes from the very first commit e83c516 (Initial
revision of "git", the information manager from hell - 2005-04-07).
Back then we did not die() when errors happened so correct errno was
required.
Since 5d1a5c0 ([PATCH] Better error reporting for "git status" -
2005-10-01), read_index_from() learned to die rather than just return
-1 and these assignments became irrelevant. Remove them.
While at it, move die_errno() next to xmmap() call because it's the
mmap's error code that we care about. Otherwise if close(fd); fails,
it could overwrite mmap's errno.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a file that ends with an incomplete line is expressed as a
complete rewrite with the -B option, git diff incorrectly
appends the incomplete line indicator "\ No newline at end of
file" after such a line, rather than writing it on a line of its
own (the output codepath for normal output without -B does not
have this problem). Add a LF after the incomplete line before
writing the "\ No newline ..." out to fix this.
Add a couple of tests to confirm that the indicator comment is
generated on its own line in both plain diff and rewrite mode.
Signed-off-by: Adam Butcher <dev.lists@jessamine.co.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The seq command is GNU-ism, and is missing at least in older BSD
releases and their derivatives, not to mention antique
commercial Unixes.
We already purged it in b3431bc (Don't use seq in tests, not
everyone has it, 2007-05-02), but a few new instances have crept
in. They went unnoticed because they are in scripts that are not
run by default.
Replace them with test_seq that is implemented with a Perl snippet
(proposed by Jeff). This is better than inlining this snippet
everywhere it's needed because it's easier to read and it's easier
to change the implementation (e.g. to C) if we ever decide to remove
Perl from the test suite.
Note that test_seq is not a complete replacement for seq(1). It
just has what we need now, in addition that it makes it possible for
us to do something like "test_seq a m" if we wanted to in the
future.
There are also many places that do `for i in 1 2 3 ...` but I'm not sure
if it's worth converting them to test_seq. That would introduce running
more processes of Perl.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge-recursive: eliminate flush_buffer() in favor of write_in_full()
flush_buffer() is a thin wrapper around write_in_full() with two very
confusing properties:
* It runs a loop to handle short reads, ensuring that we write
everything. But that is precisely what write_in_full() does!
* It checks for a return value of 0 from write_in_full(), which cannot
happen: it returns this value only if count=0, but flush_buffer()
will never call write_in_full() in this case.
Remove it.
Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff_setup_done() has historically returned an error code, but lost
the last nonzero return in 943d5b7 (allow diff.renamelimit to be set
regardless of -M/-C, 2006-08-09). The callers were in a pretty
confused state: some actually checked for the return code, and some
did not.
Let it return void, and patch all callers to take this into account.
This conveniently also gets rid of a handful of different(!) error
messages that could never be triggered anyway.
Note that the function can still die().
Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
setup: clarify error messages for file/revisions ambiguity
The previous "Use '--' to separate filenames from revisions" may sound
obvious for an old-time Unix user, but does not make it clear how to use
this '--'. In addition to mentionning this '--', give an idea of what the
new command should look like.
Ideally, we could provide cut-and-paste ready commands based on the
command that just failed, but we have no easy access to argv[] in this
place of the code.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The RFC2047 unquoting, used to parse email addresses in From and Cc
headers, is broken in several ways:
* It erroneously substitutes ' ' for '_' in *the whole* header, even
outside the quoted field. [Noticed by Christoph.]
* It is too liberal in its matching, and happily matches the start
of one quoted chunk against the end of another, or even just
something that looks like such an end. [Noticed by Junio.]
* It fundamentally cannot cope with encodings that are not a
superset of ASCII, nor several (incompatible) encodings in the
same header.
This patch fixes the first two by doing a more careful decoding of
the outer quoting (e.g. "=AB" to represent an octet whose value is
0xAB). Fixing the fundamental issues is left for a future, more
intrusive, patch.
Noticed-by: Christoph Miebach <christoph.miebach@web.de> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'jl/maint-1.7.10-recurse-submodules-with-symlink' into maint
When "git submodule add" clones a submodule repository, it can get
confused where to store the resulting submodule repository in the
superproject's .git/ directory when there is a symbolic link in the
path to the current directory.
* jl/maint-1.7.10-recurse-submodules-with-symlink:
submodules: don't stumble over symbolic links when cloning recursively
Merge branch 'jc/maint-filter-branch-epoch-date' into maint
In 1.7.9 era, we taught "git rebase" about the raw timestamp format
but we did not teach the same trick to "filter-branch", which rolled
a similar logic on its own.
* jc/maint-filter-branch-epoch-date:
t7003: add test to filter a branch with a commit at epoch
date.c: Fix off by one error in object-header date parsing
filter-branch: do not forget the '@' prefix to force git-timestamp
The test happened to use "rev-parse --max-count=1 HEAD" consistently
to prepare the expected output and the actual output, so the
comparison between them gave us a correct success/failure because
both output had irrelevant "--max-count=1" in it.
But that is not an excuse to keep it broken. Replace it a more
meaningful construct "rev-parse --verify HEAD".
Noticed by Daniel Graña while working on his submodule tests.
Since commit bbc09c22 ("grep: rip out support for external grep",
12-01-2010), test number 60 ("grep -C1 hunk mark between files") is
essentially the same as test number 59.
Test 59 was intended to verify the behaviour of git-grep resulting
from multiple invocations of an external grep. As part of the test,
it creates and adds 1024 files to the index, which is now wasted
effort.
Remove test 59, since it is now redundant.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
link_alt_odb_entry: fix read over array bounds reported by valgrind
pfxlen can be longer than the path in objdir when relative_base
contains the path to gits object directory. Here we are interested
in checking if ent->base[] (the part that corresponds to .git/objects)
is the same string as objdir, and the code NUL-terminated ent->base[]
to
LEADING PATH\0XX/XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\0
in preparation for these "duplicate check" step (before we return
from the function, the first NUL is turned into '/' so that we can
fill XX when probing for loose objects). All we need to do is to
compare the string with the path to our object directory.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Short of somebody happening to beat the 1 in 2^160 odds of
actually generating content that hashes to the null sha1, we
should never see this value in a tree entry. So let's have
fsck warn if it it seen.
As in the previous commit, we test both blob and submodule
entries to future-proof the test suite against the
implementation depending on connectivity to notice the
error.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We should never need to write the null sha1 into an index
entry (short of the 1 in 2^160 chance that somebody actually
has content that hashes to it). If we attempt to do so, it
is much more likely that it is a bug, since we use the null
sha1 as a sentinel value to mean "not valid".
The presence of null sha1s in the index (which can come
from, among other things, "update-index --cacheinfo", or by
reading a corrupted tree) can cause problems for later
readers, because they cannot distinguish the literal null
sha1 from its use a sentinel value. For example, "git
diff-files" on such an entry would make it appear as if it
is stat-dirty, and until recently, the diff code assumed
such an entry meant that we should be diffing a working tree
file rather than a blob.
Ideally, we would stop such entries from entering even our
in-core index. However, we do sometimes legitimately add
entries with null sha1s in order to represent these sentinel
situations; simply forbidding them in add_index_entry breaks
a lot of the existing code. However, we can at least make
sure that our in-core sentinel representation never makes it
to disk.
To be thorough, we will test an attempt to add both a blob
and a submodule entry. In the former case, we might run into
problems anyway because we will be missing the blob object.
But in the latter case, we do not enforce connectivity
across gitlink entries, making this our only point of
enforcement. The current implementation does not care which
type of entry we are seeing, but testing both cases helps
future-proof the test suite in case that changes.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diff code represents paths using the diff_filespec
struct. This struct has a sha1 to represent the sha1 of the
content at that path, as well as a sha1_valid member which
indicates whether its sha1 field is actually useful. If
sha1_valid is not true, then the filespec represents a
working tree file (e.g., for the no-index case, or for when
the index is not up-to-date).
The diff_filespec is only used internally, though. At the
interfaces to the diff subsystem, callers feed the sha1
directly, and we create a diff_filespec from it. It's at
that point that we look at the sha1 and decide whether it is
valid or not; callers may pass the null sha1 as a sentinel
value to indicate that it is not.
We should not typically see the null sha1 coming from any
other source (e.g., in the index itself, or from a tree).
However, a corrupt tree might have a null sha1, which would
cause "diff --patch" to accidentally diff the working tree
version of a file instead of treating it as a blob.
This patch extends the edges of the diff interface to accept
a "sha1_valid" flag whenever we accept a sha1, and to use
that flag when creating a filespec. In some cases, this
means passing the flag through several layers, making the
code change larger than would be desirable.
One alternative would be to simply die() upon seeing
corrupted trees with null sha1s. However, this fix more
directly addresses the problem (while bogus sha1s in a tree
are probably a bad thing, it is really the sentinel
confusion sending us down the wrong code path that is what
makes it devastating). And it means that git is more capable
of examining and debugging these corrupted trees. For
example, you can still "diff --raw" such a tree to find out
when the bogus entry was introduced; you just cannot do a
"--patch" diff (just as you could not with any other
corrupted tree, as we do not have any content to diff).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we are leaving a detached HEAD, we do a revision traversal to
check whether we are orphaning any commits, marking the commit we're
leaving as the start of the traversal, and all existing refs as
uninteresting.
Prior to commit 468224e5, we did so by calling for_each_ref, and
feeding each resulting refname to setup_revisions. Commit 468224e5
refactored this to simply mark the pending objects, saving an extra
lookup.
However, it confused the "flags" parameter to the each_ref_fn
clalback, which is about the flags we found while looking up the ref
with the object flag. Because REF_ISSYMREF ("this ref is a symbolic
ref, e.g. refs/remotes/origin/HEAD") happens to be the same bit
pattern as SEEN ("we have picked this object up from the pending
list and moved it to revs.commits list"), we incorrectly reported
that a commit previously at the detached HEAD will become
unreachable if the only ref that can reach the commit happens to be
pointed at by a symbolic ref.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Enumerate revision range specifiers in the documentation
It was a bit hard to learn how <rev>^@, <rev>^! and various other
forms of range specifiers are used, because they were discussed
mostly in the prose part of the documentation, unlike various forms
of extended SHA-1 expressions that are listed in an enumerated list.
Also add a few more examples showing use of <rev>, <rev>..<rev> and
<rev>^! forms, stolen from a patch by Max Horn.
mergetool: support --tool-help option like difftool does
This way we do not have to risk the list of tools going out of sync
between the implementation and the documentation.
In the same spirit as bf73fc2 (difftool: print list of valid tools
with '--tool-help', 2012-03-29), trim the list of merge backends in
the documentation. We do not want to have a complete list of valid
tools; we only want a list to help people guess what kind of things
the tools do to be specified there, and refer them to --tool-help
for a complete list.
The identity of the committer will ultimately be pulled from
the ident code by commit_tree(). However, we make an attempt
to check the author and committer identity early, before the
user has done any manual work like inputting a commit
message. That lets us abort without them having to worry
about salvaging the work from .git/COMMIT_EDITMSG.
The early check for committer ident does not use the
IDENT_STRICT flag, meaning that it would not find an empty
name field. The motivation was presumably because we did not
want to be too restrictive, as later calls might be more lax
(for example, when we create the reflog entry, we do not
care too much about a real name). However, because
commit_tree will always get a strict identity to put in the
commit object itself, there is no point in being lax only to
die later (and in fact it is harmful, because the user will
have wasted time typing their commit message).
Incidentally, this bug was masked prior to 060d4bb, as the
initial loose call would taint the later strict call. So the
commit would succeed (albeit with a bogus committer line in
the commit object), and nobody noticed that our early check
did not match the later one.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
advice: pass varargs to strbuf_vaddf, not strbuf_addf
The advise() function takes a variable number of arguments
and converts them into a va_list object to pass to strbuf
for handling. However, we accidentally called strbuf_addf
(that takes a variable number of arguments) instead of
strbuf_vaddf (that takes a va_list).
This bug dates back to v1.7.8.1-1-g23cb5bf, but we never
noticed because none of the current callers passes a string
with a format specifier in it. And the compiler did not
notice because the format string is not available at
compile time.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>