mbox series

[0/4] index-pack: optionally turn off SHA-1 collision checking

Message ID 20181028225023.26427-1-avarab@gmail.com (mailing list archive)
Headers show
Series index-pack: optionally turn off SHA-1 collision checking | expand

Message

Ævar Arnfjörð Bjarmason Oct. 28, 2018, 10:50 p.m. UTC 
This patch series implements what I suggested in
https://public-inbox.org/git/87lg6jljmf.fsf@evledraar.gmail.com/

It's not a replacement for what Geert Jansen's RFC in
https://public-inbox.org/git/ED25E182-C296-4D08-8170-340567D8964A@amazon.com/
does, which was to turn this off entirely on "clone".

I left the door open for that in the new config option 4/4 implements,
but I suspect for Geert's purposes this is something he'd prefer to
turn off in git on clone entirely, i.e. because it may be running on
some random Amazon's customer's EFS instance, and they won't know
about this new core.checkCollisions option.

But maybe I'm wrong about that and Geert is happy to just turn on
core.checkCollisions=false and use this series instead.

Ævar Arnfjörð Bjarmason (4):
  pack-objects test: modernize style
  pack-objects tests: don't leave test .git corrupt at end
  index-pack tests: don't leave test repo dirty at end
  index-pack: add ability to disable SHA-1 collision check

 Documentation/config.txt     | 68 ++++++++++++++++++++++++++++++++++++
 builtin/index-pack.c         |  7 ++--
 cache.h                      |  1 +
 config.c                     | 20 +++++++++++
 config.h                     |  1 +
 environment.c                |  1 +
 t/README                     |  3 ++
 t/t1060-object-corruption.sh | 37 +++++++++++++++++++-
 t/t5300-pack-object.sh       | 51 +++++++++++++++------------
 9 files changed, 163 insertions(+), 26 deletions(-)

Comments

Geert Jansen Oct. 30, 2018, 2:49 a.m. UTC | #1 
On Sun, Oct 28, 2018 at 10:50:19PM +0000, Ævar Arnfjörð Bjarmason wrote:

> I left the door open for that in the new config option 4/4 implements,
> but I suspect for Geert's purposes this is something he'd prefer to
> turn off in git on clone entirely, i.e. because it may be running on
> some random Amazon's customer's EFS instance, and they won't know
> about this new core.checkCollisions option.
> 
> But maybe I'm wrong about that and Geert is happy to just turn on
> core.checkCollisions=false and use this series instead.

I think that the best user experience would probably be if git were fast by
default without having to give up on (future) security by removing the sha1
collision check.  Maybe core.checkCollisons could default to "on" only when
there's no loose objects in the repository? That would give a fast experience
for many common cases (git clone, git init && git fetch) while still doing the
collision check when relevant.

My patch used the --cloning flag as an approximation of "no loose objects".
Maybe a better option would be to check for the non-existence of the [00-ff]
directories under .git/objects.
Junio C Hamano Oct. 30, 2018, 9:04 a.m. UTC | #2 
Geert Jansen <gerardu@amazon.com> writes:

> Maybe a better option would be to check for the non-existence of the [00-ff]
> directories under .git/objects.

Please do not do this; I expect many people do this before they
leave work, just like I do:

	$ git repack -a -d -f --window=$largs --depth=$small
	$ git prune

which would typically leave only info/ and pack/ subdirectories
under .git/objects/ directory.