Commit Graph

5308 Commits

Author SHA1 Message Date
Junio C Hamano
cc8364c28b Merge branch 'ep/commit-static-buf-cleanup'
Code clean-up.

* ep/commit-static-buf-cleanup:
  builtin/commit.c: switch to strbuf, instead of snprintf()
  builtin/commit.c: remove the PATH_MAX limitation via dynamic allocation
2017-02-02 13:36:57 -08:00
Junio C Hamano
140d41ae87 Merge branch 'rs/receive-pack-cleanup'
Code clean-up.

* rs/receive-pack-cleanup:
  receive-pack: call string_list_clear() unconditionally
2017-02-02 13:36:57 -08:00
Junio C Hamano
6f1c08bdb7 Merge branch 'rs/absolute-pathdup'
Code cleanup.

* rs/absolute-pathdup:
  use absolute_pathdup()
  abspath: add absolute_pathdup()
2017-02-02 13:36:55 -08:00
Junio C Hamano
4ba6197887 Merge branch 'jk/fsck-connectivity-check-fix'
"git fsck --connectivity-check" was not working at all.

* jk/fsck-connectivity-check-fix:
  fsck: lazily load types under --connectivity-only
  fsck: move typename() printing to its own function
  t1450: use "mv -f" within loose object directory
  fsck: check HAS_OBJ more consistently
  fsck: do not fallback "git fsck <bogus>" to "git fsck"
  fsck: tighten error-checks of "git fsck <head>"
  fsck: prepare dummy objects for --connectivity-check
  fsck: report trees as dangling
  t1450: clean up sub-objects in duplicate-entry test
2017-01-31 13:15:01 -08:00
Junio C Hamano
b7786bb4b0 Merge branch 'js/difftool-builtin'
Rewrite a scripted porcelain "git difftool" in C.

* js/difftool-builtin:
  difftool: hack around -Wzero-length-format warning
  difftool: retire the scripted version
  difftool: implement the functionality in the builtin
  difftool: add a skeleton for the upcoming builtin
2017-01-31 13:15:00 -08:00
Junio C Hamano
a49260b17d Merge branch 'vp/show-ref-verify-head'
"git show-ref HEAD" used with "--verify" because the user is not
interested in seeing refs/remotes/origin/HEAD, and used with
"--head" because the user does not want HEAD to be filtered out,
i.e. "git show-ref --head --verify HEAD", did not work as expected.

* vp/show-ref-verify-head:
  show-ref: remove a stale comment
  show-ref: remove dead `if (verify)' check
  show-ref: detect dangling refs under --verify as well
  show-ref: move --quiet handling into show_one()
  show-ref: allow -d to work with --verify
  show-ref: accept HEAD with --verify
2017-01-31 13:14:59 -08:00
Junio C Hamano
fe575f0653 Merge branch 'js/remote-rename-with-half-configured-remote'
With anticipatory tweaking for remotes defined in ~/.gitconfig
(e.g. "remote.origin.prune" set to true, even though there may or
may not actually be "origin" remote defined in a particular Git
repository), "git remote rename" and other commands misinterpreted
and behaved as if such a non-existing remote actually existed.

* js/remote-rename-with-half-configured-remote:
  remote rename: more carefully determine whether a remote is configured
  remote rename: demonstrate a bogus "remote exists" bug
2017-01-31 13:14:59 -08:00
Junio C Hamano
237bdd9ddb Merge branch 'st/verify-tag'
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.

* st/verify-tag:
  t/t7004-tag: Add --format specifier tests
  t/t7030-verify-tag: Add --format specifier tests
  builtin/tag: add --format argument for tag -v
  builtin/verify-tag: add --format to verify-tag
  ref-filter: add function to print single ref_array_item
  gpg-interface, tag: add GPG_VERIFY_OMIT_STATUS flag
2017-01-31 13:14:58 -08:00
Junio C Hamano
42ace93e41 Merge branch 'jk/loose-object-fsck'
"git fsck" inspects loose objects more carefully now.

* jk/loose-object-fsck:
  fsck: detect trailing garbage in all object types
  fsck: parse loose object paths directly
  sha1_file: add read_loose_object() function
  t1450: test fsck of packed objects
  sha1_file: fix error message for alternate objects
  t1450: refactor loose-object removal
2017-01-31 13:14:57 -08:00
Junio C Hamano
792e22e3fd Merge branch 'bw/push-submodule-only'
"git submodule push" learned "--recurse-submodules=only option to
push submodules out without pushing the top-level superproject.

* bw/push-submodule-only:
  push: add option to push only submodules
  submodules: add RECURSE_SUBMODULES_ONLY value
  transport: reformat flag #defines to be more readable
2017-01-31 13:14:56 -08:00
Elia Pinto
4a5281917b builtin/commit.c: switch to strbuf, instead of snprintf()
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>
2017-01-31 10:09:00 -08:00
René Scharfe
4432dd6b5b receive-pack: call string_list_clear() unconditionally
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>
2017-01-30 15:08:58 -08:00
René Scharfe
0aaad415bc use absolute_pathdup()
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>
2017-01-27 10:18:15 -08:00
Jeff King
a2b22854bd fsck: lazily load types under --connectivity-only
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>
2017-01-26 10:51:09 -08:00
Jeff King
97ca7ca8ba fsck: move typename() printing to its own function
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>
2017-01-26 10:47:20 -08:00
Jeff King
94d3997ecc difftool: hack around -Wzero-length-format warning
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>
2017-01-25 13:28:34 -08:00
Junio C Hamano
0d583ff02d show-ref: remove a stale comment
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>
2017-01-23 18:51:56 -08:00
Junio C Hamano
e801be066c Merge branch 'sb/submodule-init'
Error message fix.

* sb/submodule-init:
  submodule update --init: display correct path from submodule
2017-01-23 15:59:21 -08:00
Vladimir Panteleev
02bdc9d9f6 show-ref: remove dead `if (verify)' check
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>
2017-01-23 12:06:30 -08:00
Vladimir Panteleev
d01b8203ec show-ref: detect dangling refs under --verify as well
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>
2017-01-23 12:06:29 -08:00
Vladimir Panteleev
14144d3b53 show-ref: move --quiet handling into show_one()
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>
2017-01-23 12:06:29 -08:00
Vladimir Panteleev
f1627040b9 show-ref: allow -d to work with --verify
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>
2017-01-23 12:06:29 -08:00
Vladimir Panteleev
ec7c51bc6b show-ref: accept HEAD with --verify
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>
2017-01-23 12:06:29 -08:00
Johannes Schindelin
e459b073fb remote rename: more carefully determine whether a remote is configured
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>
2017-01-19 14:04:23 -08:00
Johannes Schindelin
019678d6b1 difftool: retire the scripted version
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>
2017-01-19 13:23:43 -08:00
Johannes Schindelin
03831ef7b5 difftool: implement the functionality in the builtin
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>
2017-01-19 13:22:36 -08:00
Junio C Hamano
fe9ec8bdf6 Merge branch 'bw/pathspec-cleanup'
Code clean-up in the pathspec API.

* bw/pathspec-cleanup:
  pathspec: rename prefix_pathspec to init_pathspec_item
  pathspec: small readability changes
  pathspec: create strip submodule slash helpers
  pathspec: create parse_element_magic helper
  pathspec: create parse_long_magic function
  pathspec: create parse_short_magic function
  pathspec: factor global magic into its own function
  pathspec: simpler logic to prefix original pathspec elements
  pathspec: always show mnemonic and name in unsupported_magic
  pathspec: remove unused variable from unsupported_magic
  pathspec: copy and free owned memory
  pathspec: remove the deprecated get_pathspec function
  ls-tree: convert show_recursive to use the pathspec struct interface
  dir: convert fill_directory to use the pathspec struct interface
  dir: remove struct path_simplify
  mv: remove use of deprecated 'get_pathspec()'
2017-01-18 15:12:15 -08:00
Junio C Hamano
256d3dabbd Merge branch 'jk/blame-fixes'
"git blame --porcelain" misidentified the "previous" <commit, path>
pair (aka "source") when contents came from two or more files.

* jk/blame-fixes:
  blame: output porcelain "previous" header for each file
  blame: handle --no-abbrev
  blame: fix alignment with --abbrev=40
2017-01-18 15:12:13 -08:00
Junio C Hamano
cf417e2c1f Merge branch 'dt/disable-bitmap-in-auto-gc'
It is natural that "git gc --auto" may not attempt to pack
everything into a single pack, and there is no point in warning
when the user has configured the system to use the pack bitmap,
leading to disabling further "gc".

* dt/disable-bitmap-in-auto-gc:
  repack: die on incremental + write-bitmap-index
  auto gc: don't write bitmaps for incremental repacks
2017-01-18 15:12:12 -08:00
Junio C Hamano
3ccd681c2a Merge branch 'sb/submodule-rm-absorb'
"git rm" used to refuse to remove a submodule when it has its own
git repository embedded in its working tree.  It learned to move
the repository away to $GIT_DIR/modules/ of the superproject
instead, and allow the submodule to be deleted (as long as there
will be no loss of local modifications, that is).

* sb/submodule-rm-absorb:
  rm: absorb a submodules git dir before deletion
  submodule: rename and add flags to ok_to_remove_submodule
  submodule: modernize ok_to_remove_submodule to use argv_array
  submodule.h: add extern keyword to functions
2017-01-18 15:12:11 -08:00
Junio C Hamano
55d128ae06 Merge branch 'bw/grep-recurse-submodules'
"git grep" has been taught to optionally recurse into submodules.

* bw/grep-recurse-submodules:
  grep: search history of moved submodules
  grep: enable recurse-submodules to work on <tree> objects
  grep: optionally recurse into submodules
  grep: add submodules as a grep source type
  submodules: load gitmodules file from commit sha1
  submodules: add helper to determine if a submodule is initialized
  submodules: add helper to determine if a submodule is populated
  real_path: canonicalize directory separators in root parts
  real_path: have callers use real_pathdup and strbuf_realpath
  real_path: create real_pathdup
  real_path: convert real_path_internal to strbuf_realpath
  real_path: resolve symlinks by hand
2017-01-18 15:12:11 -08:00
Lukas Puehringer
07d347cf9a builtin/tag: add --format argument for tag -v
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>
2017-01-18 11:27:56 -08:00
Santiago Torres
ff3c8c8f12 builtin/verify-tag: add --format to verify-tag
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>
2017-01-17 16:10:22 -08:00
Junio C Hamano
8ee6fc96f0 Merge branch 'jt/fetch-no-redundant-tag-fetch-map' into maint
Code cleanup to avoid using redundant refspecs while fetching with
the --tags option.

* jt/fetch-no-redundant-tag-fetch-map:
  fetch: do not redundantly calculate tag refmap
2017-01-17 15:19:09 -08:00
Junio C Hamano
af04b1171b Merge branch 'jc/push-default-explicit' into maint
A lazy "git push" without refspec did not internally use a fully
specified refspec to perform 'current', 'simple', or 'upstream'
push, causing unnecessary "ambiguous ref" errors.

* jc/push-default-explicit:
  push: test pushing ambiguously named branches
  push: do not use potentially ambiguous default refspec
2017-01-17 15:11:07 -08:00
Junio C Hamano
b984bc58ce Merge branch 'jk/index-pack-wo-repo-from-stdin' into maint
"git index-pack --stdin" needs an access to an existing repository,
but "git index-pack file.pack" to generate an .idx file that
corresponds to a packfile does not.

* jk/index-pack-wo-repo-from-stdin:
  index-pack: skip collision check when not in repository
  t: use nongit() function where applicable
  index-pack: complain when --stdin is used outside of a repo
  t5000: extract nongit function to test-lib-functions.sh
2017-01-17 15:11:06 -08:00
Junio C Hamano
7902b72794 Merge branch 'sb/sequencer-abort-safety' into maint
Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back
to where cherry-pick started while picking multiple changes, when
the cherry-pick stopped to ask for help from the user, and the user
did "git reset --hard" to a different commit in order to re-attempt
the operation.

* sb/sequencer-abort-safety:
  Revert "sequencer: remove useless get_dir() function"
  sequencer: remove useless get_dir() function
  sequencer: make sequencer abort safer
  t3510: test that cherry-pick --abort does not unsafely change HEAD
  am: change safe_to_abort()'s not rewinding error into a warning
  am: fix filename in safe_to_abort() error message
2017-01-17 15:11:05 -08:00
Junio C Hamano
e4ec408988 Merge branch 'jc/pull-rebase-ff' into maint
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.

* jc/pull-rebase-ff:
  pull: fast-forward "pull --rebase=true"
2017-01-17 15:11:05 -08:00
Junio C Hamano
9d2a24864e Merge branch 'ak/commit-only-allow-empty' into maint
"git commit --allow-empty --only" (no pathspec) with dirty index
ought to be an acceptable way to create a new commit that does not
change any paths, but it was forbidden, perhaps because nobody
needed it so far.

* ak/commit-only-allow-empty:
  commit: remove 'Clever' message for --only --amend
  commit: make --only --allow-empty work without paths
2017-01-17 15:11:03 -08:00
Junio C Hamano
430fd1cae5 Merge branch 'nd/worktree-list-fixup' into maint
The output from "git worktree list" was made in readdir() order,
and was unstable.

* nd/worktree-list-fixup:
  worktree list: keep the list sorted
  worktree.c: get_worktrees() takes a new flag argument
  get_worktrees() must return main worktree as first item even on error
  worktree: reorder an if statement
  worktree.c: zero new 'struct worktree' on allocation
2017-01-17 14:49:27 -08:00
Junio C Hamano
7b0490f81c Merge branch 'jk/rev-parse-symbolic-parents-fix' into maint
"git rev-parse --symbolic" failed with a more recent notation like
"HEAD^-1" and "HEAD^!".

* jk/rev-parse-symbolic-parents-fix:
  rev-parse: fix parent shorthands with --symbolic
2017-01-17 14:49:26 -08:00
Jeff King
c2d17b3b6e fsck: check HAS_OBJ more consistently
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>
2017-01-17 14:24:33 -08:00
Jeff King
c3271a0e47 fsck: do not fallback "git fsck <bogus>" to "git fsck"
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>
2017-01-17 14:24:33 -08:00
Jeff King
c6c7b16d23 fsck: tighten error-checks of "git fsck <head>"
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>
2017-01-17 14:24:33 -08:00
Jeff King
3e3f8bd608 fsck: prepare dummy objects for --connectivity-check
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>
2017-01-17 14:23:20 -08:00
Johannes Schindelin
be8a90e59c difftool: add a skeleton for the upcoming builtin
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>
2017-01-17 13:32:47 -08:00
Jeff King
b4584e4f66 fsck: report trees as dangling
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>
2017-01-17 12:49:41 -08:00
Jeff King
c68b489e56 fsck: parse loose object paths directly
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>
2017-01-15 15:59:03 -08:00
Elia Pinto
8d7aa4ba6a builtin/commit.c: remove the PATH_MAX limitation via dynamic allocation
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>
2017-01-13 11:22:18 -08:00
Stefan Beller
6e7c14e65c submodule update --init: display correct path from submodule
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>
2017-01-12 14:36:20 -08:00