Skip to content

Commit

Permalink
Merge branch 'jk/fsck-connectivity-check-fix'
Browse files Browse the repository at this point in the history 
"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
  • Loading branch information
@gitster
gitster committed Jan 31, 2017
2 parents b7786bb + a2b2285 commit 4ba6197
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 40 deletions.
118 changes: 81 additions & 37 deletions builtin/fsck.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ static const char *describe_object(struct object *obj)
return buf.buf;
}

static const char *printable_type(struct object *obj)
{
const char *ret;

if (obj->type == OBJ_NONE) {
enum object_type type = sha1_object_info(obj->oid.hash, NULL);
if (type > 0)
object_as_type(obj, type, 0);
}

ret = typename(obj->type);
if (!ret)
ret = "unknown";

return ret;
}

static int fsck_config(const char *var, const char *value, void *cb)
{
if (strcmp(var, "fsck.skiplist") == 0) {
Expand Down Expand Up @@ -83,7 +100,7 @@ static void objreport(struct object *obj, const char *msg_type,
const char *err)
{
fprintf(stderr, "%s in %s %s: %s\n",
msg_type, typename(obj->type), describe_object(obj), err);
msg_type, printable_type(obj), describe_object(obj), err);
}

static int objerror(struct object *obj, const char *err)
Expand Down Expand Up @@ -114,7 +131,7 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt
if (!obj) {
/* ... these references to parent->fld are safe here */
printf("broken link from %7s %s\n",
typename(parent->type), describe_object(parent));
printable_type(parent), describe_object(parent));
printf("broken link from %7s %s\n",
(type == OBJ_ANY ? "unknown" : typename(type)), "unknown");
errors_found |= ERROR_REACHABLE;
Expand All @@ -131,9 +148,9 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt
if (!(obj->flags & HAS_OBJ)) {
if (parent && !has_object_file(&obj->oid)) {
printf("broken link from %7s %s\n",
typename(parent->type), describe_object(parent));
printable_type(parent), describe_object(parent));
printf(" to %7s %s\n",
typename(obj->type), describe_object(obj));
printable_type(obj), describe_object(obj));
errors_found |= ERROR_REACHABLE;
}
return 1;
Expand Down Expand Up @@ -205,9 +222,7 @@ static void check_reachable_object(struct object *obj)
if (!(obj->flags & HAS_OBJ)) {
if (has_sha1_pack(obj->oid.hash))
return; /* it is in pack - forget about it */
if (connectivity_only && has_object_file(&obj->oid))
return;
printf("missing %s %s\n", typename(obj->type),
printf("missing %s %s\n", printable_type(obj),
describe_object(obj));
errors_found |= ERROR_REACHABLE;
return;
Expand All @@ -225,15 +240,15 @@ static void check_unreachable_object(struct object *obj)
* to complain about it being unreachable (since it does
* not exist).
*/
if (!obj->parsed)
if (!(obj->flags & HAS_OBJ))
return;

/*
* Unreachable object that exists? Show it if asked to,
* since this is something that is prunable.
*/
if (show_unreachable) {
printf("unreachable %s %s\n", typename(obj->type),
printf("unreachable %s %s\n", printable_type(obj),
describe_object(obj));
return;
}
Expand All @@ -252,7 +267,7 @@ static void check_unreachable_object(struct object *obj)
*/
if (!obj->used) {
if (show_dangling)
printf("dangling %s %s\n", typename(obj->type),
printf("dangling %s %s\n", printable_type(obj),
describe_object(obj));
if (write_lost_and_found) {
char *filename = git_pathdup("lost-found/%s/%s",
Expand Down Expand Up @@ -326,7 +341,7 @@ static int fsck_obj(struct object *obj)

if (verbose)
fprintf(stderr, "Checking %s %s\n",
typename(obj->type), describe_object(obj));
printable_type(obj), describe_object(obj));

if (fsck_walk(obj, NULL, &fsck_obj_options))
objerror(obj, "broken links");
Expand All @@ -352,7 +367,7 @@ static int fsck_obj(struct object *obj)
struct tag *tag = (struct tag *) obj;

if (show_tags && tag->tagged) {
printf("tagged %s %s", typename(tag->tagged->type),
printf("tagged %s %s", printable_type(tag->tagged),
describe_object(tag->tagged));
printf(" (%s) in %s\n", tag->tag,
describe_object(&tag->object));
Expand Down Expand Up @@ -388,7 +403,7 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,

if (!is_null_sha1(sha1)) {
obj = lookup_object(sha1);
if (obj) {
if (obj && (obj->flags & HAS_OBJ)) {
if (timestamp && name_objects)
add_decoration(fsck_walk_options.object_names,
obj,
Expand Down Expand Up @@ -604,6 +619,29 @@ static int fsck_cache_tree(struct cache_tree *it)
return err;
}

static void mark_object_for_connectivity(const unsigned char *sha1)
{
struct object *obj = lookup_unknown_object(sha1);
obj->flags |= HAS_OBJ;
}

static int mark_loose_for_connectivity(const unsigned char *sha1,
const char *path,
void *data)
{
mark_object_for_connectivity(sha1);
return 0;
}

static int mark_packed_for_connectivity(const unsigned char *sha1,
struct packed_git *pack,
uint32_t pos,
void *data)
{
mark_object_for_connectivity(sha1);
return 0;
}

static char const * const fsck_usage[] = {
N_("git fsck [<options>] [<object>...]"),
NULL
Expand Down Expand Up @@ -660,38 +698,41 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
git_config(fsck_config, NULL);

fsck_head_link();
if (!connectivity_only) {
if (connectivity_only) {
for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
} else {
fsck_object_dir(get_object_directory());

prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt->next)
fsck_object_dir(alt->path);
}

if (check_full) {
struct packed_git *p;
uint32_t total = 0, count = 0;
struct progress *progress = NULL;
if (check_full) {
struct packed_git *p;
uint32_t total = 0, count = 0;
struct progress *progress = NULL;

prepare_packed_git();
prepare_packed_git();

if (show_progress) {
if (show_progress) {
for (p = packed_git; p; p = p->next) {
if (open_pack_index(p))
continue;
total += p->num_objects;
}

progress = start_progress(_("Checking objects"), total);
}
for (p = packed_git; p; p = p->next) {
if (open_pack_index(p))
continue;
total += p->num_objects;
/* verify gives error messages itself */
if (verify_pack(p, fsck_obj_buffer,
progress, count))
errors_found |= ERROR_PACK;
count += p->num_objects;
}

progress = start_progress(_("Checking objects"), total);
stop_progress(&progress);
}
for (p = packed_git; p; p = p->next) {
/* verify gives error messages itself */
if (verify_pack(p, fsck_obj_buffer,
progress, count))
errors_found |= ERROR_PACK;
count += p->num_objects;
}
stop_progress(&progress);
}

heads = 0;
Expand All @@ -701,9 +742,11 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
if (!get_sha1(arg, sha1)) {
struct object *obj = lookup_object(sha1);

/* Error is printed by lookup_object(). */
if (!obj)
if (!obj || !(obj->flags & HAS_OBJ)) {
error("%s: object missing", sha1_to_hex(sha1));
errors_found |= ERROR_OBJECT;
continue;
}

obj->used = 1;
if (name_objects)
Expand All @@ -714,14 +757,15 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
continue;
}
error("invalid parameter: expected sha1, got '%s'", arg);
errors_found |= ERROR_OBJECT;
}

/*
* If we've not been given any explicit head information, do the
* default ones from .git/refs. We also consider the index file
* in this case (ie this implies --cache).
*/
if (!heads) {
if (!argc) {
get_default_heads();
keep_cache_objects = 1;
}
Expand Down
4 changes: 4 additions & 0 deletions fsck.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,10 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options)
{
if (!obj)
return -1;

if (obj->type == OBJ_NONE)
parse_object(obj->oid.hash);

switch (obj->type) {
case OBJ_BLOB:
return 0;
Expand Down
76 changes: 73 additions & 3 deletions t/t1450-fsck.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,16 @@ test_expect_success 'commit with NUL in header' '
'

test_expect_success 'tree object with duplicate entries' '
test_when_finished "remove_object \$T" &&
test_when_finished "for i in \$T; do remove_object \$i; done" &&
T=$(
GIT_INDEX_FILE=test-index &&
export GIT_INDEX_FILE &&
rm -f test-index &&
>x &&
git add x &&
git rev-parse :x &&
T=$(git write-tree) &&
echo $T &&
(
git cat-file tree $T &&
git cat-file tree $T
Expand Down Expand Up @@ -521,9 +523,21 @@ test_expect_success 'fsck --connectivity-only' '
touch empty &&
git add empty &&
test_commit empty &&
# Drop the index now; we want to be sure that we
# recursively notice the broken objects
# because they are reachable from refs, not because
# they are in the index.
rm -f .git/index &&
# corrupt the blob, but in a way that we can still identify
# its type. That lets us see that --connectivity-only is
# not actually looking at the contents, but leaves it
# free to examine the type if it chooses.
empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
rm -f $empty &&
echo invalid >$empty &&
blob=$(echo unrelated | git hash-object -w --stdin) &&
mv -f $(sha1_file $blob) $empty &&
test_must_fail git fsck --strict &&
git fsck --strict --connectivity-only &&
tree=$(git rev-parse HEAD:) &&
Expand All @@ -535,6 +549,19 @@ test_expect_success 'fsck --connectivity-only' '
)
'

test_expect_success 'fsck --connectivity-only with explicit head' '
rm -rf connectivity-only &&
git init connectivity-only &&
(
cd connectivity-only &&
test_commit foo &&
rm -f .git/index &&
tree=$(git rev-parse HEAD^{tree}) &&
remove_object $(git rev-parse HEAD:foo.t) &&
test_must_fail git fsck --connectivity-only $tree
)
'

test_expect_success 'fsck --name-objects' '
rm -rf name-objects &&
git init name-objects &&
Expand Down Expand Up @@ -619,4 +646,47 @@ test_expect_success 'fsck detects trailing loose garbage (blob)' '
test_i18ngrep "garbage.*$blob" out
'

# for each of type, we have one version which is referenced by another object
# (and so while unreachable, not dangling), and another variant which really is
# dangling.
test_expect_success 'fsck notices dangling objects' '
git init dangling &&
(
cd dangling &&
blob=$(echo not-dangling | git hash-object -w --stdin) &&
dblob=$(echo dangling | git hash-object -w --stdin) &&
tree=$(printf "100644 blob %s\t%s\n" $blob one | git mktree) &&
dtree=$(printf "100644 blob %s\t%s\n" $blob two | git mktree) &&
commit=$(git commit-tree $tree) &&
dcommit=$(git commit-tree -p $commit $tree) &&
cat >expect <<-EOF &&
dangling blob $dblob
dangling commit $dcommit
dangling tree $dtree
EOF
git fsck >actual &&
# the output order is non-deterministic, as it comes from a hash
sort <actual >actual.sorted &&
test_cmp expect actual.sorted
)
'

test_expect_success 'fsck $name notices bogus $name' '
test_must_fail git fsck bogus &&
test_must_fail git fsck $_z40
'

test_expect_success 'bogus head does not fallback to all heads' '
# set up a case that will cause a reachability complaint
echo to-be-deleted >foo &&
git add foo &&
blob=$(git rev-parse :foo) &&
test_when_finished "git rm --cached foo" &&
remove_object $blob &&
test_must_fail git fsck $_z40 >out 2>&1 &&
! grep $blob out
'

test_done

0 comments on commit 4ba6197

Please sign in to comment.