Skip to content

Commit 3c63503

Browse files
gitstergitster
committed
Merge branch 'tb/pack-revindex-on-disk' into ds/fsck-pack-revindex
* tb/pack-revindex-on-disk: t: invert `GIT_TEST_WRITE_REV_INDEX` config: enable `pack.writeReverseIndex` by default pack-revindex: introduce `pack.readReverseIndex` pack-revindex: introduce GIT_TEST_REV_INDEX_DIE_ON_DISK pack-revindex: make `load_pack_revindex` take a repository t5325: mark as leak-free pack-write.c: plug a leak in stage_tmp_packfiles()
2 parents 9857273 + 9f7f10a commit 3c63503

14 files changed

+62
-26
lines changed

‎Documentation/config/pack.txt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,15 @@ pack.writeBitmapLookupTable::
171171
beneficial in repositories that have relatively large bitmap
172172
indexes. Defaults to false.
173173

174+
pack.readReverseIndex::
175+
When true, git will read any .rev file(s) that may be available
176+
(see: linkgit:gitformat-pack[5]). When false, the reverse index
177+
will be generated from scratch and stored in memory. Defaults to
178+
true.
179+
174180
pack.writeReverseIndex::
175181
When true, git will write a corresponding .rev file (see:
176182
linkgit:gitformat-pack[5])
177183
for each new packfile that it writes in all places except for
178184
linkgit:git-fast-import[1] and in the bulk checkin mechanism.
179-
Defaults to false.
185+
Defaults to true.

‎builtin/index-pack.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1753,12 +1753,13 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
17531753
fsck_options.walk = mark_link;
17541754

17551755
reset_pack_idx_option(&opts);
1756+
opts.flags |= WRITE_REV;
17561757
git_config(git_index_pack_config, &opts);
17571758
if (prefix && chdir(prefix))
17581759
die(_("Cannot come back to cwd"));
17591760

1760-
if (git_env_bool(GIT_TEST_WRITE_REV_INDEX, 0))
1761-
rev_index = 1;
1761+
if (git_env_bool(GIT_TEST_NO_WRITE_REV_INDEX, 0))
1762+
rev_index = 0;
17621763
else
17631764
rev_index = !!(opts.flags & (WRITE_REV_VERIFY | WRITE_REV));
17641765

‎builtin/pack-objects.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4293,9 +4293,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
42934293
}
42944294

42954295
reset_pack_idx_option(&pack_idx_opts);
4296+
pack_idx_opts.flags |= WRITE_REV;
42964297
git_config(git_pack_config, NULL);
4297-
if (git_env_bool(GIT_TEST_WRITE_REV_INDEX, 0))
4298-
pack_idx_opts.flags |= WRITE_REV;
4298+
if (git_env_bool(GIT_TEST_NO_WRITE_REV_INDEX, 0))
4299+
pack_idx_opts.flags &= ~WRITE_REV;
42994300

43004301
progress = isatty(2);
43014302
argc = parse_options(argc, argv, prefix, pack_objects_options,

‎ci/run-build-and-tests.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ linux-TEST-vars)
2727
export GIT_TEST_MULTI_PACK_INDEX=1
2828
export GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1
2929
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
30-
export GIT_TEST_WRITE_REV_INDEX=1
30+
export GIT_TEST_NO_WRITE_REV_INDEX=1
3131
export GIT_TEST_CHECKOUT_WORKERS=2
3232
;;
3333
linux-clang)

‎pack-bitmap.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
463463
return 0;
464464
}
465465

466-
static int load_reverse_index(struct bitmap_index *bitmap_git)
466+
static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_git)
467467
{
468468
if (bitmap_is_midx(bitmap_git)) {
469469
uint32_t i;
@@ -477,23 +477,23 @@ static int load_reverse_index(struct bitmap_index *bitmap_git)
477477
* since we will need to make use of them in pack-objects.
478478
*/
479479
for (i = 0; i < bitmap_git->midx->num_packs; i++) {
480-
ret = load_pack_revindex(bitmap_git->midx->packs[i]);
480+
ret = load_pack_revindex(r, bitmap_git->midx->packs[i]);
481481
if (ret)
482482
return ret;
483483
}
484484
return 0;
485485
}
486-
return load_pack_revindex(bitmap_git->pack);
486+
return load_pack_revindex(r, bitmap_git->pack);
487487
}
488488

489-
static int load_bitmap(struct bitmap_index *bitmap_git)
489+
static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git)
490490
{
491491
assert(bitmap_git->map);
492492

493493
bitmap_git->bitmaps = kh_init_oid_map();
494494
bitmap_git->ext_index.positions = kh_init_oid_pos();
495495

496-
if (load_reverse_index(bitmap_git))
496+
if (load_reverse_index(r, bitmap_git))
497497
goto failed;
498498

499499
if (!(bitmap_git->commits = read_bitmap_1(bitmap_git)) ||
@@ -580,7 +580,7 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r)
580580
{
581581
struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
582582

583-
if (!open_bitmap(r, bitmap_git) && !load_bitmap(bitmap_git))
583+
if (!open_bitmap(r, bitmap_git) && !load_bitmap(r, bitmap_git))
584584
return bitmap_git;
585585

586586
free_bitmap_index(bitmap_git);
@@ -589,9 +589,10 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r)
589589

590590
struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
591591
{
592+
struct repository *r = the_repository;
592593
struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
593594

594-
if (!open_midx_bitmap_1(bitmap_git, midx) && !load_bitmap(bitmap_git))
595+
if (!open_midx_bitmap_1(bitmap_git, midx) && !load_bitmap(r, bitmap_git))
595596
return bitmap_git;
596597

597598
free_bitmap_index(bitmap_git);
@@ -1592,7 +1593,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
15921593
* from disk. this is the point of no return; after this the rev_list
15931594
* becomes invalidated and we must perform the revwalk through bitmaps
15941595
*/
1595-
if (load_bitmap(bitmap_git) < 0)
1596+
if (load_bitmap(revs->repo, bitmap_git) < 0)
15961597
goto cleanup;
15971598

15981599
object_array_clear(&revs->pending);
@@ -1742,6 +1743,7 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
17421743
uint32_t *entries,
17431744
struct bitmap **reuse_out)
17441745
{
1746+
struct repository *r = the_repository;
17451747
struct packed_git *pack;
17461748
struct bitmap *result = bitmap_git->result;
17471749
struct bitmap *reuse;
@@ -1752,7 +1754,7 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
17521754

17531755
assert(result);
17541756

1755-
load_reverse_index(bitmap_git);
1757+
load_reverse_index(r, bitmap_git);
17561758

17571759
if (bitmap_is_midx(bitmap_git))
17581760
pack = bitmap_git->midx->packs[midx_preferred_pack(bitmap_git)];
@@ -2132,11 +2134,12 @@ int rebuild_bitmap(const uint32_t *reposition,
21322134
uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git,
21332135
struct packing_data *mapping)
21342136
{
2137+
struct repository *r = the_repository;
21352138
uint32_t i, num_objects;
21362139
uint32_t *reposition;
21372140

21382141
if (!bitmap_is_midx(bitmap_git))
2139-
load_reverse_index(bitmap_git);
2142+
load_reverse_index(r, bitmap_git);
21402143
else if (load_midx_revindex(bitmap_git->midx) < 0)
21412144
BUG("rebuild_existing_bitmaps: missing required rev-cache "
21422145
"extension");

‎pack-revindex.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ static int load_revindex_from_disk(char *revindex_name,
205205
size_t revindex_size;
206206
struct revindex_header *hdr;
207207

208+
if (git_env_bool(GIT_TEST_REV_INDEX_DIE_ON_DISK, 0))
209+
die("dying as requested by '%s'", GIT_TEST_REV_INDEX_DIE_ON_DISK);
210+
208211
fd = git_open(revindex_name);
209212

210213
if (fd < 0) {
@@ -283,12 +286,15 @@ static int load_pack_revindex_from_disk(struct packed_git *p)
283286
return ret;
284287
}
285288

286-
int load_pack_revindex(struct packed_git *p)
289+
int load_pack_revindex(struct repository *r, struct packed_git *p)
287290
{
288291
if (p->revindex || p->revindex_data)
289292
return 0;
290293

291-
if (!load_pack_revindex_from_disk(p))
294+
prepare_repo_settings(r);
295+
296+
if (r->settings.pack_read_reverse_index &&
297+
!load_pack_revindex_from_disk(p))
292298
return 0;
293299
else if (!create_pack_revindex_in_memory(p))
294300
return 0;
@@ -356,7 +362,7 @@ int offset_to_pack_pos(struct packed_git *p, off_t ofs, uint32_t *pos)
356362
{
357363
unsigned lo, hi;
358364

359-
if (load_pack_revindex(p) < 0)
365+
if (load_pack_revindex(the_repository, p) < 0)
360366
return -1;
361367

362368
lo = 0;

‎pack-revindex.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,13 @@
3434
#define RIDX_SIGNATURE 0x52494458 /* "RIDX" */
3535
#define RIDX_VERSION 1
3636

37-
#define GIT_TEST_WRITE_REV_INDEX "GIT_TEST_WRITE_REV_INDEX"
37+
#define GIT_TEST_NO_WRITE_REV_INDEX "GIT_TEST_NO_WRITE_REV_INDEX"
3838
#define GIT_TEST_REV_INDEX_DIE_IN_MEMORY "GIT_TEST_REV_INDEX_DIE_IN_MEMORY"
39+
#define GIT_TEST_REV_INDEX_DIE_ON_DISK "GIT_TEST_REV_INDEX_DIE_ON_DISK"
3940

4041
struct packed_git;
4142
struct multi_pack_index;
43+
struct repository;
4244

4345
/*
4446
* load_pack_revindex populates the revindex's internal data-structures for the
@@ -47,7 +49,7 @@ struct multi_pack_index;
4749
* If a '.rev' file is present it is mmap'd, and pointers are assigned into it
4850
* (instead of using the in-memory variant).
4951
*/
50-
int load_pack_revindex(struct packed_git *p);
52+
int load_pack_revindex(struct repository *r, struct packed_git *p);
5153

5254
/*
5355
* load_midx_revindex loads the '.rev' file corresponding to the given

‎pack-write.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,
568568
rename_tmp_packfile(name_buffer, rev_tmp_name, "rev");
569569
if (mtimes_tmp_name)
570570
rename_tmp_packfile(name_buffer, mtimes_tmp_name, "mtimes");
571+
572+
free((char *)rev_tmp_name);
571573
}
572574

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

‎packfile.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2151,7 +2151,7 @@ int for_each_object_in_pack(struct packed_git *p,
21512151
int r = 0;
21522152

21532153
if (flags & FOR_EACH_OBJECT_PACK_ORDER) {
2154-
if (load_pack_revindex(p))
2154+
if (load_pack_revindex(the_repository, p))
21552155
return -1;
21562156
}
21572157

‎repo-settings.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ void prepare_repo_settings(struct repository *r)
6363
repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
6464
repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0);
6565
repo_cfg_bool(r, "index.skiphash", &r->settings.index_skip_hash, r->settings.index_skip_hash);
66+
repo_cfg_bool(r, "pack.readreverseindex", &r->settings.pack_read_reverse_index, 1);
6667

6768
/*
6869
* The GIT_TEST_MULTI_PACK_INDEX variable is special in that

0 commit comments

Comments
 (0)