This not only prevents regressions, but also serves as documentation
what this new feature is expected to do.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Factor out the commonalities from test_submodule_switch() and
test_submodule_forced_switch() in lib-submodule-update.sh, and document
their usage.
This also makes explicit (through the KNOWN_FAILURE_FORCED_SWITCH_TESTS
variable) the fact that, currently, all functionality tested using
test_submodule_forced_switch() do not correctly handle the situation in
which a submodule is replaced with an ordinary directory.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A new option --ignore-cr-at-eol tells the diff machinery to treat a
carriage-return at the end of a (complete) line as if it does not
exist.
Just like other "--ignore-*" options to ignore various kinds of
whitespace differences, this will help reviewing the real changes
you made without getting distracted by spurious CRLF<->LF conversion
made by your editor program.
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
[jch: squashed in command line completion by Dscho]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since ff6f1f564 (submodule-config: lazy-load a repository's .gitmodules
file, 2017-08-03) rebase interactive fails if there are any submodules
with unstaged changes which have been configured with a value for
'submodule.<name>.ignore' in the repository's config.
This is due to how configured values of 'submodule.<name>.ignore' are
handled in addition to a change in how the submodule config is loaded.
When the diff machinery hits a submodule (gitlink as well as a
corresponding entry in the submodule subsystem) it will read the value
of 'submodule.<name>.ignore' stored in the repository's config and if
the config is present it will clear the 'IGNORE_SUBMODULES' (which is
the flag explicitly requested by rebase interactive),
'IGNORE_UNTRACKED_IN_SUBMODULES', and 'IGNORE_DIRTY_SUBMODULES' diff
flags and then set one of them based on the configured value.
Historically this wasn't a problem because the submodule subsystem
wasn't initialized because the .gitmodules file wasn't explicitly loaded
by the rebase interactive command. So when the diff machinery hit a
submodule it would skip over reading any configured values of
'submodule.<name>.ignore'.
In order to preserve the behavior of submodules being ignored by rebase
interactive, also set the 'OVERRIDE_SUBMODULE_CONFIG' diff flag when
submodules are requested to be ignored when checking for unstaged
changes.
Reported-by: Orgad Shaneh <orgads@gmail.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
And while at it let's simplify t0021/rot13-filter.pl by
using Git/Packet.pm.
This will make it possible to reuse packet related
functions in other test scripts.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These function help read and write capabilities.
To make them more generic and make it easy to reuse them,
the following changes are made:
- we don't require capabilities to come in a fixed order,
- we allow duplicates,
- we check that the remote supports the capabilities we
advertise,
- we don't check if the remote declares any capability we
don't know about.
The reason behind the last change is that the protocol
should work using only the capabilities that both ends
support, and it should not stop working if one end starts
to advertise a new capability.
Despite those changes, we can still require a set of
capabilities, and die if one of them is not supported.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As checking for a lf character at the end of a buffer
will be useful in another function, let's refactor this
functionality into a small remove_final_lf_or_die()
helper function.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Let's refactor the code to initialize communication into its own
packet_initialize() function, so that we can reuse this
functionality in following patches.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If there is no new line at the end of something it receives,
the packet_txt_read() function die()s, but it's difficult to
debug without much context.
Let's give a bit more information when that happens.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before further refactoring the "t0021/rot13-filter.pl" script,
let's modernize the style of its 'if .. elsif .. else' clauses
to improve its readability by making it more similar to our
other perl scripts.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To make it possible in a following commit to move packet
reading and writing functions into a Packet.pm module,
let's refactor these functions, so they don't handle
printing debug output and exiting.
While at it let's create packet_required_key_val_read()
to still handle erroring out in a common case.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since edcc8581 ("convert: add filter.<driver>.process
option", 2016-10-16) when t0021/rot13-filter.pl was created, list
comparison in this perl script have been quite broken.
packet_txt_read() returns a 2-element list, and the right hand
side of "eq" also has a list with (two, elements), but "eq" takes
the last element of the list on each side, and compares them. The
first elements (0 or 1) on the right hand side lists do not matter,
which means we do not require to see a flush at the end of the
version -- a simple empty string or an EOF would do, which is
definitely not what we want.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "--push-option=<string>" option to "git push" now defaults to a
list of strings configured via push.pushOption variable.
* mp/push-pushoption-config:
builtin/push.c: add push.pushOption config
"git fetch --recurse-submodules" now knows that submodules can be
moved around in the superproject in addition to getting updated,
and finds the ones that need to be fetched accordingly.
* hv/fetch-moved-submodules-on-demand:
submodule: simplify decision tree whether to or not to fetch
implement fetching of moved submodules
fetch: add test to make sure we stay backwards compatible
"git check-ref-format --branch @{-1}" bit a "BUG()" when run
outside a repository for obvious reasons; clarify the documentation
and make sure we do not even try to expand the at-mark magic in
such a case, but still call the validation logic for branch names.
* jc/check-ref-format-oor:
check-ref-format doc: --branch validates and expands <branch>
check-ref-format --branch: strip refs/heads/ using skip_prefix
check-ref-format --branch: do not expand @{...} outside repository
Conversion from uchar[20] to struct object_id continues.
* bc/object-id: (25 commits)
refs/files-backend: convert static functions to object_id
refs: convert read_raw_ref backends to struct object_id
refs: convert peel_object to struct object_id
refs: convert resolve_ref_unsafe to struct object_id
worktree: convert struct worktree to object_id
refs: convert resolve_gitlink_ref to struct object_id
Convert remaining callers of resolve_gitlink_ref to object_id
sha1_file: convert index_path and index_fd to struct object_id
refs: convert reflog_expire parameter to struct object_id
refs: convert read_ref_at to struct object_id
refs: convert peel_ref to struct object_id
builtin/pack-objects: convert to struct object_id
pack-bitmap: convert traverse_bitmap_commit_list to object_id
refs: convert dwim_log to struct object_id
builtin/reflog: convert remaining unsigned char uses to object_id
refs: convert dwim_ref and expand_ref to struct object_id
refs: convert read_ref and read_ref_full to object_id
refs: convert resolve_refdup and refs_resolve_refdup to struct object_id
Convert check_connected to use struct object_id
refs: update ref transactions to use struct object_id
...
Optimize the code to find shortest unique prefix of object names.
* ds/find-unique-abbrev-optim:
sha1_name: minimize OID comparisons during disambiguation
sha1_name: parse less while finding common prefix
sha1_name: unroll len loop in find_unique_abbrev_r()
p4211-line-log.sh: add log --online --raw --parents perf test
"git pull" has been taught to accept "--[no-]signoff" option and
pass it down to "git merge".
* wk/pull-signoff:
pull: pass --signoff/--no-signoff to "git merge"
An early part of piece-by-piece rewrite of "git bisect".
* pb/bisect-helper:
bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
t6030: explicitly test for bisection cleanup
bisect--helper: `bisect_clean_state` shell function in C
bisect--helper: `write_terms` shell function in C
bisect--helper: rewrite `check_term_format` shell function in C
bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
A hook script that is set unexecutable is simply ignored. Git
notifies when such a file is ignored, unless the message is
squelched via advice.ignoredHook configuration.
* dm/run-command-ignored-hook-advise:
run-command: add hint when a hook is ignored
The final step to make an empty string as a pathspec element
illegal. We started this by first deprecating and warning a
pathspec that has such an element in 2.11 (Nov 2016).
Hopefully we can merge this down to the 'master' by the end of the
year? A deprecation warning period that is about 1 year does not
sound too bad.
* ex/deprecate-empty-pathspec-as-match-all:
pathspec: die on empty strings as pathspec
t0027: do not use an empty string as a pathspec element
A recent regression in "git rebase -i" that broke execution of git
commands from subdirectories via "exec" insn has been fixed.
* jk/rebase-i-exec-gitdir-fix:
sequencer: pass absolute GIT_DIR to exec commands
We try to see if somebody runs our test suite with a shell that
does not support "local" like bash/dash does.
* mh/test-local-canary:
t0000: check whether the shell supports the "local" keyword
"git status --ignored -u" did not stop at a working tree of a
separate project that is embedded in an ignored directory and
listed files in that other project, instead of just showing the
directory itself as ignored.
* js/submodule-in-excluded:
status: do not get confused by submodules in excluded directories
Some codepaths did not check for errors when asking what branch the
HEAD points at, which have been fixed.
* jk/misc-resolve-ref-unsafe-fixes:
worktree: handle broken symrefs in find_shared_symref()
log: handle broken HEAD in decoration check
remote: handle broken symrefs
test-ref-store: avoid passing NULL to printf
The experimental "color moved lines differently in diff output"
feature was buggy around "ignore whitespace changes" edges, whihch
has been corrected.
* jk/diff-color-moved-fix:
diff: handle NULs in get_string_hash()
diff: fix whitespace-skipping with --color-moved
t4015: test the output of "diff --color-moved -b"
t4015: check "negative" case for "-w --color-moved"
t4015: refactor --color-moved whitespace test
"auto" as a value for the columnar output configuration ought to
judge "is the output consumed by humans?" with the same criteria as
"auto" for coloured output configuration, i.e. either the standard
output stream is going to tty, or a pager is in use. We forgot the
latter, which has been fixed.
* kd/auto-col-with-pager-fix:
column: do not include pager.c
column: show auto columns when pager is active
All other error messages in the file use quotes around the file name.
This change removes two translations as "could not write to '%s'" and
"could not close '%s'" are already translated and these two are the only
occurrences without quotes.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
[jc: adjusted tests I noticed were broken by the change]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "2>&1" notation in Powershell and in Unix shells implies that stderr
is redirected to the same handle into which stdout is already written.
Let's use this special value to allow the same trick with
GIT_REDIRECT_STDERR and GIT_REDIRECT_STDOUT: if the former's value is
`2>&1`, then stderr will simply be written to the same handle as stdout.
The functionality was suggested by Jeff Hostetler.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Particularly when calling Git from applications, such as Visual Studio's
Team Explorer, it is important that stdin/stdout/stderr are closed
properly. However, when spawning processes on Windows, those handles
must be marked as inheritable if we want to use them, but that flag is a
global flag and may very well be used by other spawned processes which
then do not know to close those handles.
Let's introduce a set of environment variables (GIT_REDIRECT_STDIN and
friends) that specify paths to files, or even better, named pipes (which
are similar to Unix sockets) and that are used by the spawned Git
process. This helps work around above-mentioned issue: those named
pipes will be opened in a non-inheritable way upon startup, and no
handles are passed around (and therefore no inherited handles need to be
closed by any spawned child).
This feature shipped with Git for Windows (marked as experimental) since
v2.11.0(2), so it has seen some serious testing in the meantime.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we replaced the old shell script based interactive rebase in
commmit 18633e1a22 ("rebase -i: use the rebase--helper builtin",
2017-02-09) we introduced a regression of functionality in that the
GIT_DIR would be sent to the environment of the exec command as-is.
This generally meant that it would be passed as "GIT_DIR=.git", which
causes problems for any exec command that wants to run git commands in
a subdirectory.
This isn't a very large regression, since it is not that likely that the
exec command will run a git command, and even less likely that it will
need to do so in a subdir. This regression was discovered by a build
system which uses git-describe to find the current version of the build
system, and happened to do so from the src/ sub directory of the
project.
Fix this by passing in the absolute path of the git directory into the
child environment.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make sure the helper does not crash when blank username and password is
provided. If the helper can save such credentials, it should be able to
read them back.
Signed-off-by: Jakub Bereżański <kuba@berezanscy.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t5580 tests that specifying Windows UNC paths works with Git. Cygwin
supports UNC paths, albeit only using forward slashes, not backslashes,
so run the compatible tests on Cygwin as well as MinGW.
The only complication is Cygwin's `pwd`, which returns a *nix-style
path, and that's not suitable for calculating the UNC path to the
current directory. Instead use Cygwin's `cygpath` utility to get the
Windows-style path.
Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add tests around status reporting ignord files that match an exclude
pattern for both --untracked-files=normal and --untracked-files=all.
Signed-off-by: Jameson Miller <jamill@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a test balloon to see if we get complaints from anybody who is
using a shell that doesn't support the "local" keyword. If so, this
test can be reverted. If not, we might want to consider using "local"
in shell code throughout the git code base.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Even when we are deleting references, we needn't overwrite the
`packed-refs` file if the references that we are deleting only exist
as loose references. Implement this optimization as follows:
* Add a function `is_packed_transaction_needed()`, which checks
whether a given packed-refs transaction actually needs to be carried
out (i.e., it returns false if the transaction obviously wouldn't
have any effect). This function must be called while holding the
`packed-refs` lock to avoid races.
* Change `files_transaction_prepare()` to check whether the
packed-refs transaction is actually needed. If not, squelch it, but
continue holding the `packed-refs` lock until the end of the
transaction to avoid races.
This fixes a mild regression caused by dc39e09942 (files_ref_store:
use a transaction to update packed refs, 2017-09-08). Before that
commit, unnecessary rewrites of `packed-refs` were suppressed by
`repack_without_refs()`. But the transaction-based writing introduced
by that commit didn't perform that optimization.
Note that the pre-dc39e09942 code still had to *read* the whole
`packed-refs` file to determine that the rewrite could be skipped, so
the performance for the cases that the write could be elided was
`O(N)` in the number of packed references both before and after
dc39e09942. But after that commit the constant factor increased.
This commit reimplements the optimization of eliding unnecessary
`packed-refs` rewrites. That, plus the fact that since
cfa2e29c34 (packed_ref_store: get rid of the `ref_cache` entirely,
2017-03-17) we don't necessarily have to read the whole `packed-refs`
file at all, means that deletes of one or a few loose references can
now be done with `O(n lg N)` effort, where `n` is the number of loose
references being deleted and `N` is the total number of packed
references.
This commit fixes two tests in t1409.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no need to rewrite the `packed-refs` file except for the case
that we are deleting a reference that has a packed version. Verify
that `packed-refs` is not rewritten when it shouldn't be.
In fact, two of these tests fail:
* A new (empty) `packed-refs` file is created when deleting any loose
reference and no `packed-refs` file previously existed.
* The `packed-refs` file is rewritten unnecessarily when deleting a
loose reference that has no packed counterpart.
Both problems will be fixed in the next commit.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Transactions to update multiple references that involves a deletion
was quite broken in an error codepath and did not abort everything
correctly.
* mh/ref-locking-fix:
files_transaction_prepare(): fix handling of ref lock failure
t1404: add a bunch of tests of D/F conflicts
We meticulously pass the `exclude` flag to the `treat_directory()`
function so that we can indicate that files in it are excluded rather
than untracked when recursing.
But we did not yet treat submodules the same way.
Because of that, `git status --ignored --untracked` with a submodule
`submodule` in a gitignored `tracked/` would show the submodule in the
"Untracked files" section, e.g.
On branch master
Untracked files:
(use "git add <file>..." to include in what will be committed)
tracked/submodule/
Ignored files:
(use "git add -f <file>..." to include in what will be committed)
tracked/submodule/initial.t
Instead, we would want it to show the submodule in the "Ignored files"
section:
On branch master
Ignored files:
(use "git add -f <file>..." to include in what will be committed)
tracked/submodule/
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the commits 1fc458d9 (builtin/checkout: add --recurse-submodules
switch, 2017-03-14), 08d595dc (checkout: add --ignore-skip-worktree-bits
in sparse checkout mode, 2013-04-13) and 32669671 (checkout: introduce
--detach synonym for "git checkout foo^{commit}", 2011-02-08) checkout
gained new flags but the completion was not updated, although these flags
are useful completions. Add them.
The flags --force and --ignore-other-worktrees are not added as they are
potentially dangerous.
The flags --progress and --no-progress are only useful for scripting and are
therefore also not included.
Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since dc39e09942 (files_ref_store: use a transaction to update packed
refs, 2017-09-08), failure to lock a reference has been handled
incorrectly by `files_transaction_prepare()`. If
`lock_ref_for_update()` fails in the lock-acquisition loop of that
function, it sets `ret` then breaks out of that loop. Prior to
dc39e09942, that was OK, because the only thing following the loop was
the cleanup code. But dc39e09942 added another blurb of code between
the loop and the cleanup. That blurb sometimes resets `ret` to zero,
making the cleanup code think that the locking was successful.
Specifically, whenever
* One or more reference deletions have been processed successfully in
the lock-acquisition loop. (Processing the first such reference
causes a packed-ref transaction to be initialized.)
* Then `lock_ref_for_update()` fails for a subsequent reference. Such
a failure can happen for a number of reasons, such as the old SHA-1
not being correct, lock contention, etc. This causes a `break` out
of the lock-acquisition loop.
* The `packed-refs` lock is acquired successfully and
`ref_transaction_prepare()` succeeds for the packed-ref transaction.
This has the effect of resetting `ret` back to 0, and making the
cleanup code think that lock acquisition was successful.
In that case, any reference updates that were processed prior to
breaking out of the loop would be carried out (loose and packed), but
the reference that couldn't be locked and any subsequent references
would silently be ignored.
This can easily cause data loss if, for example, the user was trying
to push a new name for an existing branch while deleting the old name.
After the push, the branch could be left unreachable, and could even
subsequently be garbage-collected.
This problem was noticed in the context of deleting one reference and
creating another in a single transaction, when the two references D/F
conflict with each other, like
git update-ref --stdin <<EOF
delete refs/foo
create refs/foo/bar HEAD
EOF
This triggers the above bug because the deletion is processed
successfully for `refs/foo`, then the D/F conflict causes
`lock_ref_for_update()` to fail when `refs/foo/bar` is processed. In
this case the transaction *should* fail, but instead it causes
`refs/foo` to be deleted without creating `refs/foo`. This could
easily result in data loss.
The fix is simple: instead of just breaking out of the loop, jump
directly to the cleanup code. This fixes some tests in t1404 that were
added in the previous commit.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is currently not allowed, in a single transaction, to add one
reference and delete another reference if the two reference names D/F
conflict with each other (e.g., like `refs/foo/bar` and `refs/foo`).
The reason is that the code would need to take locks
$GIT_DIR/refs/foo.lock
$GIT_DIR/refs/foo/bar.lock
But the latter lock couldn't coexist with the loose reference file
$GIT_DIR/refs/foo
, because `$GIT_DIR/refs/foo` cannot be both a directory and a file at
the same time (hence the name "D/F conflict).
Add a bunch of tests that we cleanly reject such transactions.
In fact, many of the new tests currently fail. They will be fixed in
the next commit along with an explanation.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Push options need to be given explicitly, via the command line as "git
push --push-option <option>". Add the config option push.pushOption,
which is a multi-valued option, containing push options that are sent
by default.
When push options are set in the lower-priority configulation file
(e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
the more specific repository config by the empty string.
Add tests and update documentation as well.
Signed-off-by: Marius Paliga <marius.paliga@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many codepaths did not diagnose write failures correctly when disks
go full, due to their misuse of write_in_full() helper function,
which have been corrected.
* jk/write-in-full-fix:
read_pack_header: handle signed/unsigned comparison in read result
config: flip return value of store_write_*()
notes-merge: use ssize_t for write_in_full() return value
pkt-line: check write_in_full() errors against "< 0"
convert less-trivial versions of "write_in_full() != len"
avoid "write_in_full(fd, buf, len) != len" pattern
get-tar-commit-id: check write_in_full() return against 0
config: avoid "write_in_full(fd, buf, len) < len" pattern
The checkpoint command "git fast-import" did not flush updates to
refs and marks unless at least one object was created since the
last checkpoint, which has been corrected, as these things can
happen without any new object getting created.
* er/fast-import-dump-refs-on-checkpoint:
fast-import: checkpoint: dump branches/tags/marks even if object_count==0
"git fast-export" with -M/-C option issued "copy" instruction on a
path that is simultaneously modified, which was incorrect.
* jt/fast-export-copy-modify-fix:
fast-export: do not copy from modified file
"git branch -M a b" while on a branch that is completely unrelated
to either branch a or branch b misbehaved when multiple worktree
was in use. This has been fixed.
* nd/worktree-kill-parse-ref:
branch: fix branch renaming not updating HEADs correctly
It's possible for resolve_ref_unsafe() to return NULL (e.g.,
if we are reading and the ref does not exist), in which case
we'll pass NULL to printf. On glibc systems this produces
"(null)", but on others it may segfault.
The tests don't expect any such case, but if we ever did
trigger this, we would prefer to cleanly fail the test with
unexpected input rather than segfault. Let's manually
replace NULL with "(null)". The exact value doesn't matter,
as it won't match any possible ref the caller could expect
(and anyway, the exit code of the program will tell whether
"ref" is valid or not).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code for handling whitespace with --color-moved
represents partial strings as a pair of pointers. There are
two possible conventions for the end pointer:
1. It points to the byte right after the end of the
string.
2. It points to the final byte of the string.
But we seem to use both conventions in the code:
a. we assign the initial pointers from the NUL-terminated
string using (1)
b. we eat trailing whitespace by checking the second
pointer for isspace(), which needs (2)
c. the next_byte() function checks for end-of-string with
"if (cp > endp)", which is (2)
d. in next_byte() we skip past internal whitespace with
"while (cp < end)", which is (1)
This creates fewer bugs than you might think, because there
are some subtle interactions. Because of (a) and (c), we
always return the NUL-terminator from next_byte(). But all
of the callers of next_byte() happen to handle that
gracefully.
Because of the mismatch between (d) and (c), next_byte()
could accidentally return a whitespace character right at
endp. But because of the interaction of (a) and (b), we fail
to actually chomp trailing whitespace, meaning our endp
_always_ points to a NUL, canceling out the problem.
But that does leave (b) as a real bug: when ignoring
whitespace only at the end-of-line, we don't correctly trim
it, and fail to match up lines.
We can fix the whole thing by moving consistently to one
convention. Since convention (1) is idiomatic in our code
base, we'll pick that one.
The existing "-w" and "-b" tests continue to pass, and a new
"--ignore-space-at-eol" shows off the breakage we're fixing.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit fa5ba2c1dd (diff: fix infinite loop with
--color-moved --ignore-space-change, 2017-10-12) added a
test to make sure that "--color-moved -b" doesn't run
forever, but the test in question doesn't actually have any
moved lines in it.
Let's scrap that test and add a variant of the existing
"--color-moved -w" test, but this time we'll check that we
find the move with whitespace changes, but not arbitrary
whitespace additions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We test that lines with whitespace changes are not found by
"--color-moved" by default, but are found if "-w" is added.
Let's add one more twist: a line that has non-whitespace
changes should not be marked as a pure move.
This is perhaps an obvious case for us to get right (and we
do), but as we add more whitespace tests, they will form a
pattern of "make sure this case is a move and this other
case is not".
Note that we have to add a line to our moved block, since
having a too-small block doesn't trigger the "moved"
heuristics. And we also add a line of context to ensure
that there's more context lines than moved lines (so the
diff shows us moving the lines up, rather than moving the
context down).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for testing several different whitespace
options, let's split out the setup and cleanup steps of the
whitespace test.
While we're here, let's also switch to using "<<-" to indent
our here-documents properly, and use q_to_tab to more
explicitly mark where we expect whitespace to appear.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is the "theoretically more correct" approach of simply
stepping back to the state before plumbing commands started paying
attention to "color.ui" configuration variable.
* jk/ref-filter-colors-fix:
tag: respect color.ui config
Revert "color: check color.ui in git_default_config()"
Revert "t6006: drop "always" color config tests"
Revert "color: make "always" the same as "auto" in config"
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
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()
In the "--format=..." option of the "git for-each-ref" command (and
its friends, i.e. the listing mode of "git branch/tag"), "%(atom:)"
(e.g. "%(refname:)", "%(body:)" used to error out. Instead, treat
them as if the colon and an empty string that follows it were not
there.
* tb/ref-filter-empty-modifier:
ref-filter.c: pass empty-string as NULL to atom parsers
Spell the name of our system as "Git" in the output from
request-pull script.
* ar/request-pull-phrasofix:
request-pull: capitalise "Git" to make it a proper noun
"git describe --match" learned to take multiple patterns in v2.13
series, but the feature ignored the patterns after the first one
and did not work at all. This has been fixed.
* jk/describe-omit-some-refs:
describe: fix matching to actually match all patterns
"git archive", especially when used with pathspec, stored an empty
directory in its output, even though Git itself never does so.
This has been fixed.
* rs/archive-excluded-directory:
archive: don't add empty directories to archives
In addition to "cc: <a@dd.re.ss> # cruft", "cc: a@dd.re.ss # cruft"
was taught to "git send-email" as a valid way to tell it that it
needs to also send a carbon copy to <a@dd.re.ss> in the trailer
section.
* mm/send-email-cc-cruft:
send-email: don't use Mail::Address, even if available
send-email: fix garbage removal after address
This is the "theoretically more correct" approach of simply
stepping back to the state before plumbing commands started paying
attention to "color.ui" configuration variable.
Let's run with this one.
* jk/ref-filter-colors-fix:
tag: respect color.ui config
Revert "color: check color.ui in git_default_config()"
Revert "t6006: drop "always" color config tests"
Revert "color: make "always" the same as "auto" in config"
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>
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>
When using the 'ssh' transport, the '-o' option is used to specify an
environment variable which should be set on the remote end. This allows
git to send additional information when contacting the server,
requesting the use of a different protocol version via the
'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL".
Unfortunately not all ssh variants support the sending of environment
variables to the remote end. To account for this, only use the '-o'
option for ssh variants which are OpenSSH compliant. This is done by
checking that the basename of the ssh command is 'ssh' or the ssh
variant is overridden to be 'ssh' (via the ssh.variant config).
Other options like '-p' and '-P', which are used to specify a specific
port to use, or '-4' and '-6', which are used to indicate that IPV4 or
IPV6 addresses should be used, may also not be supported by all ssh
variants.
Currently if an ssh command's basename wasn't 'plink' or
'tortoiseplink' git assumes that the command is an OpenSSH variant.
Since user configured ssh commands may not be OpenSSH compliant, tighten
this constraint and assume a variant of 'simple' if the basename of the
command doesn't match the variants known to git. The new ssh variant
'simple' will only have the host and command to execute ([username@]host
command) passed as parameters to the ssh command.
Update the Documentation to better reflect the command-line options sent
to ssh commands based on their variant.
Reported-by: Jeffrey Yasskin <jyasskin@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Tell a server that protocol v1 can be used by sending the http header
'Git-Protocol' with 'version=1' indicating this.
Also teach the apache http server to pass through the 'Git-Protocol'
header as an environment variable 'GIT_PROTOCOL'.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach the connection logic to tell a serve that it understands protocol
v1. This is done in 2 different ways for the builtin transports, both
of which ultimately set 'GIT_PROTOCOL' to 'version=1' on the server.
1. git://
A normal request to git-daemon is structured as
"command path/to/repo\0host=..\0" and due to a bug introduced in
49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we
aren't able to place any extra arguments (separated by NULs) besides
the host otherwise the parsing of those arguments would enter an
infinite loop. This bug was fixed in 73bb33a94 (daemon: Strictly
parse the "extra arg" part of the command, 2009-06-04) but a check
was put in place to disallow extra arguments so that new clients
wouldn't trigger this bug in older servers.
In order to get around this limitation git-daemon was taught to
recognize additional request arguments hidden behind a second
NUL byte. Requests can then be structured like:
"command path/to/repo\0host=..\0\0version=1\0key=value\0".
git-daemon can then parse out the extra arguments and set
'GIT_PROTOCOL' accordingly.
By placing these extra arguments behind a second NUL byte we can
skirt around both the infinite loop bug in 49ba83fb6 (Add
virtualization support to git-daemon, 2006-09-19) as well as the
explicit disallowing of extra arguments introduced in 73bb33a94
(daemon: Strictly parse the "extra arg" part of the command,
2009-06-04) because both of these versions of git-daemon check for a
single NUL byte after the host argument before terminating the
argument parsing.
2. ssh://, file://
Set 'GIT_PROTOCOL' environment variable with the desired protocol
version. With the file:// transport, 'GIT_PROTOCOL' can be set
explicitly in the locally running git-upload-pack or git-receive-pack
processes. With the ssh:// transport and OpenSSH compliant ssh
programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o
SendEnv=GIT_PROTOCOL' and having the server whitelist this
environment variable.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We store the changed submodules paths to calculate which submodule needs
fetching. This does not work for moved submodules since their paths do
not stay the same in case of a moved submodules. In case of new
submodules we do not have a path in the current checkout, since they
just appeared in this fetch.
It is more general to collect the submodule names for changes instead of
their paths to include the above cases. If we do not have a
configuration for a gitlink we rely on constructing a default name from
the path if a git repository can be found at its path. We skip
non-configured gitlinks whose default name collides with a configured
one.
With the change described above we implement 'on-demand' fetching of
changes in moved submodules.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
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>
Convert resolve_ref_unsafe to take a pointer to struct object_id by
converting one remaining caller to use struct object_id, removing the
temporary NULL pointer check in expand_ref, converting the declaration
and definition, and applying the following semantic patch:
@@
expression E1, E2, E3, E4;
@@
- resolve_ref_unsafe(E1, E2, E3.hash, E4)
+ resolve_ref_unsafe(E1, E2, &E3, E4)
@@
expression E1, E2, E3, E4;
@@
- resolve_ref_unsafe(E1, E2, E3->hash, E4)
+ resolve_ref_unsafe(E1, E2, E3, E4)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert peel_ref (and its corresponding backend) to struct object_id.
This transformation was done with an update to the declaration,
definition, comments, and test helper and the following semantic patch:
@@
expression E1, E2;
@@
- peel_ref(E1, E2.hash)
+ peel_ref(E1, &E2)
@@
expression E1, E2;
@@
- peel_ref(E1, E2->hash)
+ peel_ref(E1, E2)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert update_ref, refs_update_ref, and write_pseudoref to use struct
object_id. Update the existing callers as well. Remove update_ref_oid,
as it is no longer needed.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert delete_ref and refs_delete_ref to take a pointer to struct
object_id. Update the documentation accordingly, including referring to
null_oid in lowercase, as it is not a #define constant.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge can take --signoff, but without pull passing --signoff down, it
is inconvenient to use; allow 'pull' to take the option and pass it
through.
The order of options in merge-options.txt is mostly alphabetical by
long option since 7c85d274 (Documentation/merge-options.txt: order
options in alphabetical groups, 2009-10-22). The long-option bit
didn't make it into the commit message, but it's under the fold in
[1]. I've put --signoff between --log and --stat to preserve the
alphabetical order.
[1]: https://public-inbox.org/git/87iqe7zspn.fsf@jondo.cante.net/
Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a new perf test for testing the performance of log while computing
OID abbreviations. Using --oneline --raw and --parents options maximizes
the number of OIDs to abbreviate while still spending some time computing
diffs.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
When an hook is present but the file is not set as executable then git will
ignore the hook.
For now this is silent which can be confusing.
This commit adds this warning to improve the situation:
hint: The 'pre-commit' hook was ignored because it's not set as executable.
hint: You can disable this warning with `git config advice.ignoredHook false`
To allow the old use-case of enabling/disabling hooks via the executable flag a
new setting is introduced: advice.ignoredHook.
Signed-off-by: Damien Marié <damien@dam.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current implementation of submodules supports on-demand fetch if
there is no .gitmodules entry for a submodule. Let's add a test to
document this behavior.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the "--format=..." option of the "git for-each-ref" command (and
its friends, i.e. the listing mode of "git branch/tag"), "%(atom:)"
(e.g. "%(refname:)", "%(body:)" used to error out. Instead, treat
them as if the colon and an empty string that follows it were not
there.
* tb/ref-filter-empty-modifier:
ref-filter.c: pass empty-string as NULL to atom parsers
Fix regression of "git add -p" for users with "color.ui = always"
in their configuration, by merging the topic below and adjusting it
for the 'master' front.
* jk/ui-color-always-to-auto:
t7301: use test_terminal to check color
t4015: use --color with --color-moved
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
The feature that allows --pretty='%(trailers)' to take modifiers
like "fold" and "only" used to separate these modifiers with a
comma, i.e. "%(trailers:fold:only)", but we changed our mind and
use a comma, i.e. "%(trailers:fold,only)". Fast track this change
before this new feature becomes part of any official release.
* tb/delimit-pretty-trailers-args-with-comma:
pretty.c: delimit "%(trailers)" arguments with ","
Fix the argument order for test_cmp. When given the expected
result first the diff shows the actual output with '+' and the
expectation with '-', which is the convention for our tests.
Signed-off-by: Stefan Beller <sbeller@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If our call to refs_read_raw_ref() fails, we check errno to
see if the ref is simply missing, or if we encountered a
more serious error. If it's just missing, then in "write"
mode (i.e., when RESOLVE_REFS_READING is not set), this is
perfectly fine.
However, checking for ENOENT isn't sufficient to catch all
missing-ref cases. In the filesystem backend, we may also
see EISDIR when we try to resolve "a" and "a/b" exists.
Likewise, we may see ENOTDIR if we try to resolve "a/b" and
"a" exists. In both of those cases, we know that our
resolved ref doesn't exist, but we return an error (rather
than reporting the refname and returning a null sha1).
This has been broken for a long time, but nobody really
noticed because the next step after resolving without the
READING flag is usually to lock the ref and write it. But in
both of those cases, the write will fail with the same
errno due to the directory/file conflict.
There are two cases where we can notice this, though:
1. If we try to write "a" and there's a leftover directory
already at "a", even though there is no ref "a/b". The
actual write is smart enough to move the empty "a" out
of the way.
This is reasonably rare, if only because the writing
code has to do an independent resolution before trying
its write (because the actual update_ref() code handles
this case fine). The notes-merge code does this, and
before the fix in the prior commit t3308 erroneously
expected this case to fail.
2. When resolving symbolic refs, we typically do not use
the READING flag because we want to resolve even
symrefs that point to unborn refs. Even if those unborn
refs could not actually be written because of d/f
conflicts with existing refs.
You can see this by asking "git symbolic-ref" to report
the target of a symref pointing past a d/f conflict.
We can fix the problem by recognizing the other "missing"
errnos and treating them like ENOENT. This should be safe to
do even for callers who are then going to actually write the
ref, because the actual writing process will fail if the d/f
conflict is a real one (and t1404 checks these cases).
Arguably this should be the responsibility of the
files-backend to normalize all "missing ref" errors into
ENOENT (since something like EISDIR may not be meaningful at
all to a database backend). However other callers of
refs_read_raw_ref() may actually care about the distinction;
putting this into resolve_ref() is the minimal fix for now.
The new tests in t1401 use git-symbolic-ref, which is the
most direct way to check the resolution by itself.
Interestingly we actually had a test that setup this case
already, but we only used it to verify that the funny state
could be overwritten, not that it could be resolved.
We also add a new test in t3200, as "branch -m" was the
original motivation for looking into this. What happens is
this:
0. HEAD is pointing to branch "a"
1. The user asks to rename "a" to "a/b".
2. We create "a/b" and delete "a".
3. We then try to update any worktree HEADs that point to
the renamed ref (including the main repo HEAD). To do
that, we have to resolve each HEAD. But now our HEAD is
pointing at "a", and we get EISDIR due to the loose
"a/b". As a result, we think there is no HEAD, and we
do not update it. It now points to the bogus "a".
Interestingly this case used to work, but only accidentally.
Before 31824d180d (branch: fix branch renaming not updating
HEADs correctly, 2017-08-24), we'd update any HEAD which we
couldn't resolve. That was wrong, but it papered over the
fact that we were incorrectly failing to resolve HEAD.
So while the bug demonstrated by the git-symbolic-ref is
quite old, the regression to "branch -m" is recent.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A test in t3308 wants to make sure that we don't
accidentally merge into "refs/notes/dir" when it exists as a
directory, so it does:
mkdir .git/refs/notes/dir
git -c core.notesRef=refs/notes/dir merge ...
and expects the second command to fail. But that
understimates the refs code, which is smart enough to remove
useless directories in the refs hierarchy. The test
succeeded only because of a bug which prevented resolving
refs/notes/dir for writing, even though an actual ref update
would succeed.
In preparation for fixing that bug, let's switch to creating
a real ref in refs/notes/dir, which is a more realistic
situation.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add test to explicitly check that 'git bisect reset' is working as
expected. This is already covered implicitly by the test suite.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
lookup_blob() and lookup_tree() can return NULL if they find an object
of an unexpected type. Accessing the object member is undefined in that
case. Cast the result to a struct object pointer instead; we can do
that because object is the first member of all object types. This trick
is already used in other places in the code.
An error message is already shown by object_as_type(), which is called
by the lookup functions. The walk callback functions are expected to
handle NULL object pointers passed to them, but put_object_name() needs
a valid object, so avoid calling it without one.
Suggested-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Spell the name of our system as "Git" in the output from
request-pull script.
* ar/request-pull-phrasofix:
request-pull: capitalise "Git" to make it a proper noun
The checkpoint command "git fast-import" did not flush updates to
refs and marks unless at least one object was created since the
last checkpoint, which has been corrected, as these things can
happen without any new object getting created.
* er/fast-import-dump-refs-on-checkpoint:
fast-import: checkpoint: dump branches/tags/marks even if object_count==0
Peff points out that different atom parsers handle the empty
"sub-argument" list differently. An example of this is the format
"%(refname:)".
Since callers often use `string_list_split` (which splits the empty
string with any delimiter as a 1-ary string_list containing the empty
string), this makes handling empty sub-argument strings non-ergonomic.
Let's fix this by declaring that atom parser implementations must
not care about distinguishing between the empty string "%(refname:)"
and no sub-arguments "%(refname)". Current code aborts, either with
"unrecognised arg" (e.g. "refname:") or "does not take args"
(e.g. "body:") as an error message.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Reviewed-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update the test fsmonitor-watchman integration script to properly
preserve utf8 filenames when outputting the .git/watchman-output.out log
file.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In Perl, setting $/ sets the string that is used as the "record
separator," which sets the boundary that the `<>` construct reads to.
Setting `local $/ = 0666;` evaluates the octal, getting 438, and
stringifies it. Thus, the later read from `<CHLD_OUT>` stops as soon
as it encounters the string "438" in the watchman output, yielding
invalid JSON; repositories containing filenames with SHA1 hashes are
able to trip this easily.
Set `$/` to undefined, thus slurping all output from watchman. Also
close STDIN which is provided to watchman, to better guarantee that we
cannot deadlock with watchman while both attempting to read.
Signed-off-by: Alex Vandiver <alexmv@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Check if the strbuf containing data to sort is empty before attempting
to trim a trailing newline character.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jk/ui-color-always-to-auto-maint:
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
This test wants to confirm that "clean -i" shows color
output. Using test_terminal gives us a more realistic
environment than "color.ui=always", and prepares us for the
behavior of "always" changing in a future patch.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tests for --color-moved write their output to a file,
but doing so suppresses color output under "auto". Right now
this is solved by running the whole script under
"color.diff=always". In preparation for the behavior of
"always" changing, let's explicitly enable color.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It can be handy to use `--color=always` (or it's synonym
`--color`) on the command-line to convince a command to
produce color even if it's stdout isn't going to the
terminal or a pager.
What's less clear is whether it makes sense to set config
variables like color.ui to `always`. For a one-shot like:
git -c color.ui=always ...
it's potentially useful (especially if the command doesn't
directly support the `--color` option). But setting `always`
in your on-disk config is much muddier, as you may be
surprised when piped commands generate colors (and send them
to whatever is consuming the pipe downstream).
Some people have done this anyway, because:
1. The documentation for color.ui makes it sound like
using `always` is a good idea, when you almost
certainly want `auto`.
2. Traditionally not every command (and especially not
plumbing) respected color.ui in the first place. So
the confusion came up less frequently than it might
have.
The situation changed in 136c8c8b8f (color: check color.ui
in git_default_config(), 2017-07-13), which negated point
(2): now scripts using only plumbing commands (like
add-interactive) are broken by this setting.
That commit was fixing real issues (e.g., by making
`color.ui=never` work, since `auto` is the default), so we
don't want to just revert it. We could turn `always` into a
noop in plumbing commands, but that creates a hard-to-explain
inconsistency between the plumbing and other commands.
Instead, let's just turn `always` into `auto` for all config.
This does break the "one-shot" config shown above, but again,
we're probably better to have simple and consistent rules than
to try to special-case command-line config.
There is one place where `always` should retain its meaning:
on the command line, `--color=always` should continue to be
the same as `--color`, overriding any isatty checks. Since the
command-line parser also depends on git_config_colorbool(), we
can use the existence of the "var" string to deterine whether
we are serving the command-line or the config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When ref-filter learned about want_color() in 11b087adfd
(ref-filter: consult want_color() before emitting colors,
2017-07-13), it became useful to be able to turn colors off
and on for specific commands. For git-branch, you can do so
with --color/--no-color.
But for git-for-each-ref and git-tag, the other users of
ref-filter, you have no option except to tweak the
"color.ui" config setting. Let's give both of these commands
the usual color command-line options.
This is a bit more obvious as a method for overriding the
config. And it also prepares us for the behavior of "always"
changing (so that we are still left with a way of forcing
color when our output goes to a non-terminal).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To test the color output, we must convince "git branch" to
write colors to a non-terminal. We do that now by setting
the color config to "always". In preparation for the
behavior of "always" changing, let's switch to using the
"--color" command-line option, which is more direct.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for the behavior of "always" changing to
match "auto", we can simply drop this test. We already check
other forms (like "--color") independently.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We test the %C() format placeholders with a variety of
color-inducing options, including "--color" and
"-c color.ui=always". In preparation for the behavior of
"always" changing, we need to do something with those
"always" tests.
We can drop ones that expect "always" to turn on color even
to a file, as that will become a synonym for "auto", which
is already tested.
For the "--no-color" test, we need to make sure that color
would otherwise be shown. To do this, we can use
test_terminal, which enables colors in the default setup.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To check that "status -v" respects diff config, we set
"color.diff" and look at the output of "status". We could
equally well use any diff config. Since color output depends
on a lot of other factors (like whether stdout is a tty, and
how we interpret "always"), let's use a more mundane option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This script tests the output of status with various formats
when color is enabled. It uses the "always" setting so that
the output is valid even though we capture it in a file.
Using test_terminal gives us a more realistic environment,
and prepares us for the behavior of "always" changing.
Arguably we are testing less than before, since "auto" is
already the default, and we can no longer tell if the config
is actually doing anything.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When testing whether "add -p" can generate colors, we set
color.ui to "always". This isn't a very good test, as in the
real-world a user typically has "auto" coupled with stdout
going to a terminal (and it's plausible that this could mask
a real bug in add--interactive if we depend on plumbing's
isatty check).
Let's switch to test_terminal, which gives us a more
realistic environment. This also prepare us for future
changes to the "always" color option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t4015 contains many color-related tests which need to
override the "is stdout a tty" check. They do so by setting
the color.diff config, but we can accomplish the same with
the --color option. Besides being shorter to type, switching
will prepare us for upcoming changes to "always" when see it
in config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of the test-terminal script is to simulate in the
test scripts an environment where output is going to a real
terminal.
But since test-lib.sh also sets TERM=dumb, the simulation
isn't very realistic. The color code will skip auto-coloring
for TERM=dumb, leading to us liberally sprinkling
test_terminal env TERM=vt100 git ...
through the test suite to convince the tests to actually
generate colors. Let's set TERM for programs run under
test_terminal, which is one less thing for test-writers to
remember.
In most cases the callers can be simplified, but note there
is one interesting case in t4202. It uses test_terminal to
check the auto-enabling of --decorate, but the expected
output _doesn't_ contain colors (because TERM=dumb
suppresses them). Using TERM=vt100 is closer to what the
real world looks like; adjust the expected output to match.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of just taking $ENV{'PWD'}, use the same logic that converts
PWD to $git_work_tree on MSYS_NT in the watchman integration hook
script also on MINGW.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The output from "git diff --summary" was broken in a recent topic
that has been merged to 'master' and lost a LF after reporting of
mode change. This has been fixed.
* sb/diff-color-move:
diff: correct newline in summary for renamed files
Some commands (most notably "git status") makes an opportunistic
update when performing a read-only operation to help optimize later
operations in the same repository. The new "--no-optional-locks"
option can be passed to Git to disable them.
* jk/no-optional-locks:
git: add --no-optional-locks option
"git branch" learned "-c/-C" to create a new branch by copying an
existing one.
* sd/branch-copy:
branch: fix "copy" to never touch HEAD
branch: add a --copy (-c) option to go with --move (-m)
branch: add test for -m renaming multiple config sections
config: create a function to format section headers
Recent versions of "git rev-parse --parseopt" did not parse the
option specification that does not have the optional flags (*=?!)
correctly, which has been corrected.
* bc/rev-parse-parseopt-fix:
parse-options: only insert newline in help text if needed
parse-options: write blank line to correct output stream
t0040,t1502: Demonstrate parse_options bugs
git-rebase: don't ignore unexpected command line arguments
rev-parse parseopt: interpret any whitespace as start of help text
rev-parse parseopt: do not search help text for flag chars
t1502: demonstrate rev-parse --parseopt option mis-parsing
The final batch to "git rebase -i" updates to move more code from
the shell script to C.
* js/rebase-i-final:
rebase -i: rearrange fixup/squash lines using the rebase--helper
t3415: test fixup with wrapped oneline
rebase -i: skip unnecessary picks using the rebase--helper
rebase -i: check for missing commits in the rebase--helper
t3404: relax rebase.missingCommitsCheck tests
rebase -i: also expand/collapse the SHA-1s via the rebase--helper
rebase -i: do not invent onelines when expanding/collapsing SHA-1s
rebase -i: remove useless indentation
rebase -i: generate the script via rebase--helper
t3415: verify that an empty instructionFormat is handled as before
Of the many ways to spell the three-letter word, the variant "Git"
should be used when referring to a repository in a description; or, in
general, when it is used as a proper noun.
We thus change the pull-request template message so that it reads
"...in the Git repository at:"
Besides, this brings us in line with the documentation, see
Documentation/howto/using-signed-tag-in-pull-request.txt
Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The %(contents) atom takes a contents "field" as its argument. Since
"trailers" is one of those fields, extend contents_atom_parser to parse
"trailers"'s arguments when used through "%(contents)", like:
%(contents:trailers:unfold,only)
A caveat: trailers_atom_parser expects NULL when no arguments are given
(see: `parse_ref_filter_atom`). This is because string_list_split (given
a maxsplit of -1) returns a 1-ary string_list* containing the given
string if the delimiter could not be found using `strchr`.
To simulate this behavior without teaching trailers_atom_parser to
accept strings with length zero, conditionally pass NULL to
trailers_atom_parser if the arguments portion of the argument to
%(contents) is empty.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fill trailer_opts with "unfold" and "only" to match the sub-arguments
given to the "%(trailers)" atom. Then, let's use the filled trailer_opts
instance with 'format_trailers_from_commit' in order to format trailers
in the desired manner.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We currently have one test for %(trailers) in `git-for-each-ref(1)`,
through "%(contents:trailers)". In preparation for more, let's add a few
things:
- Move the commit creation step to its own test so that it can be
re-used.
- Add a non-trailer to the commit's trailers to test that non-trailers
aren't shown using "%(trailers:only)".
- Add a multi-line trailer to ensure that trailers are unfolded
correctly using "%(trailers:unfold)".
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Tests in t4205 test the following:
git log --format='%(trailers:unfold)' ...
By ensuring the multi-line trailers are unfolded back onto the same
line. t4205 only includes tests for 2-line trailers, but `unfold()` will
fail for folded trailers on 3 or more lines.
In preparation for adding subsequent tests in t6300 that test similar
behavior in `git-for-each-ref(1)`, let's harden t4205 (and make it
consistent with the changes in t6300) by ensuring that 3 or more
line folded trailers are unfolded correctly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for adding consistent "%(trailers)" atom options to
`git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in
pretty.c to separate sub-arguments with a ",", instead of a ":".
Multiple sub-arguments are given either as "%(trailers:unfold,only)" or
"%(trailers:only,unfold)".
This change disambiguates between "top-level" arguments, and arguments
given to the trailers atom itself. It is consistent with the behavior of
"%(upstream)" and "%(push)" atoms.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a test utility (test-drop-caches) that flushes all changes to disk
then drops file system cache on Windows, Linux, and OSX.
Add a perf test (p7519-fsmonitor.sh) for fsmonitor.
By default, the performance test will utilize the Watchman file system
monitor if it is installed. If Watchman is not installed, it will use a
dummy integration script that does not report any new or modified files.
The dummy script has very little overhead which provides optimistic results.
The performance test will also use the untracked cache feature if it is
available as fsmonitor uses it to speed up scanning for untracked files.
There are 4 environment variables that can be used to alter the default
behavior of the performance test:
GIT_PERF_7519_UNTRACKED_CACHE: used to configure core.untrackedCache
GIT_PERF_7519_SPLIT_INDEX: used to configure core.splitIndex
GIT_PERF_7519_FSMONITOR: used to configure core.fsmonitor
GIT_PERF_7519_DROP_CACHE: if set, the OS caches are dropped between tests
The big win for using fsmonitor is the elimination of the need to scan the
working directory looking for changed and untracked files. If the file
information is all cached in RAM, the benefits are reduced.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test the ability to add/remove the fsmonitor index extension via
update-index.
Test that dirty files returned from the integration script are properly
represented in the index extension and verify that ls-files correctly
reports their state.
Test that ensure status results are correct when using the new fsmonitor
extension. Test untracked, modified, and new files by ensuring the
results are identical to when not using the extension.
Test that if the fsmonitor extension doesn't tell git about a change, it
doesn't discover it on its own. This ensures git is honoring the
extension and that we get the performance benefits desired.
Three test integration scripts are provided:
fsmonitor-all - marks all files as dirty
fsmonitor-none - marks no files as dirty
fsmonitor-watchman - integrates with Watchman with debug logging
To run tests in the test suite while utilizing fsmonitor:
First copy t/t7519/fsmonitor-all to a location in your path and then set
GIT_FORCE_PRELOAD_TEST=true and GIT_FSMONITOR_TEST=fsmonitor-all and run
your tests.
Note: currently when using the test script fsmonitor-watchman on
Windows, many tests fail due to a reported but not yet fixed bug in
Watchman where it holds on to handles for directories and files which
prevents the test directory from being cleaned up properly.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The split index test t1700-split-index.sh has hard coded SHA values for
the index. Currently it supports index V4 and V3 but assumes there are
no index extensions loaded.
When manually forcing the fsmonitor extension to be turned on when
running the test suite, the SHA values no longer match which causes the
test to fail.
The potential matrix of index extensions and index versions can is quite
large so instead temporarily disable the extension before attempting to
run the test until the underlying problem of hard coded SHA values is fixed.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a test utility (test-dump-fsmonitor) that will dump the fsmonitor
index extension.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checkpoint command cycles packfiles if object_count != 0, a sensible
test or there would be no pack files to write. Since 820b931012, the
command also dumps branches, tags and marks, but still conditionally.
However, it is possible for a command stream to modify refs or create
marks without creating any new objects.
For example, reset a branch (and keep fast-import running):
$ git fast-import
reset refs/heads/master
from refs/heads/master^
checkpoint
but refs/heads/master remains unchanged.
Other example: a commit command that re-creates an object that already
exists in the object database.
The man page also states that checkpoint "updates the refs" and that
"placing a progress command immediately after a checkpoint will inform
the reader when the checkpoint has been completed and it can safely
access the refs that fast-import updated". This wasn't always true
without this patch.
This fix unconditionally calls dump_{branches,tags,marks}() for all
checkpoint commands. dump_branches() and dump_tags() are cheap to call
in the case of a no-op.
Add tests to t9300 that observe the (non-packfiles) effects of
checkpoint.
Signed-off-by: Eric Rannaud <e@nanocritical.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git fast-export" with -M/-C option issued "copy" instruction on a
path that is simultaneously modified, which was incorrect.
* jt/fast-export-copy-modify-fix:
fast-export: do not copy from modified file
"git describe --match <pattern>" has been taught to play well with
the "--all" option.
* mk/describe-match-with-all:
describe: teach --match to handle branches and remotes
Many codepaths have been updated to squelch -Wimplicit-fallthrough
warnings from Gcc 7 (which is a good code hygiene).
* jk/fallthrough:
consistently use "fallthrough" comments in switches
curl_trace(): eliminate switch fallthrough
test-line-buffer: simplify command parsing
"git describe --match" learned to take multiple patterns in v2.13
series, but the feature ignored the patterns after the first one
and did not work at all. This has been fixed.
* jk/describe-omit-some-refs:
describe: fix matching to actually match all patterns
When a submodule diff should be displayed we currently just add the
submodule objects to the main object store and then e.g. walk the
revision graph and create a summary for that submodule.
It is possible that we are missing the submodule either completely or
partially, which we currently differentiate with different error messages
depending on whether (1) the whole submodule object store is missing or
(2) just the needed for this particular diff. (1) is reported as
"not initialized", and (2) is reported as "commits not present".
If a submodule is deinit'ed its repository data is still around inside
the superproject, such that the diff can still be produced. In that way
the error message (1) is misleading as we can have a diff despite the
submodule being not initialized.
Downgrade the error message (1) to be the same as (2) and just say
the commits are not present, as that is the true reason why the diff
cannot be shown.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 146fdb0dfe (diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY,
2017-06-29), the conversion from direct printing to the symbol emission
dropped the new line character for renamed, copied and rewritten files.
Add the emission of a newline, add a test for this case.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Stefan Beller <sbeller@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some tools like IDEs or fancy editors may periodically run
commands like "git status" in the background to keep track
of the state of the repository. Some of these commands may
refresh the index and write out the result in an
opportunistic way: if they can get the index lock, then they
update the on-disk index with any updates they find. And if
not, then their in-core refresh is lost and just has to be
recomputed by the next caller.
But taking the index lock may conflict with other operations
in the repository. Especially ones that the user is doing
themselves, which _aren't_ opportunistic. In other words,
"git status" knows how to back off when somebody else is
holding the lock, but other commands don't know that status
would be happy to drop the lock if somebody else wanted it.
There are a couple possible solutions:
1. Have some kind of "pseudo-lock" that allows other
commands to tell status that they want the lock.
This is likely to be complicated and error-prone to
implement (and maybe even impossible with just
dotlocks to work from, as it requires some
inter-process communication).
2. Avoid background runs of commands like "git status"
that want to do opportunistic updates, preferring
instead plumbing like diff-files, etc.
This is awkward for a couple of reasons. One is that
"status --porcelain" reports a lot more about the
repository state than is available from individual
plumbing commands. And two is that we actually _do_
want to see the refreshed index. We just don't want to
take a lock or write out the result. Whereas commands
like diff-files expect us to refresh the index
separately and write it to disk so that they can depend
on the result. But that write is exactly what we're
trying to avoid.
3. Ask "status" not to lock or write the index.
This is easy to implement. The big downside is that any
work done in refreshing the index for such a call is
lost when the process exits. So a background process
may end up re-hashing a changed file multiple times
until the user runs a command that does an index
refresh themselves.
This patch implements the option 3. The idea (and the test)
is largely stolen from a Git for Windows patch by Johannes
Schindelin, 67e5ce7f63 (status: offer *not* to lock the
index and update it, 2016-08-12). The twist here is that
instead of making this an option to "git status", it becomes
a "git" option and matching environment variable.
The reason there is two-fold:
1. An environment variable is carried through to
sub-processes. And whether an invocation is a
background process or not should apply to the whole
process tree. So you could do "git --no-optional-locks
foo", and if "foo" is a script or alias that calls
"status", you'll still get the effect.
2. There may be other programs that want the same
treatment.
I've punted here on finding more callers to convert,
since "status" is the obvious one to call as a repeated
background job. But "git diff"'s opportunistic refresh
of the index may be a good candidate.
The test is taken from 67e5ce7f63, and it's worth repeating
Johannes's explanation:
Note that the regression test added in this commit does
not *really* verify that no index.lock file was written;
that test is not possible in a portable way. Instead, we
verify that .git/index is rewritten *only* when `git
status` is run without `--no-optional-locks`.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
submodule.<name>.update can be assigned an arbitrary command via setting
it to "!command". When this command is found in the regular config, Git
ought to just run that command instead of other update mechanisms.
However if that command is just found in the .gitmodules file, it is
potentially untrusted, which is why we do not run it. Add a test
confirming the behavior.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint:
Git 2.14.2
Git 2.13.6
Git 2.12.5
Git 2.11.4
Git 2.10.5
cvsimport: shell-quote variable used in backticks
archimport: use safe_pipe_capture for user input
shell: drop git-cvsserver support by default
cvsserver: use safe_pipe_capture for `constant commands` as well
cvsserver: use safe_pipe_capture instead of backticks
cvsserver: move safe_pipe_capture() to the main package
The ability to exclude paths with a negative pathspec is not mentioned
in the man pages for git grep and other commands where it might be
useful.
Add an example and a pointer to the pathspec glossary entry in the man
page for git grep to help the user to discover this ability.
Add similar pointers from the git-add and git-status man pages.
Additionally,
- Add a test for the behaviour when multiple exclusions are present.
- Add a test for the ^ alias.
- Improve name of existing test.
- Improve grammar in glossary description of the exclude pathspec.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Manav Rathi <mnvrth@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git rev-parse" learned "--is-shallow-repository", that is to be
used in a way similar to existing "--is-bare-repository" and
friends.
* ow/rev-parse-is-shallow-repo:
rev-parse: rev-parse: add --is-shallow-repository
On Cygwin, "ulimit -s" does not report failure but it does not work
at all, which causes an unexpected success of some tests that
expect failures under a limited stack situation. This has been
fixed.
* rj/test-ulimit-on-windows:
t9010-*.sh: skip all tests if the PIPE prereq is missing
test-lib: use more compact expression in PIPE prerequisite
test-lib: don't use ulimit in test prerequisites on cygwin
A test to demonstrate "git mv" failing to adjust nested submodules
has been added.
* hv/mv-nested-submodules-test:
add test for bug in git-mv for recursive submodules
"git archive", especially when used with pathspec, stored an empty
directory in its output, even though Git itself never does so.
This has been fixed.
* rs/archive-excluded-directory:
archive: don't add empty directories to archives
Step #0 of a planned & larger series to make the in-core object
store per in-core repository object.
* jn/per-repo-object-store-fixes:
replace-objects: evaluate replacement refs without using the object store
push, fetch: error out for submodule entries not pointing to commits
pack: make packed_git_mru global a value instead of a pointer
Many codepaths did not diagnose write failures correctly when disks
go full, due to their misuse of write_in_full() helper function,
which have been corrected.
* jk/write-in-full-fix:
read_pack_header: handle signed/unsigned comparison in read result
config: flip return value of store_write_*()
notes-merge: use ssize_t for write_in_full() return value
pkt-line: check write_in_full() errors against "< 0"
convert less-trivial versions of "write_in_full() != len"
avoid "write_in_full(fd, buf, len) != len" pattern
get-tar-commit-id: check write_in_full() return against 0
config: avoid "write_in_full(fd, buf, len) < len" pattern
A hotfix to a topic already in 'master'.
* kw/write-index-reduce-alloc:
read-cache: fix index corruption with index v4
Add t/helper/test-write-cache to .gitignore
A handful of tests to demonstrates a recursive implementation of
"name-rev" hurts.
* mg/name-rev-tests-with-short-stack:
t6120: test describe and name-rev with deep repos
t6120: clean up state after breaking repo
t6120: test name-rev --all and --stdin
t7004: move limited stack prereq to test-lib
Currently, when parse_options() produces a help message it always emits
a blank line after the usage text to separate it from the options text.
If the option spec does not define any switches, or only defines hidden
switches that will not be displayed, then the help text will end up with
two trailing blank lines instead of one. Let's defer emitting the blank
line between the usage text and the options text until it is clear that
the options section will not be empty.
Fixes t1502.5, t1502.6.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When commit 54e6dc7 added translation support to parse-options, an
fprintf was mistakenly replaced by a call to putchar(). Let's use fputc
instead.
Fixes t0040.11, t0040.12, t0040.33, and t1502.8.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
When tests are run for a subsection defined in a config file, it is
better if the results for the current subsection are not overwritting
the results of a previous subsection.
So let's store the results for a subsection in a subdirectory of
"test-results/" with the subsection name.
The aggregate.perl, when it is run for a subsection, should then
aggregate the results found in "test-results/$GIT_PERF_SUBSECTION/".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is nice for the user to not just show the sha1 of the
current revision being built but also the actual name of
this revision.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Let's actually use the subsections we find in the config file
to run the perf tests separately for each subsection.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As we will set some config options in subsections, let's
teach get_var_from_env_or_config() to get the config options
from the subsections if they are set there.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function makes it possible to find subsections, so that
we will be able to run different tests for different subsections
in a later commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These calls make it possible to have the make command or the
make options in a config file, instead of in environment
variables.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This environment variable can be set to some revisions or
directories whose Git versions should be tested, in addition
to the revisions or directories passed as arguments to the
'run' script.
This enables a "perf.dirsOrRevs" configuration variable to
be used to set revisions or directories whose Git versions
should be tested.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add get_var_from_env_or_config() to easily set variables
from a config file if they are defined there and not already set.
This can also set them to a default value if one is provided.
As an example, use this function to set GIT_PERF_REPEAT_COUNT
from the perf.repeatCount config option or from the default
value.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is error prone and tiring to use many long environment
variables to give parameters to the 'run' script.
Let's make it easy to store some parameters in a config
file and to pass them to the run script.
The GIT_PERF_CONFIG_FILE variable will be set to the
argument of the '--config' option. This variable is not
used yet. It will be used in a following commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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
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
