Explanation
-----------
The problem here is flawed `poll()` implementation. When it tries to
see if pipe can be written without blocking, it eventually calls
`NtQueryInformationFile()` and tests `WriteQuotaAvailable`. However,
the meaning of quota was misunderstood. The value of quota is reduced
when either some data was written to a pipe, *or* there is a pending
read on the pipe. Therefore, if there is a pending read of size >= than
the pipe's buffer size, poll() will think that pipe is not writable and
will hang forever, usually that means deadlocking both pipe users.
I have studied the problem and found that Windows pipes track two values:
`QuotaUsed` and `BytesInQueue`. The code in `poll()` apparently wants to
know `BytesInQueue` instead of quota. Unfortunately, `BytesInQueue` can
only be requested from read end of the pipe, while `poll()` receives
write end.
The git's implementation of `poll()` was copied from gnulib, which also
contains a flawed implementation up to today.
I also had a look at implementation in cygwin, which is also broken in a
subtle way. It uses this code in `pipe_data_available()`:
fpli.WriteQuotaAvailable = (fpli.OutboundQuota - fpli.ReadDataAvailable)
However, `ReadDataAvailable` always returns 0 for the write end of the pipe,
turning the code into an obfuscated version of returning pipe's total
buffer size, which I guess will in turn have `poll()` always say that pipe
is writable. The commit that introduced the code doesn't say anything about
this change, so it could be some debugging code that slipped in.
These are the typical sizes used in git:
0x2000 - default read size in `strbuf_read()`
0x1000 - default read size in CRT, used by `strbuf_getwholeline()`
0x2000 - pipe buffer size in compat\mingw.c
As a consequence, as soon as child process uses `strbuf_read()`,
`poll()` in parent process will hang forever, deadlocking both
processes.
This results in two observable behaviors:
1) If parent process begins sending STDIN quickly (and usually that's
the case), then first `poll()` will succeed and first block will go
through. MAX_IO_SIZE_DEFAULT is 8MB, so if STDIN exceeds 8MB, then
it will deadlock.
2) If parent process waits a little bit for any reason (including OS
scheduler) and child is first to issue `strbuf_read()`, then it will
deadlock immediately even on small STDINs.
The problem is illustrated by `git stash push`, which will currently
read the entire patch into memory and then send it to `git apply` via
STDIN. If patch exceeds 8MB, git hangs on Windows.
Possible solutions
------------------
1) Somehow obtain `BytesInQueue` instead of `QuotaUsed`
I did a pretty thorough search and didn't find any ways to obtain
the value from write end of the pipe.
2) Also give read end of the pipe to `poll()`
That can be done, but it will probably invite some dirty code,
because `poll()`
* can accept multiple pipes at once
* can accept things that are not pipes
* is expected to have a well known signature.
3) Make `poll()` always reply "writable" for write end of the pipe
Afterall it seems that cygwin (accidentally?) does that for years.
Also, it should be noted that `pump_io_round()` writes 8MB blocks,
completely ignoring the fact that pipe's buffer size is only 8KB,
which means that pipe gets clogged many times during that single
write. This may invite a deadlock, if child's STDERR/STDOUT gets
clogged while it's trying to deal with 8MB of STDIN. Such deadlocks
could be defeated with writing less than pipe's buffer size per
round, and always reading everything from STDOUT/STDERR before
starting next round. Therefore, making `poll()` always reply
"writable" shouldn't cause any new issues or block any future
solutions.
4) Increase the size of the pipe's buffer
The difference between `BytesInQueue` and `QuotaUsed` is the size
of pending reads. Therefore, if buffer is bigger than size of reads,
`poll()` won't hang so easily. However, I found that for example
`strbuf_read()` will get more and more hungry as it reads large inputs,
eventually surpassing any reasonable pipe buffer size.
Chosen solution
---------------
Make `poll()` always reply "writable" for write end of the pipe.
Hopefully one day someone will find a way to implement it properly.
Reproduction
------------
printf "%8388608s" X >large_file.txt
git stash push --include-untracked -- large_file.txt
I have decided not to include this as test to avoid slowing down the
test suite. I don't expect the specific problem to come back, and
chances are that `git stash push` will be reworked to avoid sending the
entire patch via STDIN.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We seem to be getting new users who get confused every 20 months or
so with this "-h consistently wants to give help, but the commands
to which `-h` may feel like a good short-form option want it to mean
something else." compromise.
Let's make sure that the readers know that `git cmd -h` (with no
other arguments) is a way to get usage text, even for commands like
ls-remote and grep.
Also extend the description that is already in gitcli.txt, as it is
clear that users still get confused with the current text.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a repository has two files:
foo/bar/baz
foo/bar-2/baz
then a simple lexicographic ordering of files and directories shows
...
foo/bar
foo/bar-2
foo/bar/baz
...
and the appearance of foo/bar-2 between foo/bar and foo/bar/baz can trip
up some codepaths. Add a test to catch such cases.
t6020 might be a slight misfit since this testcase does not test any
kind of file/directory conflict. However, it is similar in spirit to
some tests (4-6) already in t6020 that check cases where a *file* sorted
between a directory and the files underneath that directory. This
testcase differs in that now there is a *directory* that sorts in the
middle.
Although merge-recursive currently has no problems with this simple
testcase, I discovered that it's very possible to accidentally mess it
up. Further, we have no other merge or cherry-pick or rebase testcases
in the entire testsuite that cover such a case, so I felt like it would
be a worthwhile addition to the testsuite.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In t6022, we were testing for file being overwritten (or not) based on
an output message instead of checking for the file being overwritten.
Since we can check for the file being overwritten via mtime updates,
check that instead.
In t6046, we were largely checking for both the expected behavior and a
proxy for it, which is unnecessary. The calls to test-tool also were a
bit cryptic. Make them a little clearer.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make use of test_path_is_file, test_write_lines, and similar helpers
in these old test files.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Indent code, and include it inside test_expect* blocks.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the en/merge-path-collision topic (see commit ac193e0e0a, "Merge
branch 'en/merge-path-collision'", 2019-01-04), all the "file collision"
conflict types were modified for consistency. In particular,
rename/add, rename/rename(2to1) and each rename/add piece of a
rename/rename(1to2)/add[/add] conflict were made to behave like add/add
conflicts have always been handled.
However, this consistency was not enforced when opt->priv->call_depth >
0 for rename/rename conflicts. Update rename/rename(1to2) and
rename/rename(2to1) conflicts in the recursive case to also be
consistent. As an added bonus, this simplifies the code considerably.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The most recent Azure Pipelines macOS agents enable what Apple calls
"System Integrity Protection". This makes `p4d -V` hang: there is some
sort of GUI dialog waiting for the user to acknowledge that the copied
binaries are legit and may be executed, but on build agents, there is no
user who could acknowledge that.
Let's ask Homebrew specifically to _not_ quarantine the Perforce
binaries.
Helped-by: Aleksandr Chebotov
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Among other differences relative to GNU sed, macOS' sed always ends its
output with a trailing newline, even if the input did not have such a
trailing newline.
Surprisingly, this makes three httpd-based tests fail on macOS: t5616,
t5702 and t5703. ("Surprisingly" because those tests have been around
for some time, but apparently nobody runs them on macOS with a working
Apache2 setup.)
The reason is that we use `sed` in those tests to filter the response of
the web server. Apart from the fact that we use GNU constructs (such as
using a space after the `c` command instead of a backslash and a
newline), we have another problem: macOS' sed LF-only newlines while
webservers are supposed to use CR/LF ones.
Even worse, t5616 uses `sed` to replace a binary part of the response
with a new binary part (kind of hoping that the replaced binary part
does not contain a 0x0a byte which would be interpreted as a newline).
To that end, it calls on Perl to read the binary pack file and
hex-encode it, then calls on `sed` to prefix every hex digit pair with a
`\x` in order to construct the text that the `c` statement of the `sed`
invocation is supposed to insert. So we call Perl and sed to construct a
sed statement. The final nail in the coffin is that macOS' sed does not
even interpret those `\x<hex>` constructs.
Let's just replace all of that by Perl snippets. With Perl, at least, we
do not have to deal with GNU vs macOS semantics, we do not have to worry
about unwanted trailing newlines, and we do not have to spawn commands
to construct arguments for other commands to be spawned (i.e. we can
avoid a whole lot of shell scripting complexity).
The upshot is that this fixes t5616, t5702 and t5703 on macOS with
Apache2.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge_commit_graphs() copies the (translated) progress message into a
strbuf and passes the copy to start_delayed_progress() at each loop
iteration. The latter function takes a string pointer, so let's avoid
the detour and hand the string to it directly. That's shorter, simpler
and slightly more efficient.
Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When searching the commit graph for tag candidates, `git-describe`
will stop as soon as there is only one active branch left and
it already found an annotated tag as a candidate.
This works well as long as all branches eventually connect back
to a common root, but if the tags are found across branches
with no common ancestor
B
o----.
\
o-----o---o----x
A
it can happen that the search on one branch terminates prematurely
because a tag was found on another, independent branch. This scenario
isn't quite as obscure as it sounds, since cloning with a limited
depth often introduces many independent "dead ends" into the commit
graph.
The help text of `git-describe` states pretty clearly that when
describing a commit D, the number appended to the emitted tag X should
correspond to the number of commits found by `git log X..D`.
Thus, this commit modifies the stopping condition to only abort
the search when only one branch is left to search *and* all current
best candidates are descendants from that branch.
For repositories with a single root, this condition is always
true: When the search is reduced to a single active branch, the
current commit must be an ancestor of *all* tag candidates. This
means that in the common case, this change will have no negative
performance impact since the same number of commits as before will
be traversed.
Signed-off-by: Benno Evers <benno@bmevers.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When `options.switch_to' is set, `options.orig_head' is populated right
after with the object name the ref/commit argument points at.
Therefore, there is no need to parse `switch_to' again.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git remote rename X Y" needs to adjust configuration variables
(e.g. branch.<name>.remote) whose value used to be X to Y.
branch.<name>.pushRemote is now also updated.
* bw/remote-rename-update-config:
remote rename/remove: gently handle remote.pushDefault config
config: provide access to the current line number
remote rename/remove: handle branch.<name>.pushRemote config values
remote: clean-up config callback
remote: clean-up by returning early to avoid one indentation
pull --rebase/remote rename: document and honor single-letter abbreviations rebase types
Previously, performing "git clone --recurse-submodules --single-branch"
resulted in submodules cloning all branches even though the superproject
cloned only one branch. Pipe --single-branch through the submodule
helper framework to make it to 'clone' later on.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Start using a named initializer list for SUBMODULE_UPDATE_CLONE_INIT, as
the struct is becoming cumbersome for a typical struct initializer list.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Log graph comparision logic is duplicated many times in:
- t3430-rebase-merges.sh
- t4202-log.sh
- t4214-log-graph-octopus.sh
- t4215-log-skewed-merges.sh
Consolidate the core of the comparision and sanitization logic in
lib-log-graph, and use it to replace the existing tests.
While at it, lose the singular/plural transition magic from the
sanitize_output helper, which was necessary around 7f814632 ("Use
correct grammar in diffstat summary line", 2012-02-01), that has
long outlived its usefulness.
Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git worktree add <path>" performs various checks before approving
<path> as a valid location for the new worktree. Aside from ensuring
that <path> does not already exist, one of the questions it asks is
whether <path> is already a registered worktree. To perform this check,
it queries find_worktree() and disallows the "add" operation if
find_worktree() finds a match for <path>. As a convenience, however,
find_worktree() casts an overly wide net to allow users to identify
worktrees by shorthand in order to keep typing to a minimum. For
instance, it performs suffix matching which, given subtrees "foo/bar"
and "foo/baz", can correctly select the latter when asked only for
"baz".
"add" validation knows the exact path it is interrogating, so this sort
of heuristic-based matching is, at best, questionable for this use-case
and, at worst, may may accidentally interpret <path> as matching an
existing worktree and incorrectly report it as already registered even
when it isn't. (In fact, validate_worktree_add() already contains a
special case to avoid accidentally matching against the main worktree,
precisely due to this problem.)
Avoid the problem of potential accidental matching against an existing
worktree by instead taking advantage of find_worktree_by_path() which
matches paths deterministically, without applying any sort of magic
shorthand matching performed by find_worktree().
Reported-by: Cameron Gunnin <cameron.gunnin@synopsys.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
find_worktree() employs heuristics to match user provided input -- which
may be a pathname or some sort of shorthand -- with an actual worktree.
Although this convenience allows a user to identify a worktree with
minimal typing, the black-box nature of these heuristics makes it
potentially difficult for callers which already know the exact path of a
worktree to be confident that the correct worktree will be returned for
any specific pathname (particularly a relative one), especially as the
heuristics are enhanced and updated.
Therefore, add a companion function, find_worktree_by_path(), which
deterministically identifies a worktree strictly by pathname with no
interpretation and no magic matching.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Do a better job of explaining that find_worktree()'s main purpose is to
locate a worktree based upon input from a user which may be some sort of
shorthand for identifying a worktree rather than an actual path. For
instance, one shorthand a user can use to identify a worktree is by
unique path suffix (i.e. given worktrees at paths "foo/bar" and
"foo/baz", the latter can be identified simply as "baz"). The actual
heuristics find_worktree() uses to select a worktree may be expanded in
the future (for instance, one day it may allow worktree selection by
<id> of the .git/worktrees/<id>/ administrative directory), thus the
documentation does not provide a precise description of how matching is
performed, instead leaving it open-ended to allow for future
enhancement.
While at it, drop mention of the non-NULL requirement of `prefix` since
NULL has long been allowed. For instance, prefix_filename() has
explicitly allowed NULL since 116fb64e43 (prefix_filename: drop length
parameter, 2017-03-20), and find_worktree() itself since e4da43b1f0
(prefix_filename: return newly allocated string, 2017-03-20).
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Once upon a time, nth_packed_object_sha1() was the primary way to get
the oid of a packfile's index position. But these days we have the more
type-safe nth_packed_object_id() wrapper, and all callers have been
converted.
Let's drop the "sha1" version (turning the safer wrapper into a single
function) so that nobody is tempted to introduce new callers.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit changed the public interface of packed_object_info()
to return a struct object_id rather than a bare hash. That enables us to
convert our internal helper, as well. We can use nth_packed_object_id()
directly for OFS_DELTA, but we'll still have to use oidread() to pull
the hash for a REF_DELTA out of the packfile.
There should be no additional cost, since we're copying directly into
the object_id the caller provided us (just as we did before; it's just
happening now via nth_packed_object_id()).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a caller sets the object_info.delta_base_sha1 to a non-NULL pointer,
we'll write the oid of the object's delta base to it. But we can
increase our type safety by switching this to a real object_id struct.
All of our callers are just pointing into the hash member of an
object_id anyway, so there's no inconvenience.
Note that we do still keep it as a pointer-to-struct, because the NULL
sentinel value tells us whether the caller is even interested in the
information.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we're checking a pack with fsck or verify-pack, we first sort the
idx entries by offset, since accessing them in pack order is more
efficient. To do so, we loop over them and fill in an array of structs
with the offset, object_id, and index position of each, sort the result,
and only then do we iterate over the sorted array and process each
entry.
In order to avoid the memory cost of storing the hash of each object, we
just store a pointer into the copy in the mmap'd pack index file. To
keep that property even as the rest of the code converted to "struct
object_id", commit 9fd750461b (Convert the verify_pack callback to
struct object_id, 2017-05-06) introduced a union in order to type-pun
the pointer-to-hash into an object_id struct.
But we can make this even simpler by observing that the sort operation
doesn't need the object id at all! We only need them one at a time while
we actually process each entry. So we can just omit the oid from the
struct entirely and load it on the fly into a local variable in the
second loop.
This gets rid of the type-punning, and lets us directly use the more
type-safe nth_packed_object_id(), simplifying the code. And as a bonus,
it saves 8 bytes of memory per object.
Note that this does mean we'll do the offset lookup for each object
before the oid lookup. The oid lookup has more safety checks in it
(e.g., for looking past p->num_objects) which in theory protected the
offset lookup. But since violating those checks was already a BUG()
condition (as described in the previous commit), it's not worth worrying
about.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we fail to load the oid from the index of a packfile, we'll die()
with an "internal error". But this should never happen: we'd fail here
only if the idx needed to be lazily opened (but we've already opened it)
or if we asked for an out-of-range index (but we're iterating using the
same count that we'd check the range against). A corrupted index might
have a bogus count (e.g., too large for its size), but we'd have
complained and aborted already when opening the index initially.
While we're here, we can add a few details so that if this bug ever
_does_ trigger, we'll have a bit more information.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A pack bitmap file contains the index position of the commit for each
bitmap, which we then translate into an object id via
nth_packed_object_sha1(). In preparation for that function going away,
we can switch to the more type-safe nth_packed_object_id().
Note that even though the result ends up in an object_id this does incur
an extra copy of the hash (into our temporary object_id, and then into
the final malloc'd stored_bitmap struct). This shouldn't make any
measurable difference. If it did, we could avoid this copy _and_ the
copy of the rest of the items by allocating the stored_bitmap struct
beforehand and reading directly into it from the bitmap file. Or better
still, if this is a bottleneck, we could introduce an on-disk index to
the bitmap file so we don't have to read every single entry to use just
one of them. So it's not worth worrying about micro-optimizing out this
one hash copy.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the pack-reuse code is dumping an OFS_DELTA entry to a client that
doesn't support it, we re-write it as a REF_DELTA. To do so, we use
nth_packed_object_sha1() to get the oid, but that function is soon going
away in favor of the more type-safe nth_packed_object_id(). Let's switch
now in preparation.
Note that this does incur an extra hash copy (from the pack idx mmap to
the object_id and then to the output, rather than straight from mmap to
the output). But this is not worth worrying about. It's probably not
measurable even when it triggers, and this is fallback code that we
expect to trigger very rarely (since everybody supports OFS_DELTA these
days anyway).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We already store an object_id internally, and now our sole caller also
has one. Let's stop passing around the internal hash array, which adds a
bit of type safety.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we're considering reusing an on-disk delta, we get the oid of the
base as a pointer to unsigned char bytes of the hash, either into the
packfile itself (for REF_DELTA) or into the pack idx (using the revindex
to convert the offset into an index entry).
Instead, we'd prefer to use a more type-safe object_id as much as
possible. We can get the pack idx using nth_packed_object_id() instead.
For the packfile bytes, we can copy them out using oidread().
This doesn't even incur an extra copy overall, since the next thing we'd
always do with that pointer is pass it to can_reuse_delta(), which needs
an object_id anyway (and called oidread() itself). So this patch also
converts that function to take the object_id directly.
Note that we did previously use NULL as a sentinel value when the object
isn't a delta. We could probably get away with using the null oid for
this, but instead we'll use an explicit boolean flag, which should make
things more obvious for people reading the code later.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our nth_packed_object_sha1() function returns NULL for error. So when we
wrapped it with nth_packed_object_oid(), we kept the same semantics. But
it's a bit funny, because the caller actually passes in an out
parameter, and the pointer we return is just that same struct they
passed to us (or NULL).
It's not too terrible, but it does make the interface a little
non-idiomatic. Let's switch to our usual "0 for success, negative for
error" return value. Most callers either don't check it, or are
trivially converted. The one that requires the biggest change is
actually improved, as we can ditch an extra aliased pointer variable.
Since we are changing the interface in a subtle way that the compiler
wouldn't catch, let's also change the name to catch any topics in
flight. We can drop the 'o' and make it nth_packed_object_id(). That's
slightly shorter, but also less redundant since the 'o' stands for
"object" already.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This code has been unused since fa099d2322 (worktree.c: kill parse_ref()
in favor of refs_resolve_ref_unsafe(), 2017-04-24), so drop it.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The fingerprints member of struct blame_origin is a void pointer that is
only ever used to reference objects of type struct fingerprint. Declare
its type to allow the compiler to do type checks. We can keep its type
opaque in blame.h, though -- only functions in blame.c need to know the
actual definition of struct fingerprint.
Signed-off-by: René Scharfe <l.s.r@web.de>
Reviewed-by: Barret Rhoden <brho@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The invocation "git rebase <upstream> <branch>" switches to <branch>
before performing the rebase operation. However, unlike git-switch,
git-checkout, and git-worktree which all refuse to switch to a branch
that is already checked out in some other worktree, git-rebase switches
to <branch> unconditionally. Curb this careless behavior by making
git-rebase also refuse to switch to a branch checked out elsewhere.
Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test intentionally creates a file which causes rebase to fail, thus
it is important that this file be deleted before subsequent tests are
run which are not expecting such a failure. In the past, the common way
to ensure cleanup (regardless of whether the test succeeded or failed)
was either for the next test to perform the previous test's cleanup as
its first step or to do the cleanup at global scope outside of any
tests. With the introduction of 'test_when_finished', however, tests can
be responsible for their own cleanup. Therefore, update this test to
clean up after itself.
A bit of history: This 'rm' invocation was moved from within the body of
the following test to global scope by bffd750adf (rebase: improve error
message when upstream argument is missing, 2010-05-31), which postdates,
by about a month, introduction of 'test_when_finished' in 3bf7886705
(test-lib: Let tests specify commands to be run at end of test,
2010-05-02).
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We `cat` files, but don't inspect or grab the contents in any way.
Unlike in an earlier commit, there is no reason to suspect that these
files could be missing, so `cat`-ing them is just wasted effort.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We `cat` kwdelfile.c, but don't inspect or grab the contents in any way.
This looks like a remnant from a debug session. Similar to the previous
commit, one could argue that `cat`-ing the file verifies that it didn't
disappear somehow. But because the very next thing we do after `cat`-ing
the file is to `grep` in it, we can safely drop the call to `cat`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We `cat` files, but don't inspect or grab the contents in any way. These
`cat` calls look like remnants from a debug session, so it's tempting to
get rid of them. But they do actually verify that the files exist, which
might not necessarily be the case for some failure modes of `git apply
--reject`. Let's not lose that.
Convert the `cat` calls to use `test_path_is_file` instead. This is of
course still a minor change since we no longer verify that the files can
be opened for reading, but that is not something we usually worry about.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The receive.denyCurrentBranch config option controls what happens if
you push to a branch that is checked out into a non-bare repository.
By default, it rejects it. It can be disabled via `ignore` or `warn`.
Another yet trickier option is `updateInstead`.
However, this setting was forgotten when the git worktree command was
introduced: only the main worktree's current branch is respected.
With this change, all worktrees are respected.
That change also leads to revealing another bug,
i.e. `receive.denyCurrentBranch = true` was ignored when pushing into a
non-bare repository's unborn current branch using ref namespaces. As
`is_ref_checked_out()` returns 0 which means `receive-pack` does not get
into conditional statement to switch `deny_current_branch` accordingly
(ignore, warn, refuse, unconfigured, updateInstead).
receive.denyCurrentBranch uses the function `refs_resolve_ref_unsafe()`
(called via `resolve_refdup()`) to resolve the symbolic ref HEAD, but
that function fails when HEAD does not point at a valid commit.
As we replace the call to `refs_resolve_ref_unsafe()` with
`find_shared_symref()`, which has no problem finding the worktree for a
given branch even if it is unborn yet, this bug is fixed at the same
time: receive.denyCurrentBranch now also handles worktrees with unborn
branches as intended even while using ref namespaces.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`receive.denyCurrentBranch` currently has a bug where it allows pushing
into non-bare repository using namespaces as long as it does not have any
commits. This would cause t5509 to fail once that bug is fixed because it
pushes into an unborn current branch.
In t5509, no operations are performed inside `pushee`, as it is only a
target for `git push` and `git ls-remote` calls. Therefore it does not
need to have a worktree. So, it is safe to change `pushee` to a bare
repository.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When called in the Git directory of a non-bare repository, this function
would not return the directory of the main worktree, but of the Git
directory instead.
The reason: when the Git directory is the current working directory, the
absolute path of the common directory will be reported with a trailing
`/.git/.`, which the code of `get_main_worktree()` does not handle
correctly.
Let's fix this.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These options are available since git v2.15, but somehow
eluded from the completion script.
Note that while --color-moved-ws= accepts comma-separated
list of values, there is no (easy?) way to make it work
with completion (see e.g. [1]).
[1]: https://github.com/scop/bash-completion/issues/240
Acked-by: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We can check if certain characters are present in a string by calling
strchr(3) on each of them, or we can pass them all to a single
strpbrk(3) call. The latter is shorter, less repetitive and slightly
more efficient, so let's do that instead.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
isalnum(c) is equivalent to isalpha(c) || isdigit(c), so use the
former instead. The result is shorter, simpler and slightly more
efficient.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use test_path_is_file() instead of 'test -f' for better debugging
information.
Signed-off-by: Rasmus Jonsson <wasmus@zom.bi>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When using partial clone, find_non_local_tags() in builtin/fetch.c
checks each remote tag to see if its object also exists locally. There
is no expectation that the object exist locally, but this function
nevertheless triggers a lazy fetch if the object does not exist. This
can be extremely expensive when asking for a commit, as we are
completely removed from the context of the non-existent object and
thus supply no "haves" in the request.
6462d5eb9a (fetch: remove fetch_if_missing=0, 2019-11-05) removed a
global variable that prevented these fetches in favor of a bitflag.
However, some object existence checks were not updated to use this flag.
Update find_non_local_tags() to use OBJECT_INFO_SKIP_FETCH_OBJECT in
addition to OBJECT_INFO_QUICK. The _QUICK option only prevents
repreparing the pack-file structures. We need to be extremely careful
about supplying _SKIP_FETCH_OBJECT when we expect an object to not exist
due to updated refs.
This resolves a broken test in t5616-partial-clone.sh.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Alexandr Miloslavskiy