"git check-ref-format --branch $name" feature was originally
introduced (and was advertised) as a way for scripts to take any
end-user supplied string (like "master", "@{-1}" etc.) and see if it
is usable when Git expects to see a branch name, and also obtain the
concrete branch name that the at-mark magic expands to.
Emphasize that "see if it is usable" role in the description and
clarify that the @{...} expansion only occurs when run from within a
repository.
[jn: split out from a larger patch]
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The expansion returned from strbuf_check_branch_ref always starts with
"refs/heads/" by construction, but there is nothing about its name or
advertised API making that obvious. This command is used to process
human-supplied input from the command line and is usually not the
inner loop, so we can spare some cycles to be more defensive. Instead
of hard-coding the offset strlen("refs/heads/") to skip, verify that
the expansion actually starts with refs/heads/.
[jn: split out from a larger patch, added explanation]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running "git check-ref-format --branch @{-1}" from outside any
repository produces
$ git check-ref-format --branch @{-1}
BUG: environment.c:182: git environment hasn't been setup
This is because the expansion of @{-1} must come from the HEAD reflog,
which involves opening the repository. @{u} and @{push} (which are
more unusual because they typically would not expand to a local
branch) trigger the same assertion.
This has been broken since day one. Before v2.13.0-rc0~48^2
(setup_git_env: avoid blind fall-back to ".git", 2016-10-02), the
breakage was more subtle: Git would read reflogs from ".git" within
the current directory even if it was not a valid repository. Usually
that is harmless because Git is not being run from the root directory
of an invalid repository, but in edge cases such accesses can be
confusing or harmful. Since v2.13.0, the problem is easier to
diagnose because Git aborts with a BUG message.
Erroring out is the right behavior: when asked to interpret a branch
name like "@{-1}", there is no reasonable answer in this context.
But we should print a message saying so instead of an assertion failure.
We do not forbid "check-ref-format --branch" from outside a repository
altogether because it is ok for a script to pre-process branch
arguments without @{...} in such a context. For example, with
pre-2.13 Git, a script that does
branch='master'; # default value
parse_options
branch=$(git check-ref-format --branch "$branch")
to normalize an optional branch name provided by the user would work
both inside a repository (where the user could provide '@{-1}') and
outside (where '@{-1}' should not be accepted).
So disable the "expand @{...}" half of the feature when run outside a
repository, but keep the check of the syntax of a proposed branch
name. This way, when run from outside a repository, "git
check-ref-format --branch @{-1}" will gracefully fail:
$ git check-ref-format --branch @{-1}
fatal: '@{-1}' is not a valid branch name
and "git check-ref-format --branch master" will succeed as before:
$ git check-ref-format --branch master
master
restoring the usual pre-2.13 behavior.
[jn: split out from a larger patch; moved conditional to
strbuf_check_branch_ref instead of its caller; fleshed out commit
message; some style tweaks in tests]
Reported-by: Marko Kungla <marko.kungla@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change an error message in sequencer.c for the case that
we could not write to a file to match other instances.
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since a9d34933 ("Merge branch 'fm/fetch-raw-sha1'", 2015-06-01) we
allow to fetch by an object name when the other side accepts such a
request, but we never updated the documentation to match.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 11b087adfd (ref-filter: consult want_color() before
emitting colors, 2017-07-13), we expect that setting
"color.ui" to "always" will enable color tag formats even
without a tty. As that commit was built on top of
136c8c8b8f (color: check color.ui in git_default_config(),
2017-07-13) from the same series, we didn't need to touch
tag's config parsing at all.
However, since we reverted 136c8c8b8f, we now need to
explicitly call git_color_default_config() to make this
work.
Let's do so, and also restore the test dropped in 0c88bf5050
(provide --color option for all ref-filter users,
2017-10-03). That commit swapped out our "color.ui=always"
test for "--color" in preparation for "always" going away.
But since it is here to stay, we should test both cases.
Note that for-each-ref also lost its color.ui support as
part of reverting 136c8c8b8f. But as a plumbing command, it
should _not_ respect the color.ui config. Since it also
gained a --color option in 0c88bf5050, that's the correct
way to ask it for color. We'll continue to test that, and
confirm that "color.ui" is not respected.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 136c8c8b8f.
That commit was trying to address a bug caused by 4c7f1819b3
(make color.ui default to 'auto', 2013-06-10), in which
plumbing like diff-tree defaulted to "auto" color, but did
not respect a "color.ui" directive to disable it.
But it also meant that we started respecting "color.ui" set
to "always". This was a known problem, but 4c7f1819b3 argued
that nobody ought to be doing that. However, that turned out
to be wrong, and we got a number of bug reports related to
"add -p" regressing in v2.14.2.
Let's revert 136c8c8b8, fixing the regression to "add -p".
This leaves the problem from 4c7f1819b3 unfixed, but:
1. It's a pretty obscure problem in the first place. I
only noticed it while working on the color code, and we
haven't got a single bug report or complaint about it.
2. We can make a more moderate fix on top by respecting
"never" but not "always" for plumbing commands. This
is just the minimal fix to go back to the working state
we had before v2.14.2.
Note that this isn't a pure revert. We now have a test in
t3701 which shows off the "add -p" regression. This can be
flipped to success.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit c5bdfe677c.
That commit was done primarily to prepare for the weakening
of "always" in 6be4595edb (color: make "always" the same as
"auto" in config, 2017-10-03). But since we've now reverted
6be4595edb, there's no need for us to remove "-c
color.ui=always" from the tests. And in fact it's a good
idea to restore these tests, to make sure that "always"
continues to work.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 6be4595edb.
That commit weakened the "always" setting of color config so
that it acted as "auto". This was meant to solve regressions
in v2.14.2 in which setting "color.ui=always" in the on-disk
config broke scripts like add--interactive, because the
plumbing diff commands began to generate color output.
This was due to 136c8c8b8f (color: check color.ui in
git_default_config(), 2017-07-13), which was in turn trying
to fix issues caused by 4c7f1819b3 (make color.ui default to
'auto', 2013-06-10). But in weakening "always", we created
even more problems, as people expect to be able to use "git
-c color.ui=always" to force color (especially because some
commands don't have their own --color flag). We can fix that
by special-casing the command-line "-c", but now things are
getting pretty confusing.
Instead of piling hacks upon hacks, let's start peeling off
the hacks. The first step is dropping the weakening of
"always", which this revert does.
Note that we could actually revert the whole series merged
in by da15b78e52. Most of that
series consists of preparations to the tests to handle the
weakening of "-c color.ui=always". But it's worth keeping
for a few reasons:
- there are some other preparatory cleanups, like
e433749d86 (test-terminal: set TERM=vt100, 2017-10-03)
- it adds "--color" options more consistently in
0c88bf5050 (provide --color option for all ref-filter
users, 2017-10-03)
- some of the cases dropping "-c" end up being more robust
and realistic tests, as in 01c94e9001 (t7508: use
test_terminal for color output, 2017-10-03)
- the preferred tool for overriding config is "--color",
and we should be modeling that consistently
We can individually revert the few commits necessary to
restore some useful tests (which will be done on top of this
patch).
Note that this isn't a pure revert; we'll keep the test
added in t3701, but mark it as failure for now.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* 'jk/ui-color-always-to-auto-maint' (early part):
color: make "always" the same as "auto" in config
provide --color option for all ref-filter users
t3205: use --color instead of color.branch=always
t3203: drop "always" color test
t6006: drop "always" color config tests
t7502: use diff.noprefix for --verbose test
t7508: use test_terminal for color output
t3701: use test-terminal to collect color output
t4015: prefer --color to -c color.diff=always
test-terminal: set TERM=vt100
"while sh t5601-clone.sh; do :; done" seems to fail sporadically at
around test #45 where fake-ssh wrapper is copied create plink.exe,
with an error message that says the "text is busy".
I have a mild suspicion that the root cause of the bug is that the
fake SSH process from the previous test is still running by the time
the next test wants to replace it with a new binary, but in the
meantime, removing the target that could still be executing before
copying something else over seems to work it around.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A recently added "--color-moved" feature of "diff" fell into
infinite loop when ignoring whitespace changes, which has been
fixed.
* sb/diff-color-move:
diff: fix infinite loop with --color-moved --ignore-space-change
When columns are set to automatic for git tag and the output is
paginated by git, the output is a single column instead of multiple
columns.
Standard behaviour in git is to honor auto values when the pager is
active, which happens for example with commands like git log showing
colors when being paged.
Since ff1e72483 (tag: change default of `pager.tag` to "on",
2017-08-02), the pager has been enabled by default, exposing this
problem to more people.
finalize_colopts in column.c only checks whether the output is a TTY to
determine if columns should be enabled with columns set to auto. Also
check if the pager is active.
Adding a test for git column is possible but requires some care to work
around a race on stdin. See commit 18d8c2693 (test_terminal: redirect
child process' stdin to a pty, 2015-08-04). Test git tag instead, since
that does not involve stdin, and since that was the original motivation
for this patch.
Helped-by: Rafael Ascensão <rafa.almas@gmail.com>
Signed-off-by: Kevin Daudt <me@ikke.info>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --color-moved code uses next_byte() to advance through
the blob contents. When the user has asked to ignore
whitespace changes, we try to collapse any whitespace change
down to a single space.
However, we enter the conditional block whenever we see the
IGNORE_WHITESPACE_CHANGE flag, even if the next byte isn't
whitespace.
This means that the combination of "--color-moved and
--ignore-space-change" was completely broken. Worse, because
we return from next_byte() without having advanced our
pointer, the function makes no forward progress in the
buffer and loops infinitely.
Fix this by entering the conditional only when we actually
see whitespace. We can apply this also to the
IGNORE_WHITESPACE change. That code path isn't buggy
(because it falls through to returning the next
non-whitespace byte), but it makes the logic more clear if
we only bother to look at whitespace flags after seeing that
the next byte is whitespace.
Reported-by: Orgad Shaneh <orgads@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the write opertion fails, we write that we could
not read. Change the error message to match the operation
and remove the full stop at the end.
When ftruncate() fails, we write that we couldn't finish
the operation on the todo file. It is more accurate to write
that we couldn't truncate as we do in other calls of ftruncate().
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pull has supported these since ea230d8 (pull: add the --gpg-sign
option, 2014-02-10). Insert in long-option alphabetical order
following 7c85d274 (Documentation/merge-options.txt: order options
in alphabetical groups, 2009-10-22).
Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are "git checkout [-p][<tree-ish>][--][<paths>...]" in the
SYNOPSIS section, and "git checkout [-p][<tree-ish>][--]<paths>..."
as the header for the section that explains the "check out paths
from index/tree-ish" mode. It is unclear if we require at least one
path, or it is entirely optional.
Actually, both are wrong. Without the "-p(atch)" option, you must
have <pathspec> (otherwise, with a commit that is a <tree-ish>, you
would be checking out that commit to build a new history on top of
it). With it, it is already clear that you are checking out paths,
it is optional. In other words, you cannot omit both.
The source of the confusion is that -p(atch) is described as if it
is just another "optional" part and its description is lumped
together with the non patch mode, even though the actual end user
experience is vastly different.
Let's split the entry into two, and describe the regular mode and
the patch mode separately. This allows us to make it clear that the
regular mode MUST be given at least one pathspec, that the patch
mode can be invoked with either '-p' or '--patch' but one of these
must be given, and that the pathspec is entirely optional in the
patch mode.
Also, revamp the explanation of "checkout paths" by removing
extraneous description at the beginning, that says "checking out
paths is not checking out a branch". Explaining what it is for and
when the user wants to use it upfront is the most direct way to help
the readers.
Noticed-by: Robert P J Day
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bugfixes to an already graduated series.
* ls/filter-process-delayed:
write_entry: untangle symlink and regular-file cases
write_entry: avoid reading blobs in CE_RETRY case
write_entry: fix leak when retrying delayed filter
entry.c: check if file exists after checkout
entry.c: update cache entry only for existing files
An ancient bug that made Git misbehave with creation/renaming of
refs has been fixed.
* jk/refs-df-conflict:
refs_resolve_ref_unsafe: handle d/f conflicts for writes
t3308: create a real ref directory/file conflict
Improve behaviour of "git fsck" upon finding a missing object.
* rs/fsck-null-return-from-lookup:
fsck: handle NULL return of lookup_blob() and lookup_tree()
"git for-each-ref --format=..." learned a new format element,
%(trailers), to show only the commit log trailer part of the log
message.
* tb/show-trailers-in-ref-filter:
ref-filter.c: parse trailers arguments with %(contents) atom
ref-filter.c: use trailer_opts to format trailers
t6300: refactor %(trailers) tests
doc: use "`<literal>`"-style quoting for literal strings
doc: 'trailers' is the preferred way to format trailers
t4205: unfold across multiple lines
The write_entry() function switches on the mode of the entry
we're going to write out. The cases for S_IFLNK and S_IFREG
are lumped together. In earlier versions of the code, this
made some sense. They have a shared preamble (which reads
the blob content), a short type-specific body, and a shared
conclusion (which writes out the file contents; always for
S_IFREG and only sometimes for S_IFLNK).
But over time this has grown to make less sense. The preamble
now has conditional bits for each type, and the S_IFREG body
has grown a lot more complicated. It's hard to follow the
logic of which code is running for which mode.
Let's give each mode its own case arm. We will still share
the conclusion code, which means we now jump to it with a
goto. Ideally we'd pull that shared code into its own
function, but it touches so much internal state in the
write_entry() function that the end result is actually
harder to follow than the goto.
While we're here, we'll touch up a few bits of whitespace to
make the beginning and endings of the cases easier to read.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When retrying a delayed filter-process request, we don't
need to send the blob to the filter a second time. However,
we read it unconditionally into a buffer, only to later
throw away that buffer. We can make this more efficient by
skipping the read in the first place when it isn't
necessary.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When write_entry() retries a delayed filter request, we
don't need to send the blob content to the filter again, and
set the pointer to NULL. But doing so means we leak the
contents we read earlier from read_blob_entry(). Let's make
sure to free it before dropping the pointer.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A common mistake when writing binary search is to allow possible
integer overflow by using the simple average:
mid = (min + max) / 2;
Instead, use the overflow-safe version:
mid = min + (max - min) / 2;
This translation is safe since the operation occurs inside a loop
conditioned on "min < max". The included changes were found using
the following git grep:
git grep '/ *2;' '*.c'
Making this cleanup will prevent future review friction when a new
binary search is contructed based on existing code.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The message spans over 2 lines but the C conconcatenation does not add
the needed space between the two lines.
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
