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

Skip 'bad date' check for some imports #92

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

spraints
Copy link
Member

In general, we do not want malformed data on our servers. However, there are some corner cases where we do accept it. A bad date format in an old commit message is one such place. Again, in general, we won't accept pushes with bad dates. The correct fix is for the user to rewrite the offending commit(s) so that Git can parse all of the dates. Failing to do so will result in errors in many applications (including GitHub), so it's better to avoid pushing malformed data. However, some repos (particularly those that are migrating to GitHub) have deep history and a malformed bit of data far enough back that it is very unlikely to cause issues. In those cases, we do sometimes decide that that cost of a complete history rewrite isn't worth it, and we allow the import to proceed.

cc @dpmex4527 @ttaylorr @mhagger https://github.com/github/git-systems/issues/3882

Pushing an empty pack should be fine. There was an earlier issue where
deleting refs didn't work because we tried looking for objects in the
uploaded pack and didn't find any. So we do want to preserve that
functionality. We also should be able to accept new refs that point to
commits that are already on the server.

missing objects are actually bad, so we don't need to have pushes work
when a partial graph is supplied. For now, the push still appears to
succeed, but in the next step it won't.
We have this enabled in our prod config, and it changes
spokes-receive-pack in important ways. In particular, it makes
spokes-receive-pack care about bad dates, which we want to disable
sometimes. So let's enable it here (and update the existing tests to
match the new behavior.
… requested

In general, it's better if customers rewrite their repos to remove the
bad dates because bad dates aren't guaranteed to work with all of our
systems and APIs. But, sometimes there are bad dates in a corner of a
repo where they're unlikely to cause problems, and the cost of rewriting
the repo is too high to be worth it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant