When there is a "sorted" trait in the header of the "packed-refs" file,
it means that each entry is sorted increasingly by comparing the
refname. We should add checks to verify whether the "packed-refs" is
sorted in this case.
Update the "packed_fsck_ref_header" to know whether there is a "sorted"
trail in the header. It may seem that we could record all refnames
during the parsing process and then compare later. However, this is not
a good design due to the following reasons:
1. Because we need to store the state across the whole checking
lifetime, we would consume a lot of memory if there are many entries
in the "packed-refs" file.
2. We cannot reuse the existing compare function "cmp_packed_ref_records"
which cause repetition.
Because "cmp_packed_ref_records" needs an extra parameter "struct
snaphost", extract the common part into a new function
"cmp_packed_ref_records" to reuse this function to compare.
Then, create a new function "packed_fsck_ref_sorted" to parse the file
again and user the new fsck message "packedRefUnsorted(ERROR)" to report
to the user if the file is not sorted.
Mentored-by: Patrick Steinhardt <ps@xxxxxx>
Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
---
Documentation/fsck-msgids.adoc | 3 +
fsck.h | 1 +
refs/packed-backend.c | 118 ++++++++++++++++++++++++++++-----
t/t0602-reffiles-fsck.sh | 87 ++++++++++++++++++++++++
4 files changed, 192 insertions(+), 17 deletions(-)
diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc
index 02a7bf0503..9601fff228 100644
--- a/Documentation/fsck-msgids.adoc
+++ b/Documentation/fsck-msgids.adoc
@@ -187,6 +187,9 @@
(ERROR) The "packed-refs" file contains an entry that is
not terminated by a newline.
+`packedRefUnsorted`::
+ (ERROR) The "packed-refs" file is not sorted.
+
`refMissingNewline`::
(INFO) A loose ref that does not end with newline(LF). As
valid implementations of Git never created such a loose ref
diff --git a/fsck.h b/fsck.h
index 14d70f6653..19f3cb2773 100644
--- a/fsck.h
+++ b/fsck.h
@@ -56,6 +56,7 @@ enum fsck_msg_type {
FUNC(MISSING_TYPE_ENTRY, ERROR) \
FUNC(MULTIPLE_AUTHORS, ERROR) \
FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \
+ FUNC(PACKED_REF_UNSORTED, ERROR) \
FUNC(TREE_NOT_SORTED, ERROR) \
FUNC(UNKNOWN_TYPE, ERROR) \
FUNC(ZERO_PADDED_DATE, ERROR) \
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index c7138aefff..ae04d8ae80 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -300,14 +300,9 @@ struct snapshot_record {
size_t len;
};
-static int cmp_packed_ref_records(const void *v1, const void *v2,
- void *cb_data)
-{
- const struct snapshot *snapshot = cb_data;
- const struct snapshot_record *e1 = v1, *e2 = v2;
- const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1;
- const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1;
+static int cmp_packed_refname(const char *r1, const char *r2)
+{
while (1) {
if (*r1 == '\n')
return *r2 == '\n' ? 0 : -1;
@@ -322,6 +317,17 @@ static int cmp_packed_ref_records(const void *v1, const void *v2,
}
}
+static int cmp_packed_ref_records(const void *v1, const void *v2,
+ void *cb_data)
+{
+ const struct snapshot *snapshot = cb_data;
+ const struct snapshot_record *e1 = v1, *e2 = v2;
+ const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1;
+ const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1;
+
+ return cmp_packed_refname(r1, r2);
+}
+
/*
* Compare a snapshot record at `rec` to the specified NUL-terminated
* refname.
@@ -1797,19 +1803,33 @@ static int packed_fsck_ref_next_line(struct fsck_options *o,
}
static int packed_fsck_ref_header(struct fsck_options *o,
- const char *start, const char *eol)
+ const char *start, const char *eol,
+ unsigned int *sorted)
{
- if (!starts_with(start, "# pack-refs with: ")) {
+ struct string_list traits = STRING_LIST_INIT_NODUP;
+ char *tmp_line;
+ int ret = 0;
+ char *p;
+
+ tmp_line = xmemdupz(start, eol - start);
+ if (!skip_prefix(tmp_line, "# pack-refs with: ", (const char **)&p)) {
struct fsck_ref_report report = { 0 };
report.path = "packed-refs.header";
- return fsck_report_ref(o, &report,
- FSCK_MSG_BAD_PACKED_REF_HEADER,
- "'%.*s' does not start with '# pack-refs with: '",
- (int)(eol - start), start);
+ ret = fsck_report_ref(o, &report,
+ FSCK_MSG_BAD_PACKED_REF_HEADER,
+ "'%.*s' does not start with '# pack-refs with: '",
+ (int)(eol - start), start);
+ goto cleanup;
}
- return 0;
+ string_list_split_in_place(&traits, p, " ", -1);
+ *sorted = unsorted_string_list_has_string(&traits, "sorted");
+
+cleanup:
+ free(tmp_line);
+ string_list_clear(&traits, 0);
+ return ret;
}
static int packed_fsck_ref_peeled_line(struct fsck_options *o,
@@ -1915,8 +1935,68 @@ static int packed_fsck_ref_main_line(struct fsck_options *o,
return ret;
}
+static int packed_fsck_ref_sorted(struct fsck_options *o,
+ struct ref_store *ref_store,
+ const char *start, const char *eof)
+{
+ size_t hexsz = ref_store->repo->hash_algo->hexsz;
+ struct strbuf packed_entry = STRBUF_INIT;
+ struct fsck_ref_report report = { 0 };
+ struct strbuf refname1 = STRBUF_INIT;
+ struct strbuf refname2 = STRBUF_INIT;
+ unsigned long line_number = 1;
+ const char *former = NULL;
+ const char *current;
+ const char *eol;
+ int ret = 0;
+
+ if (*start == '#') {
+ eol = memchr(start, '\n', eof - start);
+ start = eol + 1;
+ line_number++;
+ }
+
+ for (; start < eof; line_number++, start = eol + 1) {
+ eol = memchr(start, '\n', eof - start);
+
+ if (*start == '^')
+ continue;
+
+ if (!former) {
+ former = start + hexsz + 1;
+ continue;
+ }
+
+ current = start + hexsz + 1;
+ if (cmp_packed_refname(former, current) >= 0) {
+ const char *err_fmt =
+ "refname '%s' is less than previous refname '%s'";
+
+ eol = memchr(former, '\n', eof - former);
+ strbuf_add(&refname1, former, eol - former);
+ eol = memchr(current, '\n', eof - current);
+ strbuf_add(&refname2, current, eol - current);
+
+ strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+ report.path = packed_entry.buf;
+ ret = fsck_report_ref(o, &report,
+ FSCK_MSG_PACKED_REF_UNSORTED,
+ err_fmt, refname2.buf, refname1.buf);
+ goto cleanup;
+ }
+ former = current;
+ }
+
+cleanup:
+ strbuf_release(&packed_entry);
+ strbuf_release(&refname1);
+ strbuf_release(&refname2);
+ return ret;
+}
+
static int packed_fsck_ref_content(struct fsck_options *o,
struct ref_store *ref_store,
+ unsigned int *sorted,
const char *start, const char *eof)
{
struct strbuf refname = STRBUF_INIT;
@@ -1926,7 +2006,7 @@ static int packed_fsck_ref_content(struct fsck_options *o,
ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
if (*start == '#') {
- ret |= packed_fsck_ref_header(o, start, eol);
+ ret |= packed_fsck_ref_header(o, start, eol, sorted);
start = eol + 1;
line_number++;
@@ -1957,9 +2037,10 @@ static int packed_fsck(struct ref_store *ref_store,
struct packed_ref_store *refs = packed_downcast(ref_store,
REF_STORE_READ, "fsck");
struct strbuf packed_ref_content = STRBUF_INIT;
+ unsigned int sorted = 0;
struct stat st;
- int fd;
int ret = 0;
+ int fd;
if (!is_main_worktree(wt))
goto cleanup;
@@ -2012,8 +2093,11 @@ static int packed_fsck(struct ref_store *ref_store,
goto cleanup;
}
- ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf,
+ ret = packed_fsck_ref_content(o, ref_store, &sorted, packed_ref_content.buf,
packed_ref_content.buf + packed_ref_content.len);
+ if (!ret && sorted)
+ ret = packed_fsck_ref_sorted(o, ref_store, packed_ref_content.buf,
+ packed_ref_content.buf + packed_ref_content.len);
cleanup:
strbuf_release(&packed_ref_content);
diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 7421cc1e7f..28dc8dcddc 100755
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -735,4 +735,91 @@ test_expect_success 'packed-refs content should be checked' '
)
'
+test_expect_success 'packed-ref with sorted trait should be checked' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit default &&
+ git branch branch-1 &&
+ git branch branch-2 &&
+ git tag -a annotated-tag-1 -m tag-1 &&
+ branch_1_oid=$(git rev-parse branch-1) &&
+ branch_2_oid=$(git rev-parse branch-2) &&
+ tag_1_oid=$(git rev-parse annotated-tag-1) &&
+ tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
+ refname1="refs/heads/main" &&
+ refname2="refs/heads/foo" &&
+ refname3="refs/tags/foo" &&
+
+ cat >.git/packed-refs <<-EOF &&
+ # pack-refs with: peeled fully-peeled sorted
+ EOF
+ git refs verify 2>err &&
+ rm .git/packed-refs &&
+ test_must_be_empty err &&
+
+ cat >.git/packed-refs <<-EOF &&
+ # pack-refs with: peeled fully-peeled sorted
+ $branch_2_oid $refname1
+ EOF
+ git refs verify 2>err &&
+ rm .git/packed-refs &&
+ test_must_be_empty err &&
+
+ cat >.git/packed-refs <<-EOF &&
+ # pack-refs with: peeled fully-peeled sorted
+ $branch_2_oid $refname1
+ $branch_1_oid $refname2
+ $tag_1_oid $refname3
+ EOF
+ test_must_fail git refs verify 2>err &&
+ cat >expect <<-EOF &&
+ error: packed-refs line 3: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname1'\''
+ EOF
+ rm .git/packed-refs &&
+ test_cmp expect err &&
+
+ cat >.git/packed-refs <<-EOF &&
+ # pack-refs with: peeled fully-peeled sorted
+ $tag_1_oid $refname3
+ ^$tag_1_peeled_oid
+ $branch_2_oid $refname2
+ EOF
+ test_must_fail git refs verify 2>err &&
+ cat >expect <<-EOF &&
+ error: packed-refs line 4: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname3'\''
+ EOF
+ rm .git/packed-refs &&
+ test_cmp expect err
+ )
+'
+
+test_expect_success 'packed-ref without sorted trait should not be checked' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit default &&
+ git branch branch-1 &&
+ git branch branch-2 &&
+ git tag -a annotated-tag-1 -m tag-1 &&
+ branch_1_oid=$(git rev-parse branch-1) &&
+ branch_2_oid=$(git rev-parse branch-2) &&
+ tag_1_oid=$(git rev-parse annotated-tag-1) &&
+ tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
+ refname1="refs/heads/main" &&
+ refname2="refs/heads/foo" &&
+ refname3="refs/tags/foo" &&
+
+ cat >.git/packed-refs <<-EOF &&
+ # pack-refs with: peeled fully-peeled
+ $branch_2_oid $refname1
+ $branch_1_oid $refname2
+ EOF
+ git refs verify 2>err &&
+ test_must_be_empty err
+ )
+'
+
test_done
--
2.48.1
