We're not really removing slashes, but slash-separated path
components. Let's make that more clear.
Reported-by: kelly elton <its.the.doc@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
b2dc968e60 (t5516: refactor oddball tests, 2008-11-07) accidentaly
broke the &&-chain in the test 'push does not update local refs on
failure', but since it was in a subshell, chain-lint couldn't notice
it.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a while-at-it cleanup replacing a 'cd dir && <...> && cd ..' with a
subshell, commit 28391a80a9 (receive-pack: allow deletion of corrupt
refs, 2007-11-29) also moved the assignment of the $old_commit
variable to that subshell. This variable, however, is used outside of
that subshell as a parameter of check_push_result(), to check that a
ref still points to the commit where it is supposed to. With the
variable remaining unset outside the subshell check_push_result()
doesn't perform that check at all.
Use 'git -C <dir> cmd...', so we don't need to change directory, and
thus don't need the subshell either when setting $old_commit.
Furthermore, change check_push_result() to require at least three
parameters (the repo, the oid, and at least one ref), so it will catch
similar issues earlier should they ever arise.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A subsequent change will make use of this static function in the
get_short_oid() function, which is defined above where the
collect_ambiguous() function is now. Without this we'd then have a
compilation error due to a forward declaration.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The two JGit tests 'we can read jgit bitmaps' and 'jgit can read our
bitmaps' in 't5310-pack-bitmaps.sh' fail when run with
GIT_TEST_SPLIT_INDEX=YesPlease. Both tests create a clone of the test
repository to check bitmap interoperability with JGit. With split
indexes enabled the index in the clone repositories contains the
'link' extension, which JGit doesn't support and, consequently, an
exception aborts it:
<...>
org.eclipse.jgit.api.errors.JGitInternalException: DIRC extension 'link' not supported by this version.
at org.eclipse.jgit.dircache.DirCache.readFrom(DirCache.java:562)
<...>
Since testing bitmaps doesn't need a worktree in the first place,
let's just create bare clones for the two JGit tests, so the cloned
won't have an index, and these two tests can be executed even with
split index enabled.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This was the only occurrence of "commitish" in the tree, but as the
log will reveal we've had others in the past. Fixes up code added in
00ad6e3182 ("git-p4: work with a detached head", 2015-11-21).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The arguments weren't lined up with the opening parenthesis, after
910650d2 ("Rename sha1_array to oid_array", 2017-03-31) renamed the
function.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This stray newline was accidentally introduced in
d2b7d9c7ed ("sha1_name: convert disambiguate_hint_fn to take
object_id", 2017-03-26).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In commit dc8441fdb ("config: don't implicitly use gitdir or commondir",
2017-06-14) the function git_config_with_options was renamed to
config_with_options to better reflect the fact that it does not access
the git global config or the repo config by default.
However Documentation/technical/api-config.txt still refers to the
previous name, fix that.
While at it also update the documentation about the extra parameters,
because they too changed since the initial definition.
Signed-off-by: Antonio Ospite <ao2@ao2.it>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This aims to make git-submodule foreach a builtin. 'foreach' is ported to
the submodule--helper, and submodule--helper is called from
git-submodule.sh.
Helped-by: Brandon Williams <bmwill@google.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After the last patch, common-cmds.h is no longer used (and it was
actually broken). Remove all related code. command-list.h will take
its place from now on.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit added code generation for all_cmd_desc[] which
includes almost everything we need to generate common command list.
Convert help code to use that array instead and drop common_cmds[] array.
The description of each common command group is removed from
command-list.txt. This keeps this file format simpler. common-cmds.h
will not be generated correctly after this change due to the
command-list.txt format change. But it does not matter and
common-cmds.h will be removed.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current generate-cmds.sh generates just enough to print "git help"
output. That is, it only extracts help text for common commands.
The script is now updated to extract help text for all commands and
keep command classification a new file, command-list.h. This will be
useful later:
- "git help -a" could print a short summary of all commands instead of
just the common ones.
- "git" could produce a list of commands of one or more category. One
of its use is to reduce another command classification embedded in
git-completion.bash.
The new file can be generated but is not used anywhere yet. The plan
is we migrate away from common-cmds.h. Then we can kill off
common-cmds.h build rules and generation code (and also delete
duplicate content in command-list.h which we keep for now to not mess
generate-cmds.sh up too much).
PS. The new fixed column requirement on command-list.txt is
technically not needed. But it helps simplify the code a bit at this
stage. We could lift this restriction later if we want to.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes it easier to reuse the same code in another place (very
soon).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This was missed in 5982da9d2c (replace-object: allow
prepare_replace_object to handle arbitrary repositories, 2018-04-11)
Technically the code works correctly as the replace_map is the same
size in different repositories, however it is hard to read. So convert
the code to the familiar pattern of dereferencing the pointer that we
assign in the sizeof itself.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The replace map for objects was missed to free in the object store in
the conversion of 174774cd51 (Merge branch 'sb/object-store-replace',
2018-05-08)
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
the_repository is special. One of the special things about it is that
it does not allocate a new index_state object like submodules but
points to the global the_index variable instead. As a global variable,
the_index cannot be free()'d.
Add an exception for this in repo_clear(). In the future perhaps we
would be able to allocate the_repository's index on heap too. Then we
can revert this.
the_repository->index remains pointed to a clean the_index even after
repo_clear() so that it could still be used next time (e.g. in a crazy
use case where a dev switches repo in the same process).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clang 6 reports the following warning, which is turned into an error in a
DEVELOPER build:
builtin/fast-export.c:162:28: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
return ((uint32_t *)NULL) + mark;
~~~~~~~~~~~~~~~~~~ ^
1 error generated.
The compiler is correct, and the error message speaks for itself. There
is no need for any undefined operation -- just cast mark to void * or
uint32_t after an intermediate cast to uintptr_t. That encodes the
integer value into a pointer and later decodes it as intended.
While at it remove an outdated comment -- intptr_t has been used since
ffe659f94d (parse-options: make some arguments optional, add callbacks),
committed in October 2007.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Placing `struct lock_file`s on the stack used to be a bad idea, because
the temp- and lockfile-machinery would keep a pointer into the struct.
But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap,
2017-09-05), we can safely have lockfiles on the stack. (This applies
even if a user returns early, leaving a locked lock behind.)
Each of these `struct lock_file`s is used from within a single function.
Move them into the respective functions to make the scope clearer and
drop the staticness.
For good measure, I have inspected these sites and come to believe that
they always release the lock, with the possible exception of bailing out
using `die()` or `exit()` or by returning from a `cmd_foo()`.
As pointed out by Jeff King, it would be bad if someone held on to a
`struct lock_file *` for some reason. After some grepping, I agree with
his findings: no-one appears to be doing that.
After this commit, the remaining occurrences of "static struct
lock_file" are locks that are used from within different functions. That
is, they need to remain static. (Short of more intrusive changes like
passing around pointers to non-static locks.)
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Placing `struct lock_file`s on the stack used to be a bad idea, because
the temp- and lockfile-machinery would keep a pointer into the struct.
But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap,
2017-09-05), we can safely have lockfiles on the stack. (This applies
even if a user returns early, leaving a locked lock behind.)
These `struct lock_file`s are local to their respective functions and we
can drop their staticness.
For good measure, I have inspected these sites and come to believe that
they always release the lock, with the possible exception of bailing out
using `die()` or `exit()` or by returning from a `cmd_foo()`.
As pointed out by Jeff King, it would be bad if someone held on to a
`struct lock_file *` for some reason. After some grepping, I agree with
his findings: no-one appears to be doing that.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After taking the lock we check whether we got it and die otherwise. But
since we take the lock using `LOCK_DIE_ON_ERROR`, we would already have
died.
Considering the choice between dropping the dead code and dropping the
flag, let's go for option number three: Drop the flag, write an error
instead of dying, then return -1. This function already returns -1 for
another error, so the caller (or rather, its callers) should be able to
handle this. There is some inconsistency around how we handle errors in
this function and elsewhere in this file, but let's take this small step
towards gentle error-reporting now and leave the rest for another time.
While at it, make the lock non-static and reduce its scope. (Placing
`struct lock_file`s on the stack used to be a bad idea, because the
temp- and lockfile-machinery would keep a pointer into the struct. But
after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05),
we can safely have lockfiles on the stack.)
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we could not take the lock, we add an error to the `strbuf err` and
return. However, this code is dead. The reason is that we take the lock
using `LOCK_DIE_ON_ERROR`. Drop the flag to allow our more gentle
error-handling to actually kick in.
We could instead just drop the dead code and die here. But everything is
prepared for gently propagating the error, so let's do that instead.
There is similar dead code in `delete_pseudoref()`, but let's save that
for the next patch.
While at it, make the lock non-static. (Placing `struct lock_file`s on
the stack used to be a bad idea, because the temp- and
lockfile-machinery would keep a pointer into the struct. But after
076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we
can safely have lockfiles on the stack.)
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Die in case writing the index fails, so that the caller can notice
(instead of, say, being impressed by how performant the writing is).
While at it, note that after opening a lock with `LOCK_DIE_ON_ERROR`, we
do not need to worry about whether we succeeded. Also, we can move the
`struct lock_file` into the function and drop the staticness. (Placing
`struct lock_file`s on the stack used to be a bad idea, because the
temp- and lockfile-machinery would keep a pointer into the struct. But
after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05),
we can safely have lockfiles on the stack.)
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It was observed that the variable '$displaypath' was accessible but
undocumented. Hence, document it.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As using a variable '$path' may be harmful to users due to
capitalization issues, see 64394e3ae9 (git-submodule.sh: Don't
use $path variable in eval_gettext string, 2012-04-17). Adjust
the documentation to advocate for using $sm_path, which contains
the same value. We still make the 'path' variable available and
document it as a deprecated synonym of 'sm_path'.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When running 'git submodule foreach --recursive' from a subdirectory of
your repository, nested submodules get a bogus value for $path:
For a submodule 'sub' that contains a nested submodule 'nested',
running 'git -C dir submodule foreach echo $path' from the root of the
superproject would report path='../nested' for the nested submodule.
The first part '../' is derived from the logic computing the relative
path from $pwd to the root of the superproject. The second part is the
submodule path inside the submodule. This value is of little use and is
hard to document.
Also, in git-submodule.txt, $path is documented to be the "name of the
submodule directory relative to the superproject", but "the
superproject" is ambiguous.
To resolve both these issues, we could:
(a) Change "the superproject" to "its immediate superproject", so
$path would be "nested" instead of "../nested".
(b) Change "the superproject" to "the superproject the original
command was run from", so $path would be "sub/nested" instead of
"../nested".
(c) Change "the superproject" to "the directory the original command
was run from", so $path would be "../sub/nested" instead of
"../nested".
The behavior for (c) was attempted to be introduced in 091a6eb0fe
(submodule: drop the top-level requirement, 2013-06-16) with the intent
for $path to be relative from $pwd to the submodule worktree, but that
did not work for nested submodules, as the intermittent submodules
were not included in the path.
If we were to fix the meaning of the $path using (a), we would break
any existing submodule user that runs foreach from non-root of the
superproject as the non-nested submodule '../sub' would change its
path to 'sub'.
If we were to fix the meaning of $path using (b), then we would break
any user that uses nested submodules (even from the root directory)
as the 'nested' would become 'sub/nested'.
If we were to fix the meaning of $path using (c), then we would break
the same users as in (b) as 'nested' would become 'sub/nested' from
the root directory of the superproject.
All groups can be found in the wild. The author has no data if one group
outweighs the other by large margin, and offending each one seems equally
bad at first. However in the authors imagination it is better to go with
(a) as running from a sub directory sounds like it is carried out by a
human rather than by some automation task. With a human on the keyboard
the feedback loop is short and the changed behavior can be adapted to
quickly unlike some automation that can break silently.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a small mechanical change; it doesn't change the
implementation to handle repositories other than the_repository yet.
Use a macro to catch callers passing a repository other than
the_repository at compile time.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a repository argument to allow the caller of grow_object_hash to
be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a repository argument to allow the callers of create_object
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the existing global cache for parsed objects (obj_hash) into
repository-specific parsed object caches. Existing code that uses
obj_hash are modified to use the parsed object cache of
the_repository; future patches will use the parsed object caches of
other repositories.
Another future use case for a pool of objects is ease of memory management
in revision walking: If we can free the rev-list related memory early in
pack-objects (e.g. part of repack operation) then it could lower memory
pressure significantly when running on large repos. While this has been
discussed on the mailing list lately, this series doesn't implement this.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test script 't6050-replace.sh' starts off with redirecting the
whole test script's stdin from /dev/null. This redirection has been
there since the test script was introduced in a3e8267225
(replace_object: add a test case, 2009-01-23), but the commit message
doesn't explain why it was deemed necessary. AFAICT, it has never
been necessary, and t6050 runs just fine and succeeds even without it,
not only the current version but past versions as well.
Besides being unnecessary, this redirection is also harmful, as it
prevents the test helper functions 'test_pause' and 'debug' from
working properly in t6050, because we can't enter any commands to the
shell and the debugger, respectively.
So let's remove that redirection.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Set aggressive flag in git_merge_trees() when rename detection is turned off.
This allows read_tree() to auto resolve more cases that would have otherwise
been handled by the rename detection.
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the ability to control rename detection for merge via a config setting.
This setting behaves the same and defaults to the value of diff.renames but only
applies to merge.
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update the documentation to better indicate that the renameLimit setting is
ignored if rename detection is turned off via command line options or config
settings.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The can-working-tree-updates-be-skipped check has had a long and blemished
history. The update can be skipped iff:
a) The merge is clean
b) The merge matches what was in HEAD (content, mode, pathname)
c) The target path is usable (i.e. not involved in D/F conflict)
Traditionally, we split b into parts:
b1) The merged result matches the content and mode found in HEAD
b2) The merged target path existed in HEAD
Steps a & b1 are easy to check; we have always gotten those right. While
it is easy to overlook step c, this was fixed seven years ago with commit
4ab9a157d0 ("merge_content(): Check whether D/F conflicts are still
present", 2010-09-20). merge-recursive didn't have a readily available
way to directly check step b2, so various approximations were used:
* In commit b2c8c0a762 ("merge-recursive: When we detect we can skip
an update, actually skip it", 2011-02-28), it was noted that although
the code claimed it was skipping the update, it did not actually skip
the update. The code was made to skip it, but used lstat(path, ...)
as an approximation to path-was-tracked-in-index-before-merge.
* In commit 5b448b8530 ("merge-recursive: When we detect we can skip
an update, actually skip it", 2011-08-11), the problem with using
lstat was noted. It was changed to the approximation
path2 && strcmp(path, path2)
which is also wrong. !path2 || strcmp(path, path2) would have been
better, but would have fallen short with directory renames.
* In c5b761fb27 ("merge-recursive: ensure we write updates for
directory-renamed file", 2018-02-14), the problem with the previous
approximation was noted and changed to
was_tracked(path)
That looks close to what we were trying to answer, but was_tracked()
as implemented at the time should have been named is_tracked(); it
returned something different than what we were looking for.
* To make matters more complex, fixing was_tracked() isn't sufficient
because the splitting of b into b1 and b2 is wrong. Consider the
following merge with a rename/add conflict:
side A: modify foo, add unrelated bar
side B: rename foo->bar (but don't modify the mode or contents)
In this case, the three-way merge of original foo, A's foo, and B's
bar will result in a desired pathname of bar with the same
mode/contents that A had for foo. Thus, A had the right mode and
contents for the file, and it had the right pathname present (namely,
bar), but the bar that was present was unrelated to the contents, so
the working tree update was not skippable.
Fix this by introducing a new function:
was_tracked_and_matches(o, path, &mfi.oid, mfi.mode)
and use it to directly check for condition b.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, merge_content() would print "Auto-merging" whenever the final
content and mode aren't already available from HEAD. There are a few
problems with this:
1) There are other code paths doing merges that should probably have the
same message printed, in particular rename/rename(2to1) which cannot
call into the normal rename logic.
2) If both sides of the merge have modifications, then a content merge
is needed. It may turn out that the end result matches one of the
sides (because the other only had a subset of the same changes), but
the merge was still needed. Currently, the message will not print in
that case, though it seems like it should.
Move the printing of this message to merge_file_1() in order to address
both issues.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
was_dirty() uses was_tracked(), which has been updated to use the original
index rather than the current one. However, was_dirty() also had a
separate call to cache_file_exists(), causing it to still implicitly use
the current index. Update that to instead use index_file_exists().
Also, was_dirty() had a hack where it would mark any file as non-dirty if
we simply didn't know its modification time. This was due to using the
current index rather than the original index, because D/F conflicts and
such would cause unpack_trees() to not copy the modification times from
the original index to the current one. Now that we are using the original
index, we can dispense with this hack.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In commit aacb82de3f ("merge-recursive: Split was_tracked() out of
would_lose_untracked()", 2011-08-11), was_tracked() was split out of
would_lose_untracked() with the intent to provide a function that could
answer whether a path was tracked in the index before the merge. Sadly,
it instead returned whether the path was in the working tree due to having
been tracked in the index before the merge OR having been written there by
unpack_trees(). The distinction is important when renames are involved,
e.g. for a merge where:
HEAD: modifies path b
other: renames b->c
In this case, c was not tracked in the index before the merge, but would
have been added to the index at stage 0 and written to the working tree by
unpack_trees(). would_lose_untracked() is more interested in the
in-working-copy-for-either-reason behavior, while all other uses of
was_tracked() want just was-it-tracked-in-index-before-merge behavior.
Unsplit would_lose_untracked() and write a new was_tracked() function
which answers whether a path was tracked in the index before the merge
started.
This will also affect was_dirty(), helping it to return better results
since it can base answers off the original index rather than an index that
possibly only copied over some of the stat information. However,
was_dirty() will need an additional change that will be made in a
subsequent patch.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add several tests checking whether updates can be skipped in a merge.
Also add several similar testcases for where updates cannot be skipped in
a merge to make sure that we skip if and only if we should.
In particular:
* Testcase 1a (particularly 1a-check-L) would have pointed out the
problem Linus has been dealing with for year with his merges[1].
* Testcase 2a (particularly 2a-check-L) would have pointed out the
problem with my directory-rename-series before it broke master[2].
* Testcases 3[ab] (particularly 3a-check-L) provide a simpler testcase
than 12b of t6043 making that one easier to understand.
* There are several complementary testcases to make sure we're not just
fixing those particular issues while regressing in the opposite
direction.
* There are also a pair of tests for the special case when a merge
results in a skippable update AND the user has dirty modifications to
the path.
[1] https://public-inbox.org/git/CA+55aFzLZ3UkG5svqZwSnhNk75=fXJRkvU1m_RHBG54NOoaZPA@mail.gmail.com/
[2] https://public-inbox.org/git/xmqqmuya43cs.fsf@gitster-ct.c.googlers.com/
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a cherry-pick or merge with a rename results in a skippable update
(due to the merged content matching what HEAD already had), but the
working directory is dirty, avoid trying to refresh the index as that
will fail.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
conflict_rename_normal() was doing some handling for dirty files that
more naturally belonged in merge_content. Move it, and rename a
parameter for clarity while at it.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
