git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC Patch] Preventing corrupt objects from entering the repository

From
MKMartin Koegler <mkoegler@auto.tuwien.ac.at>
Date
Feb 13, 2008, 08:11 UTC
Message-ID
<20080213081128.GA27730@auto.tuwien.ac.at>
In-Reply-To
<20080213074209.GG24004@spearce.org>
On Wed, Feb 13, 2008 at 02:42:09AM -0500, Shawn O. Pearce wrote:
Show 9 quoted lines
> Nicolas Pitre <nico@cam.org> wrote:
> > I think this is a good idea to always have some sanity checks on any 
> > incoming objects so to make sure they're well formed and valid before 
> > giving them a SHA1 value, and bail out as soon as any error is found.  
> 
> When we get the raw data for an object so we can compute its SHA-1
> and/or write its loose object to disk we should first verify its
> content is sane, then do the SHA-1/store loose.
> 
[...]
Show 14 quoted lines
> > As to making sure those objects are well connected... well this is a 
> > technically different issue entirely, and I wonder if a special mode to 
> > fsck might not be a better solution.
> 
> Nah, just do what quickfetch does in builtin-fetch.c, but run it
> in receive-pack, between unpack() and execute_commands():
> 
> 	rev-list --quiet --objects $new... --not --all
> 
> If it aborts, reachability testing failed and the push is rejected
> without updating any refs.  Yes your repository now has objects
> that are missing things, but none of those are considered to be
> reachable, so this isn't a big deal.  They will get cleaned up on
> the next `gc --prune`, whenever that is.

This would mean, that we must make git-rev-list and git-pack-objects not segfault on incorrect links between objects.

Show 20 quoted lines
> In this configuration (--quiet) rev-list tries to be pretty low
> on its memory usage, it doesn't save buffers, etc.  Further since
> everything that is already considered reachable is not interesting,
> we are only doing a walk over the objects that we just received,
> not our entire ODB.  Its also after index-pack exited, so we just
> freed up a good chunk of memory.
> 
> Rememeber we are talking about receive-pack here.  The cost on
> the to perform the rev-list is lower than the cost will be to pack
> these objects for distribution back to just one client.  Since this
> is a server of some sorts (otherwise why did you push here?), odds
> are its going to be doing a lot of packing requests for clients to
> receive these newly uploaded objects by the native git protocol.
> This new rev-list is nothing compared to that already existing load.
> And if your OS is any good the just created .idx and .pack is still
> in OS buffer cache, so there shouldn't be any additional disk IO.
> 
> Yes, we could make this optional in receive-pack, but really I don't
> see a reason to.  Just run it.  The client shouldn't be giving us
> unreachable crap.
Looks sane to me.
mfg Martin Kögler
Previous: Shawn O. PearceNext: Johannes Schindelin
Message 17 of 23 in “Preventing corrupt objects from entering the repository”
  1. Preventing corrupt objects from entering the repositoryMartin Koegler, Feb 10, 2008
  2. Junio C HamanoFeb 11, 2008
  3. Nicolas PitreFeb 11, 2008
  4. Martin KoeglerFeb 11, 2008
  5. Nicolas PitreFeb 11, 2008
  6. Martin KoeglerFeb 11, 2008
  7. Nicolas PitreFeb 12, 2008
  8. Martin KoeglerFeb 12, 2008
  9. Nicolas PitreFeb 12, 2008
  10. Martin KoeglerFeb 12, 2008
  11. Nicolas PitreFeb 12, 2008
  12. Shawn O. PearceFeb 13, 2008
  13. Martin KoeglerFeb 13, 2008
  14. Remove object-refs from fsckShawn O. Pearce, Feb 14, 2008
  15. Martin KoeglerFeb 14, 2008
  16. Shawn O. PearceFeb 13, 2008
  17. Martin KoeglerFeb 13, 2008
  18. Johannes SchindelinFeb 13, 2008
  19. Shawn O. PearceFeb 14, 2008
  20. Martin KoeglerFeb 14, 2008
  21. Johannes SchindelinFeb 15, 2008
  22. Martin KoeglerFeb 15, 2008
  23. Martin KoeglerFeb 12, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.