Skip to content

Commit e1c9548

Browse files
shejialuogitster
shejialuo
authored and
gitster
committed
packed-backend: check whether the "packed-refs" is sorted
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 <[email protected]> Mentored-by: Karthik Nayak <[email protected]> Signed-off-by: shejialuo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e6ba4c0 commit e1c9548

File tree

4 files changed

+191
-16
lines changed

4 files changed

+191
-16
lines changed

‎Documentation/fsck-msgids.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@
187187
(ERROR) The "packed-refs" file contains an entry that is
188188
not terminated by a newline.
189189

190+
`packedRefUnsorted`::
191+
(ERROR) The "packed-refs" file is not sorted.
192+
190193
`refMissingNewline`::
191194
(INFO) A loose ref that does not end with newline(LF). As
192195
valid implementations of Git never created such a loose ref

‎fsck.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ enum fsck_msg_type {
5656
FUNC(MISSING_TYPE_ENTRY, ERROR) \
5757
FUNC(MULTIPLE_AUTHORS, ERROR) \
5858
FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \
59+
FUNC(PACKED_REF_UNSORTED, ERROR) \
5960
FUNC(TREE_NOT_SORTED, ERROR) \
6061
FUNC(UNKNOWN_TYPE, ERROR) \
6162
FUNC(ZERO_PADDED_DATE, ERROR) \

‎refs/packed-backend.c‎

Lines changed: 100 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +300,9 @@ struct snapshot_record {
300300
size_t len;
301301
};
302302

303-
static int cmp_packed_ref_records(const void *v1, const void *v2,
304-
void *cb_data)
305-
{
306-
const struct snapshot *snapshot = cb_data;
307-
const struct snapshot_record *e1 = v1, *e2 = v2;
308-
const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1;
309-
const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1;
310303

304+
static int cmp_packed_refname(const char *r1, const char *r2)
305+
{
311306
while (1) {
312307
if (*r1 == '\n')
313308
return *r2 == '\n' ? 0 : -1;
@@ -322,6 +317,17 @@ static int cmp_packed_ref_records(const void *v1, const void *v2,
322317
}
323318
}
324319

320+
static int cmp_packed_ref_records(const void *v1, const void *v2,
321+
void *cb_data)
322+
{
323+
const struct snapshot *snapshot = cb_data;
324+
const struct snapshot_record *e1 = v1, *e2 = v2;
325+
const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1;
326+
const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1;
327+
328+
return cmp_packed_refname(r1, r2);
329+
}
330+
325331
/*
326332
* Compare a snapshot record at `rec` to the specified NUL-terminated
327333
* refname.
@@ -1797,19 +1803,33 @@ static int packed_fsck_ref_next_line(struct fsck_options *o,
17971803
}
17981804

17991805
static int packed_fsck_ref_header(struct fsck_options *o,
1800-
const char *start, const char *eol)
1806+
const char *start, const char *eol,
1807+
unsigned int *sorted)
18011808
{
1802-
if (!starts_with(start, "# pack-refs with: ")) {
1809+
struct string_list traits = STRING_LIST_INIT_NODUP;
1810+
char *tmp_line;
1811+
int ret = 0;
1812+
char *p;
1813+
1814+
tmp_line = xmemdupz(start, eol - start);
1815+
if (!skip_prefix(tmp_line, "# pack-refs with: ", (const char **)&p)) {
18031816
struct fsck_ref_report report = { 0 };
18041817
report.path = "packed-refs.header";
18051818

1806-
return fsck_report_ref(o, &report,
1807-
FSCK_MSG_BAD_PACKED_REF_HEADER,
1808-
"'%.*s' does not start with '# pack-refs with: '",
1809-
(int)(eol - start), start);
1819+
ret = fsck_report_ref(o, &report,
1820+
FSCK_MSG_BAD_PACKED_REF_HEADER,
1821+
"'%.*s' does not start with '# pack-refs with: '",
1822+
(int)(eol - start), start);
1823+
goto cleanup;
18101824
}
18111825

1812-
return 0;
1826+
string_list_split_in_place(&traits, p, " ", -1);
1827+
*sorted = unsorted_string_list_has_string(&traits, "sorted");
1828+
1829+
cleanup:
1830+
free(tmp_line);
1831+
string_list_clear(&traits, 0);
1832+
return ret;
18131833
}
18141834

18151835
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,
19151935
return ret;
19161936
}
19171937

1938+
static int packed_fsck_ref_sorted(struct fsck_options *o,
1939+
struct ref_store *ref_store,
1940+
const char *start, const char *eof)
1941+
{
1942+
size_t hexsz = ref_store->repo->hash_algo->hexsz;
1943+
struct strbuf packed_entry = STRBUF_INIT;
1944+
struct fsck_ref_report report = { 0 };
1945+
struct strbuf refname1 = STRBUF_INIT;
1946+
struct strbuf refname2 = STRBUF_INIT;
1947+
unsigned long line_number = 1;
1948+
const char *former = NULL;
1949+
const char *current;
1950+
const char *eol;
1951+
int ret = 0;
1952+
1953+
if (*start == '#') {
1954+
eol = memchr(start, '\n', eof - start);
1955+
start = eol + 1;
1956+
line_number++;
1957+
}
1958+
1959+
for (; start < eof; line_number++, start = eol + 1) {
1960+
eol = memchr(start, '\n', eof - start);
1961+
1962+
if (*start == '^')
1963+
continue;
1964+
1965+
if (!former) {
1966+
former = start + hexsz + 1;
1967+
continue;
1968+
}
1969+
1970+
current = start + hexsz + 1;
1971+
if (cmp_packed_refname(former, current) >= 0) {
1972+
const char *err_fmt =
1973+
"refname '%s' is less than previous refname '%s'";
1974+
1975+
eol = memchr(former, '\n', eof - former);
1976+
strbuf_add(&refname1, former, eol - former);
1977+
eol = memchr(current, '\n', eof - current);
1978+
strbuf_add(&refname2, current, eol - current);
1979+
1980+
strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1981+
report.path = packed_entry.buf;
1982+
ret = fsck_report_ref(o, &report,
1983+
FSCK_MSG_PACKED_REF_UNSORTED,
1984+
err_fmt, refname2.buf, refname1.buf);
1985+
goto cleanup;
1986+
}
1987+
former = current;
1988+
}
1989+
1990+
cleanup:
1991+
strbuf_release(&packed_entry);
1992+
strbuf_release(&refname1);
1993+
strbuf_release(&refname2);
1994+
return ret;
1995+
}
1996+
19181997
static int packed_fsck_ref_content(struct fsck_options *o,
19191998
struct ref_store *ref_store,
1999+
unsigned int *sorted,
19202000
const char *start, const char *eof)
19212001
{
19222002
struct strbuf refname = STRBUF_INIT;
@@ -1926,7 +2006,7 @@ static int packed_fsck_ref_content(struct fsck_options *o,
19262006

19272007
ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
19282008
if (*start == '#') {
1929-
ret |= packed_fsck_ref_header(o, start, eol);
2009+
ret |= packed_fsck_ref_header(o, start, eol, sorted);
19302010

19312011
start = eol + 1;
19322012
line_number++;
@@ -1957,6 +2037,7 @@ static int packed_fsck(struct ref_store *ref_store,
19572037
struct packed_ref_store *refs = packed_downcast(ref_store,
19582038
REF_STORE_READ, "fsck");
19592039
struct strbuf packed_ref_content = STRBUF_INIT;
2040+
unsigned int sorted = 0;
19602041
struct stat st;
19612042
int ret = 0;
19622043
int fd = -1;
@@ -2004,8 +2085,11 @@ static int packed_fsck(struct ref_store *ref_store,
20042085
goto cleanup;
20052086
}
20062087

2007-
ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf,
2088+
ret = packed_fsck_ref_content(o, ref_store, &sorted, packed_ref_content.buf,
20082089
packed_ref_content.buf + packed_ref_content.len);
2090+
if (!ret && sorted)
2091+
ret = packed_fsck_ref_sorted(o, ref_store, packed_ref_content.buf,
2092+
packed_ref_content.buf + packed_ref_content.len);
20092093

20102094
cleanup:
20112095
if (fd >= 0)

‎t/t0602-reffiles-fsck.sh‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,4 +743,91 @@ test_expect_success 'packed-refs content should be checked' '
743743
)
744744
'
745745

746+
test_expect_success 'packed-ref with sorted trait should be checked' '
747+
test_when_finished "rm -rf repo" &&
748+
git init repo &&
749+
(
750+
cd repo &&
751+
test_commit default &&
752+
git branch branch-1 &&
753+
git branch branch-2 &&
754+
git tag -a annotated-tag-1 -m tag-1 &&
755+
branch_1_oid=$(git rev-parse branch-1) &&
756+
branch_2_oid=$(git rev-parse branch-2) &&
757+
tag_1_oid=$(git rev-parse annotated-tag-1) &&
758+
tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
759+
refname1="refs/heads/main" &&
760+
refname2="refs/heads/foo" &&
761+
refname3="refs/tags/foo" &&
762+
763+
cat >.git/packed-refs <<-EOF &&
764+
# pack-refs with: peeled fully-peeled sorted
765+
EOF
766+
git refs verify 2>err &&
767+
rm .git/packed-refs &&
768+
test_must_be_empty err &&
769+
770+
cat >.git/packed-refs <<-EOF &&
771+
# pack-refs with: peeled fully-peeled sorted
772+
$branch_2_oid $refname1
773+
EOF
774+
git refs verify 2>err &&
775+
rm .git/packed-refs &&
776+
test_must_be_empty err &&
777+
778+
cat >.git/packed-refs <<-EOF &&
779+
# pack-refs with: peeled fully-peeled sorted
780+
$branch_2_oid $refname1
781+
$branch_1_oid $refname2
782+
$tag_1_oid $refname3
783+
EOF
784+
test_must_fail git refs verify 2>err &&
785+
cat >expect <<-EOF &&
786+
error: packed-refs line 3: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname1'\''
787+
EOF
788+
rm .git/packed-refs &&
789+
test_cmp expect err &&
790+
791+
cat >.git/packed-refs <<-EOF &&
792+
# pack-refs with: peeled fully-peeled sorted
793+
$tag_1_oid $refname3
794+
^$tag_1_peeled_oid
795+
$branch_2_oid $refname2
796+
EOF
797+
test_must_fail git refs verify 2>err &&
798+
cat >expect <<-EOF &&
799+
error: packed-refs line 4: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname3'\''
800+
EOF
801+
rm .git/packed-refs &&
802+
test_cmp expect err
803+
)
804+
'
805+
806+
test_expect_success 'packed-ref without sorted trait should not be checked' '
807+
test_when_finished "rm -rf repo" &&
808+
git init repo &&
809+
(
810+
cd repo &&
811+
test_commit default &&
812+
git branch branch-1 &&
813+
git branch branch-2 &&
814+
git tag -a annotated-tag-1 -m tag-1 &&
815+
branch_1_oid=$(git rev-parse branch-1) &&
816+
branch_2_oid=$(git rev-parse branch-2) &&
817+
tag_1_oid=$(git rev-parse annotated-tag-1) &&
818+
tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
819+
refname1="refs/heads/main" &&
820+
refname2="refs/heads/foo" &&
821+
refname3="refs/tags/foo" &&
822+
823+
cat >.git/packed-refs <<-EOF &&
824+
# pack-refs with: peeled fully-peeled
825+
$branch_2_oid $refname1
826+
$branch_1_oid $refname2
827+
EOF
828+
git refs verify 2>err &&
829+
test_must_be_empty err
830+
)
831+
'
832+
746833
test_done

0 commit comments

Comments
 (0)