In 2.29, "--committer-date-is-author-date" option of "rebase" and
"am" subcommands lost the e-mail address by mistake, which has been
corrected.
* jk/committer-date-is-author-date-fix:
rebase: fix broken email with --committer-date-is-author-date
am: fix broken email with --committer-date-is-author-date
t3436: check --committer-date-is-author-date result more carefully
A previous commit adjusted the code in ref-filter.c so that messages
containing CRLF are now correctly parsed and displayed.
Add tests to also check that `git log` and `git show` correctly handle
such messages, to prevent futur regressions if these commands are
refactored to use the ref-filter API.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ref-filter code does not correctly handle commit or tag messages
that use CRLF as the line terminator. Such messages can be created with
the `--cleanup=verbatim` option of `git commit` and `git tag`, or by
using `git commit-tree` directly.
The function `find_subpos` in ref-filter.c looks for two consecutive
LFs to find the end of the subject line, a sequence which is absent in
messages using CRLF. This results in the whole message being parsed as
the subject line (`%(contents:subject)`), and the body of the message
(`%(contents:body)`) being empty.
Moreover, in `copy_subject`, which wants to return the subject as a
single line, '\n' is replaced by space, but '\r' is
untouched.
This impacts the output of `git branch`, `git tag` and `git
for-each-ref`.
This behaviour is a regression for `git branch --verbose`, which
bisects down to 949af0684c (branch: use ref-filter printing APIs,
2017-01-10).
Adjust the ref-filter code to be more lenient by hardening the logic in
`copy_subject` and `find_subpos` to correctly parse messages containing
CRLF.
Add a new test script, 't3920-crlf-messages.sh', to test the behaviour
of commands using either the ref-filter or the pretty APIs with messages
using CRLF line endings. The function `test_crlf_subject_body_and_contents`
can be used to test that the `--format` option of `branch`, `tag`,
`for-each-ref`, `log` and `show` correctly displays the subject, body
and raw content of commit and tag messages using CRLF. Test the
output of `branch`, `tag` and `for-each-ref` with such commits.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In demultiplex_sideband(), there are two oddities when we check an
incoming packet:
- if it has zero length, then we assume it's a flush packet. This
means we fail to notice the difference between a real flush and a
true zero-length packet that's missing its sideband designator. It's
not a huge problem in practice because we'd never send a zero-length
data packet (even our keepalives are otherwise-empty sideband-1
packets).
But it would be nice to detect and report the error, since it's
likely to cause other confusion (we think the other side flushed,
but they do not).
- we try to detect packets missing their designator by checking for
"if (len < 1)". But this will never trigger for "len == 0"; we've
already detected that and left the function before then.
It _could_ detect a negative "len" parameter. But in that case, the
error message is wrong. The issue is not "no sideband" but rather
"eof while reading the packet". However, this can't actually be
triggered in practice, because neither of the two callers uses
pkt_read's GENTLE_ON_EOF flag. Which means they'd die with "the
remote end hung up unexpectedly" before we even get here.
So this truly is dead code.
We can improve these cases by passing in a pkt-line status to the
demultiplexer, and by having recv_sideband() use GENTLE_ON_EOF. This
gives us two improvements:
- we can now reliably detect flush packets, and will report a normal
packet missing its sideband designator as an error
- we'll report an eof with a more detailed "protocol error: eof while
reading sideband packet", rather than the generic "the remote end
hung up unexpectedly"
- when we see an eof, we'll flush the sideband scratch buffer, which
may provide some hints from the remote about why they hung up
(though note we already flush on newlines, so it's likely that most
such messages already made it through)
In some sense this patch goes against fbd76cd450 (sideband: reverse its
dependency on pkt-line, 2019-01-16), which caused the sideband code not
to depend on the pkt-line code. But that commit was really just trying
to deal with the circular header dependency. The two modules are
conceptually interlinked, and it was just trying to keep things
compiling. And indeed, there's a sticking point in this patch: because
pkt-line.h includes sideband.h, we can't add the reverse include we need
for the sideband code to have an "enum packet_read_status" parameter.
Nor can we forward declare it, because you can't forward declare an enum
in C. However, C does guarantee that enums fit in an int, so we can just
use that type.
One alternative would be for the callers to check themselves that they
got something sane from the pkt-line code. But besides duplicating
logic, this gets quite tricky. Any error condition requires flushing the
sideband #2 scratch buffer, which only demultiplex_sideband() knows how
to do.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's been eight years, more than enough time to move on.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's no need to set a variable we are not going to use.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A lot of people are confused about which completion script they are
using; Zsh's Git script, or Git's Zsh script.
Add a simple helper so they can type 'git zsh<tab>' and find out if they
are running the correct one: this.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's no need to use _alternative and repeat a lot of the code.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's exactly the same as __gitcomp_file() with no prefix.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's exactly the same as __gitcomp_nl(), no need to duplicate code.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of manually removing the suffix so zsh can add its own, we can
tell zsh to add no suffix, so we don't have to remove it.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't need to override IFS, zsh has a native way of splitting by new
lines: the expansion flag (f).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Files don't need to be split by '=:', words do.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 688077910b forgot to add the corresponding zsh function.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't want to override the 'complete()' function in zsh, which can be
used by bashcomp.
Reported-by: Mark Lodato <lodato@google.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This was introduced in upstream's bash script, but never in zsh's:
b221b5ab9b (completion: collapse extra --no-.. options)
It has been failing since v2.19.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It has been deprecated for more than eight years now, it's never up to
date, and it's a hassle to maintain.
It's time to move on.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A lot of people want to define aliases like gc='git commit', and zsh
allows that (when not using 'complete_aliases'), but we need to handle
services that call a function other than the main one.
With this patch we can do:
compdef _git gc=git_commit
Additionally, add compatibility for Zsh Git functions which have the
form git-commit (with dash, not underscore).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't need PROMPT_COMMAND in Zsh; we are already using %F{color} %f,
which in turn use %{ and %}, which are the equivalent of Bash's
\[ and \].
We can use as many colors as we want and output directly into PS1
(or RPS1) without the risk of buffer wrapping issues.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update the default locations of typical system bash-completion,
including the default bash-completion location for user scripts, and the
recommended way to find the system location (with pkg-config).
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git checkout" learned to use checkout.guess configuration variable
and enable/disable its "--[no-]guess" option accordingly.
* dl/checkout-guess:
checkout: learn to respect checkout.guess
Documentation/config/checkout: replace sq with backticks
"git checkout -p A...B [-- <path>]" did not work, even though the
same command without "-p" correctly used the merge-base between
commits A and B.
* dl/checkout-p-merge-base:
t2016: add a NEEDSWORK about the PERL prerequisite
add-patch: add NEEDSWORK about comparing commits
Doc: document "A...B" form for <tree-ish> in checkout and switch
builtin/checkout: fix `git checkout -p HEAD...` bug
"git clone" learned clone.defaultremotename configuration variable
to customize what nickname to use to call the remote the repository
was cloned from.
* sb/clone-origin:
clone: allow configurable default for `-o`/`--origin`
clone: read new remote name from remote_name instead of option_origin
clone: validate --origin option before use
refs: consolidate remote name validation
remote: add tests for add and rename with invalid names
clone: use more conventional config/option layering
clone: add tests for --template and some disallowed option pairs
"git push --force-with-lease[=<ref>]" can easily be misused to lose
commits unless the user takes good care of their own "git fetch".
A new option "--force-if-includes" attempts to ensure that what is
being force-pushed was created after examining the commit at the
tip of the remote ref that is about to be force-replaced.
* sk/force-if-includes:
t, doc: update tests, reference for "--force-if-includes"
push: parse and set flag for "--force-if-includes"
push: add reflog check for "--force-if-includes"
"git worktree list" now shows if each worktree is locked. This
possibly may open us to show other kinds of states in the future.
* rs/worktree-list-show-locked:
worktree: teach `list` to annotate locked worktree
Use "git archive" more to produce the release tarball.
* rs/dist-doc-with-git-archive:
Makefile: remove the unused variable TAR_DIST_EXTRA_OPTS
Makefile: use git init/add/commit/archive for dist-doc
If we encounter an error while checking out an explicit path, we print a
message to stderr but do not actually exit with a non-zero code. While
this is a plumbing command and the behavior goes all the way back to
33db5f4d90 (Add a "checkout-cache" command which does what the name
suggests., 2005-04-09), this is almost certainly an oversight:
- we _do_ return an exit code from checkout_file(); the caller just
never reads it
- errors while checking out all paths (with "-a") do result in a
non-zero exit code.
- it would be quite unusual not to use the exit code for an error,
as otherwise the caller has no idea the command failed except by
scraping stderr
To keep our tests simple and portable, we can use the most obvious
error: asking to checkout a path which is not in the index at all.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If checkout-index is given --stage=all for a specific path, it will try
to write stages 1-3 (if present) for that path to temporary files.
However, if the file is present only at stage 0, it writes nothing but
gives a confusing message:
$ git checkout-index --stage=all -- Makefile
git checkout-index: Makefile does not exist at stage 4
This is nonsense. There is no stage 4 (it's just an internal enum value
we use for "all"), and the documentation clearly states:
Paths which only have a stage 0 entry will always be omitted from the
output.
Here it's talking about the list of tempfiles written to stdout, but it
seems clear that this case was not meant to be an error. We even have a
test which covers it, but it only checks that the command reports an
exit code of 0, not its stderr. And it reports 0 only because of another
bug which fails to propagate errors (which will be fixed in a subsequent
patch).
So let's make the test more thorough. We'll also cover the case that we
found _no_ entry, not even a stage zero, which should still be an error.
However, because of the other bug, we'll have to mark this as expecting
failure for the moment.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We pass "sideband: " as our identity for errors to recv_sideband(). But
it already adds the trailing colon and space. This doesn't invalidate
any tests, but it looks funny when you examine the test output.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the exit code for the likes of "git remote add/rename" to exit
with 2 if the remote in question doesn't exist, and 3 if it
does. Before we'd just die() and exit with the general 128 exit code.
This changes the output message from e.g.:
fatal: remote origin already exists.
To:
error: remote origin already exists.
Which I believe is a feature, since we generally use "fatal" for the
generic errors, and "error" for the more specific ones with a custom
exit code, but this part of the change may break code that already
relies on stderr parsing (not that we ever supported that...).
The motivation for this is a discussion around some code in GitLab's
gitaly which wanted to check this, and had to parse stderr to do so:
https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2695
It's worth noting as an aside that a method of checking this that
doesn't rely on that is to check with "git config" whether the value
in question does or doesn't exist. That introduces a TOCTOU race
condition, but on the other hand this code (e.g. "git remote add")
already has a TOCTOU race.
We go through the config.lock for the actual setting of the config,
but the pseudocode logic is:
read_config();
check_config_and_arg_sanity();
save_config();
So e.g. if a sleep() is added right after the remote_is_configured()
check in add() we'll clobber remote.NAME.url, and add another (usually
duplicate) remote.NAME.fetch entry (and other values, depending on
invocation).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano
Philippe Blain
Jeff King
Felipe Contreras
Ævar Arnfjörð Bjarmason