# Darcs-git: a few notes for Git hackers 12 messages from 2005-05-09 to 2005-05-10. Participants: Juliusz Chroboczek, Petr Baudis, H. Peter Anvin, Brad Roberts, Junio C Hamano, Daniel Barkalow. Thread: https://gitlist.dev/t/553 ## Juliusz Chroboczek, 2005-05-09 18:01 Subject: Darcs-git: a few notes for Git hackers Message-ID: <7ihdhc5le2.fsf@lanthane.pps.jussieu.fr> URL: https://gitlist.dev/e/7ihdhc5le2.fsf%40lanthane.pps.jussieu.fr ``` Hi, Here are a few notes about Git that should probably be taken into account by people working on Git itself or on Git wrappers. The notes apply to Linus' Git-0.6, which is the code I'm using in Darcs-git; some of them might no longer be applicable to Darcs. 1. Darcs-git uses the fact that Git updates are atomic when reading from a Git repository. Darcs-git almost writes to Git repositories atomically, with one exception: it performs a non-atomic read/update/write cycle on .git/HEAD. For that reason, I'm taking a high-level lock on .git repositories whenever I write them. The lockfile is ``.git/lock''. I haven't thought about whether Darcs can be easily coerced into accessing Git repos atomically; have people writing Git wrappers found the need for a global lock? 2. The files git.h and git.c in Darcs-git are a simple ``libgit'' that contains just enough functionality for Darcs-git; they use the functionality of sha1_file.c and read_cache.c from Git-0.6. I've found a few problems with the interfaces in these files: - the global variables sha1_file_directory, active_cache, active_nr and active_alloc are not marked ``extern'' in cache.h. This breaks linkers that don't grok common symbols, such as the one in GHCi (silly GHCi). - the function write_sha1_file takes the metadata and the data in a contiguous buffer, which is a problem when the data has been allocated by a higher layer. I'm currently working around the problem by memcpy-ing everything into a temp buffer, but that's obviously not a good thing. I don't care whether write_sha1_file is changed to use a writev-like interface, or to take the metadata explicitly (as in char *type, unsigned long length). - there is no (usable) function to write a tree; there's the code in write_tree.c, but it's not generally useful. See the function ``git_write_tree_done'' in git.c for the type of interface I'm thinking of. - there's no way to have multiple simultaneous caches, short of hacking at the values of Git's global variables by hand. As I'd rather not maintain my own version of Git, I'd be mighty grateful if some friendly Git hacker could fix the above. Juliusz ``` ## Petr Baudis, 2005-05-09 21:28 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <20050509212842.GC15712@pasky.ji.cz> URL: https://gitlist.dev/e/20050509212842.GC15712%40pasky.ji.cz In-Reply-To: <7ihdhc5le2.fsf@lanthane.pps.jussieu.fr> ``` Dear diary, on Mon, May 09, 2005 at 08:01:25PM CEST, I got a letter where Juliusz Chroboczek told me that... > 1. Darcs-git uses the fact that Git updates are atomic when reading > from a Git repository. Darcs-git almost writes to Git repositories > atomically, with one exception: it performs a non-atomic > read/update/write cycle on .git/HEAD. > > For that reason, I'm taking a high-level lock on .git repositories > whenever I write them. The lockfile is ``.git/lock''. I haven't > thought about whether Darcs can be easily coerced into accessing Git > repos atomically; have people writing Git wrappers found the need for > a global lock? FWIW, Cogito does not lock at all yet - this is one of the things which should be fixed soon. > - there's no way to have multiple simultaneous caches, short of > hacking at the values of Git's global variables by hand. See the Brad Robert's patches of Apr 21. I've decided not to apply them since it appears a lot has changed since then and it would be some pain; but they may be a worthy starting point for a more up-to-date patch. -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor ``` ## Juliusz Chroboczek, 2005-05-09 22:08 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <7iu0lc129m.fsf@lanthane.pps.jussieu.fr> URL: https://gitlist.dev/e/7iu0lc129m.fsf%40lanthane.pps.jussieu.fr In-Reply-To: <20050509212842.GC15712@pasky.ji.cz> ``` Ahoj, > FWIW, Cogito does not lock at all yet - this is one of the things which > should be fixed soon. I see. Let me know if you decide to use a different name for the lock file so I can switch to using the same one as yours. Julek ``` ## H. Peter Anvin, 2005-05-09 22:20 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <427FE248.7040403@zytor.com> URL: https://gitlist.dev/e/427FE248.7040403%40zytor.com In-Reply-To: <7iu0lc129m.fsf@lanthane.pps.jussieu.fr> ``` Juliusz Chroboczek wrote: > Ahoj, > > >>FWIW, Cogito does not lock at all yet - this is one of the things which >>should be fixed soon. > > > I see. Let me know if you decide to use a different name for the lock > file so I can switch to using the same one as yours. > Are you using flock(), or some other contraption that breaks if a process dies unexpectedly? -hpa ``` ## Juliusz Chroboczek, 2005-05-09 22:46 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <7ipsw010i5.fsf@lanthane.pps.jussieu.fr> URL: https://gitlist.dev/e/7ipsw010i5.fsf%40lanthane.pps.jussieu.fr In-Reply-To: <427FE248.7040403@zytor.com> ``` >> I see. Let me know if you decide to use a different name for the >> lock file so I can switch to using the same one as yours. > Are you using flock(), or some other contraption that breaks if a > process dies unexpectedly? No, I'm using a file that is created by the NFS-safe equivalent of open(O_CREAT | O_EXCL). This is what Darcs has been doing basically forever. Darcs usually doesn't die unexpectedly -- it's a Haskell program, so bugs usually manifest themselves with an exception being thrown allowing Darcs to clean-up after itself. The one exception is when Darcs gets killed by the OOM killer (which, as you doubtless know, doesn't give any advance warning to a process, thus making it impossible for a process to deal with it gracefully). In such cases, manual intervention is necessary anyway -- a file could have been written half-way. Juliusz ``` ## H. Peter Anvin, 2005-05-09 22:50 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <427FE938.7050904@zytor.com> URL: https://gitlist.dev/e/427FE938.7050904%40zytor.com In-Reply-To: <7ipsw010i5.fsf@lanthane.pps.jussieu.fr> ``` Juliusz Chroboczek wrote: >>>I see. Let me know if you decide to use a different name for the >>>lock file so I can switch to using the same one as yours. > > >>Are you using flock(), or some other contraption that breaks if a >>process dies unexpectedly? > > No, I'm using a file that is created by the NFS-safe equivalent of > open(O_CREAT | O_EXCL). This is what Darcs has been doing basically > forever. > > Darcs usually doesn't die unexpectedly -- it's a Haskell program, so > bugs usually manifest themselves with an exception being thrown > allowing Darcs to clean-up after itself. > > The one exception is when Darcs gets killed by the OOM killer (which, > as you doubtless know, doesn't give any advance warning to a process, > thus making it impossible for a process to deal with it gracefully). > In such cases, manual intervention is necessary anyway -- a file could > have been written half-way. > In the case of git, it should not be necessary even then; there might be a broken file in the repository but nothing would reference it so it shouldn't have any effect. Functionally speaking, operations on the git repository are in themselves atomic. -hpa ``` ## Brad Roberts, 2005-05-09 22:50 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.44.0505091549210.2136-100000%40bellevue.puremagic.com In-Reply-To: <20050509212842.GC15712@pasky.ji.cz> ``` > > - there's no way to have multiple simultaneous caches, short of > > hacking at the values of Git's global variables by hand. > > See the Brad Robert's patches of Apr 21. I've decided not to apply them > since it appears a lot has changed since then and it would be some pain; > but they may be a worthy starting point for a more up-to-date patch. > > -- > Petr "Pasky" Baudis Since there's interest, I'll pull tip of your tree and re-do them. I haven't bothered todate since no one seemed interested. Do you want them piece meal like I did last time or just one big diff? Later, Brad ``` ## Petr Baudis, 2005-05-09 23:02 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <20050509230211.GD15712@pasky.ji.cz> URL: https://gitlist.dev/e/20050509230211.GD15712%40pasky.ji.cz In-Reply-To: ``` Dear diary, on Tue, May 10, 2005 at 12:50:33AM CEST, I got a letter where Brad Roberts told me that... > > > - there's no way to have multiple simultaneous caches, short of > > > hacking at the values of Git's global variables by hand. > > > > See the Brad Robert's patches of Apr 21. I've decided not to apply them > > since it appears a lot has changed since then and it would be some pain; > > but they may be a worthy starting point for a more up-to-date patch. > > > > -- > > Petr "Pasky" Baudis > > Since there's interest, I'll pull tip of your tree and re-do them. I > haven't bothered todate since no one seemed interested. Do you want them > piece meal like I did last time or just one big diff? Piece meal would be excellent. -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor ``` ## Juliusz Chroboczek, 2005-05-09 23:08 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <7i3bsw0zhr.fsf@lanthane.pps.jussieu.fr> URL: https://gitlist.dev/e/7i3bsw0zhr.fsf%40lanthane.pps.jussieu.fr In-Reply-To: <427FE938.7050904@zytor.com> ``` >> In such cases, manual intervention is necessary anyway -- a file could >> have been written half-way. > In the case of git, it should not be necessary even then; there might > be a broken file in the repository but nothing would reference it so > it shouldn't have any effect. Functionally speaking, operations on > the git repository are in themselves atomic. Yes, you're right. I still prefer using a lockfile to flock -- NFS-safety is important for us. And experience with the Darcs user base (who are probably less Unix-savvy then the Git userbase) shows that they have no problem doing $ ps $ rm _darcs/lock $ darcs check when Darcs complains about a stray lockfile. Juliusz ``` ## Junio C Hamano, 2005-05-09 23:34 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: <7vwtq8vuqr.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwtq8vuqr.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` I'd rather see it done against the tip of the Linus tree. ``` ## Daniel Barkalow, 2005-05-10 00:07 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.21.0505091913250.30848-100000%40iabervon.org In-Reply-To: <7ihdhc5le2.fsf@lanthane.pps.jussieu.fr> ``` On Mon, 9 May 2005, Juliusz Chroboczek wrote: > Hi, > > Here are a few notes about Git that should probably be taken into > account by people working on Git itself or on Git wrappers. The notes > apply to Linus' Git-0.6, which is the code I'm using in Darcs-git; > some of them might no longer be applicable to Darcs. > > > 1. Darcs-git uses the fact that Git updates are atomic when reading > from a Git repository. Darcs-git almost writes to Git repositories > atomically, with one exception: it performs a non-atomic > read/update/write cycle on .git/HEAD. > > For that reason, I'm taking a high-level lock on .git repositories > whenever I write them. The lockfile is ``.git/lock''. I haven't > thought about whether Darcs can be easily coerced into accessing Git > repos atomically; have people writing Git wrappers found the need for > a global lock? I think most things are using the O_CREAT | O_EXCL write to a file and then rename or link/unlink to the desired location. I have some code to do this with refs/*/* as well, and I think people have generally settled on symlinking HEAD to something in refs/heads/. So it shouldn't be necessary to lock the whole repository, unless you're doing some operation like swapping two heads. > 2. The files git.h and git.c in Darcs-git are a simple ``libgit'' that > contains just enough functionality for Darcs-git; they use the > functionality of sha1_file.c and read_cache.c from Git-0.6. > > I've found a few problems with the interfaces in these files: > > - the global variables sha1_file_directory, active_cache, active_nr > and active_alloc are not marked ``extern'' in cache.h. This breaks > linkers that don't grok common symbols, such as the one in GHCi > (silly GHCi). Should be trivial to fix. > - the function write_sha1_file takes the metadata and the data in a > contiguous buffer, which is a problem when the data has been > allocated by a higher layer. I'm currently working around the > problem by memcpy-ing everything into a temp buffer, but that's > obviously not a good thing. I don't care whether write_sha1_file > is changed to use a writev-like interface, or to take the metadata > explicitly (as in char *type, unsigned long length). I've got some patches to make new functions of the write_sha1_file sort easier to write cleanly (for making git-*-pull clean); it wouldn't be too hard to have an open/write/close set. > - there is no (usable) function to write a tree; there's the code in > write_tree.c, but it's not generally useful. See the function > ``git_write_tree_done'' in git.c for the type of interface I'm > thinking of. I'm working on making this cleaner. Are you wanting to write a tree from something other than a cache? I can post my patches, but Linus is on vacation, so they couldn't go into the mainline until Friday or so anyway. -Daniel *This .sig left intentionally blank* ``` ## Brad Roberts, 2005-05-10 12:55 Subject: Re: Darcs-git: a few notes for Git hackers Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.44.0505100546010.2136-100000%40bellevue.puremagic.com In-Reply-To: ``` On Mon, 9 May 2005, Brad Roberts wrote: > Date: Mon, 9 May 2005 15:50:33 -0700 (PDT) > From: Brad Roberts > To: Petr Baudis > Cc: Juliusz Chroboczek , > Git Mailing List , darcs-devel@abridgegame.org > Subject: Re: Darcs-git: a few notes for Git hackers > > > > - there's no way to have multiple simultaneous caches, short of > > > hacking at the values of Git's global variables by hand. > > > > See the Brad Robert's patches of Apr 21. I've decided not to apply them > > since it appears a lot has changed since then and it would be some pain; > > but they may be a worthy starting point for a more up-to-date patch. > > > > -- > > Petr "Pasky" Baudis > > Since there's interest, I'll pull tip of your tree and re-do them. I > haven't bothered todate since no one seemed interested. Do you want them > piece meal like I did last time or just one big diff? > > Later, > Brad I wasn't able to finish redoing these against linus tip, but I got most of it done (patches 1-14 of the original 19): http://gameboy2.puremagic.com:8090/ rsync://gameboy2.puremagic.com/git/ The second, third, and forth to last changes need a careful review, they're direct applications of the original patches which were lightly tested during the first round and nothing other than compile tested in this round. I suspect the remaining parts of the original patch series will go in fairly smoothly. If no one gets to them before tonight I'll finish it up after work. Later, Brad The commit comments: Signed-off-by: Brad Roberts !-------------------------------------------------------------flip- - remove the no-longer-true comment about the cache being in native byte order - move the cache_header struct into read-cache.c since it's in internal detail of the cache, not a publicly accessed element !-------------------------------------------------------------flip- Drop the active_cache and active_nr parameters to write_cache !-------------------------------------------------------------flip- - Introduce set_cache_entry(ce, pos) - Migrate update-cache.c, the only place that does a active_cache[pos] = ce to use it - Migrate all the same style code in read-cache.c to use it also, except for read_cache itself which is setting up the initial active_cache entries TODO: rewrite the code that deal with pointers into the active_cache array such as read-tree.c's merging code. !-------------------------------------------------------------flip- Introduce get_cache_entry(int pos) and use it for all trivial calls like: ce = active_cache[pos] TODO: rework the non-trivial active_cache manipulations !-------------------------------------------------------------flip- remove active_cache_changed from cache.h !-------------------------------------------------------------flip- Introduce get_num_cache_entries() and migrate all the trivial callers to it !-------------------------------------------------------------flip- Remove active_alloc from cache.h !-------------------------------------------------------------flip- Restructure the diff algorythm to use indexes rather than pointer math. The resulting code is probably a little less efficient but abstracts the data structure. !-------------------------------------------------------------flip- Restructure the write tree algorythm to use indexes rather than moving the base pointer and reducing the num entries and start using the cache abstraction apis. !-------------------------------------------------------------flip- Move from pointer math to indexes and use the abstractions !-------------------------------------------------------------flip- - convert the last caller from touching active_cache directly - drop active_cache and active_nr from cache.h !-------------------------------------------------------------flip- ```