As described in https://github.com/gitgitgadget/git/pull/873, I managed to
corrupt my Git checkout in a rather thorough manner yesterday, and it took
me a long time to undo the damage. One of my tools for that should have been
git fsck --name-objects, but that command produced bogus output: It said
that a commit with the name ~2 had a missing tree, but ~2 is not even a
legal rev name.
Turns out that this is an ancient bug, and the fact that nobody complained
about it suggests to me that the --name-objects probably has exactly one
user, and he uses it only every four years, when he manages to hose his Git
checkout.
Johannes Schindelin (2):
t1450: robustify `remove_object()`
fsck --name-objects: be more careful parsing generation numbers
fsck.c | 5 +++++
t/t1450-fsck.sh | 26 ++++++++++++--------------
2 files changed, 17 insertions(+), 14 deletions(-)
base-commit: 7397ca33730626f682845f8691b39c305535611e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-874%2Fdscho%2Ffix-fsck-name-objects-generation-parsing-bug-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-874/dscho/fix-fsck-name-objects-generation-parsing-bug-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/874
--
gitgitgadget
From: Johannes Schindelin <redacted>
This function can be simplified by using the `test_oid_to_path()`
helper, which incidentally also makes it more robust by not relying on
the exact file system layout of the loose object files.
While at it, do not define those functions in a test case, it buys us
nothing.
Signed-off-by: Johannes Schindelin <redacted>
---
t/t1450-fsck.sh | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 02478bc4ece2..779f700ac4a0 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -41,17 +41,13 @@ test_expect_success 'HEAD is part of refs, valid objects appear valid' '
# specific corruption you test afterwards, lest a later test trip over
# it.
-test_expect_success 'setup: helpers for corruption tests' '
- sha1_file() {
- remainder=${1#??} &&
- firsttwo=${1%$remainder} &&
- echo ".git/objects/$firsttwo/$remainder"
- } &&
+sha1_file () {
+ git rev-parse --git-path objects/$(test_oid_to_path "$1")
+}
- remove_object() {
- rm "$(sha1_file "$1")"
- }
-'
+remove_object() {
+ rm "$(sha1_file "$1")"
+}
test_expect_success 'object with bad sha1' '
sha=$(echo blob | git hash-object -w --stdin) &&--
gitgitgadget
From: Johannes Schindelin <redacted>
In 7b35efd734e (fsck_walk(): optionally name objects on the go,
2016-07-17), the `fsck` machinery learned to optionally name the
objects, so that it is easier to see what part of the repository is in a
bad shape, say, when objects are missing.
To save on complexity, this machinery uses a parser to determine the
name of a parent given a commit's name: any `~<n>` suffix is parsed and
the parent's name is formed from the prefix together with `~<n+1>`.
However, this parser has a bug: if it finds a suffix `<n>` that is _not_
`~<n>`, it will mistake the empty string for the prefix and `<n>` for
the generation number. In other words, it will generate a name of the
form `~<bogus-number>`.
Let's fix this.
Signed-off-by: Johannes Schindelin <redacted>
---
fsck.c | 5 +++++
t/t1450-fsck.sh | 10 ++++++----
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/fsck.c b/fsck.c
index 73f30773f28a..83d727c6fe33 100644
--- a/fsck.c
+++ b/fsck.c
@@ -461,6 +461,11 @@ static int fsck_walk_commit(struct commit *commit, void *data, struct fsck_optio
generation += power * (name[--len] - '0');
if (power > 1 && len && name[len - 1] == '~')
name_prefix_len = len - 1;
+ else {
+ /* Maybe a non-first parent, e.g. HEAD^2 */
+ generation = 0;
+ name_prefix_len = len;
+ }
}
}
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 779f700ac4a0..bfa3588f37ab 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -607,13 +607,15 @@ test_expect_success 'fsck --name-objects' '
git init name-objects &&
(
cd name-objects &&
+ git config core.logAllRefUpdates false &&
test_commit julius caesar.t &&
- test_commit augustus &&
- test_commit caesar &&
+ test_commit augustus44 &&
+ test_commit caesar &&
remove_object $(git rev-parse julius:caesar.t) &&
- test_must_fail git fsck --name-objects >out &&
tree=$(git rev-parse --verify julius:) &&
- test_i18ngrep "$tree (refs/tags/julius:" out
+ git tag -d julius &&
+ test_must_fail git fsck --name-objects >out &&
+ test_i18ngrep "$tree (refs/tags/augustus44\\^:" out
)
'
--
gitgitgadget