* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Martin Langhoff (CatalystIT) @ 2006-09-04 10:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkp0ndmj.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> "Martin Langhoff (CatalystIT)" <martin@catalyst.net.nz> writes:
>
>
>>BTW, I think there's a small error.
>>
>>Your packtmp includes $$ which means that rm -f "$PACKTMP" will only
>>clear out old packs..
>
>
> That was deliberate. I hate programs that clean things up
> behind user's back. The first "rm" is to get rid of what would
> collide with what we are going to do (i.e. protecting ourselves)
> and "trap rm" is to make sure we do not leave the cruft we know
> we are going to create. I'd rather leave other people's cruft
> around, unless the purpose of the command is to clean things up,
> and repack is hardly that.
>
Ah, ok. I misunderstood the use of trap -- of course, re-reading the man
pages, it makes sense.
A-ok with me, then, and sorry about the noise.
martin
--
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington
WEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224 MOB: +64(21)364-017
Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------
--
VGER BF report: H 0.0618878
^ permalink raw reply
* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Junio C Hamano @ 2006-09-04 10:13 UTC (permalink / raw)
To: Martin Langhoff (CatalystIT); +Cc: git
In-Reply-To: <44FBF9E0.9050800@catalyst.net.nz>
"Martin Langhoff (CatalystIT)" <martin@catalyst.net.nz> writes:
> BTW, I think there's a small error.
>
> Your packtmp includes $$ which means that rm -f "$PACKTMP" will only
> clear out old packs..
That was deliberate. I hate programs that clean things up
behind user's back. The first "rm" is to get rid of what would
collide with what we are going to do (i.e. protecting ourselves)
and "trap rm" is to make sure we do not leave the cruft we know
we are going to create. I'd rather leave other people's cruft
around, unless the purpose of the command is to clean things up,
and repack is hardly that.
--
VGER BF report: H 0.149712
^ permalink raw reply
* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Martin Langhoff (CatalystIT) @ 2006-09-04 10:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Martin Waitz
In-Reply-To: <7vr6ysneor.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> In other words...
Can't be offline 2 hs to read a book... ;-) Actually, I had thought the
pack reading code would focus on filenames following pack-<id>.pack
pattern and corresponding idx files, and that .tmp-* was safe to have
there. My bad.
BTW, I think there's a small error.
...
> --- a/git-repack.sh
> +++ b/git-repack.sh
> @@ -24,8 +24,10 @@ do
> shift
> done
>
> -rm -f .tmp-pack-*
> PACKDIR="$GIT_OBJECT_DIRECTORY/pack"
> +PACKTMP="$GIT_DIR/.tmp-$$-pack"
> +rm -f "$PACKTMP"-*
> +trap 'rm -f "$PACKTMP"-*' 0 1 2 3 15
Your packtmp includes $$ which means that rm -f "$PACKTMP" will only
clear out old packs only if the pid of the old-and-probably-dead process
matches ours... and then a hyphen.
so instead I propose...
+trap 'rm -f "$GIT_DIR/.tmp-*-pack"' 0 1 2 3 15
cheers,
martin
--
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington
WEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224 MOB: +64(21)364-017
Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------
--
VGER BF report: U 0.900798
^ permalink raw reply
* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Junio C Hamano @ 2006-09-04 9:50 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git, Martin Waitz
In-Reply-To: <7vveo4nfbg.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Writing into $cwd was certainly a carelessness; we tend to use
> $GIT_DIR/ for this kind of thing.
In other words...
-- >8 --
From: Martin Langhoff <martin@catalyst.net.nz>
Date: Mon, 4 Sep 2006 17:42:32 +1200
Subject: [PATCH] git-repack: create new packs inside $GIT_DIR, not cwd
Avoid failing when cwd is !writable by writing the
packfiles in $GIT_DIR, which is more in line with other commands.
Without this, git-repack was failing when run from crontab
by non-root user accounts. For large repositories, this
also makes the mv operation a lot cheaper, and avoids leaving
temp packfiles around the fs upon failure.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
git-repack.sh | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/git-repack.sh b/git-repack.sh
index 584a732..b525fc5 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -24,8 +24,10 @@ do
shift
done
-rm -f .tmp-pack-*
PACKDIR="$GIT_OBJECT_DIRECTORY/pack"
+PACKTMP="$GIT_DIR/.tmp-$$-pack"
+rm -f "$PACKTMP"-*
+trap 'rm -f "$PACKTMP"-*' 0 1 2 3 15
# There will be more repacking strategies to come...
case ",$all_into_one," in
@@ -42,11 +44,12 @@ case ",$all_into_one," in
find . -type f \( -name '*.pack' -o -name '*.idx' \) -print`
;;
esac
+
pack_objects="$pack_objects $local $quiet $no_reuse_delta$extra"
name=$( { git-rev-list --objects --all $rev_list ||
echo "git-rev-list died with exit code $?"
} |
- git-pack-objects --non-empty $pack_objects .tmp-pack) ||
+ git-pack-objects --non-empty $pack_objects "$PACKTMP") ||
exit 1
if [ -z "$name" ]; then
echo Nothing new to pack.
@@ -64,8 +67,8 @@ else
"$PACKDIR/old-pack-$name.$sfx"
fi
done &&
- mv -f .tmp-pack-$name.pack "$PACKDIR/pack-$name.pack" &&
- mv -f .tmp-pack-$name.idx "$PACKDIR/pack-$name.idx" &&
+ mv -f "$PACKTMP-$name.pack" "$PACKDIR/pack-$name.pack" &&
+ mv -f "$PACKTMP-$name.idx" "$PACKDIR/pack-$name.idx" &&
test -f "$PACKDIR/pack-$name.pack" &&
test -f "$PACKDIR/pack-$name.idx" || {
echo >&2 "Couldn't replace the existing pack with updated one."
--
1.4.2.g99d7d
--
VGER BF report: U 0.870206
^ permalink raw reply related
* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Junio C Hamano @ 2006-09-04 9:36 UTC (permalink / raw)
To: Martin Waitz; +Cc: git, Martin Langhoff
In-Reply-To: <20060904090833.GF17042@admingilde.org>
Martin Waitz <tali@admingilde.org> writes:
> hoi :)
>
> On Mon, Sep 04, 2006 at 05:42:32PM +1200, Martin Langhoff wrote:
>> Avoid failing when cwd is !writable by writing the
>> packfiles directly in the $PACKDIR.
>
> what if other GIT commands are being called while you repack?
> Wouldn't they try to use the new packs, even while they are not
> completely written?
Given that a new pack is created first by writing out .pack and
then .idx, and that the using side ignores .pack without
corresponding .idx, we are talking about very small window, but
you are right.
Writing into $cwd was certainly a carelessness; we tend to use
$GIT_DIR/ for this kind of thing.
--
VGER BF report: U 0.926108
^ permalink raw reply
* Bogofilter woes at git-list..
From: Matti Aarnio @ 2006-09-04 9:33 UTC (permalink / raw)
To: git
Oops, sorry.
It looks like I forgot to add "learn what is acceptable traffic"
thing onto the git-list, and as an indirect result of that, a lot
of list traffic got blocked. (I added this 'learn ham out of traffic'
on GIT -list too.)
Even though the messages were rejected at SMTP input phase,
they were diverted into a "freezer" from whence I have been
digging the (mostly dung-) heap for misclassifieds, and
when found some, defrosted them.
So far I have had to add a rule to exempt also majordomo
from any pre-filtering.. way too much rejects there.
If you have any questions, do contact:
postmaster@vger.kernel.org
that address is (also) excepted of any content filtering.
/Matti Aarnio -- one of <postmaster@vger.kernel.org>
--
VGER BF report: U 0.550908
^ permalink raw reply
* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Martin Waitz @ 2006-09-04 9:08 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git, junkio
In-Reply-To: <11573485523752-git-send-email-martin@catalyst.net.nz>
[-- Attachment #1: Type: text/plain, Size: 447 bytes --]
hoi :)
On Mon, Sep 04, 2006 at 05:42:32PM +1200, Martin Langhoff wrote:
> Avoid failing when cwd is !writable by writing the
> packfiles directly in the $PACKDIR.
what if other GIT commands are being called while you repack?
Wouldn't they try to use the new packs, even while they are not
completely written?
Perhaps it is better to create a new subdirectory $PACKDIR/.tmp/
and create the new pack files there?
--
Martin Waitz
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Martin Langhoff @ 2006-09-04 5:42 UTC (permalink / raw)
To: git, junkio; +Cc: Martin Langhoff
Avoid failing when cwd is !writable by writing the
packfiles directly in the $PACKDIR.
Without this, git-repack was failing when run from crontab
by non-root user accounts. For large repositories, this
also makes the mv operation a lot cheaper, and avoids leaving
temp packfiles around the fs upon failure.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
git-repack.sh | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/git-repack.sh b/git-repack.sh
index 584a732..ccc8e43 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -42,11 +42,13 @@ case ",$all_into_one," in
find . -type f \( -name '*.pack' -o -name '*.idx' \) -print`
;;
esac
+
+mkdir -p "$PACKDIR" || exit
pack_objects="$pack_objects $local $quiet $no_reuse_delta$extra"
name=$( { git-rev-list --objects --all $rev_list ||
echo "git-rev-list died with exit code $?"
} |
- git-pack-objects --non-empty $pack_objects .tmp-pack) ||
+ git-pack-objects --non-empty $pack_objects "$PACKDIR/.tmp-pack") ||
exit 1
if [ -z "$name" ]; then
echo Nothing new to pack.
@@ -54,7 +56,6 @@ else
if test "$quiet" != '-q'; then
echo "Pack pack-$name created."
fi
- mkdir -p "$PACKDIR" || exit
for sfx in pack idx
do
@@ -64,8 +65,8 @@ else
"$PACKDIR/old-pack-$name.$sfx"
fi
done &&
- mv -f .tmp-pack-$name.pack "$PACKDIR/pack-$name.pack" &&
- mv -f .tmp-pack-$name.idx "$PACKDIR/pack-$name.idx" &&
+ mv -f "$PACKDIR/.tmp-pack-$name.pack" "$PACKDIR/pack-$name.pack" &&
+ mv -f "$PACKDIR/.tmp-pack-$name.idx" "$PACKDIR/pack-$name.idx" &&
test -f "$PACKDIR/pack-$name.pack" &&
test -f "$PACKDIR/pack-$name.idx" || {
echo >&2 "Couldn't replace the existing pack with updated one."
--
1.4.2.gdfe7
--
VGER BF report: S 1
^ permalink raw reply related
* Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.
From: Junio C Hamano @ 2006-09-04 5:50 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Linus Torvalds, git
In-Reply-To: <20060904041052.GA29369@spearce.org>
Shawn Pearce <spearce@spearce.org> writes:
> Junio C Hamano <junkio@cox.net> wrote:
>...
>> You could introduce repack.active in .git/config that points at
>> the latest active, make git-repack to notice and update when it
>> updates it.
>...
> Better that it update a symref like thing instead. For example
> create a ".git/objects/pack/active" file holding the name
> ("pack-n{40}.pack") of the current active pack. If this file is
> missing choose the pack that is smallest as the active pack.
Actually I like the one you did not quote even better the more I
think about it.
>> We could also just use .git/objects/pack/pack-active.{pack,idx}
>> files. This needs some surgery to get rid of packed_git.sha1[],
>> sha1_pack_name() and friends and have them only require .pack
>> and .idx are linked by their basename only as was discussed in a
>> separate thread to make it dumb-transport friendly.
The argument given as the reason (rather, excuse) the dumb
transport routines wanted to rely on the packed_git.sha1[] and
sha1_pack_name() was because we would need to avoid packname
collisions _anyway_ so relying on the convention to have
"pack-[0-9a-f]{40}.(pack|idx)" is OK or even desirable.
We need to avoid packname collisions, and it is acceptable to
assume that SHA-1 is practically collision free like the rest of
the system does. However, if the dumb transport wants to avoid
packname collision, it should not rely on the way how the other
side names its packs. It first downloads the .idx files, so it
can compute the _right_ packname using the sorted object names
recorded there [*1*], and store the downloaded pack/idx under
the right name, without relying on the way how the other side
names their packs (it still needs to rely on the names in that
their name end with .pack and .idx, and .pack and .idx
corresponds with each other by their basenames. But the point
is it should not depend on more than that, especially that the
basename is of the form "pack-[0-9a-f]{40}" nor the hex part is
the correct packname).
Now if we fix dumb transport downloaders, then we could even
make a convention that the packs named pack-[0-9a-f]{40}.pack
are archive packs. And git-repack can even have a convention
that .git/objects/pack/pack-active.(pack|idx) is the active
pack.
[Footnote]
*1* a refresher course of the packname generation; it is SHA-1
over the object names (20-byte binary representation) in the
pack, sorted in byte order. See builtin-pack-objects.c for
details.
--
VGER BF report: S 0.99883
^ permalink raw reply
* Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.
From: Shawn Pearce @ 2006-09-04 4:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vhczovf6m.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
> > Junio C Hamano <junkio@cox.net> wrote:
> >
> >> If rev-list had an extention that lets you say
> >>
> >> rev-list --objects --all --unpacked=$active_pack |
> >> pack-objects $new_pack
> >
> > Hmm. Seems very reasonable actually. :-)
> >
> > How do we pick the "active pack" in git-repack.sh?
>
> I was hoping that will be a Porcelain policy issue I do not have
> to decide ;-).
But would need to update git-repack.sh. :-)
> You could introduce repack.active in .git/config that points at
> the latest active, make git-repack to notice and update when it
> updates it.
No. I don't think its a good idea for git-repack to be updating
your configuration file during a repack. If something goes badly
there you could lose settings you would rather not lose.
Better that it update a symref like thing instead. For example
create a ".git/objects/pack/active" file holding the name
("pack-n{40}.pack") of the current active pack. If this file is
missing choose the pack that is smallest as the active pack.
--
Shawn.
--
VGER BF report: S 0.999999
^ permalink raw reply
* Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.
From: Shawn Pearce @ 2006-09-04 6:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7v3bb8qixi.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Now if we fix dumb transport downloaders, then we could even
> make a convention that the packs named pack-[0-9a-f]{40}.pack
> are archive packs. And git-repack can even have a convention
> that .git/objects/pack/pack-active.(pack|idx) is the active
> pack.
Seems reasonable.
I take it you are proposing that a dumb transport always downloads
pack-active.pack as pack-n{40}.pack where the dumb protocol
downloader computed the correct pack name from its contents. Thus
any remote pack downloaded over a dumb transport is automatically
treated as a historical pack by the receiving repository.
This will cause someone tracking a remote repository over a dumb
transport to need to repack only a subset of their historical packs
frequently into their own active.pack while leaving other historical
packs untouched.
But the more that I think about this neither solution (an active
pack symref or pack-active.pack) really solves this. Being limited
to just one active pack seems to be a problem with at least the
dumb transports.
I think that's why I preferred the size threshold idea. The active
packs are cheap to repack because they are small. The larger
packs aren't cheap to repack because they are large - and probably
historical. What we are trying to get is fast repacks for the
active objects while still getting full validation anytime we do a
repack and (possibly) destroy the source. A size threshold does it.
When Jon Smirl and I started kicking around the idea of a historical
pack for Mozilla I was thinking of just storing a list of pack base
names in ".git/objects/pack/historical". Packs listed there should
generally be exempt from repacking. During an initial clone we'd
need to deliver the contents of that file to the new repository,
as if the source considers a pack historical its likely the new
repository would want to as well.
But now as I write this email I'm thinking that it may be just as
easy to change the base name of the pack to "hist-n{40}" when we
want to consider it historical.
[snipped and re-ordered]
> It first downloads the .idx files, so it can compute the
> _right_ packname using the sorted object names recorded there
Why trust the .idx? I've seen you post that the .idx is purely
a local matter. The "smart" Git protocol only receives the .pack
from the remote and computes the .idx locally or unpacks it to loose
objects locally; why should a dumb transport trust the remote .idx?
Oh, I know, when the .idx is >50 MiB, the .pack is >450 MiB, has
2 million objects and delta chains ~5000 long.
Are we thinking that .idx files may need to have a slightly wider
distribution than "local"?
--
Shawn.
--
VGER BF report: S 1
^ permalink raw reply
* Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.
From: Junio C Hamano @ 2006-09-04 7:39 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Linus Torvalds, git
In-Reply-To: <20060904064443.GB30032@spearce.org>
Shawn Pearce <spearce@spearce.org> writes:
> Junio C Hamano <junkio@cox.net> wrote:
>> Now if we fix dumb transport downloaders, then we could even
>> make a convention that the packs named pack-[0-9a-f]{40}.pack
>> are archive packs. And git-repack can even have a convention
>> that .git/objects/pack/pack-active.(pack|idx) is the active
>> pack.
>
> Seems reasonable.
>
> I take it you are proposing that a dumb transport always downloads
> pack-active.pack as pack-n{40}.pack where the dumb protocol
> downloader computed the correct pack name from its contents. Thus
> any remote pack downloaded over a dumb transport is automatically
> treated as a historical pack by the receiving repository.
It is a lot lot lot stronger than that. If the dumb transport
(or git in general) uses [0-9a-f]{40} part as the assuarance
mechanism to avoid packname collisions, it should be prepared to
recompute the correct name for not just pack-acrive but _all_
remote packs and store them under the right name.
Also, if repack uses the convention to treat pack-active.* as
the active pack, it might make sense for dumb transport to use
that as a clue and explode what the other side calls pack-active.*
upon reception. For the purpose of repacking decision for the
person at the other end, they are subject for frequent repacking,
so maybe we should treat it as such.
> [snipped and re-ordered]
>> It first downloads the .idx files, so it can compute the
>> _right_ packname using the sorted object names recorded there
>
> Why trust the .idx? I've seen you post that the .idx is purely
> a local matter. The "smart" Git protocol only receives the .pack
> from the remote and computes the .idx locally or unpacks it to loose
> objects locally; why should a dumb transport trust the remote .idx?
Remote .idx is paired with remote .pack and must be able to
access into .pack otherwise the remote person could not use the
pack locally ;-). Note that we are not talking about malicious
repository.
The issue here is not about _trusting_, but adjusting its name
to its contents to match your local convention, which is what
the recent pack-objects spits out to its standard output. Older
versions of pack-objects named the resulting pack with pretty
much randomly (it used to use SHA-1 checksum of object names in
input order, not in index order which is sorted, so the same
pack _could_ have different names), and its output files should
be considered valid packs if they are still around.
--
VGER BF report: S 0.999988
^ permalink raw reply
* Re: [PATCH] git-repack: clear out tmp packfiles from $PACKDIR instead of cwd
From: Jakub Narebski @ 2006-09-04 8:17 UTC (permalink / raw)
To: git
In-Reply-To: <46a038f90609032248h5eda61dft4150e13e49f8e59e@mail.gmail.com>
Martin Langhoff wrote:
> On 9/4/06, Martin Langhoff <martin@catalyst.net.nz> wrote:
>> Temp packfiles should never be created in cwd anyway ;-)
>
> Note! Applies after "git-repack: create new packs inside $PACKDIR, not cwd"
>
>> VGER BF report: U 0.983488
>
> Hilarious!
Isn't 'U' for Undecided? (and 'H' for Ham)?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
--
VGER BF report: S 0.999453
^ permalink raw reply
* [PATCH] unpack-objects --desperate
From: Junio C Hamano @ 2006-09-04 6:05 UTC (permalink / raw)
To: git
The command unpack-objects dies upon the first error. This is
probably considered a feature -- if a pack is corrupt, instead
of trying to extract from it and possibly risking to contaminate
a good repository with objects whose validity is dubious, we
should seek a good copy of the pack and retry. However, we may
not have any good copy anywhere. This implements the last
resort effort to extract what are salvageable from such a
corrupt pack.
This flag might have helped Sergio when recovering from a
corrupt pack. In my test, it managed to salvage 247 objects out
of a pack that had 251 objects but without it the command
stopped after extracting 73 objects.
---
* A new flag I do not know what to call so I used -r for
"recover" but used the word "salvage" when describing it in
the documentation and the variable used for this option is
called "desperate". Obviously I am not good at naming things.
Only lightly tested.
diff --git a/Documentation/git-unpack-objects.txt b/Documentation/git-unpack-objects.txt
index c20b38b..415c09b 100644
--- a/Documentation/git-unpack-objects.txt
+++ b/Documentation/git-unpack-objects.txt
@@ -8,7 +8,7 @@ git-unpack-objects - Unpack objects from
SYNOPSIS
--------
-'git-unpack-objects' [-n] [-q] <pack-file
+'git-unpack-objects' [-n] [-q] [-r] <pack-file
DESCRIPTION
@@ -34,6 +34,12 @@ OPTIONS
The command usually shows percentage progress. This
flag suppresses it.
+-r::
+ When unpacking a corrupt packfile, the command dies at
+ the first corruption. This flag tells it to keep going
+ and make the best effort to salvage as many objects as
+ possible.
+
Author
------
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 0c180b5..5f9c0d2 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -10,7 +10,7 @@ #include "tree.h"
#include <sys/time.h>
-static int dry_run, quiet;
+static int dry_run, quiet, desperate, has_errors;
static const char unpack_usage[] = "git-unpack-objects [-n] [-q] < pack-file";
/* We always read in 4kB chunks. */
@@ -71,8 +71,15 @@ static void *get_data(unsigned long size
use(len - stream.avail_in);
if (stream.total_out == size && ret == Z_STREAM_END)
break;
- if (ret != Z_OK)
- die("inflate returned %d\n", ret);
+ if (ret != Z_OK) {
+ error("inflate returned %d\n", ret);
+ free(buf);
+ buf = NULL;
+ if (!desperate)
+ exit(1);
+ has_errors = 1;
+ break;
+ }
stream.next_in = fill(1);
stream.avail_in = len;
}
@@ -110,9 +117,9 @@ static void write_object(void *buf, unsi
added_object(sha1, type, buf, size);
}
-static int resolve_delta(const char *type,
- void *base, unsigned long base_size,
- void *delta, unsigned long delta_size)
+static void resolve_delta(const char *type,
+ void *base, unsigned long base_size,
+ void *delta, unsigned long delta_size)
{
void *result;
unsigned long result_size;
@@ -125,7 +132,6 @@ static int resolve_delta(const char *typ
free(delta);
write_object(result, result_size, type);
free(result);
- return 0;
}
static void added_object(unsigned char *sha1, const char *type, void *data, unsigned long size)
@@ -145,7 +151,7 @@ static void added_object(unsigned char *
}
}
-static int unpack_non_delta_entry(enum object_type kind, unsigned long size)
+static void unpack_non_delta_entry(enum object_type kind, unsigned long size)
{
void *buf = get_data(size);
const char *type;
@@ -157,39 +163,42 @@ static int unpack_non_delta_entry(enum o
case OBJ_TAG: type = tag_type; break;
default: die("bad type %d", kind);
}
- if (!dry_run)
+ if (!dry_run && buf)
write_object(buf, size, type);
free(buf);
- return 0;
}
-static int unpack_delta_entry(unsigned long delta_size)
+static void unpack_delta_entry(unsigned long delta_size)
{
void *delta_data, *base;
unsigned long base_size;
char type[20];
unsigned char base_sha1[20];
- int result;
hashcpy(base_sha1, fill(20));
use(20);
delta_data = get_data(delta_size);
- if (dry_run) {
+ if (dry_run || !delta_data) {
free(delta_data);
- return 0;
+ return;
}
if (!has_sha1_file(base_sha1)) {
add_delta_to_list(base_sha1, delta_data, delta_size);
- return 0;
+ return;
}
base = read_sha1_file(base_sha1, type, &base_size);
- if (!base)
- die("failed to read delta-pack base object %s", sha1_to_hex(base_sha1));
- result = resolve_delta(type, base, base_size, delta_data, delta_size);
+ if (!base) {
+ error("failed to read delta-pack base object %s",
+ sha1_to_hex(base_sha1));
+ if (!desperate)
+ exit(1);
+ has_errors = 1;
+ return;
+ }
+ resolve_delta(type, base, base_size, delta_data, delta_size);
free(base);
- return result;
}
static void unpack_one(unsigned nr, unsigned total)
@@ -236,7 +245,11 @@ static void unpack_one(unsigned nr, unsi
unpack_delta_entry(size);
return;
default:
- die("bad object type %d", type);
+ error("bad object type %d", type);
+ has_errors = 1;
+ if (desperate)
+ return;
+ exit(1);
}
}
@@ -280,6 +293,10 @@ int cmd_unpack_objects(int argc, const c
quiet = 1;
continue;
}
+ if (!strcmp(arg, "-r")) {
+ desperate = 1;
+ continue;
+ }
usage(unpack_usage);
}
@@ -306,5 +323,5 @@ int cmd_unpack_objects(int argc, const c
/* All done */
if (!quiet)
fprintf(stderr, "\n");
- return 0;
+ return has_errors;
}
--
VGER BF report: S 0.999272
^ permalink raw reply related
* Re: [PATCH] git-repack: clear out tmp packfiles from $PACKDIR instead of cwd
From: Martin Langhoff @ 2006-09-04 5:48 UTC (permalink / raw)
To: git; +Cc: junkio
In-Reply-To: <11573485362686-git-send-email-martin@catalyst.net.nz>
On 9/4/06, Martin Langhoff <martin@catalyst.net.nz> wrote:
> Temp packfiles should never be created in cwd anyway ;-)
Note! Applies after "git-repack: create new packs inside $PACKDIR, not cwd"
> VGER BF report: U 0.983488
Hilarious!
m
--
VGER BF report: U 0.569396
^ permalink raw reply
* Re: Bogofilter is my emails
From: Martin Langhoff @ 2006-09-04 5:46 UTC (permalink / raw)
To: git
In-Reply-To: <20060903040222.GB29756@spearce.org>
On 9/3/06, Shawn Pearce <spearce@spearce.org> wrote:
> I'm not quite sure how to fix either message to get them to the list.
> Neither email was a patch so I'm not going to try resending them
Well, it's just eaten 2 trivial patches of mine. Grumble.
I am resending them via a different smtp host with the assumption that
the rules may be blocking emails sent via localhost on the initial hop
or other smtp-routing-related rule.
Discussion however seems to imply that bogofilter is only
email-content based? The vger mta admins need to get a grip on 2006
and use a combination of weighted rules to play the spam blocking game
with at least some hope.
Simple rulesets nowadays are triggered by ham way more often than spam.
cheers,
martin
--
VGER BF report: U 0.550887
^ permalink raw reply
* [PATCH] git-repack: clear out tmp packfiles from $PACKDIR instead of cwd
From: Martin Langhoff @ 2006-09-04 5:42 UTC (permalink / raw)
To: git, junkio; +Cc: Martin Langhoff
Temp packfiles should never be created in cwd anyway ;-)
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
git-repack.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-repack.sh b/git-repack.sh
index ccc8e43..eee0d72 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -24,8 +24,9 @@ do
shift
done
-rm -f .tmp-pack-*
PACKDIR="$GIT_OBJECT_DIRECTORY/pack"
+mkdir -p "$PACKDIR" || exit
+rm -f "$PACKDIR/.tmp-pack-*"
# There will be more repacking strategies to come...
case ",$all_into_one," in
@@ -43,7 +44,6 @@ case ",$all_into_one," in
;;
esac
-mkdir -p "$PACKDIR" || exit
pack_objects="$pack_objects $local $quiet $no_reuse_delta$extra"
name=$( { git-rev-list --objects --all $rev_list ||
echo "git-rev-list died with exit code $?"
--
1.4.2.gdfe7
--
VGER BF report: U 0.983488
^ permalink raw reply related
* Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.
From: Junio C Hamano @ 2006-09-04 4:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <7vodtwtw9v.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Linus Torvalds <torvalds@osdl.org> writes:
>
>> Ok. Is it less painful if it just checks the zlib CRC...
>
> I haven't checked myself but somebody said that zlib CRC is of
> preimage so we would need to incur inflate cost anyway if that
> is the case. But I think it may be a reasonable comproise to
> assume that an existing delta that inflates properly would apply
> to its base object, and if we can assume that we do not have to
> check the inflated xdelta data.
> ...
> Another thing the current check is _not_ doing is for this
> pathological case:
>
> - .idx table says the pack entry is N bytes
>
> - unpack_entry_gently() used in the revalidate code uses the
> usual codepath that says "here is the start of the pack
> entry; inflate using as much data as you need"; .idx is
> somehow wrong and it needed N+M bytes where 0 < M.
>
> - we copy out N bytes because we belive .idx.
So here is a patch against "master" which contains none of the
earlier patches in this thread.
When copying from an existing pack and when copying from a loose
object with new style header, the code makes sure that the piece
we are going to copy out inflates well and inflate() consumes
the data in full while doing so.
The check to see if the xdelta really apply is quite expensive
as you described, because you would need to have the image of
the base object which can be represented as a delta against
something else.
The same repack takes this much with this code.
Total 301361, written 301361 (delta 238935), reused 300995 (delta 238663)
6.66user 0.67system 0:07.40elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+62728minor)pagefaults 0swaps
---
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 46f524d..149fa28 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -65,6 +65,7 @@ static unsigned char pack_file_sha1[20];
static int progress = 1;
static volatile sig_atomic_t progress_update;
static int window = 10;
+static int pack_to_stdout;
/*
* The object names in objects array are hashed with this hashtable,
@@ -242,6 +243,82 @@ static int encode_header(enum object_typ
return n;
}
+static int check_inflate(unsigned char *data, unsigned long len, unsigned long expect)
+{
+ z_stream stream;
+ unsigned char fakebuf[4096];
+ int st;
+
+ memset(&stream, 0, sizeof(stream));
+ stream.next_in = data;
+ stream.avail_in = len;
+ stream.next_out = fakebuf;
+ stream.avail_out = sizeof(fakebuf);
+ inflateInit(&stream);
+
+ while (1) {
+ st = inflate(&stream, Z_FINISH);
+ if (st == Z_STREAM_END || st == Z_OK) {
+ st = (stream.total_out == expect &&
+ stream.total_in == len) ? 0 : -1;
+ break;
+ }
+ if (st != Z_BUF_ERROR) {
+ st = -1;
+ break;
+ }
+ stream.next_out = fakebuf;
+ stream.avail_out = sizeof(fakebuf);
+ }
+ inflateEnd(&stream);
+ return st;
+}
+
+/*
+ * we are going to reuse the existing pack entry data. make
+ * sure it is not corrupt.
+ */
+static int revalidate_pack_entry(struct object_entry *entry, unsigned char *data, unsigned long len)
+{
+ enum object_type type;
+ unsigned long size, used;
+
+ if (pack_to_stdout)
+ return 0;
+
+ /* the caller has already called use_packed_git() for us,
+ * so it is safe to access the pack data from mmapped location.
+ * make sure the entry inflates correctly.
+ */
+ used = unpack_object_header_gently(data, len, &type, &size);
+ if (!used)
+ return -1;
+ if (type == OBJ_DELTA)
+ used += 20; /* skip base object name */
+ data += used;
+ len -= used;
+ return check_inflate(data, len, entry->size);
+}
+
+static int revalidate_loose_object(struct object_entry *entry,
+ unsigned char *map,
+ unsigned long mapsize)
+{
+ /* we already know this is a loose object with new type header. */
+ enum object_type type;
+ unsigned long size, used;
+
+ if (pack_to_stdout)
+ return 0;
+
+ used = unpack_object_header_gently(map, mapsize, &type, &size);
+ if (!used)
+ return -1;
+ map += used;
+ mapsize -= used;
+ return check_inflate(map, mapsize, size);
+}
+
static unsigned long write_object(struct sha1file *f,
struct object_entry *entry)
{
@@ -276,6 +353,9 @@ static unsigned long write_object(struct
map = map_sha1_file(entry->sha1, &mapsize);
if (map && !legacy_loose_object(map)) {
/* We can copy straight into the pack file */
+ if (revalidate_loose_object(entry, map, mapsize))
+ die("corrupt loose object %s",
+ sha1_to_hex(entry->sha1));
sha1write(f, map, mapsize);
munmap(map, mapsize);
written++;
@@ -286,7 +366,7 @@ static unsigned long write_object(struct
munmap(map, mapsize);
}
- if (! to_reuse) {
+ if (!to_reuse) {
buf = read_sha1_file(entry->sha1, type, &size);
if (!buf)
die("unable to read %s", sha1_to_hex(entry->sha1));
@@ -319,6 +399,9 @@ static unsigned long write_object(struct
datalen = find_packed_object_size(p, entry->in_pack_offset);
buf = (char *) p->pack_base + entry->in_pack_offset;
+
+ if (revalidate_pack_entry(entry, buf, datalen))
+ die("corrupt delta in pack %s", sha1_to_hex(entry->sha1));
sha1write(f, buf, datalen);
unuse_packed_git(p);
hdrlen = 0; /* not really */
@@ -1163,7 +1246,7 @@ static void prepare_pack(int window, int
find_deltas(sorted_by_type, window+1, depth);
}
-static int reuse_cached_pack(unsigned char *sha1, int pack_to_stdout)
+static int reuse_cached_pack(unsigned char *sha1)
{
static const char cache[] = "pack-cache/pack-%s.%s";
char *cached_pack, *cached_idx;
@@ -1247,7 +1330,7 @@ int cmd_pack_objects(int argc, const cha
{
SHA_CTX ctx;
char line[40 + 1 + PATH_MAX + 2];
- int depth = 10, pack_to_stdout = 0;
+ int depth = 10;
struct object_entry **list;
int num_preferred_base = 0;
int i;
@@ -1367,7 +1450,7 @@ int cmd_pack_objects(int argc, const cha
if (progress && (nr_objects != nr_result))
fprintf(stderr, "Result has %d objects.\n", nr_result);
- if (reuse_cached_pack(object_list_sha1, pack_to_stdout))
+ if (reuse_cached_pack(object_list_sha1))
;
else {
if (nr_result)
--
VGER BF report: U 0.976258
^ permalink raw reply related
* Re: [PATCH] git-repack: create new packs inside $PACKDIR, not cwd
From: Martin Langhoff (CatalystIT) @ 2006-09-04 2:42 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git, junkio
In-Reply-To: <1157337502104-git-send-email-martin@catalyst.net.nz>
Martin Langhoff wrote:
> Avoid failing when cwd is !writable by writing the
> packfiles directly in the $PACKDIR.
Urgh. At the very top of the script there's
rm -f .tmp-pack-*
PACKDIR="$GIT_OBJECT_DIRECTORY/pack"
which relies on cwd again. Hmm.
m
--
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington
WEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224 MOB: +64(21)364-017
Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------
--
VGER BF report: U 0.804452
^ permalink raw reply
* Re: Mozilla version control requirements and git
From: Jon Smirl @ 2006-09-04 1:43 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Jakub Narebski, git
In-Reply-To: <46a038f90609031440s33e66499x51026aa6062566d2@mail.gmail.com>
On 9/3/06, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> On 9/4/06, Jon Smirl <jonsmirl@gmail.com> wrote:
> > You can't really fake it. Many of the projects in Mozilla are
> > dependent on each other. If you break them up into separate
> > repositories you lose the ability to do a cross project commit. This
> > is common when you are changing an interface between the subprojects.
>
> In many projects this is handled by having tags and generally
> versioning modules and interfaces. Being independent gives the
> subprojects/modules a lot more freedom to work/branch on wild
> tangents, and the versioned interfaces mean that -- with the
> contraints of the versioned interfaces -- you can mix and match
> branches/releases of the different subprojects.
>
> In short, I'd definitely break those up :-)
Mozilla always ships as a complete system so there is no need to deal
with old versions of the pieces. Only the external interfaces are
frozen and versioned. This is similar to the Linux kernel. The user
space API is frozen, but when an internal device driver interface
changes all of the drivers in the kernel tree get updated in lock
step. Doing it this way removes a lot of overhead in maintaining
binary compatibility.
--
Jon Smirl
jonsmirl@gmail.com
--
VGER BF report: U 0.502564
^ permalink raw reply
* Re: Bogofilter is my emails
From: Junio C Hamano @ 2006-09-04 1:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0609031550330.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Sun, 3 Sep 2006, Shawn Pearce wrote:
>>
>> I'm not quite sure how to fix either message to get them to the list.
>> Neither email was a patch so I'm not going to try resending them
>> but I'm certainly a little curious as to how my email writing style
>> twice tripped bogofilter's spam switch.
>
> I'm surprised and disgusted that vger started using bogofilter.
>...
> Linus
>
> --
> VGER BF report: U 0.720981
Hehe, and it still considers that your message is highly spammy
(1.0 being definite spam, 0.0 being definite ham, if I am not
mistaken).
The filter at least should learn to recognize "^\t\tLinus$" ;-).
--
VGER BF report: U 0.669453
^ permalink raw reply
* Re: Bogofilter is my emails
From: Linus Torvalds @ 2006-09-03 23:09 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20060903040222.GB29756@spearce.org>
On Sun, 3 Sep 2006, Shawn Pearce wrote:
>
> I'm not quite sure how to fix either message to get them to the list.
> Neither email was a patch so I'm not going to try resending them
> but I'm certainly a little curious as to how my email writing style
> twice tripped bogofilter's spam switch.
I'm surprised and disgusted that vger started using bogofilter.
Last I saw, the bogofilter approach was totally bogus, using purely
single-word frequencies (or, more strictly, a "does word X exist or not",
where X has often gone through what a linguist would probably call a
"lemmatizer", ie something that turns different forms of the same word
into its canonical word, aka "lemma") for its "bayesian" filtering.
Maybe they've enhanced it enough since, but it certainly used to be not
only fairly easy to fool, since it at least originally didn't take any
account at all of any more complex structure.
There's even some papers about how the bayesian thing does not work well
(even when extended to do some phrases and with lemmatization) if the
cut-off is hard.
I think the bogofilter is probably an acceptable input as _one_ of many
rules for a real spam-filter (ie as one of many spamassassin rules), but
not for what vger does.
Hard rules at mail acceptance are much better if they use some really hard
datum. For example, checking that the sending site actually also receives
email, and that it resolves back to itself. That's one thing that OSDL
does, for example, and it means that you can only send me email if your
machine is actually designated as a MX gateway. That cuts down on a _lot_
of spam.
(I'd love to speak of the details, but I wouldn't know. Kees Cook set it
all up at osdl, and I can just say that it works beautifully.)
Linus
--
VGER BF report: U 0.720981
^ permalink raw reply
* Re: Mozilla version control requirements and git
From: Junio C Hamano @ 2006-09-03 23:06 UTC (permalink / raw)
To: Jon Smirl; +Cc: git
In-Reply-To: <9e4733910609022029q9cb9ba9m87cbc37e8d6f4ad7@mail.gmail.com>
"Jon Smirl" <jonsmirl@gmail.com> writes:
> Partial repo pulls and an issue to. The mozilla repo has much more
> than a browser in it, it also has a large mail/news program. A partial
> repo pull may not be what is needed for git, instead git needs a
> partial repo checkout.
That does not contradict with the underlying data model of git,
and it would certainly be possible with a specialized Porcelain.
Since git tracks the entire tree, the tree object contained in
the commit you create _must_ describe the entire project tree,
but that does not necessarily mean that your working tree needs
to have the entire tree checked out.
The git corelevel is designed to be operable with sparsely
populated working tree, and the problem you are seeing is
primarily coming from the fact that some wrappers you are seeing
as "git UI" are specifically done to help workflows that _wants_
to have fully populated working tree (e.g. "git commit -a" and
"git checkout"). There are other wrappers that are designed to
work well in sparsely populated working tree, including "git-am"
and "git-merge". They should be usable pretty much as they are.
To support "narrow checkout" workflow, some wrappers either need
to be told about the "interesting area", or a set of different
wrappers specifically designed to support "narrow checkout"
workflow can be written. I am not sure which is more desirable.
I have a gut feeling that "git commit" can be used as is (you cd
into the area you are interested in, and say "git commit ."
instead of saying "git commit -a"), while it would be less
confusing to keep "git checkout" the whole-tree operation and
have a specialized porcelain for a narrow checkout, but this is
me saying things without thinking things through.
I imagine such a "narrow checkout" workflow Porcelain would work
like this:
- the user tells it which parts of the whole project she is
interested in.
- when checking things out, the Porcelain populates the index
in full [*1*] and then checks out only specified subtrees to
the working tree.
- when checking things in, the changes outside the specified
area are ignored (well, outside there should be no files in
working tree so ignoring them should be trivial). write-tree
writes from the whole index and creates the tree object and
then commit object just as usual. The net effect is that the
parts outside the areas of interest stay the same as the
previous HEAD commit, which is exactly what the user wanted
anyway.
You could optimize the above further by populating the index
sparsely as well (i.e. *1* above is not a requirement), but you
would need to merge in the changes made to that sparse index
into the full tree read from the HEAD commit before writing the
next tree object out anyway, so I doubt it is worth it.
--
VGER BF report: U 0.842228
^ permalink raw reply
* Re: Bogofilter is my emails
From: Davide Libenzi @ 2006-09-03 22:53 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20060903040222.GB29756@spearce.org>
On Sun, 3 Sep 2006, Shawn Pearce wrote:
> Bogofilter ate two messages today that I tried to send to this
> mailing list. At least I got back bounce messages from it.
>
> I'm not quite sure how to fix either message to get them to the list.
> Neither email was a patch so I'm not going to try resending them
> but I'm certainly a little curious as to how my email writing style
> twice tripped bogofilter's spam switch.
Maybe Matti trained the filter using the lkml corpus. Just sprinkle a few
linux-kernel hot words in your posts ;)
- Davide
--
VGER BF report: U 0.926352
^ permalink raw reply
* Bogofilter is my emails
From: Shawn Pearce @ 2006-09-03 4:02 UTC (permalink / raw)
To: git
Bogofilter ate two messages today that I tried to send to this
mailing list. At least I got back bounce messages from it.
I'm not quite sure how to fix either message to get them to the list.
Neither email was a patch so I'm not going to try resending them
but I'm certainly a little curious as to how my email writing style
twice tripped bogofilter's spam switch.
--
Shawn.
--
VGER BF report: S 1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox