* Re: git-update-server-info may be required,cannot clone and pull from a remote repository
From: Johannes Schindelin @ 2007-07-12 13:13 UTC (permalink / raw)
To: pradeep singh; +Cc: Jakub Narebski, Eric Wong, git
In-Reply-To: <a901b49a0707120552y649fba20p4fa14ca48be4be54@mail.gmail.com>
Hi,
On Thu, 12 Jul 2007, pradeep singh wrote:
> On 7/12/07, pradeep singh <pradeep.rautela@gmail.com> wrote:
> > On 7/12/07, Jakub Narebski <jnareb@gmail.com> wrote:
> > > What information does gitweb/INSTALL lack?
> >
> > May be i am running some old version on my Ubuntu Edgy machine
> > perhaps? I cannot find such a file anywhere?
> >
> > Looks like it is available in newer versions. Does it works for
> > git-1.4.4?
IMHO it is not good to start with anything prior to 1.5.0, if you did not
have any exposure to older Git. In terms of user friendliness,
1.4.4.4->1.5.0 is a leap by an astronomic unit.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/6] Add functions for parsing integers with size suffixes
From: Johannes Schindelin @ 2007-07-12 13:07 UTC (permalink / raw)
To: Brian Downing; +Cc: git, Junio C Hamano, Nicolas Pitre
In-Reply-To: <1184244952173-git-send-email-bdowning@lavos.net>
Hi,
On Thu, 12 Jul 2007, Brian Downing wrote:
> Split out the nnn{k,m,g} parsing code from git_config_int into
> git_parse_long, so command-line parameters can enjoy the same
> functionality. Also add get_parse_ulong for unsigned values.
Nice!
> + if (!*end)
> + *ret = val;
> + else if (!strcasecmp(end, "k"))
> + *ret = val * 1024;
> + else if (!strcasecmp(end, "m"))
> + *ret = val * 1024 * 1024;
> + else if (!strcasecmp(end, "g"))
> + *ret = val * 1024 * 1024 * 1024;
> + else
> + return 0;
This could be an own static function, like this:
unsigned long get_unit_factor(const char *end)
{
if (!*end)
return 1;
if (!strcasecmp(end, "k"))
return 1024;
...
error("Unknown unit: %s", end);
return 1;
}
to avoid duplicated code.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Add pack-objects window memory usage limit
From: Brian Downing @ 2007-07-12 13:07 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <20070712130400.GU4087@lavos.net>
This adds an option (--window-memory=N) and configuration variable
(pack.windowMemory = N) to limit the memory size of the pack-objects
delta search window. This works by removing the oldest unpacked objects
whenever the total size goes above the limit. It will always leave
at least one object, though, so as not to completely eliminate the
possibility of computing deltas.
This is an extra limit on top of the normal window size (--window=N);
the window will not dynamically grow above the fixed number of entries
specified to fill the memory limit.
With this, repacking a repository with a mix of large and small objects
is possible even with a very large window.
Cleaner and correct circular buffer handling courtesy of Nicolas Pitre.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
builtin-pack-objects.c | 50 +++++++++++++++++++++++++++++++++++++++++------
1 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 132ce96..5cc2148 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -16,8 +16,9 @@
#include "progress.h"
static const char pack_usage[] = "\
-git-pack-objects [{ -q | --progress | --all-progress }] [--max-pack-size=N] \n\
- [--local] [--incremental] [--window=N] [--depth=N] \n\
+git-pack-objects [{ -q | --progress | --all-progress }] \n\
+ [--max-pack-size=N] [--local] [--incremental] \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]";
@@ -79,6 +80,9 @@ static unsigned long delta_cache_size = 0;
static unsigned long max_delta_cache_size = 0;
static unsigned long cache_max_small_delta_size = 1000;
+static unsigned long window_memory_usage = 0;
+static unsigned long window_memory_limit = 0;
+
/*
* The object names in objects array are hashed with this hashtable,
* to help looking up the entry by object name.
@@ -1351,12 +1355,14 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
if (sz != trg_size)
die("object %s inconsistent object length (%lu vs %lu)",
sha1_to_hex(trg_entry->idx.sha1), sz, trg_size);
+ window_memory_usage += sz;
}
if (!src->data) {
src->data = read_sha1_file(src_entry->idx.sha1, &type, &sz);
if (sz != src_size)
die("object %s inconsistent object length (%lu vs %lu)",
sha1_to_hex(src_entry->idx.sha1), sz, src_size);
+ window_memory_usage += sz;
}
if (!src->index) {
src->index = create_delta_index(src->data, src_size);
@@ -1366,6 +1372,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
warning("suboptimal pack - out of memory");
return 0;
}
+ window_memory_usage += sizeof_delta_index(src->index);
}
delta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, max_size);
@@ -1408,9 +1415,22 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)
return m;
}
+static void free_unpacked(struct unpacked *n)
+{
+ window_memory_usage -= sizeof_delta_index(n->index);
+ free_delta_index(n->index);
+ n->index = NULL;
+ if (n->data) {
+ free(n->data);
+ n->data = NULL;
+ window_memory_usage -= n->entry->size;
+ }
+ n->entry = NULL;
+}
+
static void find_deltas(struct object_entry **list, int window, int depth)
{
- uint32_t i = nr_objects, idx = 0, processed = 0;
+ uint32_t i = nr_objects, idx = 0, count = 0, processed = 0;
unsigned int array_size = window * sizeof(struct unpacked);
struct unpacked *array;
int max_depth;
@@ -1445,12 +1465,17 @@ static void find_deltas(struct object_entry **list, int window, int depth)
if (entry->no_try_delta)
continue;
- free_delta_index(n->index);
- n->index = NULL;
- free(n->data);
- n->data = NULL;
+ free_unpacked(n);
n->entry = entry;
+ while (window_memory_limit &&
+ window_memory_usage > window_memory_limit &&
+ count > 1) {
+ uint32_t tail = (idx + window - count) % window;
+ free_unpacked(array + tail);
+ count--;
+ }
+
/*
* If the current object is at pack edge, take the depth the
* objects that depend on the current object into account
@@ -1485,6 +1510,8 @@ static void find_deltas(struct object_entry **list, int window, int depth)
next:
idx++;
+ if (count + 1 < window)
+ count++;
if (idx >= window)
idx = 0;
} while (i > 0);
@@ -1523,6 +1550,10 @@ static int git_pack_config(const char *k, const char *v)
window = git_config_int(k, v);
return 0;
}
+ if(!strcmp(k, "pack.windowmemory")) {
+ window_memory_limit = git_config_ulong(k, v);
+ return 0;
+ }
if(!strcmp(k, "pack.depth")) {
depth = git_config_int(k, v);
return 0;
@@ -1699,6 +1730,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
usage(pack_usage);
continue;
}
+ if (!prefixcmp(arg, "--window-memory=")) {
+ if (!git_parse_ulong(arg+16, &window_memory_limit))
+ usage(pack_usage);
+ continue;
+ }
if (!prefixcmp(arg, "--depth=")) {
char *end;
depth = strtoul(arg+8, &end, 0);
--
1.5.2.GIT
^ permalink raw reply related
* Re: pull-fetch-param.txt (was Re: [PATCH] escape tilde in Documentation/git-rev-parse.txt)
From: Gerrit Pape @ 2007-07-12 13:06 UTC (permalink / raw)
To: git
In-Reply-To: <45222B18.1090305@s5r6.in-berlin.de>
On Tue, Oct 03, 2006 at 11:19:20AM +0200, Stefan Richter wrote:
> Junio C Hamano wrote:
> > It's a bit sad that asciidoc's nicer quoting features
> > are not backward compatible.
>
> Yes, this is awkward. Here comes the next candidate for quoting.
> In pull-fetch-param.txt:
>
> ----8<----
> <refspec>::
> The canonical format of a <refspec> parameter is
> `+?<src>:<dst>`; that is, an optional plus `+`, followed
> by the source ref, followed by a colon `:`, followed by
> the destination ref.
> +
> The remote ref that matches <src>
> is fetched, and if <dst> is not empty string, the local
> ref that matches it is fast forwarded using <src>.
> Again, if the optional plus `+` is used, the local ref
> ---->8----
>
> "man git-fetch" and "man git-pull" show:
> ----8<----
> <refspec>
> The canonical format of a <refspec> parameter is ?<src>:<dst>;
> that is, an optional plus, followed by the source ref, followed
> by a colon :, followed by the destination ref.
>
> The remote ref that matches <src> is fetched, and if <dst> is
> not empty string, the local ref that matches it is fast for-
> warded using <src>. Again, if the optional plus + is used, the
> ---->8----
Hi, this still is a problem, at least on Debian/unstable; with asciidoc
8.2.1, the git-push(1) and git-fetch(1) man pages have this 'broken'
refspec description[0].
Additionally there're problems with callouts, whereever <n> is used to
refer to a callout list, it renders broken man pages[1], e.g.:
$ man git-reset
[...]
EXAMPLES
Undo a commit and redo
$ git commit ...
$ git reset --soft HEAD^ \fB(1)\fR
$ edit \fB(2)\fR
$ git commit -a -c ORIG_HEAD \fB(3)\fR
.sp \fB1. \fRThis is most often done when you remembered what you
just committed is incomplete, or you misspelled your commit
message, or both. Leaves working tree as it was before "reset".
.br \fB2. \fRmake corrections to working tree files.
.br \fB3. \fR"reset" copies the old head to .git/ORIG_HEAD; redo
the commit by starting with its log message. If you do not need to
edit the message further, you can give -C option instead.
See also the --amend option to git-commit(1).
.br
Undo commits permanently
$ git commit ...
$ git reset --hard HEAD~3 \fB(1)\fR
.sp \fB1. \fRThe last three commits (HEAD, HEAD^, and HEAD~2) were
bad and you do not want to ever see them again. Do not do this if
you have already given these commits to somebody else.
.br
Undo a commit, making it a topic branch
[...]
Regards, Gerrit.
[0] http://bugs.debian.org/432560
[1] http://bugs.debian.org/420114
^ permalink raw reply
* Re: mtimes of working files
From: Randal L. Schwartz @ 2007-07-12 13:05 UTC (permalink / raw)
To: Eric Wong; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <20070712062605.GD29676@muzzle>
>>>>> "Eric" == Eric Wong <normalperson@yhbt.net> writes:
Eric> open FH, "git log -r --name-only --no-color --pretty=raw -z @ARGV |" or die $!;
This breaks needlessly on @ARGV names that contain spaces. You want:
open FH, "-|", qw(git log -r --name-only --no-color --pretty=raw -z), @ARGV or die $!;
But that sounds familiar.... I think there's a function somewhere included in
the git distro that does this. I'm old and senile though. :)
--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
^ permalink raw reply
* Re: [PATCH 4/6] Add pack-objects window memory usage limit
From: Brian Downing @ 2007-07-12 13:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre
In-Reply-To: <11842449521798-git-send-email-bdowning@lavos.net>
On Thu, Jul 12, 2007 at 07:55:50AM -0500, Brian Downing wrote:
> + if (!prefixcmp(arg, "--window-memory=")) {
> + if (!git_parse_ulong(arg+15, &window_memory_limit))
> + usage(pack_usage);
> + continue;
> + }
This is incorrect. I had this fixed to +16, but somewhere in my remaking
the series it got lost. I will resend the correct patch.
-bcd
^ permalink raw reply
* [PATCH 3/6] Add functions for parsing integers with size suffixes
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <1184244952554-git-send-email-bdowning@lavos.net>
Split out the nnn{k,m,g} parsing code from git_config_int into
git_parse_long, so command-line parameters can enjoy the same
functionality. Also add get_parse_ulong for unsigned values.
Make git_config_int use git_parse_long, and add get_config_ulong
as well.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
cache.h | 3 +++
config.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/cache.h b/cache.h
index e64071e..917a7e3 100644
--- a/cache.h
+++ b/cache.h
@@ -521,7 +521,10 @@ typedef int (*config_fn_t)(const char *, const char *);
extern int git_default_config(const char *, const char *);
extern int git_config_from_file(config_fn_t fn, const char *);
extern int git_config(config_fn_t fn);
+extern int git_parse_long(const char *, long *);
+extern int git_parse_ulong(const char *, unsigned long *);
extern int git_config_int(const char *, const char *);
+extern unsigned long git_config_ulong(const char *, const char *);
extern int git_config_bool(const char *, const char *);
extern int git_config_set(const char *, const char *);
extern int git_config_set_multivar(const char *, const char *, const char *, int);
diff --git a/config.c b/config.c
index 561ee3b..ee338d1 100644
--- a/config.c
+++ b/config.c
@@ -233,21 +233,60 @@ static int git_parse_file(config_fn_t fn)
die("bad config file line %d in %s", config_linenr, config_file_name);
}
-int git_config_int(const char *name, const char *value)
+int git_parse_long(const char *value, long *ret)
+{
+ if (value && *value) {
+ char *end;
+ long val = strtol(value, &end, 0);
+ if (!*end)
+ *ret = val;
+ else if (!strcasecmp(end, "k"))
+ *ret = val * 1024;
+ else if (!strcasecmp(end, "m"))
+ *ret = val * 1024 * 1024;
+ else if (!strcasecmp(end, "g"))
+ *ret = val * 1024 * 1024 * 1024;
+ else
+ return 0;
+ return 1;
+ }
+ return 0;
+}
+
+int git_parse_ulong(const char *value, unsigned long *ret)
{
if (value && *value) {
char *end;
- int val = strtol(value, &end, 0);
+ unsigned long val = strtoul(value, &end, 0);
if (!*end)
- return val;
- if (!strcasecmp(end, "k"))
- return val * 1024;
- if (!strcasecmp(end, "m"))
- return val * 1024 * 1024;
- if (!strcasecmp(end, "g"))
- return val * 1024 * 1024 * 1024;
- }
- die("bad config value for '%s' in %s", name, config_file_name);
+ *ret = val;
+ else if (!strcasecmp(end, "k"))
+ *ret = val * 1024;
+ else if (!strcasecmp(end, "m"))
+ *ret = val * 1024 * 1024;
+ else if (!strcasecmp(end, "g"))
+ *ret = val * 1024 * 1024 * 1024;
+ else
+ return 0;
+ return 1;
+ }
+ return 0;
+}
+
+int git_config_int(const char *name, const char *value)
+{
+ long ret;
+ if (!git_parse_long(value, &ret))
+ die("bad config value for '%s' in %s", name, config_file_name);
+ return ret;
+}
+
+unsigned long git_config_ulong(const char *name, const char *value)
+{
+ unsigned long ret;
+ if (!git_parse_ulong(value, &ret))
+ die("bad config value for '%s' in %s", name, config_file_name);
+ return ret;
}
int git_config_bool(const char *name, const char *value)
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 6/6] Add documentation for --window-memory, pack.windowMemory
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <1184244952554-git-send-email-bdowning@lavos.net>
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
Documentation/config.txt | 6 ++++++
Documentation/git-pack-objects.txt | 11 +++++++++++
Documentation/git-repack.txt | 11 +++++++++++
3 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4b67f0a..11b3321 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -589,6 +589,12 @@ pack.depth::
The maximum delta depth used by gitlink:git-pack-objects[1] when no
maximum depth is given on the command line. Defaults to 50.
+pack.windowMemory::
+ The window memory size limit used by gitlink:git-pack-objects[1]
+ when no limit is given on the command line. The value can be
+ suffixed with "k", "m", or "g". Defaults to 0, meaning no
+ limit.
+
pack.compression::
An integer -1..9, indicating the compression level for objects
in a pack file. -1 is the zlib default. 0 means no
diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index e3549b5..6f17cff 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -85,6 +85,17 @@ base-name::
times to get to the necessary object.
The default value for --window is 10 and --depth is 50.
+--window-memory=[N]::
+ This option provides an additional limit on top of `--window`;
+ the window size will dynamically scale down so as to not take
+ up more than N bytes in memory. This is useful in
+ repositories with a mix of large and small objects to not run
+ out of memory with a large window, but still be able to take
+ advantage of the large window for the smaller objects. The
+ size can be suffixed with "k", "m", or "g".
+ `--window-memory=0` makes memory usage unlimited, which is the
+ default.
+
--max-pack-size=<n>::
Maximum size of each output packfile, expressed in MiB.
If specified, multiple packfiles may be created.
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 2894939..5283ef8 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -68,6 +68,17 @@ OPTIONS
to be applied that many times to get to the necessary object.
The default value for --window is 10 and --depth is 50.
+--window-memory=[N]::
+ This option provides an additional limit on top of `--window`;
+ the window size will dynamically scale down so as to not take
+ up more than N bytes in memory. This is useful in
+ repositories with a mix of large and small objects to not run
+ out of memory with a large window, but still be able to take
+ advantage of the large window for the smaller objects. The
+ size can be suffixed with "k", "m", or "g".
+ `--window-memory=0` makes memory usage unlimited, which is the
+ default.
+
--max-pack-size=<n>::
Maximum size of each output packfile, expressed in MiB.
If specified, multiple packfiles may be created.
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 5/6] Add --window-memory option to git-repack
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <1184244952554-git-send-email-bdowning@lavos.net>
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
git-repack.sh | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/git-repack.sh b/git-repack.sh
index b5c6671..156c5e8 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2005 Linus Torvalds
#
-USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--depth=N]'
+USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'
SUBDIRECTORY_OK='Yes'
. git-sh-setup
@@ -20,6 +20,7 @@ do
-l) local=--local ;;
--max-pack-size=*) extra="$extra $1" ;;
--window=*) extra="$extra $1" ;;
+ --window-memory=*) extra="$extra $1" ;;
--depth=*) extra="$extra $1" ;;
*) usage ;;
esac
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 1/6] Don't try to delta if target is much smaller than source
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <1184244952554-git-send-email-bdowning@lavos.net>
Add a new try_delta heuristic: Don't bother trying to make a delta if
the target object size is much smaller (currently 1/32) than the source,
as it's very likely not going to get a match. Even if it does, you will
have to read at least 32x the size of the new file to reassemble it,
which isn't such a good deal. This leads to a considerable performance
improvement when deltifying a mix of small and large files with a very
large window, because you don't have to wait for the large files to
percolate out of the window before things start going fast again.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
builtin-pack-objects.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 54b9d26..132ce96 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1342,6 +1342,8 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
sizediff = src_size < trg_size ? trg_size - src_size : 0;
if (sizediff >= max_size)
return 0;
+ if (trg_size < src_size / 32)
+ return 0;
/* Load data if not already done */
if (!trg->data) {
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 2/6] Support fetching the memory usage of a delta index
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <1184244952554-git-send-email-bdowning@lavos.net>
Delta indexes, at least on 64-bit platforms, tend to be larger than
the actual uncompressed data. As such, keeping track of this storage
is important if you want to successfully limit the memory size of your
pack window.
Squirrel away the total allocation size inside the delta_index struct,
and add an accessor "sizeof_delta_index" to access it.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
delta.h | 7 +++++++
diff-delta.c | 10 ++++++++++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/delta.h b/delta.h
index 7b3f86d..40ccf5a 100644
--- a/delta.h
+++ b/delta.h
@@ -24,6 +24,13 @@ create_delta_index(const void *buf, unsigned long bufsize);
extern void free_delta_index(struct delta_index *index);
/*
+ * sizeof_delta_index: returns memory usage of delta index
+ *
+ * Given pointer must be what create_delta_index() returned, or NULL.
+ */
+extern unsigned long sizeof_delta_index(struct delta_index *index);
+
+/*
* create_delta: create a delta from given index for the given buffer
*
* This function may be called multiple times with different buffers using
diff --git a/diff-delta.c b/diff-delta.c
index faf96e4..3af5835 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -119,6 +119,7 @@ struct index_entry {
};
struct delta_index {
+ unsigned long memsize;
const void *src_buf;
unsigned long src_size;
unsigned int hash_mask;
@@ -159,6 +160,7 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
mem = hash + hsize;
entry = mem;
+ index->memsize = memsize;
index->src_buf = buf;
index->src_size = bufsize;
index->hash_mask = hmask;
@@ -228,6 +230,14 @@ void free_delta_index(struct delta_index *index)
free(index);
}
+unsigned long sizeof_delta_index(struct delta_index *index)
+{
+ if (index)
+ return index->memsize;
+ else
+ return 0;
+}
+
/*
* The maximum size for any opcode sequence, including the initial header
* plus Rabin window plus biggest copy.
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 4/6] Add pack-objects window memory usage limit
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
In-Reply-To: <1184244952554-git-send-email-bdowning@lavos.net>
This adds an option (--window-memory=N) and configuration variable
(pack.windowMemory = N) to limit the memory size of the pack-objects
delta search window. This works by removing the oldest unpacked objects
whenever the total size goes above the limit. It will always leave
at least one object, though, so as not to completely eliminate the
possibility of computing deltas.
This is an extra limit on top of the normal window size (--window=N);
the window will not dynamically grow above the fixed number of entries
specified to fill the memory limit.
With this, repacking a repository with a mix of large and small objects
is possible even with a very large window.
Cleaner and correct circular buffer handling courtesy of Nicolas Pitre.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
builtin-pack-objects.c | 50 +++++++++++++++++++++++++++++++++++++++++------
1 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 132ce96..dc6a5f4 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -16,8 +16,9 @@
#include "progress.h"
static const char pack_usage[] = "\
-git-pack-objects [{ -q | --progress | --all-progress }] [--max-pack-size=N] \n\
- [--local] [--incremental] [--window=N] [--depth=N] \n\
+git-pack-objects [{ -q | --progress | --all-progress }] \n\
+ [--max-pack-size=N] [--local] [--incremental] \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]";
@@ -79,6 +80,9 @@ static unsigned long delta_cache_size = 0;
static unsigned long max_delta_cache_size = 0;
static unsigned long cache_max_small_delta_size = 1000;
+static unsigned long window_memory_usage = 0;
+static unsigned long window_memory_limit = 0;
+
/*
* The object names in objects array are hashed with this hashtable,
* to help looking up the entry by object name.
@@ -1351,12 +1355,14 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
if (sz != trg_size)
die("object %s inconsistent object length (%lu vs %lu)",
sha1_to_hex(trg_entry->idx.sha1), sz, trg_size);
+ window_memory_usage += sz;
}
if (!src->data) {
src->data = read_sha1_file(src_entry->idx.sha1, &type, &sz);
if (sz != src_size)
die("object %s inconsistent object length (%lu vs %lu)",
sha1_to_hex(src_entry->idx.sha1), sz, src_size);
+ window_memory_usage += sz;
}
if (!src->index) {
src->index = create_delta_index(src->data, src_size);
@@ -1366,6 +1372,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
warning("suboptimal pack - out of memory");
return 0;
}
+ window_memory_usage += sizeof_delta_index(src->index);
}
delta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, max_size);
@@ -1408,9 +1415,22 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)
return m;
}
+static void free_unpacked(struct unpacked *n)
+{
+ window_memory_usage -= sizeof_delta_index(n->index);
+ free_delta_index(n->index);
+ n->index = NULL;
+ if (n->data) {
+ free(n->data);
+ n->data = NULL;
+ window_memory_usage -= n->entry->size;
+ }
+ n->entry = NULL;
+}
+
static void find_deltas(struct object_entry **list, int window, int depth)
{
- uint32_t i = nr_objects, idx = 0, processed = 0;
+ uint32_t i = nr_objects, idx = 0, count = 0, processed = 0;
unsigned int array_size = window * sizeof(struct unpacked);
struct unpacked *array;
int max_depth;
@@ -1445,12 +1465,17 @@ static void find_deltas(struct object_entry **list, int window, int depth)
if (entry->no_try_delta)
continue;
- free_delta_index(n->index);
- n->index = NULL;
- free(n->data);
- n->data = NULL;
+ free_unpacked(n);
n->entry = entry;
+ while (window_memory_limit &&
+ window_memory_usage > window_memory_limit &&
+ count > 1) {
+ uint32_t tail = (idx + window - count) % window;
+ free_unpacked(array + tail);
+ count--;
+ }
+
/*
* If the current object is at pack edge, take the depth the
* objects that depend on the current object into account
@@ -1485,6 +1510,8 @@ static void find_deltas(struct object_entry **list, int window, int depth)
next:
idx++;
+ if (count + 1 < window)
+ count++;
if (idx >= window)
idx = 0;
} while (i > 0);
@@ -1523,6 +1550,10 @@ static int git_pack_config(const char *k, const char *v)
window = git_config_int(k, v);
return 0;
}
+ if(!strcmp(k, "pack.windowmemory")) {
+ window_memory_limit = git_config_ulong(k, v);
+ return 0;
+ }
if(!strcmp(k, "pack.depth")) {
depth = git_config_int(k, v);
return 0;
@@ -1699,6 +1730,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
usage(pack_usage);
continue;
}
+ if (!prefixcmp(arg, "--window-memory=")) {
+ if (!git_parse_ulong(arg+15, &window_memory_limit))
+ usage(pack_usage);
+ continue;
+ }
if (!prefixcmp(arg, "--depth=")) {
char *end;
depth = strtoul(arg+8, &end, 0);
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 0/6] Pack window memory limit, take 2
From: Brian Downing @ 2007-07-12 12:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nicolas Pitre, Brian Downing
This series has my circular buffer errors (hopefully) corrected, and
the code is a lot cleaner as a bonus. Also, the options are now named
--window-memory and pack.windowMemory, and both can take {k,m,g} suffixes.
I split out the {k,m,g} parsing code from git_config_int into
git_parse_long and git_parse_ulong, so it can be used for command-line
arguments as well. Hopefully these will be useful elsewhere.
Finally the documentation has been cleaned up a bit and information on
the defaults has been added.
Patches 1 and 2 are unmodified from last time.
[PATCH 1/6] Don't try to delta if target is much smaller than source
[PATCH 2/6] Support fetching the memory usage of a delta index
[PATCH 3/6] Add functions for parsing integers with size suffixes
[PATCH 4/6] Add pack-objects window memory usage limit
[PATCH 5/6] Add --window-memory option to git-repack
[PATCH 6/6] Add documentation for --window-memory, pack.windowMemory
Documentation/config.txt | 6 +++
Documentation/git-pack-objects.txt | 11 ++++++
Documentation/git-repack.txt | 11 ++++++
builtin-pack-objects.c | 52 ++++++++++++++++++++++++++----
cache.h | 3 ++
config.c | 61 +++++++++++++++++++++++++++++------
delta.h | 7 ++++
diff-delta.c | 10 ++++++
git-repack.sh | 3 +-
9 files changed, 145 insertions(+), 19 deletions(-)
-bcd
^ permalink raw reply
* Re: git-update-server-info may be required,cannot clone and pull from a remote repository
From: pradeep singh @ 2007-07-12 12:52 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Eric Wong, git
In-Reply-To: <a901b49a0707120550i9361e30wc5811bd5d3305f59@mail.gmail.com>
sorry forgot to CC gitlist in my last mail.
On 7/12/07, pradeep singh <pradeep.rautela@gmail.com> wrote:
> On 7/12/07, Jakub Narebski <jnareb@gmail.com> wrote:
> > [Cc: git@vger.kernel.org, Pradeep Singh <pradeep.rautela@gmail.com>,
> > Eric Wong <normalperson@yhbt.net> (instaweb creator)]
> >
> > pradeep singh wrote:
> >
> > > Anyway i could not get gitweb running after running git-instaweb.
> > >
> > > Any thoughts on how to setup a gitweb interface ?
> >
> > What information does gitweb/INSTALL lack?
>
> May be i am running some old version on my Ubuntu Edgy machine perhaps?
> I cannot find such a file anywhere?
>
> Looks like it is available in newer versions.
> Does it works for git-1.4.4?
>
> thanks for help.
> >
> > --
> > Jakub Narebski
> > Warsaw, Poland
> > ShadeHawk on #git
> >
> >
> >
>
>
> --
> Pradeep
>
--
Pradeep
^ permalink raw reply
* Re: Teach read-tree 2-way merge to ignore intermediate symlinks
From: Pierre Habouzit @ 2007-07-12 12:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Daniel Barkalow
In-Reply-To: <7vzm22vyin.fsf@assigned-by-dhcp.cox.net>
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
On Thu, Jul 12, 2007 at 01:04:16AM -0700, Junio C Hamano wrote:
> Earlier in 16a4c61, we taught "read-tree -m -u" not to be
> confused when switching from a branch that has a path frotz/filfre
> to another branch that has a symlink frotz that points at xyzzy/
> directory. The fix was incomplete in that it was still confused
> when coming back (i.e. switching from a branch with frotz -> xyzzy/
> to another branch with frotz/filfre).
>
> This fix is rather expensive in that for a path that is created
> we would need to see if any of the leading component of that
> path exists as a symbolic link in the filesystem (in which case,
> we know that path itself does not exist, and the fact we already
> decided to check it out tells us that in the index we already
> know that symbolic link is going away as there is no D/F
> conflict).
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
I confirm this fixes the issue I reported.
Thanks !
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Installation failure caused by CDPATH environment variable
From: Johannes Schindelin @ 2007-07-12 11:52 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Junio C Hamano, David Kastrup, git
In-Reply-To: <4C80B6B8-F6D6-4C8B-86F5-629B5662247C@wincent.com>
Hi,
On Thu, 12 Jul 2007, Wincent Colaiuta wrote:
> El 12/7/2007, a las 10:34, Junio C Hamano escribi?:
>
> > David Kastrup <dak@gnu.org> writes:
> >
> > > Don't educate people. Just don't trigger their problems. Of
> > > course, there are millions of ways of shooting oneself in the foot,
> > > but in this case the same foot has been hit several times already.
> >
> > Yup. We do exactly that in git-clone, git-sh-setup and t/test-lib to
> > avoid getting bugged by this stupidity.
>
> El 12/7/2007, a las 9:51, David Kastrup escribi?:
>
> > [ "X" = "X$CDPATH" ] || unset CDPATH # ignore braindamaged exports
>
> Whatever decision is taken in the end, I think we should avoid terms
> like "stupidity" and "braindamaged" to avoid causing possible offense.
> Exporting CDPATH is a simple mistake that can be made inadvertently or
> unwittingly, but very easily (Googling for "export CDPATH" yields
> 18,000+ results, many of them purporting to be Bash "tutorials").
Sorry, my words were indeed to strong. I apologise. My only excuse is
that this crops up ever so often, and it does get slightly annoying. Of
course, you did not read all those mails, and so my words were unmerited.
For an example, see
http://article.gmane.org/gmane.comp.version-control.git/13736/match=cdpath
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] gitweb: new cgi parameter: option
From: Johannes Schindelin @ 2007-07-12 11:49 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Miklos Vajna, git, Junio C Hamano
In-Reply-To: <200707121211.32813.jnareb@gmail.com>
Hi,
On Thu, 12 Jul 2007, Jakub Narebski wrote:
> On Thu, 12 Jul 2007, Miklos Vajna wrote:
> > Currently the only supported value is '--no-merges' for the 'rss', 'atom',
> > 'log', 'shortlog' and 'history' actions, but it can be easily extended to allow
> > other parameters for other actions.
> >
> > Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
>
> Micronit: it is unwritten (as of yet) requirement to word wrap commit
> message at 80 columns or less.
It's not really difficult:
- snipsnap -
[PATCH] SubmittingPatches: mention line wrap
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
Documentation/SubmittingPatches | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 01354c2..066e284 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -9,6 +9,8 @@ Checklist (and a short version for the impatient):
- provide a meaningful commit message
- the first line of the commit message should be a short
description and should skip the full stop
+ - please use no lines longer than 76 characters, since you
+ want to send this message as email
- if you want your work included in git.git, add a
"Signed-off-by: Your Name <your@email.com>" line to the
commit message (or just use the option "-s" when
^ permalink raw reply related
* Re: Installation failure caused by CDPATH environment variable
From: Johannes Schindelin @ 2007-07-12 11:37 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <86sl7u12m3.fsf@lola.quinscape.zz>
Hi,
[please Cc' me if you already quote me and answer directly to my message]
On Thu, 12 Jul 2007, David Kastrup wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Wed, 11 Jul 2007, Wincent Colaiuta wrote:
> >
> >> [describes the typical CDPATH problem]
> >
> > You exported CDPATH. You're guaranteed to run into problems with
> > that. I doubt that your patch catches all problems in git, and even
> > if we tried to avoid breakage, you can only do so much about that.
> >
> > It is _wrong_ to export CDPATH.
>
> But it is not our job to educate people about that. So I'd just add
> something like
>
> [ "X" = "X$CDPATH" ] || unset CDPATH # ignore braindamaged exports
>
> to the top of possibly affected scripts and be done.
We have that already, and we're not done.
> A one-liner that makes this somebody else's problem (if at all) is worth
> its weight in gold.
That is right. And a one-liner that pretends to be a solve-all, but is
not, is worth its weight in lead.
I bet that in ten years we find yet another instance where exporting
CDPATH, which you should not have done in the first place, breaks Git.
Ciao,
Dscho "who chants wrong, wrong, wrong"
^ permalink raw reply
* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Jakub Narebski @ 2007-07-12 11:07 UTC (permalink / raw)
To: Matt McCutchen; +Cc: Junio C Hamano, git, Petr Baudis, Luben Tuikov
In-Reply-To: <3bbc18d20707111815i1de3cb35sadfa316ddee7f3f6@mail.gmail.com>
On Thu, 12 July 2007, Matt McCutchen wrote:
> On 7/11/07, Junio C Hamano <gitster@pobox.com> wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> I'm not sure if we want to store whole 'application/x-gzip' or only
>>> 'x-gzip' part of mime type, and if we want to store compressor as
>>> '| gzip' or simply as 'gzip'.
>
> Storing only 'x-gzip' assumes that all archive formats have MIME types
> beginning with 'application/'. Even if this assumption is justified
> by the MIME specification, I felt it was inappropriate to code it into
> gitweb.
Good argument. Besides, now we use it only in one place, but if we
would in the future use it in other place having full mimetype would
make it easier.
> The advantage of '| gzip' is that the lack of a compressor is
> not a special case. This is why I wrote %known_snapshot_formats the
> way I did, but of course you all are welcome to overrule me.
I wrote about this because I'm thinking about replacing the few
pipelines we use in gitweb[*1*], including the snapshot one, with
the list form, which has the advantage of avoiding spawning the shell
(performance) and not dealing with shell quoting of arguments (security
and errors), like we did for simple calling of git commands to read
from in the commit b9182987a80f7e820cbe1f8c7c4dc26f8586e8cd
"gitweb: Use list for of open for running git commands, thorougly"
Thus I'd rather have list of extra commands and arguments instead of
pipe as a string, i.e. 'gzip' instead of '| gzip', and 'gzip', '-9'
instead of '| gzip -9'.
[*1*] We currently use pipelines for snapshots which need external
compressor, like tgz and tbz2, and for pickaxe search.
>>> This would break not only existing _gitweb_ configuration (when
>>> gitweb admin installs new gitweb it isn't that hard to correct
>>> gitweb config), but also git _repositories_ config: gitweb.snapshot
>>> no longer work as it worked before, for example neither 'gzip'
>>> nor 'bzip2' values work anymore ('zip' doesn't stop working).
>>
>> I realized after seeing your other message on this patch that
>> this can be done while retaining backward compatibility, as you
>> suggested. Matt, does Jakub's suggestion make sense to you?
>
> It's not clear to me what the suggestion is: offer format names 'gzip'
> and 'bzip2' instead of 'tgz' and 'tbz2', or in addition to them, or
> what? I prefer 'tgz' and 'tbz2' because they carry more information
> and are properly analogous to 'zip', so I don't want to offer 'gzip'
> and 'bzip2' instead of them. Furthermore, I would like the user to
> see 'snapshot (tgz tbz2)' even if the repository owner wrote 'gzip
> bzip2', so just adding two rows to %known_snapshot_formats is
> insufficient. Either an additional column could be added to
> %known_snapshot_formats for the display name, or 'gzip' and 'bzip2'
> could be specified as aliases in %known_snapshot_formats and
> feature_snapshot could be taught to resolve them. I would prefer the
> second option; shall I implement it?
I also prefer the second option, perhaps as simple as 'gzip' => 'tbz'
and 'bzip2' => 'tbz2', and of course accompaning code to deal with this.
As to "display name" column: I'm not sure if for example 'tar.gz'
instead of 'tgz' would be not easier to understand.
> It would be possible to make the gitweb site configuration
> backward-compatible too; here's one possible approach. On startup,
> gitweb would check whether $feature{'snapshot'}{'default'} is a
> three-element list that appears to be in the old format. If so, it
> would save the settings in $known_snapshot_formats{'default'} and then
> set $feature{'snapshot'}{'default'} = 'default' . This is a hack; is
> it justified by the compatibility benefit?
If you implement 'gzip' and 'bzip2' as aliases, it could be as simple
as just taking last non-false element of array if the array has more
than one element. But I'm not sure if it is worth it.
But we should probably error out with some error message on
incompatibile gitweb site configuration; I'm not sure...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: git-log --follow?
From: Johannes Schindelin @ 2007-07-12 10:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vsl7uvx8v.fsf_-_@assigned-by-dhcp.cox.net>
Hi,
On Thu, 12 Jul 2007, Junio C Hamano wrote:
> The message I am following up is a patch to unpack-trees.c, whose basic
> code structure is Daniel's work, so I wanted to CC him and the easiest
> way to look his address up was to run git-log on it.
>
> Not so.
>
> [...]
>
> "git log -- unpack-trees.c" would not follow into read-tree.c, but I
> thought "git log --follow -- unpack-trees.c" is supposed to; I tried it
> for the first time, but it does not seem to work as well as I hoped.
Your lesson as to why following renames is not as useful as some might
want to make us believe is duly noted; will use it as back reference
should I have to defend that view again.
However, I have to wonder why you did not solve your problem this way:
git log --author=Daniel
Ciao,
Dscho
^ permalink raw reply
* Re: Installation failure caused by CDPATH environment variable
From: Wincent Colaiuta @ 2007-07-12 10:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Kastrup, git
In-Reply-To: <7vodiivx50.fsf@assigned-by-dhcp.cox.net>
El 12/7/2007, a las 10:34, Junio C Hamano escribió:
> David Kastrup <dak@gnu.org> writes:
>
>> Don't educate people. Just don't trigger their problems. Of course,
>> there are millions of ways of shooting oneself in the foot, but in
>> this case the same foot has been hit several times already.
>
> Yup. We do exactly that in git-clone, git-sh-setup and
> t/test-lib to avoid getting bugged by this stupidity.
El 12/7/2007, a las 9:51, David Kastrup escribió:
> [ "X" = "X$CDPATH" ] || unset CDPATH # ignore braindamaged exports
Whatever decision is taken in the end, I think we should avoid terms
like "stupidity" and "braindamaged" to avoid causing possible
offense. Exporting CDPATH is a simple mistake that can be made
inadvertently or unwittingly, but very easily (Googling for "export
CDPATH" yields 18,000+ results, many of them purporting to be Bash
"tutorials").
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] gitweb: new cgi parameter: option
From: Jakub Narebski @ 2007-07-12 10:11 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Junio C Hamano
In-Reply-To: <20070711230038.GN19386@genesis.frugalware.org>
On Thu, 12 Jul 2007, Miklos Vajna wrote:
> Currently the only supported value is '--no-merges' for the 'rss', 'atom',
> 'log', 'shortlog' and 'history' actions, but it can be easily extended to allow
> other parameters for other actions.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
Micronit: it is unwritten (as of yet) requirement to word wrap commit
message at 80 columns or less.
> ---
>
> Na Wed, Jul 11, 2007 at 11:19:41PM +0200, Jakub Narebski <jnareb@gmail.com> pisal(a):
>> Miklos Vajna wrote:
>>
>>> +((defined $filter and $filter == "nomerges") ? ("--no-merges") : ()),
>>
>> Shouldn't it be '$filter eq "nomerges"' instead?
>
> Yes, that works too (I'm not a perl addict :) )
By the way, there is t9500-gitweb-standalone-no-errors.sh test script to
check if gitweb doesn't give any Perl warnings or errors. Please try to
use it; it should at least find errors about undefined values and such.
But it has the disadvantage of requiring git to be build (compiled),
even if theoretically testing gitweb doesn't require it.
You would see something like
Argument "nomerges" isn't numeric in numeric eq (==)
when running this test with --debug, I think.
>> Besides, I'd rather have generalized way to provide additional options
>> to git commands, like '--no-merges' for RSS and Atom feeds, log, shortlog
>> and history views, '-C' for commitdiff view, '--remove-empty' for history
>> view for a file, perhaps even '-c' or '--cc' for commitdiff for merges
>> instead of abusing 'hp' argument for that.
Now I'm not so sure about using 'option' for selecting between
combined ('-c') and compressed combined ('--cc') formats for commitdiff
for merges. The '-c' and '--cc' (or '-m') must be used with only one
commit-ish[*1*], so they can take place of the second commit-ish, i.e.
'hp' (hash_parent) parameter. What do the list think about it?
[*1*] At least in gitweb. If I understand correctly, you can use
"git diff --cc tree1 tree2 tree2 ..." to get combined diff of specified
tree-ish; I'm not sure if git-diff-tree support this. And I know that
gitweb does not support this... at least for now. Would this be useful,
I wonder?
>> But that doesn't mean that this patch should be not applied... it doesn't
>> mean it should be applied neither ;-)
>
> What about this one?
See comments below.
> +my %options = (
> + "--no-merges" => [('rss', 'atom', 'log', 'shortlog', 'history')],
> +);
First, you don't need inner () parentheses to delimit/create list, as
anonymous array reference constructor [] works like it. So it could be
written simply as
+my %options = (
+ "--no-merges" => ['rss', 'atom', 'log', 'shortlog', 'history'],
+);
Second, instead of quoting each word by hand, we can use handy Perl
quoting operator, qw(), i.e. 'word list' operator, like below.
See perlop(1), "Quote and Quote-like Operators" subsection
+my %options = (
+ "--no-merges" => [ qw(rss atom log shortlog history) ],
+);
> +our $option = $cgi->param('option');
> +if (defined $option) {
> + if (not grep(/^$option$/, keys %options)) {
> + die_error(undef, "Invalid option parameter");
> + }
> + if (not grep(/^$action$/, @{$options{$option}})) {
> + die_error(undef, "Invalid option parameter for this action");
> + }
> +}
I'd rather make it possible to pass multiple additional options, for
example both '--remove-empty' (to speed up) and '--no-merges' for the
history view. So I'd use
+our @options = $cgi->param('option');
instead. This would make option validation bit harder, but I think not that
harder. But it would also make using extra options easier: just @options
instead of (defined $option ? $option : ()).
I'd also use @extra_options, or @act_opts instead of @options as
a variable name to be more descriptive.
I'm also not sure if invalid option parameter for action should return
error, or be simply ignored. This allow to hand-edit URL, changing for
example action from 'commitdiff' to 'commit', not worrying about spurious
parameters.
By the way, gitweb uses shortened names for paramaters. Perhaps 'opt'
or 'op' instead of 'options' here and in href subroutine (below)?
> @@ -534,6 +548,7 @@ sub href(%) {
[...]
> + option => "option",
> @@ -1770,6 +1785,7 @@ sub parse_commits {
> ($arg ? ($arg) : ()),
> ("--max-count=" . $maxcount),
> ("--skip=" . $skip),
> + ((defined $option) ? ($option) : ()),
> $commit_id,
> "--",
> ($filename ? ($filename) : ())
Very clever to put this in parse_commits subroutine...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 3/5] Add pack-objects window memory usage limit
From: Brian Downing @ 2007-07-12 10:02 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.LFD.0.999.0707112356330.32552@xanadu.home>
On Thu, Jul 12, 2007 at 12:25:54AM -0400, Nicolas Pitre wrote:
> On Wed, 11 Jul 2007, Brian Downing wrote:
>
> > + while (window_memory_limit &&
> > + window_memory_usage > window_memory_limit &&
> > + count > 1) {
> > + uint32_t tail = idx - count;
> > + if (tail > idx) {
> > + tail += window + 1;
> > + tail %= window;
> > + }
> > + free_unpacked(array + tail);
> > + count--;
> > + }
>
> This is bogus. Suppose window = 10 and only array entries 8, 9, 0, 1
> and 2 are populated. In that case idx = 2 and count should be 4 (not
> counting the current entry yet). You want to evict entry 8.
The current idx has already been depopulated by the time that code is
run, and count is probably one higher than you are expecting, so this
does actually work.
However, looking at it again, I think if the window hasn't been saturated
yet in my current code count will be what you expect in this situation
and it will screw up as you describe.
Besides, it is admittedly clumsy as hell (a common affliction when
dealing with circular buffers for me it seems). I'll see if I can get
something better that works.
Thanks,
-bcd
^ permalink raw reply
* Re: git-update-server-info may be required,cannot clone and pull from a remote repository
From: Jakub Narebski @ 2007-07-12 9:58 UTC (permalink / raw)
To: git
In-Reply-To: <a901b49a0707112227m2ea746ectd367031fdc8d3537@mail.gmail.com>
[Cc: git@vger.kernel.org, Pradeep Singh <pradeep.rautela@gmail.com>,
Eric Wong <normalperson@yhbt.net> (instaweb creator)]
pradeep singh wrote:
> Anyway i could not get gitweb running after running git-instaweb.
>
> Any thoughts on how to setup a gitweb interface ?
What information does gitweb/INSTALL lack?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: finding the right remote branch for a commit
From: Jakub Narebski @ 2007-07-12 9:33 UTC (permalink / raw)
To: git
In-Reply-To: <20070712074745.GA28507@piper.oerlikon.madduck.net>
martin f krafft wrote:
> Yes, it does. I am downloading the source now and intend to work
> with the HEAD (is that the right term for what I used to call trunk
> when I was doing SVN?) from now on (instead of the Debian package).
HEAD means _current_ branch. You can work off 'master' or 'next' branches,
and if you feel really adventurous even off 'pu' branch.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ 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