Skip to content

Commit a1af533

Browse files
gitstergitster
committed
Merge branch 'tb/pack-finalize-ordering'
The order in which various files that make up a single (conceptual) packfile has been reevaluated and straightened up. This matters in correctness, as an incomplete set of files must not be shown to a running Git. * tb/pack-finalize-ordering: pack-objects: rename .idx files into place after .bitmap files pack-write: split up finish_tmp_packfile() function builtin/index-pack.c: move `.idx` files into place last index-pack: refactor renaming in final() builtin/repack.c: move `.idx` files into place last pack-write.c: rename `.idx` files after `*.rev` pack-write: refactor renaming in finish_tmp_packfile() bulk-checkin.c: store checksum directly pack.h: line-wrap the definition of finish_tmp_packfile()
2 parents 403192a + 4bc1fd6 commit a1af533

File tree

6 files changed

+96
-67
lines changed

6 files changed

+96
-67
lines changed

‎builtin/index-pack.c‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,6 +1477,22 @@ static void write_special_file(const char *suffix, const char *msg,
14771477
strbuf_release(&name_buf);
14781478
}
14791479

1480+
static void rename_tmp_packfile(const char **final_name,
1481+
const char *curr_name,
1482+
struct strbuf *name, unsigned char *hash,
1483+
const char *ext, int make_read_only_if_same)
1484+
{
1485+
if (*final_name != curr_name) {
1486+
if (!*final_name)
1487+
*final_name = odb_pack_name(name, hash, ext);
1488+
if (finalize_object_file(curr_name, *final_name))
1489+
die(_("unable to rename temporary '*.%s' file to '%s"),
1490+
ext, *final_name);
1491+
} else if (make_read_only_if_same) {
1492+
chmod(*final_name, 0444);
1493+
}
1494+
}
1495+
14801496
static void final(const char *final_pack_name, const char *curr_pack_name,
14811497
const char *final_index_name, const char *curr_index_name,
14821498
const char *final_rev_index_name, const char *curr_rev_index_name,
@@ -1505,31 +1521,13 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
15051521
write_special_file("promisor", promisor_msg, final_pack_name,
15061522
hash, NULL);
15071523

1508-
if (final_pack_name != curr_pack_name) {
1509-
if (!final_pack_name)
1510-
final_pack_name = odb_pack_name(&pack_name, hash, "pack");
1511-
if (finalize_object_file(curr_pack_name, final_pack_name))
1512-
die(_("cannot store pack file"));
1513-
} else if (from_stdin)
1514-
chmod(final_pack_name, 0444);
1515-
1516-
if (final_index_name != curr_index_name) {
1517-
if (!final_index_name)
1518-
final_index_name = odb_pack_name(&index_name, hash, "idx");
1519-
if (finalize_object_file(curr_index_name, final_index_name))
1520-
die(_("cannot store index file"));
1521-
} else
1522-
chmod(final_index_name, 0444);
1523-
1524-
if (curr_rev_index_name) {
1525-
if (final_rev_index_name != curr_rev_index_name) {
1526-
if (!final_rev_index_name)
1527-
final_rev_index_name = odb_pack_name(&rev_index_name, hash, "rev");
1528-
if (finalize_object_file(curr_rev_index_name, final_rev_index_name))
1529-
die(_("cannot store reverse index file"));
1530-
} else
1531-
chmod(final_rev_index_name, 0444);
1532-
}
1524+
rename_tmp_packfile(&final_pack_name, curr_pack_name, &pack_name,
1525+
hash, "pack", from_stdin);
1526+
if (curr_rev_index_name)
1527+
rename_tmp_packfile(&final_rev_index_name, curr_rev_index_name,
1528+
&rev_index_name, hash, "rev", 1);
1529+
rename_tmp_packfile(&final_index_name, curr_index_name, &index_name,
1530+
hash, "idx", 1);
15331531

15341532
if (do_fsck_object) {
15351533
struct packed_git *p;

‎builtin/pack-objects.c‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,6 +1222,7 @@ static void write_pack_file(void)
12221222
if (!pack_to_stdout) {
12231223
struct stat st;
12241224
struct strbuf tmpname = STRBUF_INIT;
1225+
char *idx_tmp_name = NULL;
12251226

12261227
/*
12271228
* Packs are runtime accessed in their mtime
@@ -1242,21 +1243,23 @@ static void write_pack_file(void)
12421243
warning_errno(_("failed utime() on %s"), pack_tmp_name);
12431244
}
12441245

1245-
strbuf_addf(&tmpname, "%s-", base_name);
1246+
strbuf_addf(&tmpname, "%s-%s.", base_name,
1247+
hash_to_hex(hash));
12461248

12471249
if (write_bitmap_index) {
12481250
bitmap_writer_set_checksum(hash);
12491251
bitmap_writer_build_type_index(
12501252
&to_pack, written_list, nr_written);
12511253
}
12521254

1253-
finish_tmp_packfile(&tmpname, pack_tmp_name,
1255+
stage_tmp_packfiles(&tmpname, pack_tmp_name,
12541256
written_list, nr_written,
1255-
&pack_idx_opts, hash);
1257+
&pack_idx_opts, hash, &idx_tmp_name);
12561258

12571259
if (write_bitmap_index) {
1258-
strbuf_addf(&tmpname, "%s.bitmap", hash_to_hex(hash));
1260+
size_t tmpname_len = tmpname.len;
12591261

1262+
strbuf_addstr(&tmpname, "bitmap");
12601263
stop_progress(&progress_state);
12611264

12621265
bitmap_writer_show_progress(progress);
@@ -1266,8 +1269,12 @@ static void write_pack_file(void)
12661269
bitmap_writer_finish(written_list, nr_written,
12671270
tmpname.buf, write_bitmap_options);
12681271
write_bitmap_index = 0;
1272+
strbuf_setlen(&tmpname, tmpname_len);
12691273
}
12701274

1275+
rename_tmp_packfile_idx(&tmpname, &idx_tmp_name);
1276+
1277+
free(idx_tmp_name);
12711278
strbuf_release(&tmpname);
12721279
free(pack_tmp_name);
12731280
puts(hash_to_hex(hash));

‎builtin/repack.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,10 @@ static struct {
208208
unsigned optional:1;
209209
} exts[] = {
210210
{".pack"},
211-
{".idx"},
212211
{".rev", 1},
213212
{".bitmap", 1},
214213
{".promisor", 1},
214+
{".idx"},
215215
};
216216

217217
static unsigned populate_pack_exts(char *name)

‎bulk-checkin.c‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,25 @@ static struct bulk_checkin_state {
2323
uint32_t nr_written;
2424
} state;
2525

26+
static void finish_tmp_packfile(struct strbuf *basename,
27+
const char *pack_tmp_name,
28+
struct pack_idx_entry **written_list,
29+
uint32_t nr_written,
30+
struct pack_idx_option *pack_idx_opts,
31+
unsigned char hash[])
32+
{
33+
char *idx_tmp_name = NULL;
34+
35+
stage_tmp_packfiles(basename, pack_tmp_name, written_list, nr_written,
36+
pack_idx_opts, hash, &idx_tmp_name);
37+
rename_tmp_packfile_idx(basename, &idx_tmp_name);
38+
39+
free(idx_tmp_name);
40+
}
41+
2642
static void finish_bulk_checkin(struct bulk_checkin_state *state)
2743
{
28-
struct object_id oid;
44+
unsigned char hash[GIT_MAX_RAWSZ];
2945
struct strbuf packname = STRBUF_INIT;
3046
int i;
3147

@@ -37,19 +53,20 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
3753
unlink(state->pack_tmp_name);
3854
goto clear_exit;
3955
} else if (state->nr_written == 1) {
40-
finalize_hashfile(state->f, oid.hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);
56+
finalize_hashfile(state->f, hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);
4157
} else {
42-
int fd = finalize_hashfile(state->f, oid.hash, 0);
43-
fixup_pack_header_footer(fd, oid.hash, state->pack_tmp_name,
44-
state->nr_written, oid.hash,
58+
int fd = finalize_hashfile(state->f, hash, 0);
59+
fixup_pack_header_footer(fd, hash, state->pack_tmp_name,
60+
state->nr_written, hash,
4561
state->offset);
4662
close(fd);
4763
}
4864

49-
strbuf_addf(&packname, "%s/pack/pack-", get_object_directory());
65+
strbuf_addf(&packname, "%s/pack/pack-%s.", get_object_directory(),
66+
hash_to_hex(hash));
5067
finish_tmp_packfile(&packname, state->pack_tmp_name,
5168
state->written, state->nr_written,
52-
&state->pack_idx_opts, oid.hash);
69+
&state->pack_idx_opts, hash);
5370
for (i = 0; i < state->nr_written; i++)
5471
free(state->written[i]);
5572

‎pack-write.c‎

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -461,49 +461,48 @@ struct hashfile *create_tmp_packfile(char **pack_tmp_name)
461461
return hashfd(fd, *pack_tmp_name);
462462
}
463463

464-
void finish_tmp_packfile(struct strbuf *name_buffer,
464+
static void rename_tmp_packfile(struct strbuf *name_prefix, const char *source,
465+
const char *ext)
466+
{
467+
size_t name_prefix_len = name_prefix->len;
468+
469+
strbuf_addstr(name_prefix, ext);
470+
if (rename(source, name_prefix->buf))
471+
die_errno("unable to rename temporary file to '%s'",
472+
name_prefix->buf);
473+
strbuf_setlen(name_prefix, name_prefix_len);
474+
}
475+
476+
void rename_tmp_packfile_idx(struct strbuf *name_buffer,
477+
char **idx_tmp_name)
478+
{
479+
rename_tmp_packfile(name_buffer, *idx_tmp_name, "idx");
480+
}
481+
482+
void stage_tmp_packfiles(struct strbuf *name_buffer,
465483
const char *pack_tmp_name,
466484
struct pack_idx_entry **written_list,
467485
uint32_t nr_written,
468486
struct pack_idx_option *pack_idx_opts,
469-
unsigned char hash[])
487+
unsigned char hash[],
488+
char **idx_tmp_name)
470489
{
471-
const char *idx_tmp_name, *rev_tmp_name = NULL;
472-
int basename_len = name_buffer->len;
490+
const char *rev_tmp_name = NULL;
473491

474492
if (adjust_shared_perm(pack_tmp_name))
475493
die_errno("unable to make temporary pack file readable");
476494

477-
idx_tmp_name = write_idx_file(NULL, written_list, nr_written,
478-
pack_idx_opts, hash);
479-
if (adjust_shared_perm(idx_tmp_name))
495+
*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,
496+
pack_idx_opts, hash);
497+
if (adjust_shared_perm(*idx_tmp_name))
480498
die_errno("unable to make temporary index file readable");
481499

482500
rev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,
483501
pack_idx_opts->flags);
484502

485-
strbuf_addf(name_buffer, "%s.pack", hash_to_hex(hash));
486-
487-
if (rename(pack_tmp_name, name_buffer->buf))
488-
die_errno("unable to rename temporary pack file");
489-
490-
strbuf_setlen(name_buffer, basename_len);
491-
492-
strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash));
493-
if (rename(idx_tmp_name, name_buffer->buf))
494-
die_errno("unable to rename temporary index file");
495-
496-
strbuf_setlen(name_buffer, basename_len);
497-
498-
if (rev_tmp_name) {
499-
strbuf_addf(name_buffer, "%s.rev", hash_to_hex(hash));
500-
if (rename(rev_tmp_name, name_buffer->buf))
501-
die_errno("unable to rename temporary reverse-index file");
502-
}
503-
504-
strbuf_setlen(name_buffer, basename_len);
505-
506-
free((void *)idx_tmp_name);
503+
rename_tmp_packfile(name_buffer, pack_tmp_name, "pack");
504+
if (rev_tmp_name)
505+
rename_tmp_packfile(name_buffer, rev_tmp_name, "rev");
507506
}
508507

509508
void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_sought)

‎pack.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,14 @@ int encode_in_pack_object_header(unsigned char *hdr, int hdr_len,
110110
int read_pack_header(int fd, struct pack_header *);
111111

112112
struct hashfile *create_tmp_packfile(char **pack_tmp_name);
113-
void finish_tmp_packfile(struct strbuf *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
113+
void stage_tmp_packfiles(struct strbuf *name_buffer,
114+
const char *pack_tmp_name,
115+
struct pack_idx_entry **written_list,
116+
uint32_t nr_written,
117+
struct pack_idx_option *pack_idx_opts,
118+
unsigned char hash[],
119+
char **idx_tmp_name);
120+
void rename_tmp_packfile_idx(struct strbuf *basename,
121+
char **idx_tmp_name);
114122

115123
#endif

0 commit comments

Comments
 (0)