Skip to content

Commit 5c83d85

Browse files
committed
Merge branch 'mh/packed-ref-store-prep'
Bugfix for a topic that is (only) in 'master'. * mh/packed-ref-store-prep: for_each_bisect_ref(): don't trim refnames lock_packed_refs(): fix cache validity check
2 parents 849b44c + 03df567 commit 5c83d85

File tree

5 files changed

+54
-11
lines changed

5 files changed

+54
-11
lines changed

refs.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,6 +1342,18 @@ int for_each_ref_in_submodule(const char *submodule, const char *prefix,
13421342
prefix, fn, cb_data);
13431343
}
13441344

1345+
int for_each_fullref_in_submodule(const char *submodule, const char *prefix,
1346+
each_ref_fn fn, void *cb_data,
1347+
unsigned int broken)
1348+
{
1349+
unsigned int flag = 0;
1350+
1351+
if (broken)
1352+
flag = DO_FOR_EACH_INCLUDE_BROKEN;
1353+
return do_for_each_ref(get_submodule_ref_store(submodule),
1354+
prefix, fn, 0, flag, cb_data);
1355+
}
1356+
13451357
int for_each_replace_ref(each_ref_fn fn, void *cb_data)
13461358
{
13471359
return do_for_each_ref(get_main_ref_store(),

refs.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,10 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
303303
int for_each_ref_submodule(const char *submodule,
304304
each_ref_fn fn, void *cb_data);
305305
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
306-
each_ref_fn fn, void *cb_data);
306+
each_ref_fn fn, void *cb_data);
307+
int for_each_fullref_in_submodule(const char *submodule, const char *prefix,
308+
each_ref_fn fn, void *cb_data,
309+
unsigned int broken);
307310
int for_each_tag_ref_submodule(const char *submodule,
308311
each_ref_fn fn, void *cb_data);
309312
int for_each_branch_ref_submodule(const char *submodule,

refs/files-backend.c

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,18 @@ static void files_ref_path(struct files_ref_store *refs,
370370
}
371371
}
372372

373+
/*
374+
* Check that the packed refs cache (if any) still reflects the
375+
* contents of the file. If not, clear the cache.
376+
*/
377+
static void validate_packed_ref_cache(struct files_ref_store *refs)
378+
{
379+
if (refs->packed &&
380+
!stat_validity_check(&refs->packed->validity,
381+
files_packed_refs_path(refs)))
382+
clear_packed_ref_cache(refs);
383+
}
384+
373385
/*
374386
* Get the packed_ref_cache for the specified files_ref_store,
375387
* creating and populating it if it hasn't been read before or if the
@@ -382,10 +394,8 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
382394
{
383395
const char *packed_refs_file = files_packed_refs_path(refs);
384396

385-
if (refs->packed &&
386-
!is_lock_file_locked(&refs->packed_refs_lock) &&
387-
!stat_validity_check(&refs->packed->validity, packed_refs_file))
388-
clear_packed_ref_cache(refs);
397+
if (!is_lock_file_locked(&refs->packed_refs_lock))
398+
validate_packed_ref_cache(refs);
389399

390400
if (!refs->packed)
391401
refs->packed = read_packed_refs(packed_refs_file);
@@ -1312,13 +1322,17 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
13121322
&refs->packed_refs_lock, files_packed_refs_path(refs),
13131323
flags, timeout_value) < 0)
13141324
return -1;
1325+
13151326
/*
1316-
* Get the current packed-refs while holding the lock. It is
1317-
* important that we call `get_packed_ref_cache()` before
1318-
* setting `packed_ref_cache->lock`, because otherwise the
1319-
* former will see that the file is locked and assume that the
1320-
* cache can't be stale.
1327+
* Now that we hold the `packed-refs` lock, make sure that our
1328+
* cache matches the current version of the file. Normally
1329+
* `get_packed_ref_cache()` does that for us, but that
1330+
* function assumes that when the file is locked, any existing
1331+
* cache is still valid. We've just locked the file, but it
1332+
* might have changed the moment *before* we locked it.
13211333
*/
1334+
validate_packed_ref_cache(refs);
1335+
13221336
packed_ref_cache = get_packed_ref_cache(refs);
13231337
/* Increment the reference count to prevent it from being freed: */
13241338
acquire_packed_ref_cache(packed_ref_cache);

revision.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2075,7 +2075,7 @@ static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_d
20752075
struct strbuf bisect_refs = STRBUF_INIT;
20762076
int status;
20772077
strbuf_addf(&bisect_refs, "refs/bisect/%s", term);
2078-
status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
2078+
status = for_each_fullref_in_submodule(submodule, bisect_refs.buf, fn, cb_data, 0);
20792079
strbuf_release(&bisect_refs);
20802080
return status;
20812081
}

t/t6002-rev-list-bisect.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,4 +235,18 @@ test_sequence "--bisect"
235235

236236
#
237237
#
238+
239+
test_expect_success '--bisect can default to good/bad refs' '
240+
git update-ref refs/bisect/bad c3 &&
241+
good=$(git rev-parse b1) &&
242+
git update-ref refs/bisect/good-$good $good &&
243+
good=$(git rev-parse c1) &&
244+
git update-ref refs/bisect/good-$good $good &&
245+
246+
# the only thing between c3 and c1 is c2
247+
git rev-parse c2 >expect &&
248+
git rev-list --bisect >actual &&
249+
test_cmp expect actual
250+
'
251+
238252
test_done

0 commit comments

Comments
 (0)