The code recently added in this release to move to the entry beyond
the ones in the same directory in the index in the sparse-cone mode
did not count the number of entries to skip over incorrectly, which
has been corrected.
* ds/sparse-cone:
.mailmap: fix GGG authoship screwup
unpack-trees: correctly compute result count
Tell .editorconfig that in this project, *.txt files are indented
with tabs.
* hi/indent-text-with-tabs-in-editorconfig:
editorconfig: indent text files with tabs
We heap-allocate our arrays of name_entry structs, etc, with one entry
per tree we're asked to traverse. The code does a raw multiplication in
the xmalloc() call, which I find when auditing for integer overflows
during allocation.
We could "fix" this by using ALLOC_ARRAY() instead. But as it turns out,
the maximum size of these arrays is limited at compile time:
- merge_trees() always passes in 3 trees
- unpack_trees() and its brethren never pass in more than
MAX_UNPACK_TREES
So we can simplify even further by just using a stack array and bounding
it with MAX_UNPACK_TREES. There should be no concern with overflowing
the stack, since MAX_UNPACK_TREES is only 8 and the structs themselves
are small.
Note that since we're replacing xcalloc(), we have to move one of the
NULL initializations into a loop.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We compute the length of an array of object_id's with a raw
multiplication. In theory this could trigger an integer overflow which
would cause an under-allocation (and eventually an out of bounds write).
I doubt this can be triggered in practice, since you'd need to feed it
an enormous number of target objects, which would typically come from
the ref advertisement and be using proportional memory. And even on
64-bit systems, where "int" is much smaller than "size_t", that should
hold: even though "targets" is an int, the multiplication will be done
as a size_t because of the use of sizeof().
But we can easily fix it by using ALLOC_ARRAY(), which uses st_mult()
under the hood.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We take a "dst" buffer to write into, but there's no matching "len"
parameter. The hidden assumption is that normalizing always makes things
smaller, so we're OK as long as "dst" is at least as big as "src". Let's
document that explicitly.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Check we can talk to the remote host before starting the git-fastimport
subchild.
Otherwise we fail to connect, and then exit, leaving git-fastimport
still running since we did not wait() for it.
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After an error, git-p4 calls die(). This just exits, and leaves child
processes still running.
Instead of calling die(), raise an exception and catch it where the
child process(es) (git-fastimport) are created.
This was analyzed in detail here:
https://public-inbox.org/git/20190227094926.GE19739@szeder.dev/
This change does not address the particular issue of p4CmdList()
invoking a subchild and not waiting for it on error.
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes it easier to try/catch around this block of code to ensure
cleanup following p4 failures is handled properly.
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pylint is incredibly useful for finding bugs, but git-p4 has never used
it, so there are a lot of warnings that while important, don't actually
result in bugs.
Let's turn those off for now, so we can get some useful output.
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently when there is a P4 error, git-p4 calls die() which just exits.
This then leaves the git-fast-import process still running, and can even
leave p4 itself still running.
As a result, git-p4 fails to exit cleanly. This is a particular problem
for people running the unit tests in regression.
Use this exception to report errors upwards, cleaning up as the error
propagates.
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ensure that we can safely call self.closeStreams() multiple times, and
can also call it even if there is no git fast-import stream at all.
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are several error messages in get_oid() and its children that are
clearly intended for humans, but aren't marked for translation. E.g.:
$ git show :1:foo
fatal: Path 'foo' is in the index, but not at stage 1.
Did you mean ':0:foo'?
Let's mark these for translation. While we're at it, let's switch the
style to be more like our usual error messages: start with a lowercase
letter and omit a period at the end of the line.
This does mean that multi-line messages like the one above don't have
any punctuation between the two sentences. I solved that by adding a
"hint" marker like we'd see from advise(). So the result is:
$ git show :1:foo
fatal: path 'foo' is in the index, but not at stage 1
hint: Did you mean ':0:foo'?
A few tests had to be switched to test_i18ngrep and test_i18ncmp. Since
we were touching them anyway, I also simplified the ones using i18ngrep
a bit for readability.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a filter is specified, we do not need a full connectivity check on
the contents of the packfile we just fetched; we only need to check that
the objects referenced are promisor objects.
This significantly speeds up fetches into repositories that have many
promisor objects, because during the connectivity check, all promisor
objects are enumerated (to mark them UNINTERESTING), and that takes a
significant amount of time.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit dfa33a298d ("clone: do faster object check for partial clones",
2019-04-21) optimized the connectivity check done when cloning with
--filter to check only the existence of objects directly pointed to by
refs. But this is not sufficient: they also need to be promisor objects.
Make this check more robust by instead checking that these objects are
promisor objects, that is, they appear in a promisor pack.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git --git-dir <path> is a bit confusing and sometimes doesn't work as
the user would expect it to.
For example, if the user runs `git --git-dir=<path> status`, git
will skip the repository discovery algorithm and will assign the
work tree to the user's current work directory unless otherwise
specified. When this assignment is wrong, the output will not match
the user's expectations.
This patch updates the documentation to make it clearer.
Signed-off-by: Heba Waly <heba.waly@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 13185fd241 (l10n: zh_TW.po: update translation for v2.25.0 round 1,
2019-12-31), the author mistakenly used their GitHub username for
authorship information instead of their real name. However, a commit
with their real name exists prior to this: 9917eca794 (l10n: zh_TW: add
translation for v2.24.0, 2019-11-20).
Map their email to their real name so that these contributions can be
counted together.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since grep learned to recurse into submodules in 0281e487fd
(grep: optionally recurse into submodules, 2016-12-16),
using --recurse-submodules along with --no-index makes Git
die().
This is unfortunate because if submodule.recurse is set in a user's
~/.gitconfig, invoking `git grep --no-index` either inside or outside
a Git repository results in
fatal: option not supported with --recurse-submodules
Let's allow using these options together, so that setting submodule.recurse
globally does not prevent using `git grep --no-index`.
Using `--recurse-submodules` should not have any effect if `--no-index`
is used inside a repository, as Git will recurse into the checked out
submodule directories just like into regular directories.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The documentation for push.default mentions that it is used if no
refspec is "explicitly given". Let's drop the notion of "explicit" here,
since it's vague, and just mention that any refspec from anywhere is
sufficient to override this.
I've dropped the mention of "explicitly given" from the definition of
the "nothing" value right below, too. It's close enough to our
clarification that it should be obvious we mean the same type of "given"
here.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As with the previous two commits, UBSan with clang-11 complains about
computing offsets from a NULL pointer. The failures in t4013 (and
elsewhere) look like this:
kwset.c:102:23: runtime error: applying non-zero offset 107820859019600 to null pointer
...
not ok 79 - git log -SF master # magic is (not used)
That line is not enlightening:
... = obstack_alloc(&kwset->obstack, sizeof (struct trie));
because obstack is implemented almost entirely in macros, and the actual
problem is five macros deep (I temporarily converted them to inline
functions to get better compiler errors, which was tedious but worked
reasonably well).
The actual problem is in these pointer-alignment macros:
/* If B is the base of an object addressed by P, return the result of
aligning P to the next multiple of A + 1. B and P must be of type
char *. A + 1 must be a power of 2. */
#define __BPTR_ALIGN(B, P, A) ((B) + (((P) - (B) + (A)) & ~(A)))
/* Similar to _BPTR_ALIGN (B, P, A), except optimize the common case
where pointers can be converted to integers, aligned as integers,
and converted back again. If PTR_INT_TYPE is narrower than a
pointer (e.g., the AS/400), play it safe and compute the alignment
relative to B. Otherwise, use the faster strategy of computing the
alignment relative to 0. */
#define __PTR_ALIGN(B, P, A) \
__BPTR_ALIGN (sizeof (PTR_INT_TYPE) < sizeof (void *) ? (B) : (char *) 0, \
P, A)
If we have a sufficiently-large integer pointer type, then we do the
computation using a NULL pointer constant. That turns __BPTR_ALIGN()
into something like:
NULL + (P - NULL + A) & ~A
and UBSan is complaining about adding the full value of P to that
initial NULL. We can fix this by doing our math as an integer type, and
then casting the result back to a pointer. The problem case only happens
when we know that the integer type is large enough, so there should be
no issue with truncation.
Another option would be just simplify out all the 0's from
__BPTR_ALIGN() for the NULL-pointer case. That probably wouldn't work
for a platform where the NULL pointer isn't all-zeroes, but Git already
wouldn't work on such a platform (due to our use of memset to set
pointers in structs to NULL). But I tried here to keep as close to the
original as possible.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As with the previous commit, clang-11's UBSan complains about computing
offsets from a NULL pointer, causing some tests to fail. In this case,
though, we're actually computing a non-zero offset, which is even more
dubious. From t7810:
xdiff-interface.c:268:14: runtime error: applying non-zero offset 1 to null pointer
...
not ok 131 - grep -p with userdiff
The problem is our parsing of the funcname config. We count the number
of lines in the string, allocate an array, and then loop over our
allocated entries, parsing each line and moving our cursor to one past
the trailing newline for the next iteration.
But the final line will not generally have a trailing newline (since
it's a config value), and hence we go to one past NULL. In practice this
is OK, since our loop should terminate before we look at the value. But
even computing such an invalid pointer technically violates the
standard.
We can fix it by leaving the pointer at NULL if we're at the end, rather
than one-past. And while we're thinking about it, we can also document
the variant by asserting that our initial line-count matches the
second-pass of parsing.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Undefined Behavior Sanitizer in clang-11 seems to have learned a new
trick: it complains about computing offsets from a NULL pointer, even if
that offset is 0. This causes numerous test failures. For example, from
t1090:
unpack-trees.c:1355:41: runtime error: applying zero offset to null pointer
...
not ok 6 - in partial clone, sparse checkout only fetches needed blobs
The code in question looks like this:
struct cache_entry **cache_end = cache + nr;
...
while (cache != cache_end)
and we sometimes pass in a NULL and 0 for "cache" and "nr". This is
conceptually fine, as "cache_end" would be equal to "cache" in this
case, and we wouldn't enter the loop at all. But computing even a zero
offset violates the C standard. And given the fact that UBSan is
noticing this behavior, this might be a potential problem spot if the
compiler starts making unexpected assumptions based on undefined
behavior.
So let's just avoid it, which is pretty easy. In some cases we can just
switch to iterating with a numeric index (as we do in sequencer.c here).
In other cases (like the cache_end one) the use of an end pointer is
more natural; we can keep that by just explicitly checking for the
NULL/0 case when assigning the end pointer.
Note that there are two ways you can write this latter case, checking
for the pointer:
cache_end = cache ? cache + nr : cache;
or the size:
cache_end = nr ? cache + nr : cache;
For the case of a NULL/0 ptr/len combo, they are equivalent. But writing
it the second way (as this patch does) has the property that if somebody
were to incorrectly pass a NULL pointer with a non-zero length, we'd
continue to notice and segfault, rather than silently pretending the
length was zero.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bug was that ie_match_stat() was used to compare if the stat info
for the file was compatible with the stat info in the index, rather
using ie_modified() to check if the file was in fact different from the
version in the index.
A version of this (with deinit instead of rm) was reported here:
https://lore.kernel.org/git/CAPOqYV+C-P9M2zcUBBkD2LALPm4K3sxSut+BjAkZ9T1AKLEr+A@mail.gmail.com/
It seems that in that case, the user's clone command left the index
with empty stat info. The mailing list was unable to reproduce this.
But we (Two Sigma) hit the bug while using some plumbing commands, so
I'm fixing it. I manually confirmed that the fix also repairs deinit
in this scenario.
Signed-off-by: David Turner <dturner@twosigma.com>
Reported-by: Thomas Bétous <th.betous@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some (but not all!) redirections in this file are spelled "2> error".
Let's switch them to our usual style.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Using the shell "test" here is inflexible, because we can't easily swap
it out for an i18n-aware version like we can with test_cmp and
test_i18ncmp. And it's not even saving us any processes, since we have
to use "cat" to get the output. So let's switch to using test_cmp, which
has the added bonus that it will produce better output if there's a
failure.
Note that not all of the changed outputs here are candidates for
translation, but I've converted all of them for consistency and to
benefit from the better output.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When set to "warn" or "error", `rebase.missingCommitsCheck' would make
`rebase -i' warn if the user removed commits from the todo list to
prevent mistakes. Unfortunately, `rebase --edit-todo' and `rebase
--continue' don't take it into account.
This adds the ability for `rebase --edit-todo' and `rebase --continue'
to check if commits were dropped by the user. As both edit_todo_list()
and complete_action() parse the todo list and check for dropped commits,
the code doing so in the latter is removed to reduce duplication.
`edit_todo_list_advice' is removed from sequencer.c as it is no longer
used there.
This changes when a backup of the todo list is made. Until now, it was
saved only once, before the initial edit. Now, it is also made if the
original todo list has no errors or no dropped commits. Thus, the
backup should be error-free. Without this, sequencer_continue()
(`rebase --continue') could only compare the current todo list against
the original, unedited list. Before this change, this file was only
used by edit_todo_list() and `rebase -p' to create the backup before
the initial edit, and check_todo_list_from_file(), only used by
`rebase -p' to check for dropped commits after its own initial edit.
If the edited list has an error, a file, `dropped', is created to
report the issue. Otherwise, it is deleted. Usually, the edited list
is compared against the list before editing, but if this file exists,
it will be compared to the backup. Also, if the file exists,
sequencer_continue() checks the list for dropped commits. If the
check was performed every time, it would fail when resuming a rebase
after resolving a conflict, as the backup will contain commits that
were picked, but they will not be in the new list. It's safe to
ignore this check if `dropped' does not exist, because that means that
no errors were found at the last edition, so any missing commits here
have already been picked.
Five tests are added to t3404. The tests for
`rebase.missingCommitsCheck = warn' and `rebase.missingCommitsCheck =
error' have a similar structure. First, we start a rebase with an
incorrect command on the first line. Then, we edit the todo list,
removing the first and the last lines. This demonstrates that
`--edit-todo' notices dropped commits, but not when the command is
incorrect. Then, we restore the original todo list, and edit it to
remove the last line. This demonstrates that if we add a commit after
the initial edit, then remove it, `--edit-todo' will notice that it
has been dropped. Then, the actual rebase takes place. In the third
test, it is also checked that `--continue' will refuse to resume the
rebase if commits were dropped. The fourth test checks that no errors
are raised when resuming a rebase after resolving a conflict, the fifth
checks that no errors are raised when editing the todo list after
pausing the rebase.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The message contained in `edit_todo_list_advice' (sequencer.c) is
printed after the initial edit of the todo list if it can't be parsed or
if commits were dropped. This is done either in complete_action() for
`rebase -i', or in check_todo_list_from_file() for `rebase -p'.
Since we want to add this check when editing the list, we also want to
use this message from edit_todo_list() (rebase-interactive.c). To this
end, check_todo_list_from_file() is moved to rebase-interactive.c, and
`edit_todo_list_advice' is copied there. In the next commit,
complete_action() will stop using it, and `edit_todo_list_advice' will
be removed from sequencer.c.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test_must_fail function should only be used for git commands since
we should assume that external commands work sanely. Since apply_patch
wraps a sed and git invocation, rewrite it to accept an `!` argument
which would cause only the git command to be prefixed with
`test_must_fail`.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test_must_fail() function should only be used for git commands since
we should assume that external commands work sanely. Replace
`test_must_fail test_path_exists` with `test_path_is_missing` since we
expect these paths to not exist.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have some test cases which are indented 7-spaces instead of a tab.
Reindent with a tab instead.
This patch should appear empty with `--ignore-all-space`.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test with disabled rerere should make sure that the cherry-picked
result does not have the conflict replaced with a recorded resolution.
It attempts to do so by ensuring that the file content is _not_ equal
to some other file. That by itself is a very dubious check because just
about every random result of an incomplete cherry-pick would satisfy
the condition.
In this case, the intent was to check that the conflicting file does
_not_ contain the resolved content. But the check actually uses the
wrong reference file, but not the resolved content. Needless to say
that the non-equality is satisfied. And, on top of it, it uses a commit
that does not even touch the file that is checked.
Do check for the expected result, which is content from both sides of
the merge and merge conflicts. (The latter we check for just the
middle separator for brevity.)
As a side-effect, this also removes an incorrect use of test_must_fail.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix invocation of git command so its exit codes is not lost within
a non-assignment command substitution.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We are using `test_must_fail test_auto_{fixup,squash}` which would
ensure that the function failed. However, this is a little ham-fisted as
there is no way of ensuring that the expected part of the function
actually fails.
Increase the granularity by accepting an optional `!` first argument
which will check that the rebase does not squash in any commits by
ensuring that there are still 4 commits. If `!` is not provided, the old
logic is run.
This patch may be better reviewed with `--ignore-all-space`.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix invocations of git commands so their exit codes are not lost
within non-assignment command substitutions.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have many statements which are duplicated. Extract and replace these
duplicate statements with notes_merge_files_gone().
While we're at it, replace the test_might_fail(), which should only be
used on git commands.
Also, remove the redirection from stderr to /dev/null. This is because
the test scripts automatically suppress output already. Otherwise, if a
developer asks for verbose output via the `-v` flag, the stderr output
may be useful for debugging.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We use `test_must_fail test -d` to ensure that the directory is removed.
However, test_must_fail() should only be used for git commands. Use
test_path_is_missing() instead to check that the directory has been
removed.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As part of the effort to become more hash-agnostic, replace all
instances of "sha" with "oid".
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix invocations of git commands so their exit codes are not lost
within non-assignment command substitutions.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We are running `test_must_fail do_checkout`. However,
`test_must_fail` should only be used on git commands. Teach
do_checkout() to accept `!` as a potential first argument which will
cause the function to expect the "git checkout" to fail.
This increases the granularity of the test as, instead of blindly
checking that do_checkout() failed, we check that only the specific
expected invocation of git fails.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Functions test_dirty_unmergeable() and test_dirty_mergeable()
expect git-diff to exit with the specific code 1. However, rather
than checking for that value explicitly, they instead negate the
exit code. Unfortunately, this negation makes it impossible to
distinguish the expected code from some other unexpected non-zero
code, for instance, from a segmentation fault. Therefore, be more
discerning by checking the exit code explicitly using
test_expect_code().
Furthermore, some callers of those functions want to negate the
result again, and do so with test_must_fail(). However,
test_must_fail() should only be used with git commands. Address
this by introducing a couple new tiny helper functions which test
the exact condition expected (without the unnecessarily confusing
double-negation).
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 6462d5eb9a ("fetch: remove fetch_if_missing=0", 2019-11-08)
contains a test that relies on having to lazily fetch the delta base of
a blob, but assumes that the tree being fetched (as part of the test) is
sent as a non-delta object. This assumption may not hold in the future;
for example, a change in the length of the object hash might result in
the tree being sent as a delta instead.
Make the test more robust by relying on having to lazily fetch the delta
base of the tree instead, and by making no assumptions on whether the
blobs are sent as delta or non-delta.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The flip_stage() helper uses a bit-flipping xor to switch between "2"
and "3". While clever, this relies on a property of those two numbers
that is mostly coincidence. Let's write it as a subtraction; that's more
clear and would extend to other numbers if somebody copies the logic.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The merge-recursive code uses stage number constants like this:
add = &ci->ren1->dst_entry->stages[2 ^ 1];
...
add = &ci->ren2->dst_entry->stages[3 ^ 1];
The xor has the effect of flipping the "1" bit, so that "2 ^ 1" becomes
"3" and "3 ^ 1" becomes "2", which correspond to the "ours" and "theirs"
stages respectively.
Unfortunately, clang-10 and up issue a warning for this code:
merge-recursive.c:1759:40: error: result of '2 ^ 1' is 3; did you mean '1 << 1' (2)? [-Werror,-Wxor-used-as-pow]
add = &ci->ren1->dst_entry->stages[2 ^ 1];
~~^~~
1 << 1
merge-recursive.c:1759:40: note: replace expression with '0x2 ^ 1' to silence this warning
We could silence it by using 0x2, as the compiler mentions. Or by just
using the constants "2" and "3" directly. But after digging into it, I
do think this bit-flip is telling us something. If we just wrote:
add = &ci->ren2->dst_entry->stages[2];
for the second one, you might think that "ren2" and "2" correspond. But
they don't. The logic is: ren2 is theirs, which is stage 3, but we
are interested in the opposite side's stage, so flip it to 2.
So let's keep the bit-flipping, but let's also put it behind a named
function, which will make its purpose a bit clearer. This also has the
side effect of suppressing the warning (and an optimizing compiler
should be able to easily turn it into a constant as before).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 28fcc0b71a (pathspec: avoid the need of "--" when wildcard is
used, 2015-05-02) allowed:
git rev-parse '*.c'
without the double-dash. But the rule it uses to check for wildcards
actually looks for any glob special. This is overly liberal, as it means
that a pattern that doesn't actually do any wildcard matching, like
"a\b", will be considered a pathspec.
If you do have such a file on disk, that's presumably what you wanted.
But if you don't, the results are confusing: rather than say "there's no
such path a\b", we'll quietly accept it as a pathspec which very likely
matches nothing (or at least not what you intended). Likewise, looking
for path "a\*b" doesn't expand the search at all; it would only find a
single entry, "a*b".
This commit switches the rule to trigger only when glob metacharacters
would expand the search, meaning both of those cases will now report an
error (you can still disambiguate using "--", of course; we're just
tightening the DWIM heuristic).
Note that we didn't test the original feature in 28fcc0b71a at all. So
this patch not only tests for these corner cases, but also adds a
regression test for the existing behavior.
Reported-by: David Burström <davidburstrom@spotify.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 49e268e23e (mingw: safeguard better against backslashes in file
names, 2020-01-09), the commit author is listed as
"Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>", which
is erroneous. Fix the authorship by mapping the erroneous authorship to
his canonical authorship information.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Original bash helper for "submodule status" was doing a check for
initialized but not cloned submodules and prefixed the status with
a minus sign in case no .git file or folder was found inside the
submodule directory.
This check was missed when the original port of the functionality
from bash to C was done.
Signed-off-by: Peter Kaestle <peter.kaestle@nokia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have test coverage for "git submodule status" output in
various cases, i.e.
1) not-init, not-cloned: status should initially be "missing"
2) init, not-cloned: status should be "missing"
3) not-init, cloned:
4) init, cloned: status should be "up-to-date" after update
4.1) + modified: status should be "modified" after submodule commit
4.2) + modified, committed: status should be "up-to-date" after update
but the cases 2) and 3) are not covered.
Test that submodule status reports initialized but not cloned
submodules as missing to fill the gap in test coverage; this covers
case (2) above, but case (3) remains uncovered.
Signed-off-by: Peter Kaestle <peter.kaestle@nokia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
