{"thread":{"id":"26393","subject":"Q: does index-pack work in place on windows?","startedAt":"2011-02-02T20:21:05Z","lastAt":"2011-02-04T02:45:57Z","messageCount":7,"participants":["Junio C Hamano","Erik Faye-Lund","Nicolas Pitre"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"160292","messageId":"7v39o6xk5q.fsf@alter.siamese.dyndns.org","threadId":"26393","inReplyTo":null,"subject":"Q: does index-pack work in place on windows?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-02T20:21:05Z","receivedAt":"2011-02-02T20:21:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"... or any other (operating / file) system where you cannot unlink a file\nthat is open?\n\nWhen you run\n\n    $ git clone git://some.where/repo/sitory.git local\n    $ cd local\n    $ git index-pack .git/objects/pack/pack-*.pack\n\nthere is a call to write_idx_file() in builtin/index-pack.c, that feeds\nthe correct (and existing) name of the corresponding pack idx file.\n\nThe callee in pack-write.c, after sorting the list of objects contained in\nthe pack, does this:\n\n\tif (!index_name) {\n\t\tstatic char tmpfile[PATH_MAX];\n\t\tfd = odb_mkstemp(tmpfile, sizeof(tmpfile), \"pack/tmp_idx_XXXXXX\");\n\t\tindex_name = xstrdup(tmpfile);\n\t} else {\n\t\tunlink(index_name);\n\t\tfd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n\t}\n \nand then writes out the pack index to the file descriptor.  But index-pack\nuses the usual has_sha1_file() and read_sha1_file() interface to validate\nthe \"existing\" objects, and is likely to have mmapped the .idx file when\nit called use_pack_window().  Which makes me suspect that this unlink (or\nthe open that immediately follows) may fail on systems that do not allow\nunlink on inode that has still users.\n"},{"id":"160294","messageId":"AANLkTi=m+kERSH7TJFjuRaUfA7FR03zwNn2PKFxrqiOp@mail.gmail.com","threadId":"26393","inReplyTo":"7v39o6xk5q.fsf@alter.siamese.dyndns.org","subject":"Re: Q: does index-pack work in place on windows?","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-02-02T20:30:15Z","receivedAt":"2011-02-02T20:30:15Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Feb 2, 2011 at 9:21 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> ... or any other (operating / file) system where you cannot unlink a file\n> that is open?\n>\n> When you run\n>\n>    $ git clone git://some.where/repo/sitory.git local\n>    $ cd local\n>    $ git index-pack .git/objects/pack/pack-*.pack\n>\n> there is a call to write_idx_file() in builtin/index-pack.c, that feeds\n> the correct (and existing) name of the corresponding pack idx file.\n>\n> The callee in pack-write.c, after sorting the list of objects contained in\n> the pack, does this:\n>\n>        if (!index_name) {\n>                static char tmpfile[PATH_MAX];\n>                fd = odb_mkstemp(tmpfile, sizeof(tmpfile), \"pack/tmp_idx_XXXXXX\");\n>                index_name = xstrdup(tmpfile);\n>        } else {\n>                unlink(index_name);\n>                fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n>        }\n>\n> and then writes out the pack index to the file descriptor.  But index-pack\n> uses the usual has_sha1_file() and read_sha1_file() interface to validate\n> the \"existing\" objects, and is likely to have mmapped the .idx file when\n> it called use_pack_window().  Which makes me suspect that this unlink (or\n> the open that immediately follows) may fail on systems that do not allow\n> unlink on inode that has still users.\n\n...and you're right:\n\n$ git index-pack\n.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack\nUnlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'\nfailed. Should I try again? (y/n)? n\nfatal: unable to create\n'.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':\nFile exists\n"},{"id":"160312","messageId":"alpine.LFD.2.00.1102030026430.12104@xanadu.home","threadId":"26393","inReplyTo":"AANLkTi=m+kERSH7TJFjuRaUfA7FR03zwNn2PKFxrqiOp@mail.gmail.com","subject":"Re: Q: does index-pack work in place on windows?","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2011-02-03T05:32:02Z","receivedAt":"2011-02-03T05:32:02Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 2 Feb 2011, Erik Faye-Lund wrote:\n\n> On Wed, Feb 2, 2011 at 9:21 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > ... or any other (operating / file) system where you cannot unlink a file\n> > that is open?\n> >\n> > When you run\n> >\n> >    $ git clone git://some.where/repo/sitory.git local\n> >    $ cd local\n> >    $ git index-pack .git/objects/pack/pack-*.pack\n> >\n> > there is a call to write_idx_file() in builtin/index-pack.c, that feeds\n> > the correct (and existing) name of the corresponding pack idx file.\n> >\n> > The callee in pack-write.c, after sorting the list of objects contained in\n> > the pack, does this:\n> >\n> >        if (!index_name) {\n> >                static char tmpfile[PATH_MAX];\n> >                fd = odb_mkstemp(tmpfile, sizeof(tmpfile), \"pack/tmp_idx_XXXXXX\");\n> >                index_name = xstrdup(tmpfile);\n> >        } else {\n> >                unlink(index_name);\n> >                fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n> >        }\n> >\n> > and then writes out the pack index to the file descriptor.  But index-pack\n> > uses the usual has_sha1_file() and read_sha1_file() interface to validate\n> > the \"existing\" objects, and is likely to have mmapped the .idx file when\n> > it called use_pack_window().  Which makes me suspect that this unlink (or\n> > the open that immediately follows) may fail on systems that do not allow\n> > unlink on inode that has still users.\n> \n> ...and you're right:\n> \n> $ git index-pack\n> .git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack\n> Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'\n> failed. Should I try again? (y/n)? n\n> fatal: unable to create\n> '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':\n> File exists\n\nWhy would you do such thing in the first place?\n\nIf the pack exists along with its index on disk, there is no point \nrecreating it.  Maybe index-pack should just bail out early in that \ncase.\n\n\nNicolas\n"},{"id":"160318","messageId":"7vk4hhwpfb.fsf@alter.siamese.dyndns.org","threadId":"26393","inReplyTo":"alpine.LFD.2.00.1102030026430.12104@xanadu.home","subject":"Re: Q: does index-pack work in place on windows?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-03T07:24:56Z","receivedAt":"2011-02-03T07:24:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@fluxnic.net> writes:\n\n>> $ git index-pack\n>> .git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack\n>> Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'\n>> failed. Should I try again? (y/n)? n\n>> fatal: unable to create\n>> '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':\n>> File exists\n>\n> Why would you do such thing in the first place?\n>\n> If the pack exists along with its index on disk, there is no point \n> recreating it.  Maybe index-pack should just bail out early in that \n> case.\n\nI am trying to see if an index-pack with slight modification would be a\ngood replacement for verify-pack.\n"},{"id":"160370","messageId":"alpine.LFD.2.00.1102031958000.12104@xanadu.home","threadId":"26393","inReplyTo":"7vk4hhwpfb.fsf@alter.siamese.dyndns.org","subject":"Re: Q: does index-pack work in place on windows?","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2011-02-04T01:14:34Z","receivedAt":"2011-02-04T01:14:34Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 2 Feb 2011, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@fluxnic.net> writes:\n> \n> >> $ git index-pack\n> >> .git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack\n> >> Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'\n> >> failed. Should I try again? (y/n)? n\n> >> fatal: unable to create\n> >> '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':\n> >> File exists\n> >\n> > Why would you do such thing in the first place?\n> >\n> > If the pack exists along with its index on disk, there is no point \n> > recreating it.  Maybe index-pack should just bail out early in that \n> > case.\n> \n> I am trying to see if an index-pack with slight modification would be a\n> good replacement for verify-pack.\n\nOK, then you don't intend to reuse it as is.  If you really wanted to \nhave index-pack work in place then you would have to use \nfree_pack_by_name() somewhere before writing the new pack index out, but \nthat is still a dubious use case.\n\nindex-pack _could_ be a replacement for verify-pack.  It certainly can \nvalidate a pack since it is its purpose, possibly faster than \nverify-pack.  You'd still have to compare the existing pack index \nagainst the one index-pack creates without overwriting that original \nindex, taking into accound index version differences, etc.\n\nHowever index-pack won't tell you what is broken in the pack when \ncorruptions are to be found.  It will simply tell you at the very end \nthat the checksum hash doesn't match, or that some deltas were not \nresolved, that kind of thing, whereas verify-pack has the ability to \ntell you exactly what doesn't match with the info in the index or the \nlike as soon as it encounters a problem.\n\n\nNicolas\n"},{"id":"160372","messageId":"7v1v3ozhij.fsf@alter.siamese.dyndns.org","threadId":"26393","inReplyTo":"alpine.LFD.2.00.1102031958000.12104@xanadu.home","subject":"Re: Q: does index-pack work in place on windows?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-04T01:59:48Z","receivedAt":"2011-02-04T01:59:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@fluxnic.net> writes:\n\n> On Wed, 2 Feb 2011, Junio C Hamano wrote:\n>\n>> I am trying to see if an index-pack with slight modification would be a\n>> good replacement for verify-pack.\n> ...\n> index-pack _could_ be a replacement for verify-pack.  It certainly can \n> validate a pack since it is its purpose, possibly faster than \n> verify-pack.  You'd still have to compare the existing pack index \n> against the one index-pack creates without overwriting that original \n> index, taking into accound index version differences, etc.\n\nWe already know index-pack is a lot faster when you have a lot of deep\ndeltas, as it works from a base to its immediate delta children while\npinning that base, as opposed to verify-pack that verifies each and every\nobject in the pack in the index order, inflating and then applying\npotentially long delta chains repeatedly---the only thing that could be\nhelping it right now is the in-core delta base cache.\n\n> However index-pack won't tell you what is broken in the pack when \n> corruptions are to be found.\n\nYes, but at that point, you are pretty much lost anyway, as the only thing\nyou can do to salvage salvageable parts of the broken pack, if you still\ntrust its associated .idx file, is to walk the table of contents and ask\nfor each individual object; knowing where the .pack or .idx is broken with\nthe current verify-pack does not help you very much.\n"},{"id":"160374","messageId":"alpine.LFD.2.00.1102032137230.12104@xanadu.home","threadId":"26393","inReplyTo":"7v1v3ozhij.fsf@alter.siamese.dyndns.org","subject":"Re: Q: does index-pack work in place on windows?","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2011-02-04T02:45:57Z","receivedAt":"2011-02-04T02:45:57Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 3 Feb 2011, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@fluxnic.net> writes:\n> \n> > However index-pack won't tell you what is broken in the pack when \n> > corruptions are to be found.\n> \n> Yes, but at that point, you are pretty much lost anyway, as the only thing\n> you can do to salvage salvageable parts of the broken pack, if you still\n> trust its associated .idx file, is to walk the table of contents and ask\n> for each individual object; knowing where the .pack or .idx is broken with\n> the current verify-pack does not help you very much.\n\nWell... given that you do have to compare both the generated index and \nthe on-disk index at some point, it is then possible to identify \ncorrupted objects and their location.  And with pack index version 2 \nincluding a CRC for the packed data, you should be able to tell if the \ncorruption is actually in the pack or in the index (or both if the final \npack/index SHA1 sum doesn't match).\n\n\nNicolas\n"}]}