{"thread":{"id":"461","subject":"Careful object writing..","startedAt":"2005-05-03T19:15:08Z","lastAt":"2005-05-04T16:16:53Z","messageCount":19,"participants":["Linus Torvalds","Chris Wedgwood","Jan Harkes","Daniel Barkalow","H. Peter Anvin","Alex Riesen","Junio C Hamano","Morten Welinder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"2499","messageId":"Pine.LNX.4.58.0505031204030.26698@ppc970.osdl.org","threadId":"461","inReplyTo":null,"subject":"Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T19:15:08Z","receivedAt":"2005-05-03T19:15:08Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nI just pushed out the commit that tries to finally actually write the sha1\nobjects the right way in a shared object directory environment.\n\nI used to be lazy, and just do \"O_CREAT | O_EXCL\" on the final name, but\nthat obviously is not very nice when it can result in other people seeing\nobjects that haven't been fully finalized yet.\n\nSo now I do it \"right\", and create a temporary file in the \"top\" object\ndirectory, and then when it's all done, I do a \"link()\" to the final place\nand unlink the original.\n\nI also change the permission to 0444 before it gets its final name.\n\nTwo notes:\n\n - because the objects all get created initially in .git/objects rather \n   than in the subdirectory they get moved to, you can't use symlinks \n   to other filesystems for the 256 object subdirectories. The object \n   directory has to be one filesystem (but it doesn't have to be the same \n   one as you actually keep your working ddirectories on, of course)\n\n - The upside of this is that filesystem block allocators should do the \n   right thing. Instead of spreading the objects out (because they are in \n   different directories), they should be created together.\n\nAnyway, somebody should double-check the thing. It _should_ now work\ncorrectly over NFS etc too, and everything should be nice and atomic (and\nwith any half-way decent filesystem, it also means that even if you have a\nsystem crash in the middle, you'll never see half-created objects).\n\nNOTE NOTE NOTE! I have _not_ updated all the helper stuff that also write \nobjects. So things like \"git-http-pull\" etc will still write objects \ndirectly into the object directory, and that can cause problems with \nshared usage. Same goes for \"write_sha1_from_fd()\" that rpull.c uses. I \nhope somebody will take a look at those issues..\n\nAnyway, at least the really core operations should now really be\n\"thread-safe\" in a shared object directory environment.\n\n\t\tLinus\n"},{"id":"2502","messageId":"20050503192753.GA6435@taniwha.stupidest.org","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031204030.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"Chris Wedgwood","fromEmail":"cw@f00f.org","sentAt":"2005-05-03T19:27:53Z","receivedAt":"2005-05-03T19:27:53Z","isPatch":false,"sender":{"key":"cw@f00f.org","avatar":null},"body":"On Tue, May 03, 2005 at 12:15:08PM -0700, Linus Torvalds wrote:\n\n> So now I do it \"right\", and create a temporary file in the \"top\"\n> object directory, and then when it's all done, I do a \"link()\" to\n> the final place and unlink the original.\n\nhow is this better than a single rename?  i take it there is something\nfundamental from clue.101 i slept though here?\n\nalso, if you are *really* paranoid you want to fsync *before* you do\nthe link/unklink or rename --- which is what MTAs do[1]\n\nhowever, that said it *kills* performance and if it's not critical\nit's really a terrible idea\n\nalso, shouldn't HEAD (and similar)[2] be updated with a temporary and\na rename too?\n\n> I also change the permission to 0444 before it gets its final name.\n\ncool\n\n> NOTE NOTE NOTE! I have _not_ updated all the helper stuff that also\n> write objects.\n\ni thought this was all common code?  if it's not maybe now is the time\nto change that?\n\n\n\n[1] yes, i know this depends on the fs used and various things and\n    ext3 should be fine, blah blah blah, but not everyone uses ext3\n    and quite probably not everyone will use git under Linux\n\n[2] i didn't check the code as i'm still using BK in places\n"},{"id":"2504","messageId":"Pine.LNX.4.58.0505031242330.26698@ppc970.osdl.org","threadId":"461","inReplyTo":"20050503192753.GA6435@taniwha.stupidest.org","subject":"Re: Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T19:47:36Z","receivedAt":"2005-05-03T19:47:36Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 3 May 2005, Chris Wedgwood wrote:\n> \n> how is this better than a single rename?  i take it there is something\n> fundamental from clue.101 i slept though here?\n\nA rename will overwrite any old object, which means that you cannot do any \ncollision checks. In contrast, a \"link()\" will return EEXIST if somebody \nelse raced with you and created a new object, and you can do collision \nchecks instead of overwriting another persons object.\n\n> also, if you are *really* paranoid you want to fsync *before* you do\n> the link/unklink or rename --- which is what MTAs do[1]\n\nMe, I refuse to slow down my habits for old filesystems. You can either \nfsck, or use a logging filesystem. \n\nI don't see anybody not using logging filesystems these days, so..\n\n> also, shouldn't HEAD (and similar)[2] be updated with a temporary and\n> a rename too?\n\nMaybe. Much less important, though.\n\n> > NOTE NOTE NOTE! I have _not_ updated all the helper stuff that also\n> > write objects.\n> \n> i thought this was all common code?  if it's not maybe now is the time\n> to change that?\n\nIt is all common code, except:\n - things like \"fetch from another host\" will use rsync/wget/xxx to \n   actually get the files. To those programs, we're not talking about git \n   objects, we're just talking \"regular files\"\n - rpull.c has a special different routine to write its objects. I don't \n   use it, so..\n\nAnyway, it should be reasonably easily fixable.\n\n\t\tLinus\n"},{"id":"2505","messageId":"20050503194739.GA7082@taniwha.stupidest.org","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031242330.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"Chris Wedgwood","fromEmail":"cw@f00f.org","sentAt":"2005-05-03T19:47:39Z","receivedAt":"2005-05-03T19:47:39Z","isPatch":false,"sender":{"key":"cw@f00f.org","avatar":null},"body":"On Tue, May 03, 2005 at 12:47:36PM -0700, Linus Torvalds wrote:\n\n> Me, I refuse to slow down my habits for old filesystems. You can\n> either fsck, or use a logging filesystem.\n\nok, so you're saying everyone use linux ext3 or similar more or\nless...\n\nhow about we drop all the objects in one directory then?\n"},{"id":"2508","messageId":"Pine.LNX.4.58.0505031254550.26698@ppc970.osdl.org","threadId":"461","inReplyTo":"20050503194739.GA7082@taniwha.stupidest.org","subject":"Re: Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T19:56:14Z","receivedAt":"2005-05-03T19:56:14Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 3 May 2005, Chris Wedgwood wrote:\n>\n> On Tue, May 03, 2005 at 12:47:36PM -0700, Linus Torvalds wrote:\n> \n> > Me, I refuse to slow down my habits for old filesystems. You can\n> > either fsck, or use a logging filesystem.\n> \n> ok, so you're saying everyone use linux ext3 or similar more or\n> less...\n\nNo. I'm saying\n - you can use git-fsck-cache\n - or you can use a logging filesystem.\n\nI happen to use both.\n\n> how about we drop all the objects in one directory then?\n\nI don't even have directory hashing on, and as mentioned, the logging \nfilesystem is _not_ a requirement. It's just a reality for most of us.\n\n\t\tLinus\n"},{"id":"2512","messageId":"20050503200034.GA16104@delft.aura.cs.cmu.edu","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031204030.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"Jan Harkes","fromEmail":"jaharkes@cs.cmu.edu","sentAt":"2005-05-03T20:00:34Z","receivedAt":"2005-05-03T20:00:34Z","isPatch":false,"sender":{"key":"jaharkes@cs.cmu.edu","avatar":"https://gravatar.com/avatar/cf95aecd150ca8ef33d6edc337ac4bb9e13aa4246fc3679257d578c7fddc1633?d=mp&s=160"},"body":"On Tue, May 03, 2005 at 12:15:08PM -0700, Linus Torvalds wrote:\n> I just pushed out the commit that tries to finally actually write the sha1\n> objects the right way in a shared object directory environment.\n> \n> I used to be lazy, and just do \"O_CREAT | O_EXCL\" on the final name, but\n> that obviously is not very nice when it can result in other people seeing\n> objects that haven't been fully finalized yet.\n> \n> So now I do it \"right\", and create a temporary file in the \"top\" object\n> directory, and then when it's all done, I do a \"link()\" to the final place\n> and unlink the original.\n\nAnnoyingly until this commit, git has been just about the perfect SCM\nsystem to run on top of Coda. Almost every other SCM can and will get\nconflicts on it's repository files, which are pretty much impossible to\nresolve (just try merging two diverging copies of an RCS archive..)\n\nBut the only conflicts we ever see with git are when two people create\nthe same SHA1 object. And if the contents are in fact identical this\nconflict will be trivially resolved.\n\nI tried to pull in the latest version of your tree, but it doesn't look\nlike this commit has propagated to rsync.kernel.org yet. Hopefully you\nwill accept a small patch (should be < 5 lines) that makes git work\nnicely when Coda complains about the cross-directory hardlink without\naffecting the reliability of using link/unlink on normal filesystems.\n\nJan\n\n> \n> I also change the permission to 0444 before it gets its final name.\n> \n> Two notes:\n> \n>  - because the objects all get created initially in .git/objects rather \n>    than in the subdirectory they get moved to, you can't use symlinks \n>    to other filesystems for the 256 object subdirectories. The object \n>    directory has to be one filesystem (but it doesn't have to be the same \n>    one as you actually keep your working ddirectories on, of course)\n> \n>  - The upside of this is that filesystem block allocators should do the \n>    right thing. Instead of spreading the objects out (because they are in \n>    different directories), they should be created together.\n> \n> Anyway, somebody should double-check the thing. It _should_ now work\n> correctly over NFS etc too, and everything should be nice and atomic (and\n> with any half-way decent filesystem, it also means that even if you have a\n> system crash in the middle, you'll never see half-created objects).\n> \n> NOTE NOTE NOTE! I have _not_ updated all the helper stuff that also write \n> objects. So things like \"git-http-pull\" etc will still write objects \n> directly into the object directory, and that can cause problems with \n> shared usage. Same goes for \"write_sha1_from_fd()\" that rpull.c uses. I \n> hope somebody will take a look at those issues..\n> \n> Anyway, at least the really core operations should now really be\n> \"thread-safe\" in a shared object directory environment.\n> \n> \t\tLinus\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"2514","messageId":"Pine.LNX.4.21.0505031539480.30848-100000@iabervon.org","threadId":"461","inReplyTo":"20050503192753.GA6435@taniwha.stupidest.org","subject":"Re: Careful object writing..","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-05-03T20:02:05Z","receivedAt":"2005-05-03T20:02:05Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 3 May 2005, Chris Wedgwood wrote:\n\n> i thought this was all common code?  if it's not maybe now is the time\n> to change that?\n\nThe other versions are getting the tagged compressed object along with the\nintended hash from an external source. They both verify the result as they\ngo (as opposed to the normal case which takes the uncompressed data\nand finds the hash, and therefore has to be right). The common part is\nreally \"open the file\" and \"close (and place) the file\", which weren't\npreviously sufficiently complex to justify sharing the code.\n\n\t-Daniel\n*This .sig left intentionally blank*\n\n"},{"id":"2518","messageId":"Pine.LNX.4.58.0505031306310.26698@ppc970.osdl.org","threadId":"461","inReplyTo":"20050503200034.GA16104@delft.aura.cs.cmu.edu","subject":"Re: Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T20:11:47Z","receivedAt":"2005-05-03T20:11:47Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 3 May 2005, Jan Harkes wrote:\n> \n> I tried to pull in the latest version of your tree, but it doesn't look\n> like this commit has propagated to rsync.kernel.org yet. Hopefully you\n> will accept a small patch (should be < 5 lines) that makes git work\n> nicely when Coda complains about the cross-directory hardlink without\n> affecting the reliability of using link/unlink on normal filesystems.\n\nWhat is it that coda wants to do, and is there some portable way to get \nthere? \n\nIs it just that you want to stay within the directory? Or is it any link \naction that is nasty?\n\nWhat makes resolving renames hard when the file contents are the same? \nMaybe Coda could just do a trivial resolve of that \"conflict\" too?\n\n\t\t\tLinus\n"},{"id":"2521","messageId":"20050503205957.GA25253@delft.aura.cs.cmu.edu","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031306310.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"Jan Harkes","fromEmail":"jaharkes@cs.cmu.edu","sentAt":"2005-05-03T20:59:57Z","receivedAt":"2005-05-03T20:59:57Z","isPatch":false,"sender":{"key":"jaharkes@cs.cmu.edu","avatar":"https://gravatar.com/avatar/cf95aecd150ca8ef33d6edc337ac4bb9e13aa4246fc3679257d578c7fddc1633?d=mp&s=160"},"body":"On Tue, May 03, 2005 at 01:11:47PM -0700, Linus Torvalds wrote:\n> On Tue, 3 May 2005, Jan Harkes wrote:\n> > I tried to pull in the latest version of your tree, but it doesn't look\n> > like this commit has propagated to rsync.kernel.org yet. Hopefully you\n> > will accept a small patch (should be < 5 lines) that makes git work\n> > nicely when Coda complains about the cross-directory hardlink without\n> > affecting the reliability of using link/unlink on normal filesystems.\n> \n> What is it that coda wants to do, and is there some portable way to get \n> there? \n\nShort summary:\n\n    rc = link(old, new);\n    if (rc == -1 && errno == EXDEV)\n\trc = rename(old, new);\n\nOn Coda, the cross-directory link fails, the following cross-directory\nrename will work fine.  On a normal filesystem, if the link fails with\nEXDEV, the rename will fail with the same.\n\nBecause our cache consistency model is fairly optimistic, we already\nhave to deal with potential problems with a rename removing an unwanted\ntarget. So if we are logging write operations, and the link operation\ndid not return EEXISTS, then the rename will be marked as not having\nremoved any target file. If the target did happen to exist on the server\nby the time we reintegrate the operations we end up with a reintegration\nconflict.\n\n\nLonger version:\n\nWhen a server performs conflict resolution it happens on a per-directory\nbasis. So any cross-directory operation already a special case.\n\nWe cannot guarantee which directory will be resolved first, so if it is\nthe destination of a link or rename the object itself might not exist\nyet. The advantage of a rename operation is that it contains a reference\nto both the source and the destination directories. If we don't yet know\nthe renamed object we resolve the source first. That creates the object\nand allows us to complete the rename operation.\n\nHowever with a link we only have a reference to an object and the\ndirectory where the link should be added. But again, the object might\nnot yet exist on all servers. At this point things get a bit more\ncomplicated because we don't enforce access based on per object UNIX\nmode bits, but rely on directory ACLs. So we can't just add a reference\nto an unknown object in the destination directory because until we know\nwhere else this object is located, we can't tell if the user actually is\nallowed to access the object.\n\n> Is it just that you want to stay within the directory? Or is it any link \n> action that is nasty?\n\nWe do allow links within the same directory, mostly because that often\nhappens in places like /usr/bin and we know that whenever we encounter\nthe link operation in the resolution log, that the object creation has\nalready been processed. We also know that the new link can't give a user\nany rights he didn't already have.\n\n> What makes resolving renames hard when the file contents are the same? \n\nRenames mostly work, there are only a few corner cases left. One is\nwhere something is moved up in the directory tree and the source\ndirectory is then removed. We end up screwing ourselves because we\nappend the childs logs on removal and the create operation ends up\nbehind the rename operation. But that is a dumb implementation problem.\nAnother issue is of course when someone (validly) hardlinks a file in\nthe same directory and then moves one of the links to another directory.\n\nWe definitely are not a typical filesystem with UNIX semantics, which is\nwhy it is unusual to find an application that seems so well suited for\ndisconnected and weakly connected operation.\n\nJan\n\n"},{"id":"2534","messageId":"Pine.LNX.4.58.0505031507460.26698@ppc970.osdl.org","threadId":"461","inReplyTo":"20050503205957.GA25253@delft.aura.cs.cmu.edu","subject":"Re: Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T22:13:11Z","receivedAt":"2005-05-03T22:13:11Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 3 May 2005, Jan Harkes wrote:\n> \n> Short summary:\n> \n>     rc = link(old, new);\n>     if (rc == -1 && errno == EXDEV)\n> \trc = rename(old, new);\n\nOk, that is safe enough. Will do.\n\n> On Coda, the cross-directory link fails, the following cross-directory\n> rename will work fine.  On a normal filesystem, if the link fails with\n> EXDEV, the rename will fail with the same.\n\nYup. I do suspect that since you handle the rename anyway, you probably \ncould handle the git link/unlink patterns too, but it's easy enough to \njust do the rename fallback in git itself.\n\nThe only reason not to use rename in the first place is literally just to \nbe able to check for collisions. Which we don't actually _do_ right now, \nbut I like to be able to do so in theory.\n\n\t\tLinus\n"},{"id":"2537","messageId":"Pine.LNX.4.58.0505031531270.26698@ppc970.osdl.org","threadId":"461","inReplyTo":"20050503200034.GA16104@delft.aura.cs.cmu.edu","subject":"Re: Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T22:37:36Z","receivedAt":"2005-05-03T22:37:36Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 3 May 2005, Jan Harkes wrote:\n> \n> I tried to pull in the latest version of your tree, but it doesn't look\n> like this commit has propagated to rsync.kernel.org yet.\n\nHmm.. It's still not there a few hours later. I wonder what the mirroring\nrules are. Or maybe mirroring is just broken right now. Peter?\n\nOne change introduced by me is that the new objects changed from 0664\n(-rw-rw-r--) to (0444) -r--r--r-- due to the object writing rules. Maybe\nthe mirroring decides that such objects shouldn't be mirrored, since they\nare \"private\"?\n\nOr maybe it's just that Peter has shut down mirroring in preparation for \nthe imminent memory upgrade on master.kernel.org. \n\n\t\t\tLinus\n"},{"id":"2538","messageId":"4277FDD9.6030502@zytor.com","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031531270.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-05-03T22:40:25Z","receivedAt":"2005-05-03T22:40:25Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> On Tue, 3 May 2005, Jan Harkes wrote:\n> \n>>I tried to pull in the latest version of your tree, but it doesn't look\n>>like this commit has propagated to rsync.kernel.org yet.\n> \n> \n> Hmm.. It's still not there a few hours later. I wonder what the mirroring\n> rules are. Or maybe mirroring is just broken right now. Peter?\n> \n> One change introduced by me is that the new objects changed from 0664\n> (-rw-rw-r--) to (0444) -r--r--r-- due to the object writing rules. Maybe\n> the mirroring decides that such objects shouldn't be mirrored, since they\n> are \"private\"?\n> \n> Or maybe it's just that Peter has shut down mirroring in preparation for \n> the imminent memory upgrade on master.kernel.org. \n> \n\nNo, I had stopped the cron job to fix a script bug and forgot to turn it \nback on.  It's pushing now.\n\n\t-hpa\n\n"},{"id":"2543","messageId":"81b0412b05050316045fa31c2a@mail.gmail.com","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031204030.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-05-03T23:04:23Z","receivedAt":"2005-05-03T23:04:23Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 5/3/05, Linus Torvalds <torvalds@osdl.org> wrote:\n> I also change the permission to 0444 before it gets its final name.\n\nMaybe umask it first? Just in case.\n"},{"id":"2546","messageId":"Pine.LNX.4.58.0505031618360.26698@ppc970.osdl.org","threadId":"461","inReplyTo":"81b0412b05050316045fa31c2a@mail.gmail.com","subject":"Re: Careful object writing..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-03T23:22:46Z","receivedAt":"2005-05-03T23:22:46Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 4 May 2005, Alex Riesen wrote:\n>\n> On 5/3/05, Linus Torvalds <torvalds@osdl.org> wrote:\n> > I also change the permission to 0444 before it gets its final name.\n> \n> Maybe umask it first? Just in case.\n\nI considered it, but it's not worth it.\n\nIf you don't want somebody else to see your objects, you should just \ndisable execute permission on your .git directory. \"umask\" is actually a \nfairly nasty interface, since it takes effect on create(), and we do want \nto fchmod _after_ the create (some filesystems don't like it when you \ncreate a non-writable object and then write to it, but more importantly, \nsince we use \"mkstemp()\" for the temp-file handling, we don't even have \ncontrol of the initial umask.\n\nSo to make it (0444 & umask) git would actually have to jump through silly\nhoops. Without actually buying you anything new, since the access\npermissions for git objects really are about the _directory_ anyway.\n\n\t\tLinus\n"},{"id":"2547","messageId":"7vr7gnsxma.fsf@assigned-by-dhcp.cox.net","threadId":"461","inReplyTo":"81b0412b05050316045fa31c2a@mail.gmail.com","subject":"Re: Careful object writing..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-03T23:22:53Z","receivedAt":"2005-05-03T23:22:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"AR\" == Alex Riesen <raa.lkml@gmail.com> writes:\n\nAR> On 5/3/05, Linus Torvalds <torvalds@osdl.org> wrote:\n>> I also change the permission to 0444 before it gets its final name.\n\nAR> Maybe umask it first? Just in case.\n\nIn general worrying about umask when you see chmod is a good\npractice, but it probably is not applicable to this particular\ncase.  A person with umask 077 should still get 0444 if the\nSHA1_FILE_DIRECTORY is shared with other people, and if it is\nnot shared, his initial git-init-db would have made it 0700, so\nit does not matter it the files files underneath it have 0444.\n\n"},{"id":"2548","messageId":"81b0412b0505031625478de1a5@mail.gmail.com","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031618360.26698@ppc970.osdl.org","subject":"Re: Careful object writing..","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-05-03T23:25:05Z","receivedAt":"2005-05-03T23:25:05Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 5/4/05, Linus Torvalds <torvalds@osdl.org> wrote:\n> > > I also change the permission to 0444 before it gets its final name.\n> > Maybe umask it first? Just in case.\n> \n> I considered it, but it's not worth it.\n> \n> If you don't want somebody else to see your objects, you should just\n> disable execute permission on your .git directory. ...\n\nOf course. It's leaves even more control to the user. Forget I asked :)\n"},{"id":"2559","messageId":"Pine.LNX.4.21.0505040004290.30848-100000@iabervon.org","threadId":"461","inReplyTo":"Pine.LNX.4.58.0505031204030.26698@ppc970.osdl.org","subject":"[PATCH] Careful object pulling","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-05-04T04:07:06Z","receivedAt":"2005-05-04T04:07:06Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This splits out the careful methods for writing a file and placing it in\nthe correct location from the function to write a buffer to the file and\nmakes the various functions used by git-*-pull programs use those\nfunctions to write their files.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\nIndex: cache.h\n===================================================================\n--- 51a882a2dc62e0d3cdc79e0badc61559fb723481/cache.h  (mode:100644 sha1:8dd812827604d510038f5d93e3718c43f9d12c30)\n+++ 4e31436bacfff09ce673665a1061b41e37ffd661/cache.h  (mode:100644 sha1:0d7411c3b86a899cee45627997f4bb7ba0df2ea7)\n@@ -134,6 +134,12 @@\n extern void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size);\n extern int write_sha1_file(char *buf, unsigned long len, const char *type, unsigned char *return_sha1);\n \n+/* Open a file for writing a sha1 file. */\n+extern int open_sha1_tmpfile(char tmpfile[PATH_MAX]);\n+\n+/* Put a sha1 file in the correct place. */\n+extern int place_sha1_file(char tmpfile[PATH_MAX], const unsigned char *sha1);\n+\n extern int check_sha1_signature(unsigned char *sha1, void *buf, unsigned long size, const char *type);\n \n /* Read a tree into the cache */\nIndex: http-pull.c\n===================================================================\n--- 51a882a2dc62e0d3cdc79e0badc61559fb723481/http-pull.c  (mode:100644 sha1:f693aba61b4dcb4b738faf1335ec74aa1545e45d)\n+++ 4e31436bacfff09ce673665a1061b41e37ffd661/http-pull.c  (mode:100644 sha1:793d8b9e125c1f164f774e2888d26beaa75e1df0)\n@@ -52,8 +52,9 @@\n \tchar real_sha1[20];\n \tchar *url;\n \tchar *posn;\n+\tchar tmpname[PATH_MAX];\n \n-\tlocal = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0666);\n+\tlocal = open_sha1_tmpfile(tmpname);\n \n \tif (local < 0)\n \t\treturn error(\"Couldn't open %s\\n\", filename);\n@@ -88,15 +89,14 @@\n \tinflateEnd(&stream);\n \tSHA1_Final(real_sha1, &c);\n \tif (zret != Z_STREAM_END) {\n-\t\tunlink(filename);\n+\t\tunlink(tmpname);\n \t\treturn error(\"File %s (%s) corrupt\\n\", hex, url);\n \t}\n \tif (memcmp(sha1, real_sha1, 20)) {\n-\t\tunlink(filename);\n+\t\tunlink(tmpname);\n \t\treturn error(\"File %s has bad hash\\n\", hex);\n \t}\n-\t\n-\treturn 0;\n+\treturn place_sha1_file(tmpname, sha1);\n }\n \n int main(int argc, char **argv)\nIndex: sha1_file.c\n===================================================================\n--- 51a882a2dc62e0d3cdc79e0badc61559fb723481/sha1_file.c  (mode:100644 sha1:e6ce455ae90bd430f2128f454bdb6e0575412486)\n+++ 4e31436bacfff09ce673665a1061b41e37ffd661/sha1_file.c  (mode:100644 sha1:85daa0b0045c3f19e697d1a7aa8ab15ff54eab99)\n@@ -276,6 +276,55 @@\n \t}\n }\n \n+int open_sha1_tmpfile(char tmpfile[PATH_MAX])\n+{\n+\tint fd;\n+\n+\tsnprintf(tmpfile, sizeof(tmpfile), \"%s/obj_XXXXXX\", get_object_directory());\n+\tfd = mkstemp(tmpfile);\n+\tif (fd < 0) {\n+\t\tfprintf(stderr,\n+\t\t\t\"unable to create temporary sha1 filename %s: %s\",\n+\t\t\ttmpfile, strerror(errno));\n+\t\treturn -1;\n+\t}\n+\treturn fd;\n+}\n+\n+int place_sha1_file(char tmpfile[PATH_MAX], const unsigned char *sha1)\n+{\n+\tchar *filename = sha1_file_name(sha1);\n+\tint ret;\n+\n+\tret = link(tmpfile, filename);\n+\tif (ret < 0) {\n+\t\tret = errno;\n+\n+\t\t/*\n+\t\t * Coda hack - coda doesn't like cross-directory links,\n+\t\t * so we fall back to a rename, which will mean that it\n+\t\t * won't be able to check collisions, but that's not a\n+\t\t * big deal.\n+\t\t *\n+\t\t * When this succeeds, we just return 0. We have nothing\n+\t\t * left to unlink.\n+\t\t */\n+\t\tif (ret == EXDEV && !rename(tmpfile, filename))\n+\t\t\treturn 0;\n+\t}\n+\tunlink(tmpfile);\n+\tif (ret) {\n+\t\tif (ret != EEXIST) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"unable to write sha1 filename %s: %s\", \n+\t\t\t\tfilename, strerror(ret));\n+\t\t\treturn -1;\n+\t\t}\n+\t\t/* FIXME!!! Collision check here ? */\n+\t}\n+\treturn 0;\n+}\n+\n int write_sha1_file(char *buf, unsigned long len, const char *type, unsigned char *returnsha1)\n {\n \tint size;\n@@ -286,7 +335,7 @@\n \tchar *filename;\n \tstatic char tmpfile[PATH_MAX];\n \tchar hdr[50];\n-\tint fd, hdrlen, ret;\n+\tint fd, hdrlen;\n \n \t/* Generate the header */\n \thdrlen = sprintf(hdr, \"%s %lu\", type, len)+1;\n@@ -316,12 +365,9 @@\n \t\treturn -1;\n \t}\n \n-\tsnprintf(tmpfile, sizeof(tmpfile), \"%s/obj_XXXXXX\", get_object_directory());\n-\tfd = mkstemp(tmpfile);\n-\tif (fd < 0) {\n-\t\tfprintf(stderr, \"unable to create temporary sha1 filename %s: %s\", tmpfile, strerror(errno));\n-\t\treturn -1;\n-\t}\n+\tfd = open_sha1_tmpfile(tmpfile);\n+\tif (fd < 0)\n+\t\treturn fd;\n \n \t/* Set it up */\n \tmemset(&stream, 0, sizeof(stream));\n@@ -349,53 +395,28 @@\n \n \tif (write(fd, compressed, size) != size)\n \t\tdie(\"unable to write file\");\n+\n \tfchmod(fd, 0444);\n \tclose(fd);\n \n-\tret = link(tmpfile, filename);\n-\tif (ret < 0) {\n-\t\tret = errno;\n-\n-\t\t/*\n-\t\t * Coda hack - coda doesn't like cross-directory links,\n-\t\t * so we fall back to a rename, which will mean that it\n-\t\t * won't be able to check collisions, but that's not a\n-\t\t * big deal.\n-\t\t *\n-\t\t * When this succeeds, we just return 0. We have nothing\n-\t\t * left to unlink.\n-\t\t */\n-\t\tif (ret == EXDEV && !rename(tmpfile, filename))\n-\t\t\treturn 0;\n-\t}\n-\tunlink(tmpfile);\n-\tif (ret) {\n-\t\tif (ret != EEXIST) {\n-\t\t\tfprintf(stderr, \"unable to write sha1 filename %s: %s\", filename, strerror(ret));\n-\t\t\treturn -1;\n-\t\t}\n-\t\t/* FIXME!!! Collision check here ? */\n-\t}\n-\n-\treturn 0;\n+\treturn place_sha1_file(tmpfile, sha1);\n }\n \n int write_sha1_from_fd(const unsigned char *sha1, int fd)\n {\n-\tchar *filename = sha1_file_name(sha1);\n-\n \tint local;\n \tz_stream stream;\n \tunsigned char real_sha1[20];\n \tchar buf[4096];\n \tchar discard[4096];\n+\tchar tmpname[PATH_MAX];\n \tint ret;\n \tSHA_CTX c;\n \n-\tlocal = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0666);\n+\tlocal = open_sha1_tmpfile(tmpname);\n \n \tif (local < 0)\n-\t\treturn error(\"Couldn't open %s\\n\", filename);\n+\t\treturn -1;\n \n \tmemset(&stream, 0, sizeof(stream));\n \n@@ -408,7 +429,7 @@\n \t\tsize = read(fd, buf, 4096);\n \t\tif (size <= 0) {\n \t\t\tclose(local);\n-\t\t\tunlink(filename);\n+\t\t\tunlink(tmpname);\n \t\t\tif (!size)\n \t\t\t\treturn error(\"Connection closed?\");\n \t\t\tperror(\"Reading from connection\");\n@@ -431,15 +452,15 @@\n \tclose(local);\n \tSHA1_Final(real_sha1, &c);\n \tif (ret != Z_STREAM_END) {\n-\t\tunlink(filename);\n+\t\tunlink(tmpname);\n \t\treturn error(\"File %s corrupted\", sha1_to_hex(sha1));\n \t}\n \tif (memcmp(sha1, real_sha1, 20)) {\n-\t\tunlink(filename);\n+\t\tunlink(tmpname);\n \t\treturn error(\"File %s has bad hash\\n\", sha1_to_hex(sha1));\n \t}\n-\t\n-\treturn 0;\n+\n+\treturn place_sha1_file(tmpname, sha1);\n }\n \n int has_sha1_file(const unsigned char *sha1)\n\n"},{"id":"2566","messageId":"118833cc050504023569e00d38@mail.gmail.com","threadId":"461","inReplyTo":"Pine.LNX.4.21.0505040004290.30848-100000@iabervon.org","subject":"Re: [PATCH] Careful object pulling","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2005-05-04T09:35:10Z","receivedAt":"2005-05-04T09:35:10Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"Something's fishy there.  You are comparing the result from link with EEXIST.\n\nMorten\n"},{"id":"2580","messageId":"Pine.LNX.4.21.0505041207480.30848-100000@iabervon.org","threadId":"461","inReplyTo":"118833cc050504023569e00d38@mail.gmail.com","subject":"Re: [PATCH] Careful object pulling","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-05-04T16:16:53Z","receivedAt":"2005-05-04T16:16:53Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 4 May 2005, Morten Welinder wrote:\n\n> Something's fishy there.  You are comparing the result from link with EEXIST.\n\nNo, it just looks that way. If ret is negative, errno gets written to\nit. If it's zero, we don't do anything with it. It can't be positive. So,\nat the point where we test it, it must be the errno from link, which would\nbe EEXIST if the case we're worried about.\n\n\t-Daniel\n*This .sig left intentionally blank*\n\n"}]}