Re: [PATCH 0/3] Strengthen fsck checks for submodule URLs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux