On 13 Nov 24 15:40, Junio C Hamano wrote:
> Neil Mayhew <neil@xxxxxxxxxxx> writes:
>
>> ... immediately corrected by another commit. However, the bad commit is
>> still in the history. It happened 6 years ago, so there's no
>> possibility of us changing the history.
>
> I think fsck.skipList was meant to cover such a case. The idea is
> that the blob object name of the bad .gitmodules file can be placed
> on the list, and the rest of the "bad commit" and the whole history
> can still be checked for consistency, without triggering the warning
> (or error) resulting from the offending .gitmodules file.
Thank you. This explanation is very helpful. I hadn't noticed the
existence of this config option, and using it does indeed enable fsck to
pass on our repo.
>> Is there any possibility of "loosening the fsck.gitmodulesUrl
>> severity", as Jeff suggested?
>
> Isn't the suggestion not about butchering the rest of the world but
> by locally configuring fsck.gitmodulesUrl down from error to
> warning?
Again, thank you for the helpful explanation. I hadn't fully understood
what was being suggested.
> I personally think excluding a single known-offending blob
> without doing such loosening is a much better idea in that it
> prevents *new* offending instances from getting into the repository,
> while allowing an existing benign and honest mistake to stay in your
> history. Loosening the severity of a class of check means you will
> accept *new* offending instances, which may very well be malicious,
> unlike the existing benign one you know about.
I agree that skipping a single object is better than loosening.
However, there is a bit of a catch-22 situation here. The problem in our
repo was reported to us by someone who was unable to clone, because they
have transfer.fsckObjects set globally (and initially didn't realise
that was where the error message was coming from). Someone in that
situation could in addition set fetch.fsck.gitmodulesUrl=warn globally,
but as you say that's too all-encompassing and it would be better to
skip the specific object. Unfortunately, this is difficult to do when
cloning, because the configuration value doesn't come automatically with
the repo. A local skiplist configuration value can't exist until after
the repo has been cloned, and adding the object name to a global
skiplist doesn't seem appropriate. It's possible to add the option to
the clone command line (with -c) but that's starting to get quite messy
and in any case many people wouldn't see the instruction even if we put
it in our README.
I don't think there's a perfect solution here. It seems like the best we
can do is to put the object name in a .git-fsck-skiplist file at the top
level of the repo, and add a section at the end of the README telling
people to configure it for all three *fsck.skipList values if they want
to use git fsck explicitly, or implicitly via other configuration
values. We would also need to mention using -c when cloning initially.
This information will probably be intimidating for most people, and I
wish we didn't have to include it, but hopefully we can make it clear
that most people won't need it.