Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Add a new core.checkCollisions setting. On by default, it can be set
> to 'false' to disable the check for existing objects in sha1_object().
> ...
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 2004e25da2..4a3508aa9f 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -791,23 +791,24 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
> {
> void *new_data = NULL;
> int collision_test_needed = 0;
> + int do_coll_check = git_config_get_collision_check();
>
> assert(data || obj_entry);
>
> - if (startup_info->have_repository) {
> + if (do_coll_check && startup_info->have_repository) {
> read_lock();
> collision_test_needed =
> has_sha1_file_with_flags(oid->hash, OBJECT_INFO_QUICK);
> read_unlock();
> }
>
> - if (collision_test_needed && !data) {
> + if (do_coll_check && collision_test_needed && !data) {
> read_lock();
> if (!check_collison(obj_entry))
> collision_test_needed = 0;
> read_unlock();
> }
> - if (collision_test_needed) {
> + if (do_coll_check && collision_test_needed) {
If I am reading the patch correctly, The latter two changes are
totally unnecessary. c-t-needed is true only when dO-coll_check
allowed the initial "do we even have that object?" check to kick in
and never set otherwise.
I am not all that enthused to the idea of sending a wrong message to
our users, i.e. it is sometimes OK to sacrifice the security of
collision detection.
A small change like this is easy to adjust to apply to the codebase,
even after today's codebase undergoes extensive modifications; quite
honestly, I'd prefer not having to worry about it so close to the
pre-release feature freeze.
