* Re: Huge win, compressing a window of delta runs as a unit
From: Nicolas Pitre @ 2006-08-21 17:55 UTC (permalink / raw)
To: Jon Smirl; +Cc: Shawn Pearce, git
In-Reply-To: <Pine.LNX.4.64.0608211309270.3851@localhost.localdomain>
On Mon, 21 Aug 2006, Nicolas Pitre wrote:
> If for example each object has 2 delta childs, and each of those deltas
> also have 2 delta childs, you could have up to 39366 delta objects
> attached to a _single_ undeltified base object.
Sorry I've got the math wrong. That is 3582 deltas, given a
conservative number of delta childs = 2.
Nicolas
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Nicolas Pitre @ 2006-08-21 17:48 UTC (permalink / raw)
To: Jon Smirl; +Cc: Shawn Pearce, git
In-Reply-To: <9e4733910608210914s1157f47eta821584928ce4dd5@mail.gmail.com>
On Mon, 21 Aug 2006, Jon Smirl wrote:
> How about making the length of delta chains an exponential function of
> the number of revs? In Mozilla configure.in has 1,700 revs and it is a
> 250K file. If you store a full copy every 10 revs that is 43MB
> (prezip) of data that almost no one is going to look at.
> The chains
> lengths should reflect the relative probability that someone is going
> to ask to see the revs. That is not at all a uniform function.
1) You can do that already with stock git-repack.
2) The current git-pack-objects code can and does produce a delta "tree"
off of a single base object. It doesn't have to be a linear list.
Therefore, even if the default depth is 10, you may as well have many
deltas pointing to the _same_ base object effectively making the
compression ratio much larger than 1/10.
If for example each object has 2 delta childs, and each of those deltas
also have 2 delta childs, you could have up to 39366 delta objects
attached to a _single_ undeltified base object.
And of course the git-pack-objects code doesn't limit the number of
delta childs in any way so this could theoritically be infinite even
though the max depth is 10. OK the delta matching window limits that
somehow but nothing prevents you from repacking with a larger window
since that parameter has no penalty on the reading of objects out of the
pack.
> Personally I am still in favor of a two pack system. One archival pack
> stores everything in a single chain and size, not speed, is it's most
> important attribute. It is marked readonly and only functions as an
> archive; git-repack never touches it. It might even use a more compact
> compression algorithm.
>
> The second pack is for storing more recent revisions. The archival
> pack would be constructed such that none of the files needed for the
> head revisions of any branch are in it. They would all be in the
> second pack.
Personally I'm still against that. All arguments put forward for a
different or multiple packing system are based on unproven assumptions
so far and none of those arguments actually present significant
advantages over the current system. For example, I was really intriged
by the potential of object grouping into a single zlib stream at first,
but it turns out not to be so great after actual testing.
I still think that a global zlib dictionary is a good idea. Not because
it looks like it'll make packs enormously smaller, but rather because it
impose no performance regression over the current system. And I
strongly believe that you have to have a really strong case for
promoting a solution that carries performance regression, something like
over 25% smaller packs for example. But so far it didn't happen.
> This may be a path to partial repositories. Instead of downloading the
> real archival pack I could download just an index for it. The index
> entries would be marked to indicate that these objects are valid but
> not-present.
An index without the actual objects is simply useless. You could do
without the index entirely in that case anyway since the mere presence
of an entry in the index doesn't give you anything really useful.
Nicolas
^ permalink raw reply
* stgit: deleting a branch doesn't seem to work
From: Torgil Svensson @ 2006-08-21 17:27 UTC (permalink / raw)
To: git
I have three branches and want my "gst10" to be "master" instead
$ stg branch --delete master
Deleting branch "master"... done
$ stg branch -l
Available branches:
s gst10 |
master |
> origin |
$ stg branch --delete master
Deleting branch "master"... done
$ stg branch -r gst10 master
stg branch: Branch "master" already exists
Is there something i'm missing in my logic?
//Torgil
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Jon Smirl @ 2006-08-21 16:23 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Nicolas Pitre, git
In-Reply-To: <20060821064659.GB24054@spearce.org>
On 8/21/06, Shawn Pearce <spearce@spearce.org> wrote:
> Based on the stats Jon provided me it appeared as though the code
> skipped about 9% of the objects, and at the time I thought I new
> why but looking at it again right now I'm not sure why. :-)
9% of the objects may be duplicates because of branching. I have no
way of detecting them since I am not maintaining a map of sha1s
encountered. The way CVS branching works some revisions are built
using forward diffs and some reverse, I can't tell when they will end
up with the same result.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Jon Smirl @ 2006-08-21 16:14 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn Pearce, git
In-Reply-To: <Pine.LNX.4.64.0608211036150.3851@localhost.localdomain>
How about making the length of delta chains an exponential function of
the number of revs? In Mozilla configure.in has 1,700 revs and it is a
250K file. If you store a full copy every 10 revs that is 43MB
(prezip) of data that almost no one is going to look at. The chains
lengths should reflect the relative probability that someone is going
to ask to see the revs. That is not at all a uniform function.
Personally I am still in favor of a two pack system. One archival pack
stores everything in a single chain and size, not speed, is it's most
important attribute. It is marked readonly and only functions as an
archive; git-repack never touches it. It might even use a more compact
compression algorithm.
The second pack is for storing more recent revisions. The archival
pack would be constructed such that none of the files needed for the
head revisions of any branch are in it. They would all be in the
second pack.
After time the second pack may grow large and another archival pack
can be created. The first one would still be maintained in it's
readonly form. git could be optimized to always search for objects in
non-archival packs before even opening the index of an archival one.
This may be a path to partial repositories. Instead of downloading the
real archival pack I could download just an index for it. The index
entries would be marked to indicate that these objects are valid but
not-present.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [RFD] gitweb: href() function to generate URLs for CGI
From: Jakub Narebski @ 2006-08-21 15:52 UTC (permalink / raw)
To: git
In-Reply-To: <200608211739.32993.jnareb@gmail.com>
Ooops. Sorry for duplicated post.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Recover from a bad push in StGit
From: Catalin Marinas @ 2006-08-21 15:55 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200608211703.59873.robin.rosenberg.lists@dewire.com>
On 21/08/06, Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> måndag 21 augusti 2006 11:35 skrev Catalin Marinas:
> > "stg push --undo" (the same option for "refresh"; maybe I should add a
> > separate "undo" command) resets the local changes and it restores the
> Not necessarily, but a hint about --undo when push/pop fail would have
> helped me.
It is in the "stg help push" output but I could just display it when
the conflict happened.
> > original boundaries of the patch (no information is lost). You could
> > use the latest snapshot as it has some bug-fixes from release 0.10.
>
> Does refresh --undo make me see the state of the patch like it was before last
> refresh, ie. stgit remembers the history of my patces? I thought it dropped
> the references as obsolete.
The references before the last command are stored and they can be
obtained by "stg id <patch>//(bottom|top).old". As long as you haven't
run a git prune on your repository, you should be able to undo the
refresh.
I'm (slowly) working on patch history support using reflogs and (when
ready) you should be able to retrieve a previous state of the patch as
long as you haven't pruned your GIT repository.
--
Catalin
^ permalink raw reply
* Re: [PATCH] Verify we know how to read a pack before trying to using it.
From: Nicolas Pitre @ 2006-08-21 15:51 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20060821071002.GA26029@spearce.org>
On Mon, 21 Aug 2006, Shawn Pearce wrote:
> If the pack format were to ever change or be extended in the future
> there is no assurance that just because the pack file lives in
> objects/pack and doesn't end in .idx that we can read and decompress
> its contents properly.
>
> If we encounter what we think is a pack file and it isn't or we don't
> recognize its version then die and suggest to the user that they
> upgrade to a newer version of GIT which can handle that pack file.
There are already 3 other places where the pack header and version are
validated: see git-grep pack_version_ok *.c output. Maybe it would be
best to abstract those tests all in one place.
Nicolas
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Nicolas Pitre @ 2006-08-21 15:46 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Jon Smirl, git
In-Reply-To: <20060821070609.GC24054@spearce.org>
On Mon, 21 Aug 2006, Shawn Pearce wrote:
> Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 18 Aug 2006, Jon Smirl wrote:
> >
> > > On 8/18/06, Nicolas Pitre <nico@cam.org> wrote:
> > > > A better way to get such a size saving is to increase the window and
> > > > depth parameters. For example, a window of 20 and depth of 20 can
> > > > usually provide a pack size saving greater than 11% with none of the
> > > > disadvantages mentioned above.
> > >
> > > Our window size is effectively infinite. I am handing him all of the
> > > revisions from a single file in optimal order. This includes branches.
> >
> > In GIT packing terms this is infinite delta _depth_ not _window_.
>
> We're not using infinite anything.
>
> fast-import is basically doing window=1 and depth=10.
Yes, that is what I figured out looking at the code.
> We only examine the last blob to see if we can get a delta against
> it. If we do we write that delta out; otherwise we reset our delta
> chain and write the complete object. We also reset our chain after
> writing out 10 deltas, each of which used the immediately prior
> object as its base.
I think this is a perfectly sensible thing to do for the initial import
pass. This will get the data in GIT format quickly and save disk space
over no deltas at all. No need to seek for optimal packing at that
point but only lay things out for a faster git-repack pass to complete
the import.
Also, like I said before, you cannot get a nice object layout from your
import process since all revisions of each file end up next to each
other while in practice what you want is all objects for each _commit_
to be next to each other. Otherwise checking out a commit will seek all
over the pack like mad. A final repack using git-repack will create
that nice object layout for you and that is something you cannot do
on the fly.
The git-pack-objects code is also where all the accumulated knowledge
for best delta packing heuristics has been created over the last year.
It would be counter productive to cut yourself from its value. If
anything more clever can be done with packing it probably should be done
there.
Finally, I don't see how you could create nice deltas for tree objects
on the fly like you do with blobs without making your code significantly
more complex and use more memory due to the recursive nature of
directory information. This is no a problem for git-pack-objects since
tree objects are no different from blob objects when deltifying them.
Again this is why I think you should focus on the quickest generation of
a reasonable intermediate pack and let git-repack make it optimal.
> Since I just found out that in some cases the Mozilla repository has
> 1000s of revisions per file[*1*] and in others only 1 revision per
> file we probably should be adjusting this depth to have a maximum
> of 500 while also having the frontend send us a "I'm switching
> files now" marker so we know to not even bother trying to delta
> the new blob against the last blob as they are likely to not
> delta well[*2*].
No please don't do that. A deep delta chain won't produce a dramatic
size difference but it _will_ make it really costly to read objects back
from the pack later on. For example it might make the next git-repack
many times longer but the size saving will not be more than 20%. If you
agree with me that a final git-repack is the best thing to do then the
intermediate pack doesn't have to be that small.
> Although complex the aggressive and grouped deltas appears to
> have saved you 18.2% on this repository. That's not something
> to ignore.
But the aggressive delta alone saved 13% already. The remaining saving
is questionable since, like I said, it prevents the delta data reuse
optimization which is so nice in order to reduce the load on servers to
nearly zero.
> A reasonably optimal local pack dictionary could save
> at least 4%[*3*]. Whacking 22% off a 400 MB pack is saving 88 MB.
This is still speculations though. The separate dictionary is certainly
a good idea to try out and it is way more flexible than the grouped
deltas. But I doubt the combined concepts will all add up size
savings linearly.
> Transferring that over the network on an initial clone is like
> downloading all of Eclipse. Or an uncompressed kernel tarball...
... something that most people can afford these days. Plus, not using
grouped objects would make future shallow clones much less costly on the
server as well, while this is something that most people will be most
interested in with such large repos and which has much higher bandwidth
saver value.
Of course this is a trade-off. Tighter packs are likely to be more
costly to use. So far we've contained that cost to the _initial_
generation of the pack (using -a -f with git-repack) while subsequent
repacking for network transfer were basically free on CPU. The grouped
object concept saves around 7 to 11% on pack size in my tests, but it
impose a non negligible cost on any further repacking as well as an
additional cost on the fetching of objects out of the pack.
> [*3*] I wrote a brain-dead simple local dictionary selecter in Perl.
> Its horribly far from being ideal. But it is consistently
> saving us 4% on the GIT and the Mozilla repository and its
> pretty darn fast. Shockingly the C keywords didn't gain
> us very much here; its project specific text that's the
> real win.
>
> Looking at chunks which are frequently copied in deltas
> from base objects and breaking those chunks up into
> smaller common chunks, then loading those most frequent
> common chunks into the pack dictionary would most likely
> produce far better results.
I think such a global dictionary per pack is something that could be
really nice. Again it limits the cost to the initial repack process
while having no cost impact neither on the delta reuse optimization nor
on the object checkout which is not the case for object grouping.
Certainly worth experimenting ... but independently from any import
process I think.
Nicolas
^ permalink raw reply
* [RFD] gitweb: href() function to generate URLs for CGI
From: Jakub Narebski @ 2006-08-21 15:39 UTC (permalink / raw)
To: git
In first version of href() function we had
(commit 06a9d86b49b826562e2b12b5c7e831e20b8f7dce)
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
"$mapping{$_}=$params{$_}"
} keys %params
) );
First, there was a question what happend if someone would enter
parameter name incorrectly, and some key of %params is not found in
%mapping hash. The above code would generate warnings (which web admins
frown upon), and empty (because undef) parameters corresponding to e.g.
mistyped parameter name.
One solution (sweeping under the carpet) would be to use parameter key
as CGI parameter name if it is not found in the %mapping, i.e.
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
if (exists $mapping{$_}) {
"$mapping{$_}=$params{$_}"
} else {
"$_=$params{$_}"
}
} keys %params
) );
Another solution would be to skip parameters which are not found in
%mapping. Correct way to do this is:
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
"$mapping{$_}=$params{$_}"
} grep { exists $mapping{$_} } keys %params
) );
(we cannot put condition in map BLOCK, because map does not filter, only
act on elements, so the result would be empty parameter (e.g. ";;" in
generated URL), I guess without warnings).
Which solutions should be chosen? If the one is chosen, I can send the
patch.
Second problem is that using href() function, although it consolidates
to generate URL for CGI, it changes the order of CGI parameters. It
used to be that 'p' (project) parameter was first, then 'a' (action)
parameter, then hashes ('h', 'hp', 'hb'), last 'f' (filename) or
'p' (page) or 's' (searchtext). The simplest and fastest solution would
be to create array with all keys of %mapping in appropriate order and
do something like this:
my @mapping_sorted = ('project', 'action', 'hash',
'hash_parent', 'hash_base', 'file_name', 'searchtext');
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
"$mapping{$_}=$params{$_}"
} grep { exists $params{$_}} @mapping_sorted;
) );
The problem is of course updating both %mappings and @mapping_sorted.
Is this really a problem, should this (ordering of CGI parameters)
addressed?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* [RFD] gitweb: href() function to generate URLs for CGI
From: Jakub Narebski @ 2006-08-21 15:21 UTC (permalink / raw)
To: git
In first version of href() function we had
(commit 06a9d86b49b826562e2b12b5c7e831e20b8f7dce)
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
"$mapping{$_}=$params{$_}"
} keys %params
) );
First, there was a question what happend if someone would enter parameter
name incorrectly, and some key of %params is not found in %mapping hash.
The above code would generate warnings (which web admins frown upon), and
empty (because undef) parameters corresponding to e.g. mistyped parameter
name.
One solution (sweeping under the carpet) would be to use parameter key as
CGI parameter name if it is not found in the %mapping, i.e.
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
if (exists $mapping{$_}) {
"$mapping{$_}=$params{$_}"
} else {
"$_=$params{$_}"
}
} keys %params
) );
Another solution would be to skip parameters which are not found in
%mapping. Correct way to do this is:
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
"$mapping{$_}=$params{$_}"
} grep { exists $mapping{$_} } keys %params
) );
(we cannot put condition in map BLOCK, because map does not filter, only act
on elements, so the result would be empty parameter (e.g. ";;" in generated
URL), I guess without warnings).
Which solutions should be chosen? If the one is chosen, I can send the
patch.
Second problem is that using href() function, although it consolidates to
generate URL for CGI, it changes the order of CGI parameters. It used to be
that 'p' (project) parameter was first, then 'a' (action) parameter, then
hashes ('h', 'hp', 'hb'), last 'f' (filename) or 'p' (page) or
's' (searchtext). The simplest and fastest solution would be to create
array with all keys of %mapping in appropriate order and do something like
this:
my @mapping_sorted = ('project', 'action', 'hash', 'hash_parent',
'hash_base', 'file_name', 'searchtext');
my $href = "$my_uri?";
$href .= esc_param( join(";",
map {
"$mapping{$_}=$params{$_}"
} grep { exists $params{$_}} @mapping_sorted;
) );
The problem is of course updating both %mappings and @mapping_sorted.
Is this really a problem, should this (ordering of CGI parameters)
addressed?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Recover from a bad push in StGit
From: Robin Rosenberg @ 2006-08-21 15:03 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <tnxk652torl.fsf@arm.com>
måndag 21 augusti 2006 11:35 skrev Catalin Marinas:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > I recently did some reordering of patches and goofed up (not totally, but
> > anyway). I pushed a number of patches and forgot one in the middle
> > resulting in a merge conflict. Pop won't work since I have local changes
> > from the conflict and I don't want to resolve the conflict either since I
> > didn't mean to push the patch at that point.
> >
> > Is there a simple way of undoing a bad push?
> >
> > In this case I had a fresh export do help me out so I could just delete
> > the patches and re-import them again, but what if I didn't?
>
> "stg push --undo" (the same option for "refresh"; maybe I should add a
> separate "undo" command) resets the local changes and it restores the
Not necessarily, but a hint about --undo when push/pop fail would have
helped me.
> original boundaries of the patch (no information is lost). You could
> use the latest snapshot as it has some bug-fixes from release 0.10.
Does refresh --undo make me see the state of the patch like it was before last
refresh, ie. stgit remembers the history of my patces? I thought it dropped
the references as obsolete.
-- robin
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Jon Smirl @ 2006-08-21 14:07 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Nicolas Pitre, git
In-Reply-To: <20060821070609.GC24054@spearce.org>
Mozilla CVS files have between 1 and 1700 deltas. The average is ten,
but the deviation is high. I believe less than 10 files have over 1000
deltas, they are all in the root directory and related to the build
process. Many files have no deltas, or because of CVS artifacts all of
the revisions are identical.
I am still IO bound. Random access IO is the problem, not stream IO. I
have to open and read 110,000 (5GB total) files. It takes about 2hrs
to do all of the IO. I'm not CPU bound yet but as we make things more
efficient, I am getting closer to being CPU bound.
Forking is not an option. It can takes days to fork 1M copies of an
app. I have used oprofile on parsecvs. It spends 60% of the time in
the kernel processing fork calls. Parsecvs runs for 6hrs on mozcvs and
dies without finishing.
I am back to working on the branch code. I'm over the cold I got from
my 2 yr old. It is slow going now. I am in the phase where the import
process runs without error 5-10 minutes and then dies from some
unusual branch case. I fix it up and try again. I am slowly
identifying and removing all the code in cvs2svn that puts the
branches and symbols into their own subdirectories.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Jakub Narebski @ 2006-08-21 10:24 UTC (permalink / raw)
To: git
In-Reply-To: <20060821064659.GB24054@spearce.org>
Shawn Pearce wrote:
> - the strategy of how I'm slamming a very large number of objects
> into a pack may be useful in situations other than a foreign
> SCM import process. I can see someone wanting to create a
> large commit with a lot of modified objects. Doing this with
> update-index and write-tree into loose objects would be far
> slower than just generating a new pack if the number of objects
> you are writing exceeds about 100 on Windows or ~1k on UNIX;
Like e.g. initial import of large project, or incorporating some subproject
into a projects (e.g. gitk and gitweb in git)?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] branch as a builtin (again)
From: Jonas Fonseca @ 2006-08-21 10:13 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <59ad55d30608201422h4a6d40f7y7782212637380438@mail.gmail.com>
Kristian Høgsberg <hoegsberg@gmail.com> wrote Sun, Aug 20, 2006:
> diff --git a/builtin-branch.c b/builtin-branch.c
> new file mode 100644
> index 0000000..25c6313
> --- /dev/null
> +++ b/builtin-branch.c
> @@ -0,0 +1,170 @@
> +/*
> + * Builtin "git branch"
> + *
> + * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
> + * Based on git-branch.sh by Junio C Hamano.
> + */
> +
> +#include "cache.h"
> +#include "refs.h"
> +#include "commit.h"
> +#include "builtin.h"
> +
> +static const char builtin_branch_usage[] =
> + "git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
The norm seems to be that the usage string should not be indented.
> +
> +
> +static int remote_only = 0;
This could be local to cmd_branch.
> +static const char *head;
> +static unsigned char head_sha1[20];
> +
> +static int in_merge_bases(const unsigned char *sha1,
> + struct commit *rev1,
> + struct commit *rev2)
> +{
> + struct commit_list *bases, *b;
> +
> + bases = get_merge_bases(rev1, rev2, 1);
> + for (b = bases; b != NULL; b = b->next) {
> + if (!hashcmp(sha1, b->item->object.sha1)) {
> + free_commit_list(bases);
> + return 1;
> + }
> + }
> +
> + free_commit_list(bases);
> + return 0;
> +}
> +
> +static void delete_branches(int argc, const char **argv, int force)
> +{
> + struct commit *rev1, *rev2;
> + unsigned char sha1[20];
> + const char *p, *name;
> + int i;
> +
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(head, argv[i]))
> + die("Cannot delete the branch you are currently on.");
> +
> + name = git_path("refs/heads/%s", argv[i]);
> + p = resolve_ref(name, sha1, 1);
> + if (p == NULL)
> + die("Branch '%s' not found.", argv[i]);
> +
> + rev1 = lookup_commit_reference(sha1);
> + rev2 = lookup_commit_reference(head_sha1);
> + if (!rev1 || !rev2)
> + die("Couldn't look up commit objects.");
> +
> + /* This checks wether the merge bases of branch and
whether
> + * HEAD contains branch -- which means that the HEAD
> + * contains everything in both.
> + */
> +
> + if (!force &&
> + !in_merge_bases(sha1, rev1, rev2)) {
> + fprintf(stderr,
> + "The branch '%s' is not a strict subset of your current HEAD.\n"
> + "If you are sure you want to delete it, run 'git branch -D %s'.\n",
> + argv[i], argv[i]);
> + exit(1);
> + }
> +
> + unlink(name);
> + printf("Deleted branch %s.\n", argv[i]);
> + }
> +}
> +
> +static int show_reference(const char *refname, const unsigned char *sha1)
> +{
> + int is_head = !strcmp(refname, head);
> +
> + printf("%c %s\n", (is_head ? '*' : ' '), refname);
> +
> + return 0;
> +}
> +
> +static void create_branch (const char *name, const char *start, int force)
> +{
> + struct ref_lock *lock;
> + unsigned char sha1[20];
> + char ref[PATH_MAX];
> +
> + snprintf(ref, sizeof ref, "refs/heads/%s", name);
> + if (check_ref_format(ref + 5))
> + die("'%s' is not a valid branch name.", name);
Why not simply check_ref_format(name)?
> +
> + if (resolve_ref(ref, sha1, 1)) {
All other places that call resolve_ref passes a ref created with
git_path. I don't know if this should too.
> + if (!force)
> + die("A branch named '%s' already exists.", name);
> + else if (!strcmp(head, name))
> + die("Cannot force update the current branch.");
> + }
> +
> + if (get_sha1(start, sha1))
> + die("Not a valid branch point: '%s'", start);
Missing punctuation at the end.
> +
> + lock = lock_any_ref_for_update(ref, NULL, 0);
> + if (!lock)
> + die("Failed to lock ref for update: %s.", strerror(errno));
> + if (write_ref_sha1(lock, sha1, NULL) < 0)
> + die("Failed to write ref: %s.", strerror(errno));
> +}
> +
> +int cmd_branch(int argc, const char **argv, const char *prefix)
> +{
> + int delete = 0, force_delete = 0, force_create = 0;
> + int i, prefix_length;
> + const char *p;
> +
> + git_config(git_default_config);
> +
> + for (i = 1; i < argc; i++) {
> + const char *arg = argv[i];
> +
> + if (arg[0] != '-')
> + break;
> + if (!strcmp(arg, "--")) {
> + i++;
> + break;
> + }
> + if (!strcmp(arg, "-d")) {
> + delete = 1;
> + continue;
> + }
> + if (!strcmp(arg, "-D")) {
> + delete = 1;
> + force_delete = 1;
> + continue;
> + }
> + if (!strcmp(arg, "-f")) {
> + force_create = 1;
> + continue;
> + }
> + if (!strcmp(arg, "-r")) {
> + remote_only = 1;
> + continue;
> + }
> + die(builtin_branch_usage);
Perhaps usage() would be more appropriate here.
> + }
> +
> + prefix_length = strlen(git_path("refs/heads/"));
> + p = resolve_ref(git_path("HEAD"), head_sha1, 0);
> + if (!p)
> + die("Failed to resolve HEAD as a valid ref");
Ending punctuation.
> + head = strdup(p + prefix_length);
> +
> + if (delete)
> + delete_branches(argc - i, argv + i, force_delete);
> + else if (i == argc && remote_only)
> + for_each_remote_ref(show_reference);
> + else if (i == argc)
> + for_each_branch_ref(show_reference);
> + else if (argc - i == 1)
> + create_branch (argv[i], head, force_create);
> + else
> + create_branch (argv[i], argv[i + 1], force_create);
It would be more consistent to leave out the space before the
paranthesis. Also goes for the implementation of create_branch as
already mentioned.
> +
> + return 0;
> +}
--
Jonas Fonseca
^ permalink raw reply
* Re: non-monotonic index when using http://?
From: Junio C Hamano @ 2006-08-21 10:08 UTC (permalink / raw)
To: Martin Waitz; +Cc: git
In-Reply-To: <20060821095419.GK30022@admingilde.org>
Martin Waitz <tali@admingilde.org> writes:
> For some reason, I have a
> .git/objects/pack/pack-793a9e93286d6c656941977d2e5b49e28566edcd.idx
> without the corresponding .pack file.
It is worrysome why. In the meantime you could remove it (if it
were a .pack file without .idx then you may want to try feeding
it to unpack-objects but .idx without corresponding .pack is
pretty much useless).
> At least, it should not SEGV later...
Absolutely. The particular code in http-fetch (your gdb session
tells us exactly where) seems quite loosely written, and there
may be other places that do not check return values.
^ permalink raw reply
* Re: non-monotonic index when using http://?
From: Martin Waitz @ 2006-08-21 9:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64gmwil5.fsf@assigned-by-dhcp.cox.net>
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
On Mon, Aug 21, 2006 at 02:20:38AM -0700, Junio C Hamano wrote:
> The "non monotonic index" check is to make sure pack .idx file
> is sane, and it appears that you are getting the error before
> you fetch new pack from the server, which means one of your
> local packs is bad.
You are right, the local archive is corrupted, it works with
a fresh clone.
For some reason, I have a
.git/objects/pack/pack-793a9e93286d6c656941977d2e5b49e28566edcd.idx
without the corresponding .pack file.
Has anybody seen something like this before?
Should GIT bail out earlier when it sees this non-monotonic-index
corruption?
At least, it should not SEGV later...
--
Martin Waitz
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Recover from a bad push in StGit
From: Catalin Marinas @ 2006-08-21 9:35 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200608190030.47257.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> I recently did some reordering of patches and goofed up (not totally, but
> anyway). I pushed a number of patches and forgot one in the middle resulting
> in a merge conflict. Pop won't work since I have local changes from the
> conflict and I don't want to resolve the conflict either since I didn't mean
> to push the patch at that point.
>
> Is there a simple way of undoing a bad push?
>
> In this case I had a fresh export do help me out so I could just delete the
> patches and re-import them again, but what if I didn't?
"stg push --undo" (the same option for "refresh"; maybe I should add a
separate "undo" command) resets the local changes and it restores the
original boundaries of the patch (no information is lost). You could
use the latest snapshot as it has some bug-fixes from release 0.10.
--
Catalin
^ permalink raw reply
* Re: non-monotonic index when using http://?
From: Junio C Hamano @ 2006-08-21 9:20 UTC (permalink / raw)
To: Martin Waitz; +Cc: git
In-Reply-To: <20060821084606.GI30022@admingilde.org>
Martin Waitz <tali@admingilde.org> writes:
> hoi :)
>
> At work I'm getting the following error message, any ideas?
>
>> git --version
> git version 1.4.2.rc4.g242a
>> git pull upstream
> Fetching refs/heads/master from http://git.kernel.org/pub/scm/git/git.git using http
> error: non-monotonic index
> walk 43134fcb350fb70d26634be163db1f71c14db19f
> walk 55c3eb434ab6d489c632263239be15a1054df7f2
> Getting alternates list for http://git.kernel.org/pub/scm/git/git.git/
> Getting pack list for http://git.kernel.org/pub/scm/git/git.git/
> error: non-monotonic index
The "non monotonic index" check is to make sure pack .idx file
is sane, and it appears that you are getting the error before
you fetch new pack from the server, which means one of your
local packs is bad.
I just cloned from the kernel.org archive over http (I usually
don't, but your report was alarming enough) and the packfile I
have there do not seem to have the problem.
^ permalink raw reply
* non-monotonic index when using http://?
From: Martin Waitz @ 2006-08-21 8:46 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]
hoi :)
At work I'm getting the following error message, any ideas?
> git --version
git version 1.4.2.rc4.g242a
> git pull upstream
Fetching refs/heads/master from http://git.kernel.org/pub/scm/git/git.git using http
error: non-monotonic index
walk 43134fcb350fb70d26634be163db1f71c14db19f
walk 55c3eb434ab6d489c632263239be15a1054df7f2
Getting alternates list for http://git.kernel.org/pub/scm/git/git.git/
Getting pack list for http://git.kernel.org/pub/scm/git/git.git/
error: non-monotonic index
/home/tcmc_mwz/i686/bin/git-fetch: line 402: 2077 Segmentation fault
git-http-fetch -v -a "$head" "$remote/"
zsh: exit 1 git pull upstream
> gdb git-http-fetch
GNU gdb Red Hat Linux (6.3.0.0-1.132.EL3rh)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i386-redhat-linux-gnu"...Using host libthread_db library "/lib/tls/libthread_db.so.1".
(gdb) run -v -a 43134fcb350fb70d26634be163db1f71c14db19f http://git.kernel.org/pub/scm/git/git.git/
Starting program: /autohome/tcmc_mwz/src/git/git-http-fetch -v -a 43134fcb350fb70d26634be163db1f71c14db19f http://git.kernel.org/pub/scm/git/git.git/
error: non-monotonic index
walk 43134fcb350fb70d26634be163db1f71c14db19f
walk 55c3eb434ab6d489c632263239be15a1054df7f2
Getting alternates list for http://git.kernel.org/pub/scm/git/git.git/
Getting pack list for http://git.kernel.org/pub/scm/git/git.git/
error: non-monotonic index
Program received signal SIGSEGV, Segmentation fault.
0x0804c1e2 in setup_index (repo=0x991a058,
sha1=0xbfffa420 "y:\236\223(mleiA\227}.[Iï¿¿\205fï¿¿ï¿¿") at http-fetch.c:501
501 new_pack->next = repo->packs;
(gdb) bt
#0 0x0804c1e2 in setup_index (repo=0x991a058,
sha1=0xbfffa420 "y:\236\223(mleiA\227}.[Iï¿¿\205fï¿¿ï¿¿") at http-fetch.c:501
#1 0x0804d17c in fetch_indices (repo=0x991a058) at http-fetch.c:946
#2 0x0804d217 in fetch_pack (repo=0x991a058,
sha1=0x994eb9c "Uï¿¿\230Dï¿¿\036 &p\aihï¿¿\rï¿¿\017ï¿¿hs.ï¿¿") at http-fetch.c:978
#3 0x0804d7f6 in fetch (sha1=0x994eb9c "Uï¿¿\230Dï¿¿\036 &p\aihï¿¿\rï¿¿\017ï¿¿hs.ï¿¿")
at http-fetch.c:1130
#4 0x0804a414 in loop () at fetch.c:179
#5 0x0804a77b in pull (targets=1, target=0xbfffb600, write_ref=0x0,
write_ref_log_details=0xbfffd3b9 "http://git.kernel.org/pub/scm/git/git.git/") at fetch.c:288
#6 0x0804dbd7 in main (argc=5, argv=0xbfffb5f4) at http-fetch.c:1282
--
Martin Waitz
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] branch as a builtin (again)
From: Shawn Pearce @ 2006-08-21 8:03 UTC (permalink / raw)
To: hoegsberg; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0608210045450.17336@chino.corp.google.com>
> On Sun, 20 Aug 2006, Kristian Høgsberg wrote:
> > diff --git a/builtin-branch.c b/builtin-branch.c
> > +static const char builtin_branch_usage[] =
> > + "git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
What happened to the -l switch to create a reflog for the new branch?
> > +static void delete_branches(int argc, const char **argv, int force)
What happened to deleting the reflog when the branch gets deleted?
In both cases please see the existing git-branch documentation and
shell script.
I applaud the effort of removing some of these smaller shell scripts
with more portable C code but at the same time I'd hate to see a
loss of functionality, especially something that I use! :-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Verify we know how to read a pack before trying to using it.
From: Junio C Hamano @ 2006-08-21 8:00 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20060821071002.GA26029@spearce.org>
I agree we would need this check somewhere, but I am wondering
if we might be better off doing this where we verify pack-idx
pair, e.g. add_packed_git().
^ permalink raw reply
* Re: [PATCH] branch as a builtin (again)
From: David Rientjes @ 2006-08-21 7:49 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <59ad55d30608201422h4a6d40f7y7782212637380438@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2958 bytes --]
On Sun, 20 Aug 2006, Kristian Høgsberg wrote:
> diff --git a/builtin-branch.c b/builtin-branch.c
> new file mode 100644
> index 0000000..25c6313
> --- /dev/null
> +++ b/builtin-branch.c
> @@ -0,0 +1,170 @@
> +/*
> + * Builtin "git branch"
> + *
> + * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
> + * Based on git-branch.sh by Junio C Hamano.
> + */
> +
> +#include "cache.h"
> +#include "refs.h"
> +#include "commit.h"
> +#include "builtin.h"
> +
> +static const char builtin_branch_usage[] =
> + "git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
> +
> +
> +static int remote_only = 0;
Unnecessary initialization
> +static const char *head;
> +static unsigned char head_sha1[20];
> +
> +static int in_merge_bases(const unsigned char *sha1,
> + struct commit *rev1,
> + struct commit *rev2)
> +{
> + struct commit_list *bases, *b;
> +
> + bases = get_merge_bases(rev1, rev2, 1);
> + for (b = bases; b != NULL; b = b->next) {
> + if (!hashcmp(sha1, b->item->object.sha1)) {
> + free_commit_list(bases);
> + return 1;
> + }
> + }
> +
> + free_commit_list(bases);
> + return 0;
Make it cleaner for the future:
{
int ret = 0;
...
for (b = bases; b; b = b->next) {
if (!hashcmp(sha1, b->item->object.sha1)) {
ret = 1;
break;
}
}
free_commit_list(bases);
return ret;
}
> +}
> +
> +static void delete_branches(int argc, const char **argv, int force)
> +{
> + struct commit *rev1, *rev2;
> + unsigned char sha1[20];
> + const char *p, *name;
> + int i;
> +
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(head, argv[i]))
> + die("Cannot delete the branch you are currently on.");
> +
> + name = git_path("refs/heads/%s", argv[i]);
> + p = resolve_ref(name, sha1, 1);
> + if (p == NULL)
> + die("Branch '%s' not found.", argv[i]);
p is unnecessary:
if (!resolve_ref(name, sha1, 1))
die(...);
> +
> + rev1 = lookup_commit_reference(sha1);
> + rev2 = lookup_commit_reference(head_sha1);
Are these both needed inside every iteration?
> + if (!rev1 || !rev2)
> + die("Couldn't look up commit objects.");
> +
> + /* This checks wether the merge bases of branch and
> + * HEAD contains branch -- which means that the HEAD
> + * contains everything in both.
> + */
> +
> + if (!force &&
> + !in_merge_bases(sha1, rev1, rev2)) {
> + fprintf(stderr,
> + "The branch '%s' is not a strict subset of your current HEAD.\n"
> + "If you are sure you want to delete it, run 'git branch -D %s'.\n",
> + argv[i], argv[i]);
> + exit(1);
> + }
> +
> + unlink(name);
> + printf("Deleted branch %s.\n", argv[i]);
> + }
> +}
> +
> +static int show_reference(const char *refname, const unsigned char *sha1)
> +{
> + int is_head = !strcmp(refname, head);
> +
> + printf("%c %s\n", (is_head ? '*' : ' '), refname);
> +
> + return 0;
Unnecessary variable
> +}
> +
> +static void create_branch (const char *name, const char *start, int force)
No space
David
^ permalink raw reply
* [PATCH] Remove unnecessary forward declaration of unpack_entry.
From: Shawn Pearce @ 2006-08-21 7:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This declaration probably used to be necessary but the code has
been refactored since to use unpack_entry_gently instead.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
sha1_file.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index bb9fa8b..b1c88de 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1075,9 +1075,6 @@ static int packed_object_info(struct pac
return 0;
}
-/* forward declaration for a mutually recursive function */
-static void *unpack_entry(struct pack_entry *, char *, unsigned long *);
-
static void *unpack_delta_entry(unsigned char *base_sha1,
unsigned long delta_size,
unsigned long left,
--
1.4.2.g0dcb
^ permalink raw reply related
* [PATCH] Verify we know how to read a pack before trying to using it.
From: Shawn Pearce @ 2006-08-21 7:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If the pack format were to ever change or be extended in the future
there is no assurance that just because the pack file lives in
objects/pack and doesn't end in .idx that we can read and decompress
its contents properly.
If we encounter what we think is a pack file and it isn't or we don't
recognize its version then die and suggest to the user that they
upgrade to a newer version of GIT which can handle that pack file.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
sha1_file.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index d6eb9b3..bb9fa8b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -481,6 +481,7 @@ int use_packed_git(struct packed_git *p)
int fd;
struct stat st;
void *map;
+ struct pack_header *hdr;
pack_mapped += p->pack_size;
while (PACK_MAX_SZ < pack_mapped && unuse_one_packed_git())
@@ -500,6 +501,17 @@ int use_packed_git(struct packed_git *p)
die("packfile %s cannot be mapped.", p->pack_name);
p->pack_base = map;
+ /* Check if we understand this pack file. If we don't we're
+ * likely too old to handle it.
+ */
+ hdr = map;
+ if (hdr->hdr_signature != htonl(PACK_SIGNATURE))
+ die("packfile %s isn't actually a pack.", p->pack_name);
+ if (!pack_version_ok(hdr->hdr_version))
+ die("packfile %s is version %i and not supported"
+ " (try upgrading GIT to a newer version)",
+ p->pack_name, ntohl(hdr->hdr_version));
+
/* Check if the pack file matches with the index file.
* this is cheap.
*/
--
1.4.2.g0dcb
^ permalink raw reply related
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