* [PATCH 4/8] git-gc --auto: move threshold check to need_to_gc() function.
From: Junio C Hamano @ 2007-09-17 8:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <11900186941912-git-send-email-gitster@pobox.com>
That is where we decide if we are going to run gc
automatically.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-gc.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin-gc.c b/builtin-gc.c
index 093b3dd..f046a2a 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -80,6 +80,13 @@ static int need_to_gc(void)
int num_loose = 0;
int needed = 0;
+ /*
+ * Setting gc.auto to 0 or negative can disable the
+ * automatic gc
+ */
+ if (gc_auto_threshold <= 0)
+ return 0;
+
if (sizeof(path) <= snprintf(path, sizeof(path), "%s/17", objdir)) {
warning("insanely long object directory %.*s", 50, objdir);
return 0;
@@ -129,8 +136,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
continue;
}
if (!strcmp(arg, "--auto")) {
- if (gc_auto_threshold <= 0)
- return 0;
auto_gc = 1;
continue;
}
--
1.5.3.1.967.g6bb01
^ permalink raw reply related
* [PATCH 5/8] git-gc --auto: add documentation.
From: Junio C Hamano @ 2007-09-17 8:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <11900186941912-git-send-email-gitster@pobox.com>
This documents the auto-packing of loose objects performed by
git-gc --auto.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 7 +++++++
Documentation/git-gc.txt | 11 ++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 866e053..3643c0b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -439,6 +439,13 @@ gc.aggressiveWindow::
algorithm used by 'git gc --aggressive'. This defaults
to 10.
+gc.auto::
+ When there are approximately more than this many loose
+ objects in the repository, `git gc --auto` that is
+ invoked by some Porcelain commands will create a new
+ pack and prune them. Setting this to 0 disables the
+ auto garbage collection.
+
gc.packrefs::
`git gc` does not run `git pack-refs` in a bare repository by
default so that older dumb-transport clients can still fetch
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index c7742ca..40c1ce4 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -8,7 +8,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository
SYNOPSIS
--------
-'git-gc' [--prune] [--aggressive]
+'git-gc' [--prune] [--aggressive] [--auto]
DESCRIPTION
-----------
@@ -43,6 +43,15 @@ OPTIONS
persistent, so this option only needs to be used occasionally; every
few hundred changesets or so.
+--auto::
+ With this option, `git gc` checks if there are too many
+ loose objects in the repository and runs
+ gitlink:git-repack[1] with `-d -l` option to pack them.
+ The threshold is set with `gc.auto` configuration
+ variable, and can be disabled by setting it to 0. Some
+ Porcelain commands use this after they perform operation
+ that could create many loose objects automatically.
+
Configuration
-------------
--
1.5.3.1.967.g6bb01
^ permalink raw reply related
* [PATCH 3/8] repack -A -d: use --keep-unreachable when repacking
From: Junio C Hamano @ 2007-09-17 8:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <11900186941912-git-send-email-gitster@pobox.com>
This is a safer variant of "repack -a -d" that does not drop
unreachable objects that are in packs.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-repack.sh | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/git-repack.sh b/git-repack.sh
index 156c5e8..204084e 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -3,17 +3,19 @@
# Copyright (c) 2005 Linus Torvalds
#
-USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'
+USAGE='[-a|-A] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'
SUBDIRECTORY_OK='Yes'
. git-sh-setup
-no_update_info= all_into_one= remove_redundant=
+no_update_info= all_into_one= remove_redundant= keep_unreachable=
local= quiet= no_reuse= extra=
while case "$#" in 0) break ;; esac
do
case "$1" in
-n) no_update_info=t ;;
-a) all_into_one=t ;;
+ -A) all_into_one=t
+ keep_unreachable=t ;;
-d) remove_redundant=t ;;
-q) quiet=-q ;;
-f) no_reuse=--no-reuse-object ;;
@@ -59,7 +61,13 @@ case ",$all_into_one," in
fi
done
fi
- [ -z "$args" ] && args='--unpacked --incremental'
+ if test -z "$args"
+ then
+ args='--unpacked --incremental'
+ elif test -n "$keep_unreachable"
+ then
+ args="$args --keep-unreachable"
+ fi
;;
esac
--
1.5.3.1.967.g6bb01
^ permalink raw reply related
* [PATCH 2/8] pack-objects --keep-unreachable
From: Junio C Hamano @ 2007-09-17 8:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <11900186941912-git-send-email-gitster@pobox.com>
This new option is meant to be used in conjunction with the
options "git repack -a -d" usually invokes the underlying
pack-objects. When this option is given, objects unreachable
from the refs in packs named with --unpacked= option are added
to the resulting pack, in addition to the reachable objects that
are not in packs marked with *.keep files.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-pack-objects.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 12509fa..ba7c8da 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -21,7 +21,7 @@ git-pack-objects [{ -q | --progress | --all-progress }] \n\
[--window=N] [--window-memory=N] [--depth=N] \n\
[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \n\
[--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n\
- [--stdout | base-name] [<ref-list | <object-list]";
+ [--stdout | base-name] [--keep-unreachable] [<ref-list | <object-list]";
struct object_entry {
struct pack_idx_entry idx;
@@ -57,7 +57,7 @@ static struct object_entry **written_list;
static uint32_t nr_objects, nr_alloc, nr_result, nr_written;
static int non_empty;
-static int no_reuse_delta, no_reuse_object;
+static int no_reuse_delta, no_reuse_object, keep_unreachable;
static int local;
static int incremental;
static int allow_ofs_delta;
@@ -1625,15 +1625,19 @@ static void read_object_list_from_stdin(void)
}
}
+#define OBJECT_ADDED (1u<<20)
+
static void show_commit(struct commit *commit)
{
add_object_entry(commit->object.sha1, OBJ_COMMIT, NULL, 0);
+ commit->object.flags |= OBJECT_ADDED;
}
static void show_object(struct object_array_entry *p)
{
add_preferred_base_object(p->name);
add_object_entry(p->item->sha1, p->item->type, p->name, 0);
+ p->item->flags |= OBJECT_ADDED;
}
static void show_edge(struct commit *commit)
@@ -1641,6 +1645,86 @@ static void show_edge(struct commit *commit)
add_preferred_base(commit->object.sha1);
}
+struct in_pack_object {
+ off_t offset;
+ struct object *object;
+};
+
+struct in_pack {
+ int alloc;
+ int nr;
+ struct in_pack_object *array;
+};
+
+static void mark_in_pack_object(struct object *object, struct packed_git *p, struct in_pack *in_pack)
+{
+ in_pack->array[in_pack->nr].offset = find_pack_entry_one(object->sha1, p);
+ in_pack->array[in_pack->nr].object = object;
+ in_pack->nr++;
+}
+
+/*
+ * Compare the objects in the offset order, in order to emulate the
+ * "git-rev-list --objects" output that produced the pack originally.
+ */
+static int ofscmp(const void *a_, const void *b_)
+{
+ struct in_pack_object *a = (struct in_pack_object *)a_;
+ struct in_pack_object *b = (struct in_pack_object *)b_;
+
+ if (a->offset < b->offset)
+ return -1;
+ else if (a->offset > b->offset)
+ return 1;
+ else
+ return hashcmp(a->object->sha1, b->object->sha1);
+}
+
+static void add_objects_in_unpacked_packs(struct rev_info *revs)
+{
+ struct packed_git *p;
+ struct in_pack in_pack;
+ uint32_t i;
+
+ memset(&in_pack, 0, sizeof(in_pack));
+
+ for (p = packed_git; p; p = p->next) {
+ const unsigned char *sha1;
+ struct object *o;
+
+ for (i = 0; i < revs->num_ignore_packed; i++) {
+ if (matches_pack_name(p, revs->ignore_packed[i]))
+ break;
+ }
+ if (revs->num_ignore_packed <= i)
+ continue;
+ if (open_pack_index(p))
+ die("cannot open pack index");
+
+ ALLOC_GROW(in_pack.array,
+ in_pack.nr + p->num_objects,
+ in_pack.alloc);
+
+ for (i = 0; i < p->num_objects; i++) {
+ sha1 = nth_packed_object_sha1(p, i);
+ o = lookup_unknown_object(sha1);
+ if (!(o->flags & OBJECT_ADDED))
+ mark_in_pack_object(o, p, &in_pack);
+ o->flags |= OBJECT_ADDED;
+ }
+ }
+
+ if (in_pack.nr) {
+ qsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),
+ ofscmp);
+ for (i = 0; i < in_pack.nr; i++) {
+ struct object *o = in_pack.array[i].object;
+ add_object_entry(o->sha1, o->type, "", 0);
+ }
+ }
+ free(in_pack.array);
+}
+
static void get_object_list(int ac, const char **av)
{
struct rev_info revs;
@@ -1672,6 +1756,9 @@ static void get_object_list(int ac, const char **av)
prepare_revision_walk(&revs);
mark_edges_uninteresting(revs.commits, &revs, show_edge);
traverse_commit_list(&revs, show_commit, show_object);
+
+ if (keep_unreachable)
+ add_objects_in_unpacked_packs(&revs);
}
static int adjust_perm(const char *path, mode_t mode)
@@ -1789,6 +1876,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
use_internal_rev_list = 1;
continue;
}
+ if (!strcmp("--keep-unreachable", arg)) {
+ keep_unreachable = 1;
+ continue;
+ }
if (!strcmp("--unpacked", arg) ||
!prefixcmp(arg, "--unpacked=") ||
!strcmp("--reflog", arg) ||
--
1.5.3.1.967.g6bb01
^ permalink raw reply related
* [PATCH 1/8] Export matches_pack_name() and fix its return value
From: Junio C Hamano @ 2007-09-17 8:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
The function sounds boolean; make it behave as one, not "0 for
success, non-zero for failure".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cache.h | 1 +
sha1_file.c | 14 +++++++-------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 70abbd5..3fa5b8e 100644
--- a/cache.h
+++ b/cache.h
@@ -529,6 +529,7 @@ extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsign
extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);
+extern int matches_pack_name(struct packed_git *p, const char *name);
/* Dumb servers support */
extern int update_server_info(int);
diff --git a/sha1_file.c b/sha1_file.c
index 9978a58..5801c3e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1684,22 +1684,22 @@ off_t find_pack_entry_one(const unsigned char *sha1,
return 0;
}
-static int matches_pack_name(struct packed_git *p, const char *ig)
+int matches_pack_name(struct packed_git *p, const char *name)
{
const char *last_c, *c;
- if (!strcmp(p->pack_name, ig))
- return 0;
+ if (!strcmp(p->pack_name, name))
+ return 1;
for (c = p->pack_name, last_c = c; *c;)
if (*c == '/')
last_c = ++c;
else
++c;
- if (!strcmp(last_c, ig))
- return 0;
+ if (!strcmp(last_c, name))
+ return 1;
- return 1;
+ return 0;
}
static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, const char **ignore_packed)
@@ -1717,7 +1717,7 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, cons
if (ignore_packed) {
const char **ig;
for (ig = ignore_packed; *ig; ig++)
- if (!matches_pack_name(p, *ig))
+ if (matches_pack_name(p, *ig))
break;
if (*ig)
goto next;
--
1.5.3.1.967.g6bb01
^ permalink raw reply related
* [PATCH 0/8] Updated git-gc --auto series.
From: Junio C Hamano @ 2007-09-17 8:43 UTC (permalink / raw)
To: git
An updated series of "git-gc --auto", on top of 'next',
consisting of 8 patches, will follow this message.
Differences from the previous round are:
- Earlier if you have too many unreachable loose objects,
automated gc would have tried to "repack -d -l" which would
not improve the situation at all every time it was run. It
now at least warns upon such a situation;
- pack-objects learned --keep-unreachable option which helps
"repack -a -d" not to lose packed but unreachable objects
while repacking existing packs into a new pack;
- repack learned -A option which is similar to -a but gives
--keep-unreachable to underlying pack-objects;
- "git-gc --auto" runs "git-repack -A -d -l" when there are too
many packs in the repository;
- These changes are documented ;-)
^ permalink raw reply
* Re: [StGit PATCH 00/13] Eliminate 'top' and 'bottom' files
From: Karl Hasselström @ 2007-09-17 8:17 UTC (permalink / raw)
To: Catalin Marinas; +Cc: David Kågedal, git
In-Reply-To: <b0943d9e0709160028h41a67474g6b379a45c4c88432@mail.gmail.com>
On 2007-09-16 08:28:53 +0100, Catalin Marinas wrote:
> We should get rid of top.old and bottom.old as well.
Yeah, I guess that data could be computed from the patch log?
> My question - does this conflict with the DAG patches in any way? I
> intend to include the them at some point, once I get a chance to
> test the performance penalty with a big tree like the Linux kernel.
I haven't been able to get rid of all the expensive DAG walking, so
I've been considering a different approach: continue using the current
applied and unapplied files if the existing HEAD == top patch check
passes, and letting the assimilate command do a full DAG walk to
regenerate those files. (And by "full DAG walk", I mean walking from
HEAD down to the first commit with parents != 1; the patches we see
are applied (in the order we see them), and the rest are unapplied.)
> Is there any patch which consists of more than one commit? Maybe
> only uncommit could generate one but I think we put some tests in
> place.
Uncommit does not generate such patches, unless I've made a thinko; I
don't approve of them. But I always assumed they could exist, since
the code (at least in places) seems careful to not assume anything
about the number of commits between top and bottom.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: RFC: German translation vocabulary
From: Alexander Wuerstlein @ 2007-09-17 7:54 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <851wcy47n4.fsf@lola.goethe.zz>
On 070916 23:34, David Kastrup <dak@gnu.org> wrote:
> Alexander Wuerstlein <snalwuer@cip.informatik.uni-erlangen.de> writes:
>
> > On 070916 14:46, Christian Stimming <stimming@tuhh.de> wrote:
> >> msgid "commit [noun]"
> >> msgstr "?bertragung (Sendung?, ?bergabe?, Einspielung?, Ablagevorgang?)"
> >
> > "Vorgang"? (think Beamtendeutsch)
>
> Buchung, Einbuchung, Verbuchung, Registrierung?
Transaktion?
Ciao,
Alexander Wuerstlein.
^ permalink raw reply
* Re: [StGit PATCH 13/13] Remove the 'top' field
From: Karl Hasselström @ 2007-09-17 7:30 UTC (permalink / raw)
To: David Kågedal; +Cc: git, catalin.marinas
In-Reply-To: <CD668999-4CD3-416C-9205-CEB46FFA2398@lysator.liu.se>
On 2007-09-16 12:22:28 +0200, David Kågedal wrote:
> 16 sep 2007 kl. 01.36 skrev Karl Hasselström:
>
> > And remove the top file, maybe? (Or I may be mistaken; I don't
> > have a copy of the surrounding code handy.)
>
> No, this is the code that updates from version 0 to version 1. The
> problem was that the update functionality used the update_top_ref()
> function in the Patch class which I changed. So I had to inline the
> code instead.
Ah. Right, you did say that you hadn't built an update function yet.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: metastore
From: Junio C Hamano @ 2007-09-17 6:06 UTC (permalink / raw)
To: david
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709162126380.24221@asgard.lang.hm>
david@lang.hm writes:
> On Sun, 16 Sep 2007, Junio C Hamano wrote:
>
>> I also need to rant here a bit.
>>
>> Fortunately we haven't had this problem too many times on this
>> list, but sometimes people say "Here is my patch. If this is
>> accepted I'll add documentation and tests". I rarely reply to
>> such patches without sugarcoating my response, but my internal
>> reaction is, "Don't you, as the person who proposes that change,
>> believe in your patch deeply enough to be willing to perfect it,
>> in order to make it suitable for consumption by the general
>> public, whether it is included in my tree or not? A change that
>> even you do not believe in yourself has very little chance of
>> benefitting the general public, so thanks but no thanks, I'll
>> pass."
>
> I hope that my questions did not seem to fall into this catagory.
Not at all.
^ permalink raw reply
* Re: [PATCH] New strbuf APIs: splice and attach.
From: Florian Weimer @ 2007-09-17 5:43 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070916205136.GE26457@artemis.corp>
* Pierre Habouzit:
> On Sun, Sep 16, 2007 at 08:20:06PM +0000, Florian Weimer wrote:
>> * Pierre Habouzit:
>>
>> > +void strbuf_grow(struct strbuf *sb, size_t extra)
>> > +{
>> > if (sb->len + extra + 1 <= sb->len)
>> > die("you want to use way too much memory");
>>
>> By the way, this comparison is always false because sb->len is signed.
>
> News to me. Actually it's not, it's a size_t :)
Ah, then this has changed somewhere. It used to be int. Good.
^ permalink raw reply
* Re: Blaming diffs
From: Junio C Hamano @ 2007-09-17 5:41 UTC (permalink / raw)
To: Christian Couder; +Cc: Shawn O. Pearce, Mike Hommey, git
In-Reply-To: <200709170740.00917.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Le lundi 17 septembre 2007, Shawn O. Pearce a écrit :
>> Christian Couder <chriscool@tuxfamily.org> wrote:
>> > I don't know if that's what you are looking for but perhaps you could
>> > use "git bisect run". You just need to pass it a script that returns 1
>> > when it finds the changes and 0 otherwise. (See git-bisect man page.)
>>
>> That's very inefficient to search for something...
>
> Perhaps but you can search using whatever script or command you want/know.
> You are not limited by those implemented in git.
>
> You can also make it more efficient with "git bisect {start,good,bad}".
I _think_ the inefficiency Shawn refers to is that "git bisect"
wrapper inherently is based on checking out the revision. It is
similar to "filter-branch --tree-filter" being much more
inefficient than "filter-branch --index-filter" (the latter only
works with index while the former does a full checkout).
The underlying "git rev-list --bisect" can be used to ask for
sequence of commits to check if your check does not require a
full checkout, but there is no wrapper like "git bisect" that
uses that mode of operation.
^ permalink raw reply
* Re: Blaming diffs
From: Christian Couder @ 2007-09-17 5:40 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Mike Hommey, git
In-Reply-To: <20070917045704.GH3099@spearce.org>
Le lundi 17 septembre 2007, Shawn O. Pearce a écrit :
> Christian Couder <chriscool@tuxfamily.org> wrote:
> > I don't know if that's what you are looking for but perhaps you could
> > use "git bisect run". You just need to pass it a script that returns 1
> > when it finds the changes and 0 otherwise. (See git-bisect man page.)
>
> That's very inefficient to search for something...
Perhaps but you can search using whatever script or command you want/know.
You are not limited by those implemented in git.
You can also make it more efficient with "git bisect {start,good,bad}".
Regards,
Christian.
^ permalink raw reply
* Re: Blaming diffs
From: Shawn O. Pearce @ 2007-09-17 4:57 UTC (permalink / raw)
To: Christian Couder; +Cc: Mike Hommey, git
In-Reply-To: <200709170659.15655.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> wrote:
> Le dimanche 16 septembre 2007, Mike Hommey a écrit :
> >
> > It seems to me there is no tool to "blame diffs", i.e. something to know
> > what commit(s) is(are) responsible for a set of changes.
>
> I don't know if that's what you are looking for but perhaps you could
> use "git bisect run". You just need to pass it a script that returns 1 when
> it finds the changes and 0 otherwise. (See git-bisect man page.)
That's very inefficient to search for something...
> Sometimes ago I sent a patch that would allow "!" after "git bisect run",
> but it seems to have been forgotten. This patch makes it possible to use:
>
> git bisect run ! grep some_stuff file1 file2...
>
> This would give you the commit where some_stuff was introduced in file1 or
> file2...
Is `git log -Ssome_stuff -- file1 file2` somehow not working for you?
--
Shawn.
^ permalink raw reply
* Re: Blaming diffs
From: Christian Couder @ 2007-09-17 4:59 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20070916163829.GA6679@glandium.org>
Le dimanche 16 septembre 2007, Mike Hommey a écrit :
> Hi,
>
> It seems to me there is no tool to "blame diffs", i.e. something to know
> what commit(s) is(are) responsible for a set of changes.
I don't know if that's what you are looking for but perhaps you could
use "git bisect run". You just need to pass it a script that returns 1 when
it finds the changes and 0 otherwise. (See git-bisect man page.)
Sometimes ago I sent a patch that would allow "!" after "git bisect run",
but it seems to have been forgotten. This patch makes it possible to use:
git bisect run ! grep some_stuff file1 file2...
This would give you the commit where some_stuff was introduced in file1 or
file2...
Regards,
Christian.
^ permalink raw reply
* Re: metastore
From: david @ 2007-09-17 4:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <7v7imp539u.fsf@gitster.siamese.dyndns.org>
On Sun, 16 Sep 2007, Junio C Hamano wrote:
> david@lang.hm writes:
>
>>> Post-checkout trigger is something I can say I can live with
>>> without looking at the actual patch, but that does not mean it
>>> would be a better approach at all.
>>
>> we agree on this much at least :-)
>>
>>> I would not be able to answer the first question right now; that
>>> needs a patch to prove that it can be done with a well contained
>>> set of changes that results in a maintainable code.
>>
>> you cannot answer the question in the affirmitive, but you could say
>> that any changes in that area would be completely unacceptable to you
>> (and for a while it sounded like you were saying exactly that). in
>> which case any effort put into preparing patches would be a waste of
>> time
>
> I tend to disagree. It's far from a waste of time. While, as I
> said, I am skeptical that such a patch would be small impact, if
> it helps people's needs, somebody will pick it up and carry
> forward, even if that somebody is not me. It can then mature
> out of tree and later could be merged. We simply do not know
> unless somebody tries. And I am quite happy that you seem to be
> motivated enough to see how it goes.
>
> On the other hand, the experiment could fail and you may end up
> with a patch that is too messy to be acceptable, in which case
> you might feel it a waste of time, but I do not think it is a
> waste even in such a case. We would learn what works and what
> doesn't, and we can bury "keeping track of /etc" topic to rest.
this is perfectly acceptable to me. I was trying to make very sure that
this topic fell in this catagory.
there are other topics that come up repeatedly that do get (and deserve)
automatic rejections ('patch to explicitly record renames' for example).
and while I didn't think that 'managing /etc' was in the same catagory,
sometimes that catagory is defined as much by the opinions and goals of
the core team as it is by techinical considerations.
there's a huge difference between 'this patch is rejected becouse we think
the implementation is bad' and 'this patch is rejected becouse we disagree
with the fundamental goal of the patch' effort spent on a patch rejected
for the first reason is never a complete waste (if nothing else it can
serve an an example of how not to do things for future developers ;-) but
effort spent on a patch that's rejected for the second reason is useually
a waste, and as such I make it a point to discuss the objective and basic
approach before spending much effort on somthing.
> I also need to rant here a bit.
>
> Fortunately we haven't had this problem too many times on this
> list, but sometimes people say "Here is my patch. If this is
> accepted I'll add documentation and tests". I rarely reply to
> such patches without sugarcoating my response, but my internal
> reaction is, "Don't you, as the person who proposes that change,
> believe in your patch deeply enough to be willing to perfect it,
> in order to make it suitable for consumption by the general
> public, whether it is included in my tree or not? A change that
> even you do not believe in yourself has very little chance of
> benefitting the general public, so thanks but no thanks, I'll
> pass."
>
I hope that my questions did not seem to fall into this catagory.
David Lang
^ permalink raw reply
* Re: rename detection limit checking, cherry picking, and git am -3
From: Junio C Hamano @ 2007-09-17 4:27 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Mark Levedahl, Git Mailing List
In-Reply-To: <20070917034742.GG3099@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> I actually don't see why cherry-pick can't be defined in terms
> of `format-patch|am -3`. It probably would be faster in almost
> all cases.
Heh, people often suggested that rebase should get --merge as
default, and I resisted that.
I think it would make sense to do the consolidated backend for
rebase, revert, cherry-pick and am (I have been tentatively
calling this "git replay") primarily based on the "patch with
fallback to 3-way" like format-patch piped to "am -3", with an
option to do merge-recursive.
^ permalink raw reply
* Re: metastore
From: Junio C Hamano @ 2007-09-17 4:23 UTC (permalink / raw)
To: david
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709161925000.24221@asgard.lang.hm>
david@lang.hm writes:
>> Post-checkout trigger is something I can say I can live with
>> without looking at the actual patch, but that does not mean it
>> would be a better approach at all.
>
> we agree on this much at least :-)
>
>> I would not be able to answer the first question right now; that
>> needs a patch to prove that it can be done with a well contained
>> set of changes that results in a maintainable code.
>
> you cannot answer the question in the affirmitive, but you could say
> that any changes in that area would be completely unacceptable to you
> (and for a while it sounded like you were saying exactly that). in
> which case any effort put into preparing patches would be a waste of
> time
I tend to disagree. It's far from a waste of time. While, as I
said, I am skeptical that such a patch would be small impact, if
it helps people's needs, somebody will pick it up and carry
forward, even if that somebody is not me. It can then mature
out of tree and later could be merged. We simply do not know
unless somebody tries. And I am quite happy that you seem to be
motivated enough to see how it goes.
On the other hand, the experiment could fail and you may end up
with a patch that is too messy to be acceptable, in which case
you might feel it a waste of time, but I do not think it is a
waste even in such a case. We would learn what works and what
doesn't, and we can bury "keeping track of /etc" topic to rest.
I also need to rant here a bit.
Fortunately we haven't had this problem too many times on this
list, but sometimes people say "Here is my patch. If this is
accepted I'll add documentation and tests". I rarely reply to
such patches without sugarcoating my response, but my internal
reaction is, "Don't you, as the person who proposes that change,
believe in your patch deeply enough to be willing to perfect it,
in order to make it suitable for consumption by the general
public, whether it is included in my tree or not? A change that
even you do not believe in yourself has very little chance of
benefitting the general public, so thanks but no thanks, I'll
pass."
^ permalink raw reply
* Re: rename detection limit checking, cherry picking, and git am -3
From: Shawn O. Pearce @ 2007-09-17 3:47 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Git Mailing List
In-Reply-To: <46EDF54F.5030503@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> wrote:
> The curious thing to me is the vast superiority of whatever
> git-format-patch|git-am -3 does, and I wonder if that isn't a
> fundementally better design for cherry picking than git-cherry-pick
> implements (it obviously is for this case).
In this case `git am -3` creates a tree object containing only
the files modified by the patch and then feeds that tree into
git-merge-recursive. Now if you go study git-revert's code you'll
see it actually just calls git-merge-recursive on three trees,
but these are three complete trees.
So what's probably happening here is there's less candidates on one
side in the `am -3` case, so we spend a lot less time generating
the rename matrix, searching for a match, and we get better changes
of finding a match.
I actually don't see why cherry-pick can't be defined in terms
of `format-patch|am -3`. It probably would be faster in almost
all cases.
--
Shawn.
^ permalink raw reply
* rename detection limit checking, cherry picking, and git am -3
From: Mark Levedahl @ 2007-09-17 3:32 UTC (permalink / raw)
To: Git Mailing List
Linus' recent patch to invoke limiting on rename detection broke my
ability to use cherry-picking on one project. This project has about
4300 files on one branch (a), 2500 on a later branch (b), 226 commits in
total between the two branches, and a convoluted history of how branch a
morphed into branch b. About 50 files were renamed in the transition,
and we need to migrate patches from the still maintained branch a onto
the new branch b.
Prior to Linus' recent patch to limit rename detection (0024a549),
cherry picking a patch from a to b, where the patch affected just one
file, often took about 45 seconds on a 3 GHz pentium 4 with the CPU
pegged at 100% for the duration. The cherry picking always succeeded and
correctly followed renames, but was very slow.
Following Linus' patch, the cherry picking fails with a merge conflict
(almost instantly), complaining the file has been deleted on b but
modified on a, i.e., the rename detection does not work. I tried raising
diff.renameLimit to 100000, that seems to have no effect whatsoever on
cherry-pick (the process aborts with a conflict almost immediately).
Curiously, using "git format-patch x..y --stdout | git am -3" succeeds
in this case, and runs in well less than a second. This performance
seems unchanged by the rename detection limit patch.
So, the rename limit patch "broke" git for this usage, though one could
reasonably argue the previous code was so slow as to be broken anyway.
The curious thing to me is the vast superiority of whatever
git-format-patch|git-am -3 does, and I wonder if that isn't a
fundementally better design for cherry picking than git-cherry-pick
implements (it obviously is for this case).
Mark
^ permalink raw reply
* [PATCH 3/3] rev-list --bisect: Bisection "distance" clean up.
From: Christian Couder @ 2007-09-17 3:28 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 18 +++++++-----------
1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8c9635a..899a31d 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -189,7 +189,7 @@ static int count_interesting_parents(struct commit *commit)
return count;
}
-static inline int halfway(struct commit_list *p, int distance, int nr)
+static inline int halfway(struct commit_list *p, int nr)
{
/*
* Don't short-cut something we are not going to return!
@@ -202,8 +202,7 @@ static inline int halfway(struct commit_list *p, int distance, int nr)
* 2 and 3 are halfway of 5.
* 3 is halfway of 6 but 2 and 4 are not.
*/
- distance *= 2;
- switch (distance - nr) {
+ switch (2 * weight(p) - nr) {
case -1: case 0: case 1:
return 1;
default:
@@ -295,7 +294,7 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)
static struct commit_list *do_find_bisection(struct commit_list *list,
int nr, int *weights)
{
- int n, counted, distance;
+ int n, counted;
struct commit_list *p;
counted = 0;
@@ -346,15 +345,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
for (p = list; p; p = p->next) {
if (p->item->object.flags & UNINTERESTING)
continue;
- n = weight(p);
- if (n != -2)
+ if (weight(p) != -2)
continue;
- distance = count_distance(p);
+ weight_set(p, count_distance(p));
clear_distance(list);
- weight_set(p, distance);
/* Does it happen to be at exactly half-way? */
- if (halfway(p, distance, nr))
+ if (halfway(p, nr))
return p;
counted++;
}
@@ -392,8 +389,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
weight_set(p, weight(q));
/* Does it happen to be at exactly half-way? */
- distance = weight(p);
- if (halfway(p, distance, nr))
+ if (halfway(p, nr))
return p;
}
}
--
1.5.3.1.59.g93705
^ permalink raw reply related
* [PATCH 2/3] rev-list --bisect: Move some bisection code into best_bisection.
From: Christian Couder @ 2007-09-17 3:28 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 43 ++++++++++++++++++++++++++-----------------
1 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 2dae287..8c9635a 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -255,6 +255,30 @@ static void show_list(const char *debug, int counted, int nr,
}
#endif /* DEBUG_BISECT */
+static struct commit_list *best_bisection(struct commit_list *list, int nr)
+{
+ struct commit_list *p, *best;
+ int best_distance = -1;
+
+ best = list;
+ for (p = list; p; p = p->next) {
+ int distance;
+ unsigned flags = p->item->object.flags;
+
+ if (revs.prune_fn && !(flags & TREECHANGE))
+ continue;
+ distance = weight(p);
+ if (nr - distance < distance)
+ distance = nr - distance;
+ if (distance > best_distance) {
+ best = p;
+ best_distance = distance;
+ }
+ }
+
+ return best;
+}
+
/*
* zero or positive weight is the number of interesting commits it can
* reach, including itself. Especially, weight = 0 means it does not
@@ -272,7 +296,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
int nr, int *weights)
{
int n, counted, distance;
- struct commit_list *p, *best;
+ struct commit_list *p;
counted = 0;
@@ -377,22 +401,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
show_list("bisection 2 counted all", counted, nr, list);
/* Then find the best one */
- counted = -1;
- best = list;
- for (p = list; p; p = p->next) {
- unsigned flags = p->item->object.flags;
-
- if (revs.prune_fn && !(flags & TREECHANGE))
- continue;
- distance = weight(p);
- if (nr - distance < distance)
- distance = nr - distance;
- if (distance > counted) {
- best = p;
- counted = distance;
- }
- }
- return best;
+ return best_bisection(list, nr);
}
static struct commit_list *find_bisection(struct commit_list *list,
--
1.5.3.1.59.g93705
^ permalink raw reply related
* [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.
From: Christian Couder @ 2007-09-17 3:28 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
This factorises some code and make a big function smaller.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 90 +++++++++++++++++++++++++++------------------------
1 files changed, 48 insertions(+), 42 deletions(-)
This patch series is a resend with the changes Junio asked for.
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index ac551d5..2dae287 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -268,39 +268,12 @@ static void show_list(const char *debug, int counted, int nr,
* unknown. After running count_distance() first, they will get zero
* or positive distance.
*/
-
-static struct commit_list *find_bisection(struct commit_list *list,
- int *reaches, int *all)
+static struct commit_list *do_find_bisection(struct commit_list *list,
+ int nr, int *weights)
{
- int n, nr, on_list, counted, distance;
- struct commit_list *p, *best, *next, *last;
- int *weights;
-
- show_list("bisection 2 entry", 0, 0, list);
-
- /*
- * Count the number of total and tree-changing items on the
- * list, while reversing the list.
- */
- for (nr = on_list = 0, last = NULL, p = list;
- p;
- p = next) {
- unsigned flags = p->item->object.flags;
+ int n, counted, distance;
+ struct commit_list *p, *best;
- next = p->next;
- if (flags & UNINTERESTING)
- continue;
- p->next = last;
- last = p;
- if (!revs.prune_fn || (flags & TREECHANGE))
- nr++;
- on_list++;
- }
- list = last;
- show_list("bisection 2 sorted", 0, nr, list);
-
- *all = nr;
- weights = xcalloc(on_list, sizeof(*weights));
counted = 0;
for (n = 0, p = list; p; p = p->next) {
@@ -357,12 +330,8 @@ static struct commit_list *find_bisection(struct commit_list *list,
weight_set(p, distance);
/* Does it happen to be at exactly half-way? */
- if (halfway(p, distance, nr)) {
- p->next = NULL;
- *reaches = distance;
- free(weights);
+ if (halfway(p, distance, nr))
return p;
- }
counted++;
}
@@ -400,12 +369,8 @@ static struct commit_list *find_bisection(struct commit_list *list,
/* Does it happen to be at exactly half-way? */
distance = weight(p);
- if (halfway(p, distance, nr)) {
- p->next = NULL;
- *reaches = distance;
- free(weights);
+ if (halfway(p, distance, nr))
return p;
- }
}
}
@@ -425,12 +390,53 @@ static struct commit_list *find_bisection(struct commit_list *list,
if (distance > counted) {
best = p;
counted = distance;
- *reaches = weight(p);
}
}
+ return best;
+}
+
+static struct commit_list *find_bisection(struct commit_list *list,
+ int *reaches, int *all)
+{
+ int nr, on_list;
+ struct commit_list *p, *best, *next, *last;
+ int *weights;
+
+ show_list("bisection 2 entry", 0, 0, list);
+
+ /*
+ * Count the number of total and tree-changing items on the
+ * list, while reversing the list.
+ */
+ for (nr = on_list = 0, last = NULL, p = list;
+ p;
+ p = next) {
+ unsigned flags = p->item->object.flags;
+
+ next = p->next;
+ if (flags & UNINTERESTING)
+ continue;
+ p->next = last;
+ last = p;
+ if (!revs.prune_fn || (flags & TREECHANGE))
+ nr++;
+ on_list++;
+ }
+ list = last;
+ show_list("bisection 2 sorted", 0, nr, list);
+
+ *all = nr;
+ weights = xcalloc(on_list, sizeof(*weights));
+
+ /* Do the real work of finding bisection commit. */
+ best = do_find_bisection(list, nr, weights);
+
if (best)
best->next = NULL;
+
+ *reaches = weight(best);
free(weights);
+
return best;
}
--
1.5.3.1.59.g93705
^ permalink raw reply related
* Re: git-gui i18n status?
From: Shawn O. Pearce @ 2007-09-17 3:20 UTC (permalink / raw)
To: Christian Stimming; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <200709161403.50780.stimming@tuhh.de>
Christian Stimming <stimming@tuhh.de> wrote:
> One question came up when seeing the i18n code really in git-gui.git: How are
> translators supposed to submit new or updated translations? Is
> git-gui-i18n.git of any use anymore? This doesn't seem so. Should updated
> translations just be submitted by email to git@vger? In any case, the
> instructions in po/README should probably be updated to explain the
> recommended way of submitting translation updates.
I was sort of hoping Dscho would be able to answer that. ;-)
I can play patch-monkey and apply things people send to the mailing
list. I'm also willing to pull from a tree if the commit history
is clean and mergable. Since each language more or less stands
on its own in its own .po file translators may find it easier to
email patches. I dunno, I'm not a translator.
> Oh, and po/git-gui.pot should probably be updated to reflect the latest string
> additions and changes.
Yes. Dscho was looking at creating a custom diff filter for git
that would better handle showing diffs here. I was sort of waiting
for progress from that (if any) before doing the pot update. I also
have a lot of UI work that I wanted to do in the 0.9.x series and
those are likely to create/change the sets of messages we need
to translate.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.
From: Christian Couder @ 2007-09-17 3:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5qr575a.fsf@gitster.siamese.dyndns.org>
Le dimanche 16 septembre 2007, Junio C Hamano a écrit :
> > +static struct commit_list *do_find_bisection(struct commit_list *list,
> > + int nr, int *weights);
> > +
> > /*
> > * zero or positive weight is the number of interesting commits it can
> > * reach, including itself. Especially, weight = 0 means it does not
>
> The comment whose top part we can see here talks about the magic
> values -1 and -2 used while do_find_bisection() after the
> refactoring does its work, and these magic values are never
> visible to the calling function. You should move the comment to
> the top of do_find_bisection() as well.
>
> Also this forward declaration is unwarranted. A bottom-up
> sequence to define do_find_bisection() first, then to define its
> sole caller find_bisection() next is easier to read at least for
> me.
>
> The latter comment also applies to your other patch.
All right, I will send new patchs with these changes.
Thanks,
Christian.
^ 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