The odb_pack_keep() function generates the name of a .keep
file and opens it. This has two problems:
1. It requires a fixed-size buffer to create the filename
and doesn't notice when the result is truncated.
2. Of the two callers, one sometimes wants to open a
filename it already has, which makes things awkward (it
has to do so manually, and skips the leading-directory
creation).
Instead, let's have odb_pack_keep() just open the file.
Generating the name isn't hard, and a future patch will
switch callers over to odb_pack_name() anyway.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't throw the memory allocated for remove_dir_recursively() away after
a single call, use it for the other entries as well instead.
This change was done before in deb8e15a (rm: reuse strbuf for all
remove_dir_recursively() calls), but was reverted as a side-effect of
55856a35 (rm: absorb a submodules git dir before deletion). Reinstate
the optimization.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
prune_cache() first identifies those entries at the start of the sorted
array that can be discarded. Then it moves the rest of the entries up.
Last it identifies the unwanted trailing entries among the moved ones
and cuts them off.
Change the order: Identify both start *and* end of the range to keep
first and then move only those entries to the top. The resulting code
is slightly shorter and a bit more efficient.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function prune_cache() relies on the fact that it is only called on
max_prefix and sneakily uses the matching global variable max_prefix_len
directly. Tighten its interface by passing both the string and its
length as parameters. While at it move the NULL check into the function
to collect all cache-pruning related logic in one place.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `verbose` and `expire` options of the `git worktree prune`
subcommand have wrong descriptions in that they pretend to relate to
objects. But as the git-worktree(1) correctly states, these options have
nothing to do with objects but only with worktrees. Fix the description
accordingly.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git difftool -h" reports an error:
fatal: BUG: setup_git_env called without repository
Defer repository setup so that the help option processing happens before
the repository is initialized.
Add tests to ensure that the basic usage works inside and outside of a
repository.
Signed-off-by: David Aguilar <davvid@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Switch to dynamic allocation with strbuf, so we can avoid dealing
with magic numbers in the code and reduce the cognitive burden from
the programmers. The original code is correct, but programmers no
longer have to count bytes needed for static allocation to know that.
As a side effect of this change, we also reduce the snprintf()
calls, that may silently truncate results if the programmer is not
careful.
Helped-by: René Scharfe <l.s.r@web.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) are not meant to change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).
However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.
This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).
Based-on-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
string_list_clear() handles empty lists just fine, so remove the
redundant check.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the semantic patch swap.cocci to convert hand-rolled swaps to use
the macro SWAP. The resulting code is shorter and easier to read, the
object code is effectively unchanged.
The patch for object.c had to be hand-edited in order to preserve the
comment before the change; Coccinelle tried to eat it for some reason.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the semantic patch for converting callers that duplicate the
result of absolute_path() to call absolute_pathdup() instead, which
avoids an extra string copy to a static buffer.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The recent fixes to "fsck --connectivity-only" load all of
the objects with their correct types. This keeps the
connectivity-only code path close to the regular one, but it
also introduces some unnecessary inefficiency. While getting
the type of an object is cheap compared to actually opening
and parsing the object (as the non-connectivity-only case
would do), it's still not free.
For reachable non-blob objects, we end up having to parse
them later anyway (to see what they point to), making our
type lookup here redundant.
For unreachable objects, we might never hit them at all in
the reachability traversal, making the lookup completely
wasted. And in some cases, we might have quite a few
unreachable objects (e.g., when alternates are used for
shared object storage between repositories, it's normal for
there to be objects reachable from other repositories but
not the one running fsck).
The comment in mark_object_for_connectivity() claims two
benefits to getting the type up front:
1. We need to know the types during fsck_walk(). (And not
explicitly mentioned, but we also need them when
printing the types of broken or dangling commits).
We can address this by lazy-loading the types as
necessary. Most objects never need this lazy-load at
all, because they fall into one of these categories:
a. Reachable from our tips, and are coerced into the
correct type as we traverse (e.g., a parent link
will call lookup_commit(), which converts OBJ_NONE
to OBJ_COMMIT).
b. Unreachable, but not at the tip of a chunk of
unreachable history. We only mention the tips as
"dangling", so an unreachable commit which links
to hundreds of other objects needs only report the
type of the tip commit.
2. It serves as a cross-check that the coercion in (1a) is
correct (i.e., we'll complain about a parent link that
points to a blob). But we get most of this for free
already, because right after coercing, we'll parse any
non-blob objects. So we'd notice then if we expected a
commit and got a blob.
The one exception is when we expect a blob, in which
case we never actually read the object contents.
So this is a slight weakening, but given that the whole
point of --connectivity-only is to sacrifice some data
integrity checks for speed, this seems like an
acceptable tradeoff.
Here are before and after timings for an extreme case with
~5M reachable objects and another ~12M unreachable (it's the
torvalds/linux repository on GitHub, connected to shared
storage for all of the other kernel forks):
[before]
$ time git fsck --no-dangling --connectivity-only
real 3m4.323s
user 1m25.121s
sys 1m38.710s
[after]
$ time git fsck --no-dangling --connectivity-only
real 0m51.497s
user 0m49.575s
sys 0m1.776s
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When an object has a problem, we mention its type. But we do
so by feeding the result of typename() directly to
fprintf(). This is potentially dangerous because typename()
can return NULL for some type values (like OBJ_NONE).
It's doubtful that this can be triggered in practice with
the current code, so this is probably not fixing a bug. But
it future-proofs us against modifications that make things
like OBJ_NONE more likely (and gives future patches a
central point to handle them).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Building with "gcc -Wall" will complain that the format in:
warning("")
is empty. Which is true, but the warning is over-eager. We
are calling the function for its side effect of printing
"warning:", even with an empty string.
Our DEVELOPER Makefile knob disables the warning, but not
everybody uses it. Let's silence the warning in the code so
that nobody reports it or tries to "fix" it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When cf0adba788 ("Store peeled refs in packed-refs file.",
2006-11-19) made the command to die with a message on error even
when --quiet is passed, it left the comment to say it changed the
semantics. But that kind of information belongs to the log message,
not in-code comment. Besides, the behaviour after the change has
been the established one for the past 10 years ;-)
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As show_ref() is only ever called on the path where --verify is not
specified, `verify' can never possibly be true here.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move detection of dangling refs into show_one(), so that they are
detected when --verify is present as well as when it is absent.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Do the same with --quiet as was done with -d, to remove the need to
perform this check at show_one()'s call site from the --verify branch.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move handling of -d into show_one(), so that it takes effect when
--verify is present as well as when it is absent. This is useful when
the user wishes to avoid the costly iteration of refs.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, when --verify was specified, show-ref would use a separate
code path which did not handle HEAD and treated it as an invalid
ref. Thus, "git show-ref --verify HEAD" (where "--verify" is used
because the user is not interested in seeing refs/remotes/origin/HEAD)
did not work as expected.
Instead of insisting that the input begins with "refs/", allow "HEAD"
as well in the codepath that handles "--verify", so that all valid
full refnames including HEAD are passed to the same output machinery.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the really nice features of the ~/.gitconfig file is that users
can override defaults by their own preferred settings for all of their
repositories.
One such default that some users like to override is whether the
"origin" remote gets auto-pruned or not. The user would simply call
git config --global remote.origin.prune true
and from now on all "origin" remotes would be pruned automatically when
fetching into the local repository.
There is just one catch: now Git thinks that the "origin" remote is
configured, even if the repository config has no [remote "origin"]
section at all, as it does not realize that the "prune" setting was
configured globally and that there really is no "origin" remote
configured in this repository.
That is a problem e.g. when renaming a remote to a new name, when Git
may be fooled into thinking that there is already a remote of that new
name.
Let's fix this by paying more attention to *where* the remote settings
came from: if they are configured in the local repository config, we
must not overwrite them. If they were configured elsewhere, we cannot
overwrite them to begin with, as we only write the repository config.
There is only one caller of remote_is_configured() (in `git fetch`) that
may want to take remotes into account even if they were configured
outside the repository config; all other callers essentially try to
prevent the Git command from overwriting settings in the repository
config.
To accommodate that fact, the remote_is_configured() function now
requires a parameter that states whether the caller is interested in all
remotes, or only in those that were configured in the repository config.
Many thanks to Jeff King whose tireless review helped with settling for
nothing less than the current strategy.
This fixes https://github.com/git-for-windows/git/issues/888
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It served its purpose, but now we have a builtin difftool. Time for the
Perl script to enjoy Florida.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch gives life to the skeleton added in the previous patch.
The motivation for converting the difftool is that Perl scripts are not at
all native on Windows, and that `git difftool` therefore is pretty slow on
that platform, when there is no good reason for it to be slow.
In addition, Perl does not really have access to Git's internals. That
means that any script will always have to jump through unnecessary
hoops, and it will often need to perform unnecessary work (e.g. when
reading the entire config every time `git config` is called to query a
single config value).
The current version of the builtin difftool does not, however, make full
use of the internals but instead chooses to spawn a couple of Git
processes, still, to make for an easier conversion. There remains a lot
of room for improvement, left later.
Note: to play it safe, the original difftool is still called unless the
config setting difftool.useBuiltin is set to true.
The reason: this new, experimental, builtin difftool was shipped as part
of Git for Windows v2.11.0, to allow for easier large-scale testing, but
of course as an opt-in feature.
The speedup is actually more noticable on Linux than on Windows: a quick
test shows that t7800-difftool.sh runs in (2.183s/0.052s/0.108s)
(real/user/sys) in a Linux VM, down from (6.529s/3.112s/0.644s), while on
Windows, it is (36.064s/2.730s/7.194s), down from (47.637s/2.407s/6.863s).
The culprit is most likely the overhead incurred from *still* having to
shell out to mergetool-lib.sh and difftool--helper.sh.
Still, it is an improvement.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.
The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.
Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two spots that call lookup_object() and assume
that a non-NULL result means we have the object:
1. When we're checking the objects given to us by the user
on the command line.
2. When we're checking if a reflog entry is valid.
This generally follows fsck's mental model that we will have
looked at and loaded a "struct object" for each object in
the repository. But it misses one case: if another object
_mentioned_ an object, but we didn't actually parse it or
verify that it exists, it will still have a struct.
It's not clear if this is a triggerable bug or not.
Certainly the later parts of the reachability check need to
be careful of this, and do so by checking the HAS_OBJ flag.
But both of these steps happen before we start traversing,
so probably we won't have followed any links yet. Still,
it's easy enough to be defensive here.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since fsck tries to continue as much as it can after seeing
an error, we still do the reachability check even if some
heads we were given on the command-line are bogus. But if
_none_ of the heads is is valid, we fallback to checking all
refs and the index, which is not what the user asked for at
all.
Instead of checking "heads", the number of successful heads
we got, check "argc" (which we know only has non-options in
it, because parse_options removed the others).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of checking reachability from the refs, you can ask
fsck to check from a particular set of heads. However, the
error checking here is quite lax. In particular:
1. It claims lookup_object() will report an error, which
is not true. It only does a hash lookup, and the user
has no clue that their argument was skipped.
2. When either the name or sha1 cannot be resolved, we
continue to exit with a successful error code, even
though we didn't check what the user asked us to.
This patch fixes both of these cases.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Normally fsck makes a pass over all objects to check their
integrity, and then follows up with a reachability check to
make sure we have all of the referenced objects (and to know
which ones are dangling). The latter checks for the HAS_OBJ
flag in obj->flags to see if we found the object in the
first pass.
Commit 02976bf85 (fsck: introduce `git fsck --connectivity-only`,
2015-06-22) taught fsck to skip the initial pass, and to
fallback to has_sha1_file() instead of the HAS_OBJ check.
However, it converted only one HAS_OBJ check to use
has_sha1_file(). But there are many other places in
builtin/fsck.c that assume that the flag is set (or that
lookup_object() will return an object at all). This leads to
several bugs with --connectivity-only:
1. mark_object() will not queue objects for examination,
so recursively following links from commits to trees,
etc, did nothing. I.e., we were checking the
reachability of hardly anything at all.
2. When a set of heads is given on the command-line, we
use lookup_object() to see if they exist. But without
the initial pass, we assume nothing exists.
3. When loading reflog entries, we do a similar
lookup_object() check, and complain that the reflog is
broken if the object doesn't exist in our hash.
So in short, --connectivity-only is broken pretty badly, and
will claim that your repository is fine when it's not.
Presumably nobody noticed for a few reasons.
One is that the embedded test does not actually test the
recursive nature of the reachability check. All of the
missing objects are still in the index, and we directly
check items from the index. This patch modifies the test to
delete the index, which shows off breakage (1).
Another is that --connectivity-only just skips the initial
pass for loose objects. So on a real repository, the packed
objects were still checked correctly. But on the flipside,
it means that "git fsck --connectivity-only" still checks
the sha1 of all of the packed objects, nullifying its
original purpose of being a faster git-fsck.
And of course the final problem is that the bug only shows
up when there _is_ corruption, which is rare. So anybody
running "git fsck --connectivity-only" proactively would
assume it was being thorough, when it was not.
One possibility for fixing this is to find all of the spots
that rely on HAS_OBJ and tweak them for the connectivity-only
case. But besides the risk that we might miss a spot (and I
found three already, corresponding to the three bugs above),
there are other parts of fsck that _can't_ work without a
full list of objects. E.g., the list of dangling objects.
Instead, let's make the connectivity-only case look more
like the normal case. Rather than skip the initial pass
completely, we'll do an abbreviated one that sets up the
HAS_OBJ flag for each object, without actually loading the
object data.
That's simple and fast, and we don't have to care about the
connectivity_only flag in the rest of the code at all.
While we're at it, let's make sure we treat loose and packed
objects the same (i.e., setting up dummy objects for both
and skipping the actual sha1 check). That makes the
connectivity-only check actually fast on a real repo (40
seconds versus 180 seconds on my copy of linux.git).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This adds a builtin difftool that still falls back to the legacy Perl
version, which has been renamed to `legacy-difftool`.
The idea is that the new, experimental, builtin difftool immediately hands
off to the legacy difftool for now, unless the config variable
difftool.useBuiltin is set to true.
This feature flag will be used in the upcoming Git for Windows v2.11.0
release, to allow early testers to opt-in to use the builtin difftool and
flesh out any bugs.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After checking connectivity, fsck looks through the list of
any objects we've seen mentioned, and reports unreachable
and un-"used" ones as dangling. However, it skips any object
which is not marked as "parsed", as that is an object that
we _don't_ have (but that somebody mentioned).
Since 6e454b9a3 (clear parsed flag when we free tree
buffers, 2013-06-05), that flag can't be relied on, and the
correct method is to check the HAS_OBJ flag. The cleanup in
that commit missed this callsite, though. As a result, we
would generally fail to report dangling trees.
We never noticed because there were no tests in this area
(for trees or otherwise). Let's add some.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we iterate over the list of loose objects to check, we
get the actual path of each object. But we then throw it
away and pass just the sha1 to fsck_sha1(), which will do a
fresh lookup. Usually it would find the same object, but it
may not if an object exists both as a loose and a packed
object. We may end up checking the packed object twice, and
never look at the loose one.
In practice this isn't too terrible, because if fsck doesn't
complain, it means you have at least one good copy. But
since the point of fsck is to look for corruption, we should
be thorough.
The new read_loose_object() interface can help us get the
data from disk, and then we replace parse_object() with
parse_object_buffer(). As a bonus, our error messages now
mention the path to a corrupted object, which should make it
easier to track down errors when they do happen.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the PATH_MAX limitation from the environment setting that
points to a filename by switching to dynamic allocation.
As a side effect of this change, we also reduce the snprintf()
calls, that may silently truncate results if the programmer is not
careful.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the submodule helper we did not correctly handled the display path
for initializing submodules when both the submodule is inside a
subdirectory as well as the command being invoked from a subdirectory
(as viewed from the superproject).
This was broken in 3604242f08, which was written at a time where
there was no super-prefix available, so we abused the --prefix option
for the same purpose and could get only one case right (the call from
within a subdirectory, not the submodule being in a subdirectory).
Test-provided-by: David Turner <novalis@novalis.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All occurrences of OPT_SET_INT were setting the value to 1;
internally OPT_BOOL is just that.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert 'show_recursive()' to use the pathspec struct interface from
using the '_raw' entry in the pathspec struct.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the 'internal_copy_pathspec()' function to 'prefix_path()'
instead of using the deprecated 'get_pathspec()' interface. Also,
rename 'internal_copy_pathspec()' to 'internal_prefix_pathspec()' to be
more descriptive of what the funciton is actually doing.
In addition to this, fix a memory leak caused by only duplicating some
of the pathspec elements. Instead always duplicate all of the the
pathspec elements as an intermediate step (with modificationed based on
the passed in flags). This way the intermediate strings can then be
freed after getting the result from 'prefix_path()'.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's possible for content currently found in one file to
have originated in two separate files, each of which may
have been modified in some single older commit. The
--porcelain output generates an incorrect "previous" header
in this case, whereas --line-porcelain gets it right. The
problem is that the porcelain output tries to omit repeated
details of commits, and treats "previous" as a property of
the commit, when it is really a property of the blamed block
of lines.
Let's look at an example. In a case like this, you might see
this output from --line-porcelain:
SOME_SHA1 1 1 1
author ...
committer ...
previous SOME_SHA1^ file_one
filename file_one
...some line content...
SOME_SHA1 2 1 1
author ...
committer ...
previous SOME_SHA1^ file_two
filename file_two
...some different content....
The "filename" fields tell us that the two lines are from
two different files. But notice that the filename also
appears in the "previous" field, which tells us where to
start a re-blame. The second content line never appeared in
file_one at all, so we would obviously need to re-blame from
file_two (or possibly even some other file, if had just been
renamed to file_two in SOME_SHA1).
So far so good. Now here's what --porcelain looks like:
SOME_SHA1 1 1 1
author ...
committer ...
previous SOME_SHA1^ file_one
filename file_one
...some line content...
SOME_SHA1 2 1 1
filename file_two
...some different content....
We've dropped the author and committer fields from the
second line, as they would just be repeats. But we can't
omit "filename", because it depends on the actual block of
blamed lines, not just the commit. This is handled by
emit_porcelain_details(), which will show the filename
either if it is the first mention of the commit _or_ if the
commit has multiple paths in it.
But we don't give "previous" the same handling. It's written
inside emit_one_suspect_detail(), which bails early if we've
already seen that commit. And so the output above is wrong;
a reader would assume that the correct place to re-blame
line two is from file_one, but that's obviously nonsense.
Let's treat "previous" the same as "filename", and show it
fresh whenever we know we are in a confusing case like this.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
You can already ask blame for full sha1s with "-l" or with
"--abbrev=40". But for consistency with other parts of Git,
we should support "--no-abbrev".
Worse, blame already accepts --no-abbrev, but it's totally
broken. When we see --no-abbrev, the abbrev variable is set
to 0, which is then used as a printf precision. For regular
sha1s, that means we print nothing at all (which is very
wrong). For boundary commits we decrement it to "-1", which
printf interprets as "no limit" (which is almost correct,
except it misses the 39-length magic explained in the
previous commit).
Let's detect --no-abbrev and behave as if --abbrev=40 was
given.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The blame command internally adds 1 to any requested sha1
abbreviation length, and then subtracts it when outputting a
boundary commit. This lets regular and boundary sha1s line
up visually, but it misses one corner case.
When the requested length is 40, we bump the value to 41.
But since we only have 40 characters, that's all we can show
(fortunately the truncation is done by a printf precision
field, so it never tries to read past the end of the
buffer). So a normal sha1 shows 40 hex characters, and a
boundary sha1 shows "^" plus 40 hex characters. The result
is misaligned.
The "-l" option to show long sha1s gets around this by
skipping the "abbrev" variable entirely and just always
using GIT_SHA1_HEXSZ. This avoids the "+1" issue, but it
does mean that boundary commits only have 39 characters
printed. This is somewhat odd, but it does look good
visually: the results are aligned and left-justified. The
alternative would be to allocate an extra column that would
contain either an extra space or the "^" boundary marker.
As this is by definition the human-readable view, it's
probably not that big a deal either way (and of course
--porcelain, etc, correctly produce correct 40-hex sha1s).
But for consistency, this patch teaches --abbrev=40 to
produce the same output as "-l" (always left-aligned, with
40-hex for normal sha1s, and "^" plus 39-hex for
boundaries).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bitmap index only works for single packs, so requesting an
incremental repack with bitmap indexes makes no sense.
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When git gc --auto does an incremental repack of loose objects, we do
not expect to be able to write a bitmap; it is very likely that
objects in the new pack will have references to objects outside of the
pack. So we shouldn't try to write a bitmap, because doing so will
likely issue a warning.
This warning was making its way into gc.log. When the gc.log was
present, future auto gc runs would refuse to run.
Patch by Jeff King.
Bug report, test, and commit message by David Turner.
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When deleting a submodule, we need to keep the actual git directory around,
such that we do not lose local changes in there and at a later checkout
of the submodule we don't need to clone it again.
Now that the functionality is available to absorb the git directory of a
submodule, rewrite the checking in git-rm to not complain, but rather
relocate the git directories inside the superproject.
An alternative solution was discussed to have a function
`depopulate_submodule`. That would couple the check for its git directory
and possible relocation before the the removal, such that it is less
likely to miss the check in the future. But the indirection with such
a function added seemed also complex. The reason for that was that this
possible move of the git directory was also implemented in
`ok_to_remove_submodule`, such that this function could truthfully
answer whether it is ok to remove the submodule.
The solution proposed here defers all these checks to the caller.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In different contexts the question "Is it ok to delete a submodule?"
may be answered differently.
In 293ab15eea (submodule: teach rm to remove submodules unless they
contain a git directory, 2012-09-26) a case was made that we can safely
ignore ignored untracked files for removal as we explicitely ask for the
removal of the submodule.
In a later patch we want to remove submodules even when the user doesn't
explicitly ask for it (e.g. checking out a tree-ish in which the submodule
doesn't exist). In that case we want to be more careful when it comes
to deletion of untracked files. As of this patch it is unclear how this
will be implemented exactly, so we'll offer flags in which the caller
can specify how the different untracked files ought to be handled.
As the flags allow the function to not die on an error when spawning
a child process, we need to find an appropriate return code for the
case when the child process could not be started. As in that case we
cannot tell if the submodule is ok to remove, we'd want to return 'false'.
As only 0 is understood as false, rename the function to invert the
meaning, i.e. the return code of 0 signals the removal of the submodule
is fine, and other values can be used to return a more precise answer
what went wrong.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a patch inserts a block of lines, whose last lines are the
same as the existing lines that appear before the inserted block,
"git diff" can choose any place between these existing lines as the
boundary between the pre-context and the added lines (adjusting the
end of the inserted block as appropriate) to come up with variants
of the same patch, and some variants are easier to read than others.
We have been trying to improve the choice of this boundary, and Git
2.11 shipped with an experimental "compaction-heuristic". Since
then another attempt to improve the logic further resulted in a new
"indent-heuristic" logic. It is agreed that the latter gives better
result overall, and the former outlived its usefulness.
Retire "compaction", and keep "indent" as an experimental feature.
The latter hopefully will be turned on by default in a future
release, but that should be done as a separate step.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a submodule was renamed at any point since it's inception then if you
were to try and grep on a commit prior to the submodule being moved, you
wouldn't be able to find a working directory for the submodule since the
path in the past is different from the current path.
This patch teaches grep to find the .git directory for a submodule in
the parents .git/modules/ directory in the event the path to the
submodule in the commit that is being searched differs from the state of
the currently checked out commit. If found, the child process that is
spawned to grep the submodule will chdir into its gitdir instead of a
working directory.
In order to override the explicit setting of submodule child process's
gitdir environment variable (which was introduced in '10f5c526')
`GIT_DIR_ENVIORMENT` needs to be pushed onto child process's env_array.
This allows the searching of history from a submodule's gitdir, rather
than from a working directory.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach grep to recursively search in submodules when provided with a
<tree> object. This allows grep to search a submodule based on the state
of the submodule that is present in a commit of the super project.
When grep is provided with a <tree> object, the name of the object is
prefixed to all output. In order to provide uniformity of output
between the parent and child processes the option `--parent-basename`
has been added so that the child can preface all of it's output with the
name of the parent's object instead of the name of the commit SHA1 of
the submodule. This changes output from the command
`git grep -e. -l --recurse-submodules HEAD` from:
HEAD:file
<commit sha1 of submodule>:sub/file
to:
HEAD:file
HEAD:sub/file
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow grep to recognize submodules and recursively search for patterns in
each submodule. This is done by forking off a process to recursively
call grep on each submodule. The top level --super-prefix option is
used to pass a path to the submodule which can in turn be used to
prepend to output or in pathspec matching logic.
Recursion only occurs for submodules which have been initialized and
checked out by the parent project. If a submodule hasn't been
initialized and checked out it is simply skipped.
In order to support the existing multi-threading infrastructure in grep,
output from each child process is captured in a strbuf so that it can be
later printed to the console in an ordered fashion.
To limit the number of theads that are created, each child process has
half the number of threads as its parents (minimum of 1), otherwise we
potentailly have a fork-bomb.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King