Skip to content

Conversation

@timrchavez
Copy link
Contributor

If a git-resource is configured to use shallow cloning and new commits
are added to the repository that are not considered new versions of the
resource (e.g. via a path filter excludes it), a get will shallow
clone HEAD which will not be equal to the version ref. This will result
in breakage.

@pivotal-issuemaster
Copy link

@timrchavez Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@timrchavez
Copy link
Contributor Author

Assuming I have the test correct, I do not believe the suggested change for #158 will work. The error I'm getting is:

running it_can_shallow_clone_ref_with_new_commits...
  Switched to a new branch 'bogus'
  Switched to branch 'master'
  Cloning into '/tmp/git-tests.dlHJcd/git-tests.JFhcGp/destination'...
  error: Server does not allow request for unadvertised object 57b3b83c0199e8d73a1d1921785f2f73b0eb5ddf
The command '/bin/sh -c /tests/all.sh' returned a non-zero code: 1

@bhcleek
Copy link

bhcleek commented Jan 19, 2018

On a shallow clone, I am able to duplicate the error you're seeing. It appears, though, that setting uploadpack.allowReachableSHA1InWant on the server will allow an arbitrary SHA to be retrieved.

RE: https://git-scm.com/docs/git-fetch-pack

@timrchavez
Copy link
Contributor Author

I use a good ol until loop to progressively fetch deeper and deeper until I can successfully check out the ref.

@timrchavez timrchavez changed the title WIP: Make shallow cloning durable Make shallow cloning more durable Jan 23, 2018
If a git-resource is configured to use shallow cloning and new commits
are added to the repository that are not considered new versions of the
resource (e.g. via a path filter excluding it), a `get` will shallow
clone HEAD, which will not be equal to the version ref, and start to
break when it then tries to checkout the version ref since there is
no longer sufficient history available to find version ref. One
solution, which is proposed here, is to always fetch enough depth to
checkout the version ref corresponding to that version of the resource.
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

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

Thanks! I left a couple inline comments since this will break in a couple scenarios. Not sure what to do in those cases to be honest.

I wonder if an approach like @bhcleek mentioned would work instead, assuming that config is set?


git fetch origin refs/notes/*:refs/notes/*
git checkout -q $ref
until `git checkout -q $ref`; do

This comment was marked as spam.

This comment was marked as spam.

git fetch origin refs/notes/*:refs/notes/*
git checkout -q $ref
until `git checkout -q $ref`; do
deepen=$((deepen+$depth))

This comment was marked as spam.

This comment was marked as spam.

@pivotal-issuemaster
Copy link

@timrchavez Thank you for signing the Contributor License Agreement!

@vito
Copy link
Member

vito commented May 28, 2018

@timrchavez Do you have time to look at this? No rush, just pinging. :)

@timrchavez
Copy link
Contributor Author

timrchavez commented May 30, 2018

@vito Yep. I should have time. I'm not sure about @bhcleek's approach honestly because it requires a configuration change to the GitHub installation itself and based on the documentation it's very resource intensive. I'm open to other suggestions, but I'm not creative enough to think of anything better than a loop which fetches deeper and deeper until it can find its ref. We should do something here, even if it comes with a warning. This could significantly improve build times for us or anyone with very larger (mono)repos (our primary one is something like 1.2GB) by allowing them to safely shallow clone with path filters.

@vito
Copy link
Member

vito commented May 30, 2018

@timrchavez I'm fine with it fetching all the way back to the first commit in the push -f case and then giving up, just as long as it's an infinite loop.

norbertbuchmueller added a commit to optiopay/git-resource that referenced this pull request Aug 6, 2018
If the given ref is not found after a shallow clone, it starts chasing
it by deepening the clone in 2-power steps, eventually falling back to a
deep clone if a threshold is reached.

This should fix concourse#158 / concourse#167.
norbertbuchmueller added a commit to optiopay/git-resource that referenced this pull request Aug 6, 2018
If the given ref is not found after a shallow clone, it starts chasing
it by deepening the clone in steps of 2-power, eventually falling back
to a deep clone if a threshold is reached.

This should fix concourse#158 / concourse#167.
@vito
Copy link
Member

vito commented Aug 8, 2018

merged #205

@vito vito closed this Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants