Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pack-objects: create new name-hash algorithm #5157

Merged
merged 11 commits into from
Sep 24, 2024
6 changes: 4 additions & 2 deletions builtin/pack-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ struct configured_exclusion {
static struct oidmap configured_exclusions;

static struct oidset excluded_by_config;
static int use_full_name_hash;
static int use_full_name_hash = -1;

static inline uint32_t pack_name_hash_fn(const char *name)
{
Expand Down Expand Up @@ -4554,8 +4554,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (pack_to_stdout || !rev_list_all)
write_bitmap_index = 0;

if (write_bitmap_index && use_full_name_hash)
if (write_bitmap_index && use_full_name_hash > 0)
die(_("currently, the --full-name-hash option is incompatible with --write-bitmap-index"));
if (use_full_name_hash < 0)
use_full_name_hash = git_env_bool("GIT_TEST_FULL_NAME_HASH", 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment variable should probably be interpreted before validating that we're not writing a bitmap, right? I guess that would let more tests fail, though...


if (use_delta_islands)
strvec_push(&rp, "--topo-order");
Expand Down
1 change: 1 addition & 0 deletions ci/run-build-and-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ linux-TEST-vars)
export GIT_TEST_NO_WRITE_REV_INDEX=1
export GIT_TEST_CHECKOUT_WORKERS=2
export GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL=1
export GIT_TEST_FULL_NAME_HASH=1
;;
linux-clang)
export GIT_TEST_DEFAULT_HASH=sha1
Expand Down
4 changes: 4 additions & 0 deletions t/README
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,10 @@ a test and then fails then the whole test run will abort. This can help to make
sure the expected tests are executed and not silently skipped when their
dependency breaks or is simply not present in a new environment.

GIT_TEST_FULL_NAME_HASH=<boolean>, when true, sets the default name-hash
function in 'git pack-objects' to be the one used by the --full-name-hash
option.

GIT_TEST_FSCACHE=<boolean> exercises the uncommon fscache code path
which adds a cache below mingw's lstat and dirent implementations.

Expand Down
7 changes: 6 additions & 1 deletion t/t5510-fetch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,12 @@ test_expect_success 'all boundary commits are excluded' '
test_tick &&
git merge otherside &&
ad=$(git log --no-walk --format=%ad HEAD) &&
git bundle create twoside-boundary.bdl main --since="$ad" &&
# If the --full-name-hash function is used here, then no delta
# pair is found and the bundle does not expand to three objects
# when fixing the thin object.
GIT_TEST_FULL_NAME_HASH=0 \
git bundle create twoside-boundary.bdl main --since="$ad" &&
test_bundle_object_count --thin twoside-boundary.bdl 3
'

Expand Down
26 changes: 24 additions & 2 deletions t/t5616-partial-clone.sh
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,18 @@ test_expect_success 'fetch lazy-fetches only to resolve deltas' '
# Exercise to make sure it works. Git will not fetch anything from the
# promisor remote other than for the big tree (because it needs to
# resolve the delta).
GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
#
# TODO: the --full-name-hash option is disabled here, since this test
# is fundamentally broken! When GIT_TEST_FULL_NAME_HASH=1, the server
# recognizes delta bases in a different way and then sends a _blob_ to
# the client with a delta base that the client does not have! This is
# because the client is cloned from "promisor-server" with tree:0 but
# is now fetching from "server" withot any filter. This is violating the
# promise to the server that all reachable objects exist and could be
# used as delta bases!
GIT_TRACE_PACKET="$(pwd)/trace" \
GIT_TEST_FULL_NAME_HASH=0 \
git -C client \
fetch "file://$(pwd)/server" main &&

# Verify the assumption that the client needed to fetch the delta base
Expand All @@ -534,7 +545,18 @@ test_expect_success 'fetch lazy-fetches only to resolve deltas, protocol v2' '
# Exercise to make sure it works. Git will not fetch anything from the
# promisor remote other than for the big blob (because it needs to
# resolve the delta).
GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
#
# TODO: the --full-name-hash option is disabled here, since this test
# is fundamentally broken! When GIT_TEST_FULL_NAME_HASH=1, the server
# recognizes delta bases in a different way and then sends a _blob_ to
# the client with a delta base that the client does not have! This is
# because the client is cloned from "promisor-server" with tree:0 but
# is now fetching from "server" withot any filter. This is violating the
# promise to the server that all reachable objects exist and could be
# used as delta bases!
GIT_TRACE_PACKET="$(pwd)/trace" \
GIT_TEST_FULL_NAME_HASH=0 \
git -C client \
fetch "file://$(pwd)/server" main &&

# Verify that protocol version 2 was used.
Expand Down
6 changes: 5 additions & 1 deletion t/t6020-bundle-misc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,11 @@ test_expect_success 'create bundle with --since option' '
EOF
test_cmp expect actual &&
git bundle create since.bdl \
# If the --full-name-hash option is used, then one fewer
# delta base is found and this counts a different number
# of objects after performing --fix-thin.
GIT_TEST_FULL_NAME_HASH=0 \
git bundle create since.bdl \
--since "Thu Apr 7 15:27:00 2005 -0700" \
--all &&
Expand Down