* Q: does index-pack work in place on windows?
@ 2011-02-02 20:21 Junio C Hamano
2011-02-02 20:30 ` Erik Faye-Lund
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2011-02-02 20:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Shawn O. Pearce
... or any other (operating / file) system where you cannot unlink a file
that is open?
When you run
$ git clone git://some.where/repo/sitory.git local
$ cd local
$ git index-pack .git/objects/pack/pack-*.pack
there is a call to write_idx_file() in builtin/index-pack.c, that feeds
the correct (and existing) name of the corresponding pack idx file.
The callee in pack-write.c, after sorting the list of objects contained in
the pack, does this:
if (!index_name) {
static char tmpfile[PATH_MAX];
fd = odb_mkstemp(tmpfile, sizeof(tmpfile), "pack/tmp_idx_XXXXXX");
index_name = xstrdup(tmpfile);
} else {
unlink(index_name);
fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
}
and then writes out the pack index to the file descriptor. But index-pack
uses the usual has_sha1_file() and read_sha1_file() interface to validate
the "existing" objects, and is likely to have mmapped the .idx file when
it called use_pack_window(). Which makes me suspect that this unlink (or
the open that immediately follows) may fail on systems that do not allow
unlink on inode that has still users.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Q: does index-pack work in place on windows?
2011-02-02 20:21 Q: does index-pack work in place on windows? Junio C Hamano
@ 2011-02-02 20:30 ` Erik Faye-Lund
2011-02-03 5:32 ` Nicolas Pitre
0 siblings, 1 reply; 7+ messages in thread
From: Erik Faye-Lund @ 2011-02-02 20:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Shawn O. Pearce
On Wed, Feb 2, 2011 at 9:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ... or any other (operating / file) system where you cannot unlink a file
> that is open?
>
> When you run
>
> $ git clone git://some.where/repo/sitory.git local
> $ cd local
> $ git index-pack .git/objects/pack/pack-*.pack
>
> there is a call to write_idx_file() in builtin/index-pack.c, that feeds
> the correct (and existing) name of the corresponding pack idx file.
>
> The callee in pack-write.c, after sorting the list of objects contained in
> the pack, does this:
>
> if (!index_name) {
> static char tmpfile[PATH_MAX];
> fd = odb_mkstemp(tmpfile, sizeof(tmpfile), "pack/tmp_idx_XXXXXX");
> index_name = xstrdup(tmpfile);
> } else {
> unlink(index_name);
> fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
> }
>
> and then writes out the pack index to the file descriptor. But index-pack
> uses the usual has_sha1_file() and read_sha1_file() interface to validate
> the "existing" objects, and is likely to have mmapped the .idx file when
> it called use_pack_window(). Which makes me suspect that this unlink (or
> the open that immediately follows) may fail on systems that do not allow
> unlink on inode that has still users.
...and you're right:
$ git index-pack
.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack
Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'
failed. Should I try again? (y/n)? n
fatal: unable to create
'.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':
File exists
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Q: does index-pack work in place on windows?
2011-02-02 20:30 ` Erik Faye-Lund
@ 2011-02-03 5:32 ` Nicolas Pitre
2011-02-03 7:24 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Nicolas Pitre @ 2011-02-03 5:32 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: Junio C Hamano, Johannes Sixt, git, Shawn O. Pearce
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1991 bytes --]
On Wed, 2 Feb 2011, Erik Faye-Lund wrote:
> On Wed, Feb 2, 2011 at 9:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > ... or any other (operating / file) system where you cannot unlink a file
> > that is open?
> >
> > When you run
> >
> > $ git clone git://some.where/repo/sitory.git local
> > $ cd local
> > $ git index-pack .git/objects/pack/pack-*.pack
> >
> > there is a call to write_idx_file() in builtin/index-pack.c, that feeds
> > the correct (and existing) name of the corresponding pack idx file.
> >
> > The callee in pack-write.c, after sorting the list of objects contained in
> > the pack, does this:
> >
> > if (!index_name) {
> > static char tmpfile[PATH_MAX];
> > fd = odb_mkstemp(tmpfile, sizeof(tmpfile), "pack/tmp_idx_XXXXXX");
> > index_name = xstrdup(tmpfile);
> > } else {
> > unlink(index_name);
> > fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
> > }
> >
> > and then writes out the pack index to the file descriptor. But index-pack
> > uses the usual has_sha1_file() and read_sha1_file() interface to validate
> > the "existing" objects, and is likely to have mmapped the .idx file when
> > it called use_pack_window(). Which makes me suspect that this unlink (or
> > the open that immediately follows) may fail on systems that do not allow
> > unlink on inode that has still users.
>
> ...and you're right:
>
> $ git index-pack
> .git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack
> Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'
> failed. Should I try again? (y/n)? n
> fatal: unable to create
> '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':
> File exists
Why would you do such thing in the first place?
If the pack exists along with its index on disk, there is no point
recreating it. Maybe index-pack should just bail out early in that
case.
Nicolas
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Q: does index-pack work in place on windows?
2011-02-03 5:32 ` Nicolas Pitre
@ 2011-02-03 7:24 ` Junio C Hamano
2011-02-04 1:14 ` Nicolas Pitre
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2011-02-03 7:24 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Erik Faye-Lund, Johannes Sixt, git, Shawn O. Pearce
Nicolas Pitre <nico@fluxnic.net> writes:
>> $ git index-pack
>> .git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack
>> Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'
>> failed. Should I try again? (y/n)? n
>> fatal: unable to create
>> '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':
>> File exists
>
> Why would you do such thing in the first place?
>
> If the pack exists along with its index on disk, there is no point
> recreating it. Maybe index-pack should just bail out early in that
> case.
I am trying to see if an index-pack with slight modification would be a
good replacement for verify-pack.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Q: does index-pack work in place on windows?
2011-02-03 7:24 ` Junio C Hamano
@ 2011-02-04 1:14 ` Nicolas Pitre
2011-02-04 1:59 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Nicolas Pitre @ 2011-02-04 1:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, Johannes Sixt, git, Shawn O. Pearce
On Wed, 2 Feb 2011, Junio C Hamano wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
> >> $ git index-pack
> >> .git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.pack
> >> Unlink of file '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx'
> >> failed. Should I try again? (y/n)? n
> >> fatal: unable to create
> >> '.git/objects/pack/pack-d634271f4d7ca70c00148e967a343c3c46cd7705.idx':
> >> File exists
> >
> > Why would you do such thing in the first place?
> >
> > If the pack exists along with its index on disk, there is no point
> > recreating it. Maybe index-pack should just bail out early in that
> > case.
>
> I am trying to see if an index-pack with slight modification would be a
> good replacement for verify-pack.
OK, then you don't intend to reuse it as is. If you really wanted to
have index-pack work in place then you would have to use
free_pack_by_name() somewhere before writing the new pack index out, but
that is still a dubious use case.
index-pack _could_ be a replacement for verify-pack. It certainly can
validate a pack since it is its purpose, possibly faster than
verify-pack. You'd still have to compare the existing pack index
against the one index-pack creates without overwriting that original
index, taking into accound index version differences, etc.
However index-pack won't tell you what is broken in the pack when
corruptions are to be found. It will simply tell you at the very end
that the checksum hash doesn't match, or that some deltas were not
resolved, that kind of thing, whereas verify-pack has the ability to
tell you exactly what doesn't match with the info in the index or the
like as soon as it encounters a problem.
Nicolas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Q: does index-pack work in place on windows?
2011-02-04 1:14 ` Nicolas Pitre
@ 2011-02-04 1:59 ` Junio C Hamano
2011-02-04 2:45 ` Nicolas Pitre
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2011-02-04 1:59 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Erik Faye-Lund, Johannes Sixt, git, Shawn O. Pearce
Nicolas Pitre <nico@fluxnic.net> writes:
> On Wed, 2 Feb 2011, Junio C Hamano wrote:
>
>> I am trying to see if an index-pack with slight modification would be a
>> good replacement for verify-pack.
> ...
> index-pack _could_ be a replacement for verify-pack. It certainly can
> validate a pack since it is its purpose, possibly faster than
> verify-pack. You'd still have to compare the existing pack index
> against the one index-pack creates without overwriting that original
> index, taking into accound index version differences, etc.
We already know index-pack is a lot faster when you have a lot of deep
deltas, as it works from a base to its immediate delta children while
pinning that base, as opposed to verify-pack that verifies each and every
object in the pack in the index order, inflating and then applying
potentially long delta chains repeatedly---the only thing that could be
helping it right now is the in-core delta base cache.
> However index-pack won't tell you what is broken in the pack when
> corruptions are to be found.
Yes, but at that point, you are pretty much lost anyway, as the only thing
you can do to salvage salvageable parts of the broken pack, if you still
trust its associated .idx file, is to walk the table of contents and ask
for each individual object; knowing where the .pack or .idx is broken with
the current verify-pack does not help you very much.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Q: does index-pack work in place on windows?
2011-02-04 1:59 ` Junio C Hamano
@ 2011-02-04 2:45 ` Nicolas Pitre
0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Pitre @ 2011-02-04 2:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, Johannes Sixt, git, Shawn O. Pearce
On Thu, 3 Feb 2011, Junio C Hamano wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
> > However index-pack won't tell you what is broken in the pack when
> > corruptions are to be found.
>
> Yes, but at that point, you are pretty much lost anyway, as the only thing
> you can do to salvage salvageable parts of the broken pack, if you still
> trust its associated .idx file, is to walk the table of contents and ask
> for each individual object; knowing where the .pack or .idx is broken with
> the current verify-pack does not help you very much.
Well... given that you do have to compare both the generated index and
the on-disk index at some point, it is then possible to identify
corrupted objects and their location. And with pack index version 2
including a CRC for the packed data, you should be able to tell if the
corruption is actually in the pack or in the index (or both if the final
pack/index SHA1 sum doesn't match).
Nicolas
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-04 2:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-02 20:21 Q: does index-pack work in place on windows? Junio C Hamano
2011-02-02 20:30 ` Erik Faye-Lund
2011-02-03 5:32 ` Nicolas Pitre
2011-02-03 7:24 ` Junio C Hamano
2011-02-04 1:14 ` Nicolas Pitre
2011-02-04 1:59 ` Junio C Hamano
2011-02-04 2:45 ` Nicolas Pitre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).