Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

plumbing/format/packfile: Fix broken "thin" packfile support. Fixes #991 #994

Merged
merged 2 commits into from
Nov 16, 2018

Conversation

jpeletier
Copy link
Contributor

We have noticed that thin-pack support by the parser stopped working after upgrading from 4.5.0 to the latest version 4.7.1, but going back, it doesn't work since 4.6.0.
When the parser is given a so-called "thin" pack file, it will panic with a nil-pointer exception in Parser.get() as this function receives a nil o *objectInfo parameter. I traced this back to indexObjects() case plumbing.REFDeltaObject where the Parent property is left uninitialized when a REF delta object base is not found in the current pack. Unit tests did not check for thin-pack eventuality, and thus this case was uncovered.

This is an attempt to fix the issue. The strategy is to initialize objInfo.Parent to a "placeholder" parent, so we can detect this during the resolve phase.

I would need your guidance in writing tests for this one, as I am not sure how to produce a thin pack file out of the fixtures: these types of pack files only exsist on the network when git is moving stuff, never stored on disk.

@jfontan
Copy link
Contributor

jfontan commented Oct 25, 2018

I've managed to create a thin pack from fixtures repo labeled worktree, dirty:

https://github.com/src-d/go-git-fixtures/blob/v3.1.1/fixtures.go#L144

It's 726 bytes so it may be added to the test source. You can find the file here: https://downloads.zooloo.org/thin.pack

The way it was generated was creating a new commit, getting the list of unpacked objects and passing it to git pack-objects:

find .git/objects -type f | grep -v -E '(pack|info)' | sed 's|.git/objects/||' | tr -d '/' | git pack-objects --thin --stdout > thin-pack

@jpeletier
Copy link
Contributor Author

@jfontan I have added a test for this, please take a look.
I was not able to use your method above to create a thin packfile. Upon exploring the generated packfile, I detected it actually was embedding the dependencies in.
To create a truly thin packfile I resorted to using git bundle to produce a differential bundle, then removing the bundle header. The result is the new thinpack fixture.
I had to add that new fixture to the fixtures repo, so this won't work until src-d/go-git-fixtures#11 is merged

Please check it out and let me know. Thanks.

@mcuadros mcuadros merged commit 7853ab6 into src-d:master Nov 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants