Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Johannes Sixt @ 2008-11-19 14:52 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Alex Riesen, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811190940480.27509@xanadu.home>

Nicolas Pitre schrieb:
> On Wed, 19 Nov 2008, Johannes Sixt wrote:
>> The unusual case is where you do this:
>>
>>  $ git rev-list -10 HEAD | git pack-objects foobar
>>
>> twice in a row: In this case the second invocation fails on Windows
>> because the destination pack file already exists *and* is open. But not
>> even git-repack does this even if it is called twice. OTOH, the test case
>> *does* exactly this.
> 
> OK.... Well, despite my earlier assertion, I think the above should be a 
> valid operation.
> 
> I'm looking at it now.  I'm therefore revoking my earlier ACK as well 
> (better keep that test case alive).

Hold on a moment: When I tested the above sequence, I was fooled by a flaw
in mingw_rename() (it doesn't replace read-only files). With that fixed,
it works as expected in repeated invocations (note that foobar is outside
the .git/objects/pack directory).

If I use .git/objects/pack/foobar instead, then I get the failures on
Windows, and I won't argue that this should be "fixed". ;)

-- Hannes

^ permalink raw reply

* Re: git and mtime
From: Arafangion @ 2008-11-19 12:37 UTC (permalink / raw)
  To: Roger Leigh; +Cc: git
In-Reply-To: <20081119113752.GA13611@ravenclaw.codelibre.net>

On Wed, 2008-11-19 at 11:37 +0000, Roger Leigh wrote:
<snip>
> different systems).  However, the fact that git isn't storing the
> mtime of the files confuses make, so it then tries to regenerate these
> (already up-to-date) files, and fails in the process since the tools
> aren't available.

Unless I'm mistaken, I was under the impression that the reason why git
doesn't, and shouldn't do this is _because_ it confuses make.

Suppose you've got two branches, and you check out the other branch,
resulting in changes in 3 files.  Should git go and modify the mtime for
every single file, and remove any file that isn't part of the repo (Such
as generated object files)?

If it modifies the dates on every file, but doesn't remove the generated
object files, how does make handle that, as it'll likely generate some
of the object files, but not all of them.

If it doesn't, but touches the files that changed, and the dates are now
older than the corresponding object files, make would fail to recompile
the project properly!

The only way this could work is if you never switch branches, which is
quite limiting for git, and never check out an older revision, which is
quite limiting for the RCS systems in general.

You should probably fix your build script, or add a hook script that
sets the dates on the files in question manually, but the former
solution would be much better.

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Nicolas Pitre @ 2008-11-19 14:42 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Alex Riesen, Junio C Hamano, Git Mailing List
In-Reply-To: <49241AEF.1080808@viscovery.net>

On Wed, 19 Nov 2008, Johannes Sixt wrote:

> Alex Riesen schrieb:
> > 2008/11/19 Nicolas Pitre <nico@cam.org>:
> >> On Wed, 19 Nov 2008, Johannes Sixt wrote:
> >>> The work-around is to write the repacked objects to a file of a different
> >>> name, and replace the original after git-pack-objects has terminated.
> >>>
> >>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> >> Acked-by: Nicolas Pitre <nico@cam.org>
> > 
> > Are you sure? Will it work in a real repository? Were noone does
> > rename the previous pack files into packtmp-something?
> 
> Oh, the patch only works around the failure in the test case. In a real
> repository there is usually no problem because the destination pack file
> does not exist.
> 
> The unusual case is where you do this:
> 
>  $ git rev-list -10 HEAD | git pack-objects foobar
> 
> twice in a row: In this case the second invocation fails on Windows
> because the destination pack file already exists *and* is open. But not
> even git-repack does this even if it is called twice. OTOH, the test case
> *does* exactly this.

OK.... Well, despite my earlier assertion, I think the above should be a 
valid operation.

I'm looking at it now.  I'm therefore revoking my earlier ACK as well 
(better keep that test case alive).


Nicolas

^ permalink raw reply

* Re: Git commit won't add an untracked file given on the command line
From: Mark Burton @ 2008-11-19 14:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vd4grsveo.fsf@gitster.siamese.dyndns.org>


Hi,

> > That's just impossible.  You cannot create a tree object, let alone a 
> > commit object, without touching the index (AKA staging area).
> 
> I do not think Mark really _means_ "not in the index".
> 
> The wish is more like "I want to let git know that I am interested in this
> path, but I'm not ready to say what exact content I want for that path in
> the next commit, not just yet".
> 
> I do not think that is an unreasonable wish.  On the other hand, it is
> unreasonable for anybody to insist that we satisfy the wish without
> touching the index.  The index is the most natural place to do that.
> 
> We have a half (probably a quarter) of what we need for that implemented
> already, by the way.

Sorry, poor choice of words on my part - you have to remember my
viewpoint is one of user more than developer.

My wish was really just based on the advertised behaviour that
specifying a file on the command line would commit the contents of that
file while leaving the index intact. Whether the index was temporarily
used/altered during the execution of the commit didn't cross my mind.

Hey, it's not a big deal and with the accepted patch to the
documentation it need not take any more of anyone's time.

Cheers,

Mark

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Nicolas Pitre @ 2008-11-19 14:31 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Alex Riesen, Junio C Hamano, Git Mailing List
In-Reply-To: <49240F6D.3030203@viscovery.net>

On Wed, 19 Nov 2008, Johannes Sixt wrote:

> Nicolas Pitre schrieb:
> > On Wed, 19 Nov 2008, Johannes Sixt wrote:
> > 
> >> Alex Riesen schrieb:
> >>> The opened packs seem to stay open forever.
> >> In my MinGW port I have the patch below that avoids that t5303 fails
> >> because of a pack file that remains open. (Open files cannot be replaced
> >> on Windows.) I had hoped that your patch would help, but it does not.
> >> Something else still keeps the pack file open. Can anything be done about
> >> that?
> >>
> >> -- Hannes
> >>
> >> From: Johannes Sixt <j6t@kdbg.org>
> >> Date: Mon, 17 Nov 2008 09:25:19 +0100
> >> Subject: [PATCH] t5303: Do not overwrite an existing pack
> >>
> ...
> > Acked-by: Nicolas Pitre <nico@cam.org>
> 
> Thanks, but I should have mentioned that at this time this patch was just
> meant for exposition, not inclusion.

Well, I'd include it right away since it is fundamentally the right 
thing to do.

> I'd prefer a solution to the problem that the pack file remains open. Do
> you have an idea where git-pack-objects keeps the pack file open, even
> with Alex's two patches applied?

That's not the issue.

The test is using pack-objects to overwrite a pack file, but to do so 
pack-objects has to open and read from the pack file about to be 
overwritten.  This is just wrong even if it happens to work by luck on 
Linux.

It is on purpose that the pack files are kept open.  This is to minimize 
the number of open() and mmap()/read() operations between successive 
object reads.  Those are not leaked file handles since they get closed 
when new packs are opened and the number of concurrent opened packs 
reaches a certain limit.


Nicolas

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Alex Riesen @ 2008-11-19 14:17 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Nicolas Pitre, Junio C Hamano, Git Mailing List
In-Reply-To: <49241AEF.1080808@viscovery.net>

2008/11/19 Johannes Sixt <j.sixt@viscovery.net>:
> Alex Riesen schrieb:
>> 2008/11/19 Nicolas Pitre <nico@cam.org>:
>>> On Wed, 19 Nov 2008, Johannes Sixt wrote:
>>>> The work-around is to write the repacked objects to a file of a different
>>>> name, and replace the original after git-pack-objects has terminated.
>>>>
>>>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>>> Acked-by: Nicolas Pitre <nico@cam.org>
>>
>> Are you sure? Will it work in a real repository? Were noone does
>> rename the previous pack files into packtmp-something?
>
> Oh, the patch only works around the failure in the test case. In a real
> repository there is usually no problem because the destination pack file
> does not exist.
>
> The unusual case is where you do this:
>
>  $ git rev-list -10 HEAD | git pack-objects foobar
>
> twice in a row: In this case the second invocation fails on Windows
> because the destination pack file already exists *and* is open. But not
> even git-repack does this even if it is called twice. OTOH, the test case
> *does* exactly this.
>

Still bad...

BTW, the patch _does_ fix the "unusual case" for me. IOW, I plainly copied
your rev-list|pack-objects into command line. As expected, it fails for Junio's
master (after the second run) and works with the patch:

  $ git rev-list -10 HEAD | ./git pack-objects .git/objects/pack/foobar
  Counting objects: 10, done.
  Compressing objects: 100% (9/9), done.
  82864ec14cd06e6089543a1419762e4cd40f7988
  Writing objects: 100% (10/10), done.
  Total 10 (delta 1), reused 10 (delta 1)

  $ git rev-list -10 HEAD | ./git pack-objects .git/objects/pack/foobar
  Counting objects: 10, done.
  Compressing objects: 100% (9/9), done.
  fatal: unable to rename temporary pack file: Permission denied

  $ git cherry-pick fix-fd-leak
  Finished one cherry-pick.
  [master]: created e629465: "Fix handle leak in builtin-pack-objects"
   1 files changed, 1 insertions(+), 0 deletions(-)

  $ git rev-list -10 HEAD | ./git pack-objects .git/objects/pack/foobar
  Counting objects: 10, done.
  Compressing objects: 100% (9/9), done.
  0e43d919a2a8336fd740d8d9b8f9f78d49e855e5
  Writing objects: 100% (10/10), done.
  Total 10 (delta 1), reused 9 (delta 1)

  $ git rev-list -10 HEAD | ./git pack-objects .git/objects/pack/foobar
  Counting objects: 10, done.
  Compressing objects: 100% (9/9), done.
  0e43d919a2a8336fd740d8d9b8f9f78d49e855e5
  Writing objects: 100% (10/10), done.
  Total 10 (delta 1), reused 10 (delta 1)

  $ git rev-list -10 HEAD | ./git pack-objects .git/objects/pack/foobar
  Counting objects: 10, done.
  Compressing objects: 100% (9/9), done.
  0e43d919a2a8336fd740d8d9b8f9f78d49e855e5
  Writing objects: 100% (10/10), done.
  Total 10 (delta 1), reused 10 (delta 1)

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Johannes Sixt @ 2008-11-19 13:55 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Nicolas Pitre, Junio C Hamano, Git Mailing List
In-Reply-To: <81b0412b0811190534r4f71f981s53de415f79e56e25@mail.gmail.com>

Alex Riesen schrieb:
> 2008/11/19 Nicolas Pitre <nico@cam.org>:
>> On Wed, 19 Nov 2008, Johannes Sixt wrote:
>>> The work-around is to write the repacked objects to a file of a different
>>> name, and replace the original after git-pack-objects has terminated.
>>>
>>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> Acked-by: Nicolas Pitre <nico@cam.org>
> 
> Are you sure? Will it work in a real repository? Were noone does
> rename the previous pack files into packtmp-something?

Oh, the patch only works around the failure in the test case. In a real
repository there is usually no problem because the destination pack file
does not exist.

The unusual case is where you do this:

 $ git rev-list -10 HEAD | git pack-objects foobar

twice in a row: In this case the second invocation fails on Windows
because the destination pack file already exists *and* is open. But not
even git-repack does this even if it is called twice. OTOH, the test case
*does* exactly this.

-- Hannes

^ permalink raw reply

* Re: How to make public repository GIT_DIR=my-git.git git-init Command not found.
From: Alex Riesen @ 2008-11-19 13:38 UTC (permalink / raw)
  To: garyyang6; +Cc: git
In-Reply-To: <138223.4849.qm@web37905.mail.mud.yahoo.com>

2008/11/19 Gary Yang <garyyang6@yahoo.com>:
> Hi,
>
> I want to make a public repository. I followed the instructions in gitcore-tutorial.
> I typed "GIT_DIR=my-git.git git init" per instruction. But, I got command not found.
> I do not think this is the correct command. How should I do? I use C-Shell.

git --bare init

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Alex Riesen @ 2008-11-19 13:34 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Johannes Sixt, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811190753420.27509@xanadu.home>

2008/11/19 Nicolas Pitre <nico@cam.org>:
> On Wed, 19 Nov 2008, Johannes Sixt wrote:
>> The work-around is to write the repacked objects to a file of a different
>> name, and replace the original after git-pack-objects has terminated.
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>
> Acked-by: Nicolas Pitre <nico@cam.org>
>

Are you sure? Will it work in a real repository? Were noone does
rename the previous pack files into packtmp-something?

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Alex Riesen @ 2008-11-19 13:30 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811190740570.27509@xanadu.home>

2008/11/19 Nicolas Pitre <nico@cam.org>:
> On Wed, 19 Nov 2008, Alex Riesen wrote:
>
>> The opened packs seem to stay open forever.
>>
>> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
>> ---
>>
>> I'm very unsure about the solution, though: it is really horrible code
>> to debug...
>
> What is the actual problem?  Pack windows are left open on purpose.
>

* expecting success: do_repack &&
     git prune-packed -v &&
     git verify-pack ${pack}.pack &&
     git cat-file blob $blob_1 > /dev/null &&
     git cat-file blob $blob_2 > /dev/null &&
     git cat-file blob $blob_3 > /dev/null
Counting objects: 3, done.
error: unknown object type 0 at offset 2032 in
.git/objects/pack/pack-8d1e04fc992cfbdb3ab72cf7abbf08cce8b1900.pack
Compressing objects: 100% (2/2), done.
fatal: unable to rename temporary pack file
.git/objects/pack/tmp_pack_s9Gijm->.git/objects/pack/pack-b8d1e04fc992cfbdbab72cf7abbf08cce8b1900.pack:
Permission denied
* FAIL 10: ... and then a repack "clears" the corruption
        do_repack &&
             git prune-packed -v &&
             git verify-pack ${pack}.pack &&
             git cat-file blob $blob_1 > /dev/null &&
             git cat-file blob $blob_2 > /dev/null &&
             git cat-file blob $blob_3 > /dev/null

^ permalink raw reply

* Re: git and mtime
From: Jakub Narebski @ 2008-11-19 13:29 UTC (permalink / raw)
  To: Roger Leigh; +Cc: git
In-Reply-To: <20081119113752.GA13611@ravenclaw.codelibre.net>

Roger Leigh <rleigh@codelibre.net> writes:

> Would it be possible for git to store the mtime of files in the tree?
> 
> This would make it possible to do this type of work in git, since it's
> currently a bit random as to whether it works or not.  This only
> started when I upgraded to an amd64 architecture from powerpc32,
> I guess it's maybe using high-resolution timestamps.

I don't think it would be done as in core change at all, or at least
soon.

You can use Metastore, or some custom clean/smudge gitattribute
filters with something like Metastore (or etckeeper) to store extra
metadata about files in your tree.

See http://git.or.cz/gitwiki/InterfacesFrontendsAndTools

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Git commit won't add an untracked file given on the command line
From: Junio C Hamano @ 2008-11-19 13:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Mark Burton, git
In-Reply-To: <alpine.DEB.1.00.0811191226530.30769@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Wed, 19 Nov 2008, Mark Burton wrote:
>
>> Having said that, I still like the concept of being able to add named 
>> files without touching the index.
>
> That's just impossible.  You cannot create a tree object, let alone a 
> commit object, without touching the index (AKA staging area).

I do not think Mark really _means_ "not in the index".

The wish is more like "I want to let git know that I am interested in this
path, but I'm not ready to say what exact content I want for that path in
the next commit, not just yet".

I do not think that is an unreasonable wish.  On the other hand, it is
unreasonable for anybody to insist that we satisfy the wish without
touching the index.  The index is the most natural place to do that.

We have a half (probably a quarter) of what we need for that implemented
already, by the way.

^ permalink raw reply

* Re: [PATCH] Fix deletion of last character in levenshtein distance
From: Junio C Hamano @ 2008-11-19 13:13 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Samuel Tardieu, git
In-Reply-To: <alpine.DEB.1.00.0811191053250.30769@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, I understand now, _after_ having looked at the original 
> levenshtein.c.
>
> IOW you could have made my task of reviewing your patch much easier.
>
> Anyway, here is my
>
> 	Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Thanks for the bugfix,

In other words, even the original author's head exploded without looking
at extra context lines around the patch.

It is a sure sign that the original implementation was too scantily
described, and that the fix was not explained well in the proposed commit
log message (i.e. in what corner cases the original was bad in what way,
and how the patch fixes it).

I shouldn't have to decipher the original and the fixed version with
pencil and paper when re-reviewing Dscho's Ack.

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Johannes Sixt @ 2008-11-19 13:06 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Alex Riesen, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811190753420.27509@xanadu.home>

Nicolas Pitre schrieb:
> On Wed, 19 Nov 2008, Johannes Sixt wrote:
> 
>> Alex Riesen schrieb:
>>> The opened packs seem to stay open forever.
>> In my MinGW port I have the patch below that avoids that t5303 fails
>> because of a pack file that remains open. (Open files cannot be replaced
>> on Windows.) I had hoped that your patch would help, but it does not.
>> Something else still keeps the pack file open. Can anything be done about
>> that?
>>
>> -- Hannes
>>
>> From: Johannes Sixt <j6t@kdbg.org>
>> Date: Mon, 17 Nov 2008 09:25:19 +0100
>> Subject: [PATCH] t5303: Do not overwrite an existing pack
>>
...
> Acked-by: Nicolas Pitre <nico@cam.org>

Thanks, but I should have mentioned that at this time this patch was just
meant for exposition, not inclusion. [It's just one of many, many patches
needed to make the test suite pass on MinGW.]

I'd prefer a solution to the problem that the pack file remains open. Do
you have an idea where git-pack-objects keeps the pack file open, even
with Alex's two patches applied?

-- Hannes

^ permalink raw reply

* Re: [PATCH 5/9] update-index: add --checkout/--no-checkout to update CE_NO_CHECKOUT bit
From: Jeff King @ 2008-11-19 13:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, Shawn O. Pearce
In-Reply-To: <7v4p24tq59.fsf@gitster.siamese.dyndns.org>

On Tue, Nov 18, 2008 at 06:18:10PM -0800, Junio C Hamano wrote:

> Having said all that, I wouldn't suggest redoing the patch using >>
> redirection.  But change from "touch 1 nondigit" to "touch nondigit 1"
> is a bit too subtle to my taste.  Let's write it this way instead:

Yes, I also dislike the subtlety, but my "obvious" idea was something
like:

  for i in 1 2 sub/1 sub/2; do
    touch $i
  done

which just seemed clunky. But:

> -	touch 1 2 sub/1 sub/2 &&
> +	touch ./1 ./2 sub/1 sub/2 &&

this is less clunky, and I have confirmed that it solves the problem. I
just wasn't clever enough to think of it in the first place. ;)

-Peff

^ permalink raw reply

* Re: git bisect do not work when good reversion is newer than bad reversion
From: Junio C Hamano @ 2008-11-19 13:00 UTC (permalink / raw)
  To: Cai, Crane; +Cc: git
In-Reply-To: <6077B97E85E7374DAE87B42B5AA997970D0B12@sshaexmb1.amd.com>

"Cai, Crane" <Crane.Cai@amd.com> writes:

> Hi Junio,
>
> Sorry to disturb you. Here the requirement I met can not find answer
> from website. That's why I directly ask you.
>
> I need to use "git bisect" to find what patch can fix a problem.
> I found v2.6.25 has this problem.
> And v2.6.26 does NOT has this problem.
>
> Then I use bisect as this:
>
> #git bisect start
> #git bisect good v2.6.26
> #git bisect bad v2.6.25
>
> No reversion cut via these steps. Do you know whether git command can
> achieve this requirement or not?

You can use "git bisect" to find a single change that fixed the issue
(_provided if_ such a single change exists).  There is a small mental
trick you need to exercise.

First, realize that bisect is about "it used to be like this, but later it
turned into that.  Where did that single transition from this to that
happened"?  Usually, "this" is "had this particular bug and was broken"
and "that" is "that bug was fixed and things work smoothly".  In your
case, however, "this" is "broken" and "that" is "fixed".  IOW, the
question you ask "bisect" is "it used to be broken but later it was fixed;
where did that exactly happen?".

The mental trick is to swap "good" and "bad" in order to adjust to your
use, because you are using bisect in a reverse way from the usual.  Usual
case uses "good" mark "this" while "bad" is used to mark "that" in the
sentence "it used to be like this, but later it turned into that".

So what you would do is...

	(1) First of all, write on a piece of paper:

	    good - the version _STILL_ has the bug.
            bad  - the version does not have the bug _ANYMORE_

	(2) Start with "git bisect v2.6.25 v2.6.26";

        (3) As you test each revision, look at the memo you made.  Say
            "good" if it still has the bug you are interested in; say
            "bad" if it does not have the bug anymore..

    cf. http://thread.gmane.org/gmane.comp.version-control.git/86063

By the way, I usually drop requests-for-help on the floor unless it is
CC'ed to the mailing list, because my time spent on writing the response
like this message will be wasted and wouldn't help the git community at
all.  I simply do not have enough time to give unpaid support for
individuals.  I am making exception this time only because I am waiting
for something and I happen to have nothing else to do while doing so.

Please send this kind of request-for-help to <git@vger.kernel.org>, the
main mailing list, to which anybody can send messages without subscribing.

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Nicolas Pitre @ 2008-11-19 12:55 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Alex Riesen, Junio C Hamano, Git Mailing List
In-Reply-To: <4923FE58.3090503@viscovery.net>

On Wed, 19 Nov 2008, Johannes Sixt wrote:

> Alex Riesen schrieb:
> > The opened packs seem to stay open forever.
> 
> In my MinGW port I have the patch below that avoids that t5303 fails
> because of a pack file that remains open. (Open files cannot be replaced
> on Windows.) I had hoped that your patch would help, but it does not.
> Something else still keeps the pack file open. Can anything be done about
> that?
> 
> -- Hannes
> 
> From: Johannes Sixt <j6t@kdbg.org>
> Date: Mon, 17 Nov 2008 09:25:19 +0100
> Subject: [PATCH] t5303: Do not overwrite an existing pack
> 
> This test corrupts a pack file, then repacks the objects. The consequence
> is that the repacked pack file has the same name as the original file
> (that has been corrupted).
> 
> During its operation, git-pack-objects opens the corrupted file and keeps
> it open at all times. On Windows, this is a problem because a file that is
> open in any process cannot be delete or replaced, but that is what we do
> in some of the test cases, and so they fail.
> 
> The work-around is to write the repacked objects to a file of a different
> name, and replace the original after git-pack-objects has terminated.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Acked-by: Nicolas Pitre <nico@cam.org>


> ---
>  t/t5303-pack-corruption-resilience.sh |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t5303-pack-corruption-resilience.sh
> b/t/t5303-pack-corruption-resilience.sh
> index 5132d41..41c83e3 100755
> --- a/t/t5303-pack-corruption-resilience.sh
> +++ b/t/t5303-pack-corruption-resilience.sh
> @@ -43,8 +43,11 @@ create_new_pack() {
> 
>  do_repack() {
>      pack=`printf "$blob_1\n$blob_2\n$blob_3\n" |
> -          git pack-objects $@ .git/objects/pack/pack` &&
> -    pack=".git/objects/pack/pack-${pack}"
> +          git pack-objects $@ .git/objects/pack/packtmp` &&
> +    packtmp=".git/objects/pack/packtmp-${pack}" &&
> +    pack=".git/objects/pack/pack-${pack}" &&
> +    mv "${packtmp}.pack" "${pack}.pack" &&
> +    mv "${packtmp}.idx" "${pack}.idx"
>  }
> 
>  do_corrupt_object() {
> -- 
> 1.6.0.4.1683.g35125
> 
> 


Nicolas

^ permalink raw reply

* Re: [PATCH] Fix handle leak in sha1_file/unpack_objects if there were damaged object data
From: Nicolas Pitre @ 2008-11-19 12:49 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <81b0412b0811190314k92a0acbn3ea820cce2a7a40b@mail.gmail.com>

On Wed, 19 Nov 2008, Alex Riesen wrote:

> In the case of bad packed object CRC, unuse_pack wasn't called after
> check_pack_crc which calls use_pack.
> 
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

Acked-by: Nicolas Pitre <nico@cam.org>


> ---
>  sha1_file.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 


Nicolas

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Nicolas Pitre @ 2008-11-19 12:45 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <81b0412b0811190313p643c0cb4vad620ea942aeea93@mail.gmail.com>

On Wed, 19 Nov 2008, Alex Riesen wrote:

> The opened packs seem to stay open forever.
> 
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
> 
> I'm very unsure about the solution, though: it is really horrible code
> to debug...

What is the actual problem?  Pack windows are left open on purpose.


Nicolas

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Johannes Sixt @ 2008-11-19 12:26 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Nicolas Pitre, Junio C Hamano, Git Mailing List
In-Reply-To: <81b0412b0811190413s29644092y86d1c8840e10c28b@mail.gmail.com>

Alex Riesen schrieb:
> 2008/11/19 Johannes Sixt <j.sixt@viscovery.net>:
>> Alex Riesen schrieb:
>>> The opened packs seem to stay open forever.
>> In my MinGW port I have the patch below that avoids that t5303 fails
>> because of a pack file that remains open. (Open files cannot be replaced
>> on Windows.) I had hoped that your patch would help, but it does not.
>> Something else still keeps the pack file open. Can anything be done about
>> that?
>>
> 
> Do this _and_ the other handle-leak-patch together help?
> (I think the second should)

No, that doesn't help, either. :-(

-- Hannes

^ permalink raw reply

* Re: git and mtime
From: Johannes Schindelin @ 2008-11-19 12:31 UTC (permalink / raw)
  To: Roger Leigh; +Cc: git
In-Reply-To: <20081119113752.GA13611@ravenclaw.codelibre.net>

Hi,

On Wed, 19 Nov 2008, Roger Leigh wrote:

> Would it be possible for git to store the mtime of files in the tree?

No, since this would wreck people's workflows:

	- compile in branch "master"
	- switch to branch "topic"
	- compile
	- switch back to branch "master"

Now you _want_ files in "master" that were changed in "topic" to be 
recompiled.

This is a quite common case.

However, nothing hinders you having your own ".gitmtimes" in the tree, and 
a script people can use as a hook, which applies the mtimes to the files.

Ciao,
Dscho

^ permalink raw reply

* Re: git and mtime
From: Matthias Kestenholz @ 2008-11-19 12:22 UTC (permalink / raw)
  To: Roger Leigh; +Cc: git
In-Reply-To: <20081119113752.GA13611@ravenclaw.codelibre.net>

Hi,

On 19.11.2008, at 12:37, Roger Leigh wrote:

> Hi folks,
>
> I'm using git to store some generated files, as well as their sources.
> (This is in the context of Debian package development, where entire
> upstream release tarballs are injected into an upstream branch, with
> Debian releases merging the upstream branch, and adding the Debian
> packaging files.)
>
> The upstream release tarballs contains files such as
> - yacc/lex code, and the corresponding generated sources
> - Docbook/XML code, and corresponding HTML/PDF documentation
>
> These are provided by upstream so that end users don't need these  
> tools
> installed (particularly docbook, since the toolchain is so flaky on
> different systems).  However, the fact that git isn't storing the
> mtime of the files confuses make, so it then tries to regenerate these
> (already up-to-date) files, and fails in the process since the tools
> aren't available.
>
> Would it be possible for git to store the mtime of files in the tree?
>

This subject comes up from time to time, but the answer always
stays the same: No. The trees are purely defined by their content, and
that's by design.

If you do not want to regenerate files that are already up-to-date,
you need multiple checkouts of the same repository.



Thanks,
Matthias

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Alex Riesen @ 2008-11-19 12:13 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Nicolas Pitre, Junio C Hamano, Git Mailing List
In-Reply-To: <4923FE58.3090503@viscovery.net>

2008/11/19 Johannes Sixt <j.sixt@viscovery.net>:
> Alex Riesen schrieb:
>> The opened packs seem to stay open forever.
>
> In my MinGW port I have the patch below that avoids that t5303 fails
> because of a pack file that remains open. (Open files cannot be replaced
> on Windows.) I had hoped that your patch would help, but it does not.
> Something else still keeps the pack file open. Can anything be done about
> that?
>

Do this _and_ the other handle-leak-patch together help?
(I think the second should)

> From: Johannes Sixt <j6t@kdbg.org>
> Date: Mon, 17 Nov 2008 09:25:19 +0100

BTW, you better report such things. Even though it is typical
for this platform brokenness, leaking open files is never good.

^ permalink raw reply

* git and mtime
From: Roger Leigh @ 2008-11-19 11:37 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]

Hi folks,

I'm using git to store some generated files, as well as their sources.
(This is in the context of Debian package development, where entire
upstream release tarballs are injected into an upstream branch, with
Debian releases merging the upstream branch, and adding the Debian
packaging files.)

The upstream release tarballs contains files such as
- yacc/lex code, and the corresponding generated sources
- Docbook/XML code, and corresponding HTML/PDF documentation

These are provided by upstream so that end users don't need these tools
installed (particularly docbook, since the toolchain is so flaky on
different systems).  However, the fact that git isn't storing the
mtime of the files confuses make, so it then tries to regenerate these
(already up-to-date) files, and fails in the process since the tools
aren't available.

Would it be possible for git to store the mtime of files in the tree?

This would make it possible to do this type of work in git, since it's
currently a bit random as to whether it works or not.  This only
started when I upgraded to an amd64 architecture from powerpc32,
I guess it's maybe using high-resolution timestamps.


Thanks,
Roger


P.S. The repo I'm working on here is at
     git://git.debian.org/git/collab-maint/gutenprint.git

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] Fix handle leak in builtin-pack-objects
From: Johannes Sixt @ 2008-11-19 11:54 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Nicolas Pitre, Junio C Hamano, Git Mailing List
In-Reply-To: <81b0412b0811190313p643c0cb4vad620ea942aeea93@mail.gmail.com>

Alex Riesen schrieb:
> The opened packs seem to stay open forever.

In my MinGW port I have the patch below that avoids that t5303 fails
because of a pack file that remains open. (Open files cannot be replaced
on Windows.) I had hoped that your patch would help, but it does not.
Something else still keeps the pack file open. Can anything be done about
that?

-- Hannes

From: Johannes Sixt <j6t@kdbg.org>
Date: Mon, 17 Nov 2008 09:25:19 +0100
Subject: [PATCH] t5303: Do not overwrite an existing pack

This test corrupts a pack file, then repacks the objects. The consequence
is that the repacked pack file has the same name as the original file
(that has been corrupted).

During its operation, git-pack-objects opens the corrupted file and keeps
it open at all times. On Windows, this is a problem because a file that is
open in any process cannot be delete or replaced, but that is what we do
in some of the test cases, and so they fail.

The work-around is to write the repacked objects to a file of a different
name, and replace the original after git-pack-objects has terminated.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t5303-pack-corruption-resilience.sh |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/t5303-pack-corruption-resilience.sh
b/t/t5303-pack-corruption-resilience.sh
index 5132d41..41c83e3 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -43,8 +43,11 @@ create_new_pack() {

 do_repack() {
     pack=`printf "$blob_1\n$blob_2\n$blob_3\n" |
-          git pack-objects $@ .git/objects/pack/pack` &&
-    pack=".git/objects/pack/pack-${pack}"
+          git pack-objects $@ .git/objects/pack/packtmp` &&
+    packtmp=".git/objects/pack/packtmp-${pack}" &&
+    pack=".git/objects/pack/pack-${pack}" &&
+    mv "${packtmp}.pack" "${pack}.pack" &&
+    mv "${packtmp}.idx" "${pack}.idx"
 }

 do_corrupt_object() {
-- 
1.6.0.4.1683.g35125

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox