{"thread":{"id":"553","subject":"Darcs-git: a few notes for Git hackers","startedAt":"2005-05-09T18:01:25Z","lastAt":"2005-05-10T12:55:54Z","messageCount":12,"participants":["Juliusz Chroboczek","Petr Baudis","H. Peter Anvin","Brad Roberts","Junio C Hamano","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"2899","messageId":"7ihdhc5le2.fsf@lanthane.pps.jussieu.fr","threadId":"553","inReplyTo":null,"subject":"Darcs-git: a few notes for Git hackers","fromName":"Juliusz Chroboczek","fromEmail":"juliusz.chroboczek@pps.jussieu.fr","sentAt":"2005-05-09T18:01:25Z","receivedAt":"2005-05-09T18:01:25Z","isPatch":false,"sender":{"key":"juliusz.chroboczek@pps.jussieu.fr","avatar":null},"body":"Hi,\n\nHere are a few notes about Git that should probably be taken into\naccount by people working on Git itself or on Git wrappers. The notes\napply to Linus' Git-0.6, which is the code I'm using in Darcs-git;\nsome of them might no longer be applicable to Darcs.\n\n\n1. Darcs-git uses the fact that Git updates are atomic when reading\nfrom a Git repository. Darcs-git almost writes to Git repositories\natomically, with one exception: it performs a non-atomic\nread/update/write cycle on .git/HEAD.\n\nFor that reason, I'm taking a high-level lock on .git repositories\nwhenever I write them. The lockfile is ``.git/lock''. I haven't\nthought about whether Darcs can be easily coerced into accessing Git\nrepos atomically; have people writing Git wrappers found the need for\na global lock?\n\n\n2. The files git.h and git.c in Darcs-git are a simple ``libgit'' that\ncontains just enough functionality for Darcs-git; they use the\nfunctionality of sha1_file.c and read_cache.c from Git-0.6.\n\nI've found a few problems with the interfaces in these files:\n\n - the global variables sha1_file_directory, active_cache, active_nr\n and active_alloc are not marked ``extern'' in cache.h. This breaks\n linkers that don't grok common symbols, such as the one in GHCi\n (silly GHCi).\n\n - the function write_sha1_file takes the metadata and the data in a\n contiguous buffer, which is a problem when the data has been\n allocated by a higher layer. I'm currently working around the\n problem by memcpy-ing everything into a temp buffer, but that's\n obviously not a good thing. I don't care whether write_sha1_file\n is changed to use a writev-like interface, or to take the metadata\n explicitly (as in char *type, unsigned long length).\n\n - there is no (usable) function to write a tree; there's the code in\n write_tree.c, but it's not generally useful. See the function\n ``git_write_tree_done'' in git.c for the type of interface I'm\n thinking of.\n\n - there's no way to have multiple simultaneous caches, short of\n hacking at the values of Git's global variables by hand.\n\nAs I'd rather not maintain my own version of Git, I'd be mighty\ngrateful if some friendly Git hacker could fix the above.\n\n Juliusz\n"},{"id":"2912","messageId":"20050509212842.GC15712@pasky.ji.cz","threadId":"553","inReplyTo":"7ihdhc5le2.fsf@lanthane.pps.jussieu.fr","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-09T21:28:42Z","receivedAt":"2005-05-09T21:28:42Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Mon, May 09, 2005 at 08:01:25PM CEST, I got a letter\nwhere Juliusz Chroboczek told me that...\n> 1. Darcs-git uses the fact that Git updates are atomic when reading\n> from a Git repository. Darcs-git almost writes to Git repositories\n> atomically, with one exception: it performs a non-atomic\n> read/update/write cycle on .git/HEAD.\n> \n> For that reason, I'm taking a high-level lock on .git repositories\n> whenever I write them. The lockfile is ``.git/lock''. I haven't\n> thought about whether Darcs can be easily coerced into accessing Git\n> repos atomically; have people writing Git wrappers found the need for\n> a global lock?\n\nFWIW, Cogito does not lock at all yet - this is one of the things which\nshould be fixed soon.\n\n> - there's no way to have multiple simultaneous caches, short of\n> hacking at the values of Git's global variables by hand.\n\nSee the Brad Robert's patches of Apr 21. I've decided not to apply them\nsince it appears a lot has changed since then and it would be some pain;\nbut they may be a worthy starting point for a more up-to-date patch.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"2913","messageId":"7iu0lc129m.fsf@lanthane.pps.jussieu.fr","threadId":"553","inReplyTo":"20050509212842.GC15712@pasky.ji.cz","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Juliusz Chroboczek","fromEmail":"juliusz.chroboczek@pps.jussieu.fr","sentAt":"2005-05-09T22:08:05Z","receivedAt":"2005-05-09T22:08:05Z","isPatch":false,"sender":{"key":"juliusz.chroboczek@pps.jussieu.fr","avatar":null},"body":"Ahoj,\n\n> FWIW, Cogito does not lock at all yet - this is one of the things which\n> should be fixed soon.\n\nI see. Let me know if you decide to use a different name for the lock\nfile so I can switch to using the same one as yours.\n\n Julek\n"},{"id":"2916","messageId":"427FE248.7040403@zytor.com","threadId":"553","inReplyTo":"7iu0lc129m.fsf@lanthane.pps.jussieu.fr","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-05-09T22:20:56Z","receivedAt":"2005-05-09T22:20:56Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Juliusz Chroboczek wrote:\n> Ahoj,\n> \n> \n>>FWIW, Cogito does not lock at all yet - this is one of the things which\n>>should be fixed soon.\n> \n> \n> I see. Let me know if you decide to use a different name for the lock\n> file so I can switch to using the same one as yours.\n> \n\nAre you using flock(), or some other contraption that breaks if a \nprocess dies unexpectedly?\n\n\t-hpa\n"},{"id":"2918","messageId":"7ipsw010i5.fsf@lanthane.pps.jussieu.fr","threadId":"553","inReplyTo":"427FE248.7040403@zytor.com","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Juliusz Chroboczek","fromEmail":"juliusz.chroboczek@pps.jussieu.fr","sentAt":"2005-05-09T22:46:10Z","receivedAt":"2005-05-09T22:46:10Z","isPatch":false,"sender":{"key":"juliusz.chroboczek@pps.jussieu.fr","avatar":null},"body":">> I see. Let me know if you decide to use a different name for the\n>> lock file so I can switch to using the same one as yours.\n\n> Are you using flock(), or some other contraption that breaks if a\n> process dies unexpectedly?\n\nNo, I'm using a file that is created by the NFS-safe equivalent of\nopen(O_CREAT | O_EXCL). This is what Darcs has been doing basically\nforever.\n\nDarcs usually doesn't die unexpectedly -- it's a Haskell program, so\nbugs usually manifest themselves with an exception being thrown\nallowing Darcs to clean-up after itself.\n\nThe one exception is when Darcs gets killed by the OOM killer (which,\nas you doubtless know, doesn't give any advance warning to a process,\nthus making it impossible for a process to deal with it gracefully).\nIn such cases, manual intervention is necessary anyway -- a file could\nhave been written half-way.\n\n Juliusz\n"},{"id":"2919","messageId":"427FE938.7050904@zytor.com","threadId":"553","inReplyTo":"7ipsw010i5.fsf@lanthane.pps.jussieu.fr","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-05-09T22:50:32Z","receivedAt":"2005-05-09T22:50:32Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Juliusz Chroboczek wrote:\n>>>I see. Let me know if you decide to use a different name for the\n>>>lock file so I can switch to using the same one as yours.\n> \n> \n>>Are you using flock(), or some other contraption that breaks if a\n>>process dies unexpectedly?\n> \n> No, I'm using a file that is created by the NFS-safe equivalent of\n> open(O_CREAT | O_EXCL). This is what Darcs has been doing basically\n> forever.\n> \n> Darcs usually doesn't die unexpectedly -- it's a Haskell program, so\n> bugs usually manifest themselves with an exception being thrown\n> allowing Darcs to clean-up after itself.\n> \n> The one exception is when Darcs gets killed by the OOM killer (which,\n> as you doubtless know, doesn't give any advance warning to a process,\n> thus making it impossible for a process to deal with it gracefully).\n> In such cases, manual intervention is necessary anyway -- a file could\n> have been written half-way.\n> \n\nIn the case of git, it should not be necessary even then; there might be \na broken file in the repository but nothing would reference it so it \nshouldn't have any effect. Functionally speaking, operations on the git \nrepository are in themselves atomic.\n\n\t-hpa\n"},{"id":"2920","messageId":"Pine.LNX.4.44.0505091549210.2136-100000@bellevue.puremagic.com","threadId":"553","inReplyTo":"20050509212842.GC15712@pasky.ji.cz","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Brad Roberts","fromEmail":"braddr@puremagic.com","sentAt":"2005-05-09T22:50:33Z","receivedAt":"2005-05-09T22:50:33Z","isPatch":false,"sender":{"key":"braddr@puremagic.com","avatar":null},"body":"> > - there's no way to have multiple simultaneous caches, short of\n> > hacking at the values of Git's global variables by hand.\n>\n> See the Brad Robert's patches of Apr 21. I've decided not to apply them\n> since it appears a lot has changed since then and it would be some pain;\n> but they may be a worthy starting point for a more up-to-date patch.\n>\n> --\n> \t\t\t\tPetr \"Pasky\" Baudis\n\nSince there's interest, I'll pull tip of your tree and re-do them. I\nhaven't bothered todate since no one seemed interested. Do you want them\npiece meal like I did last time or just one big diff?\n\nLater,\nBrad\n\n"},{"id":"2921","messageId":"20050509230211.GD15712@pasky.ji.cz","threadId":"553","inReplyTo":"Pine.LNX.4.44.0505091549210.2136-100000@bellevue.puremagic.com","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-09T23:02:11Z","receivedAt":"2005-05-09T23:02:11Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, May 10, 2005 at 12:50:33AM CEST, I got a letter\nwhere Brad Roberts told me that...\n> > > - there's no way to have multiple simultaneous caches, short of\n> > > hacking at the values of Git's global variables by hand.\n> >\n> > See the Brad Robert's patches of Apr 21. I've decided not to apply them\n> > since it appears a lot has changed since then and it would be some pain;\n> > but they may be a worthy starting point for a more up-to-date patch.\n> >\n> > --\n> > \t\t\t\tPetr \"Pasky\" Baudis\n> \n> Since there's interest, I'll pull tip of your tree and re-do them. I\n> haven't bothered todate since no one seemed interested. Do you want them\n> piece meal like I did last time or just one big diff?\n\nPiece meal would be excellent.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"2922","messageId":"7i3bsw0zhr.fsf@lanthane.pps.jussieu.fr","threadId":"553","inReplyTo":"427FE938.7050904@zytor.com","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Juliusz Chroboczek","fromEmail":"juliusz.chroboczek@pps.jussieu.fr","sentAt":"2005-05-09T23:08:00Z","receivedAt":"2005-05-09T23:08:00Z","isPatch":false,"sender":{"key":"juliusz.chroboczek@pps.jussieu.fr","avatar":null},"body":">> In such cases, manual intervention is necessary anyway -- a file could\n>> have been written half-way.\n\n> In the case of git, it should not be necessary even then; there might\n> be a broken file in the repository but nothing would reference it so\n> it shouldn't have any effect. Functionally speaking, operations on\n> the git repository are in themselves atomic.\n\nYes, you're right.\n\nI still prefer using a lockfile to flock -- NFS-safety is important\nfor us. And experience with the Darcs user base (who are probably\nless Unix-savvy then the Git userbase) shows that they have no problem\ndoing\n\n $ ps\n $ rm _darcs/lock\n $ darcs check\n\nwhen Darcs complains about a stray lockfile.\n\n Juliusz\n\n"},{"id":"2924","messageId":"7vwtq8vuqr.fsf@assigned-by-dhcp.cox.net","threadId":"553","inReplyTo":"Pine.LNX.4.44.0505091549210.2136-100000@bellevue.puremagic.com","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-09T23:34:52Z","receivedAt":"2005-05-09T23:34:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'd rather see it done against the tip of the Linus tree.\n\n"},{"id":"2927","messageId":"Pine.LNX.4.21.0505091913250.30848-100000@iabervon.org","threadId":"553","inReplyTo":"7ihdhc5le2.fsf@lanthane.pps.jussieu.fr","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-05-10T00:07:36Z","receivedAt":"2005-05-10T00:07:36Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 9 May 2005, Juliusz Chroboczek wrote:\n\n> Hi,\n> \n> Here are a few notes about Git that should probably be taken into\n> account by people working on Git itself or on Git wrappers. The notes\n> apply to Linus' Git-0.6, which is the code I'm using in Darcs-git;\n> some of them might no longer be applicable to Darcs.\n> \n> \n> 1. Darcs-git uses the fact that Git updates are atomic when reading\n> from a Git repository. Darcs-git almost writes to Git repositories\n> atomically, with one exception: it performs a non-atomic\n> read/update/write cycle on .git/HEAD.\n> \n> For that reason, I'm taking a high-level lock on .git repositories\n> whenever I write them. The lockfile is ``.git/lock''. I haven't\n> thought about whether Darcs can be easily coerced into accessing Git\n> repos atomically; have people writing Git wrappers found the need for\n> a global lock?\n\nI think most things are using the O_CREAT | O_EXCL write to a file and\nthen rename or link/unlink to the desired location. I have some code to do\nthis with refs/*/* as well, and I think people have generally settled on\nsymlinking HEAD to something in refs/heads/. So it shouldn't be necessary\nto lock the whole repository, unless you're doing some operation like\nswapping two heads.\n\n> 2. The files git.h and git.c in Darcs-git are a simple ``libgit'' that\n> contains just enough functionality for Darcs-git; they use the\n> functionality of sha1_file.c and read_cache.c from Git-0.6.\n> \n> I've found a few problems with the interfaces in these files:\n> \n> - the global variables sha1_file_directory, active_cache, active_nr\n> and active_alloc are not marked ``extern'' in cache.h. This breaks\n> linkers that don't grok common symbols, such as the one in GHCi\n> (silly GHCi).\n\nShould be trivial to fix.\n\n> - the function write_sha1_file takes the metadata and the data in a\n> contiguous buffer, which is a problem when the data has been\n> allocated by a higher layer. I'm currently working around the\n> problem by memcpy-ing everything into a temp buffer, but that's\n> obviously not a good thing. I don't care whether write_sha1_file\n> is changed to use a writev-like interface, or to take the metadata\n> explicitly (as in char *type, unsigned long length).\n\nI've got some patches to make new functions of the write_sha1_file sort\neasier to write cleanly (for making git-*-pull clean); it wouldn't be too\nhard to have an open/write/close set.\n\n> - there is no (usable) function to write a tree; there's the code in\n> write_tree.c, but it's not generally useful. See the function\n> ``git_write_tree_done'' in git.c for the type of interface I'm\n> thinking of.\n\nI'm working on making this cleaner. Are you wanting to write a tree from\nsomething other than a cache?\n\nI can post my patches, but Linus is on vacation, so they couldn't go into\nthe mainline until Friday or so anyway.\n\n\t-Daniel\n*This .sig left intentionally blank*\n\n"},{"id":"2973","messageId":"Pine.LNX.4.44.0505100546010.2136-100000@bellevue.puremagic.com","threadId":"553","inReplyTo":"Pine.LNX.4.44.0505091549210.2136-100000@bellevue.puremagic.com","subject":"Re: Darcs-git: a few notes for Git hackers","fromName":"Brad Roberts","fromEmail":"braddr@puremagic.com","sentAt":"2005-05-10T12:55:54Z","receivedAt":"2005-05-10T12:55:54Z","isPatch":false,"sender":{"key":"braddr@puremagic.com","avatar":null},"body":"On Mon, 9 May 2005, Brad Roberts wrote:\n\n> Date: Mon, 9 May 2005 15:50:33 -0700 (PDT)\n> From: Brad Roberts \n> To: Petr Baudis \n> Cc: Juliusz Chroboczek ,\n> Git Mailing List , darcs-devel@abridgegame.org\n> Subject: Re: Darcs-git: a few notes for Git hackers\n>\n> > > - there's no way to have multiple simultaneous caches, short of\n> > > hacking at the values of Git's global variables by hand.\n> >\n> > See the Brad Robert's patches of Apr 21. I've decided not to apply them\n> > since it appears a lot has changed since then and it would be some pain;\n> > but they may be a worthy starting point for a more up-to-date patch.\n> >\n> > --\n> > \t\t\t\tPetr \"Pasky\" Baudis\n>\n> Since there's interest, I'll pull tip of your tree and re-do them. I\n> haven't bothered todate since no one seemed interested. Do you want them\n> piece meal like I did last time or just one big diff?\n>\n> Later,\n> Brad\n\nI wasn't able to finish redoing these against linus tip, but I got most of\nit done (patches 1-14 of the original 19):\n\n http://gameboy2.puremagic.com:8090/\n rsync://gameboy2.puremagic.com/git/\n\nThe second, third, and forth to last changes need a careful review,\nthey're direct applications of the original patches which were lightly\ntested during the first round and nothing other than compile tested in\nthis round.\n\nI suspect the remaining parts of the original patch series will go in\nfairly smoothly. If no one gets to them before tonight I'll finish\nit up after work.\n\nLater,\nBrad\n\n\nThe commit comments:\n\nSigned-off-by: Brad Roberts \n\n!-------------------------------------------------------------flip-\n\n- remove the no-longer-true comment about the cache being in native byte order\n- move the cache_header struct into read-cache.c since it's in internal detail\n of the cache, not a publicly accessed element\n\n!-------------------------------------------------------------flip-\n\nDrop the active_cache and active_nr parameters to write_cache\n\n!-------------------------------------------------------------flip-\n\n- Introduce set_cache_entry(ce, pos)\n- Migrate update-cache.c, the only place that does a active_cache[pos] = ce to use it\n- Migrate all the same style code in read-cache.c to use it also, except for\n read_cache itself which is setting up the initial active_cache entries\n\nTODO: rewrite the code that deal with pointers into the active_cache array such as\nread-tree.c's merging code.\n\n!-------------------------------------------------------------flip-\n\nIntroduce get_cache_entry(int pos) and use it for all trivial calls like:\n ce = active_cache[pos]\n\nTODO: rework the non-trivial active_cache manipulations\n\n!-------------------------------------------------------------flip-\n\nremove active_cache_changed from cache.h\n\n!-------------------------------------------------------------flip-\n\nIntroduce get_num_cache_entries() and migrate all the trivial callers to it\n\n!-------------------------------------------------------------flip-\n\nRemove active_alloc from cache.h\n\n!-------------------------------------------------------------flip-\n\nRestructure the diff algorythm to use indexes rather than pointer math.\nThe resulting code is probably a little less efficient but abstracts\nthe data structure.\n\n!-------------------------------------------------------------flip-\n\nRestructure the write tree algorythm to use indexes rather than moving\nthe base pointer and reducing the num entries and start using the\ncache abstraction apis.\n\n!-------------------------------------------------------------flip-\n\nMove from pointer math to indexes and use the abstractions\n\n!-------------------------------------------------------------flip-\n\n- convert the last caller from touching active_cache directly\n- drop active_cache and active_nr from cache.h\n\n!-------------------------------------------------------------flip-\n\n\n\n"}]}