* [PATCH 2/5] Support fetching the memory usage of a delta index
From: Brian Downing @ 2007-07-12 3:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brian Downing
In-Reply-To: <11842100581060-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 0/5] Memory-limited pack-object window support
From: Brian Downing @ 2007-07-12 3:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brian Downing
This patch series implements a memory limit on the window size for
pack-objects and repack. Basically, the window size will temporarily
grow smaller than the --window option specifies if the total memory
usage of the window is over the specified limit.
With this series I can run even with --window=1000 on my troublesome
repository (described in an email a couple of days ago; basically, it
has a 20MB file with around 200 revisions, plus a bunch of normal sized
files) with memory usage limited to 512MB and it will happily scale down
the window to accommodate the file without blowing out my machine.
The --window option still specifies the size of the window and is still
required; the window will not grow to reach the memory limit, it will
only shrink to fit. I think this may be a feature, as running with a
very large window depth and a memory limit basically means that you will
pack at an approximately constant slow speed, rather than rushing ahead
as it does now for very small objects.
I took the easy way out and expire objects out of the window after
allocation has occurred, rather than figuring out how much needed to
be cleared before allocating. This made the logic much more feasible,
though.
I chose --window-bytes=N and pack.windowBytes=N as my option and
configuration names. I'm not in love with them, though; no other
configuration in Git mentions bytes in the name as far as I can see,
but I wanted to clearly differentiate it from the window "size" that
--window gets you.
The first patch in this series is optional, but I recommend it. It makes
it so that files that are much smaller than a potential delta source
don't even try to delta with it. This is handy for running in a mixed
file size repository with a large window, as it means that when you get
to small files again you start moving fast without having to wait for
the large objects to trickle out of the window. The cut-off is currently
if it is 1/32 the size, but that number was completely arbitrary.
[PATCH 1/5] Don't try to delta if target is much smaller than source
[PATCH 2/5] Support fetching the memory usage of a delta index
[PATCH 3/5] Add pack-objects window memory usage limit
[PATCH 4/5] Add --window-bytes option to git-repack
[PATCH 5/5] Add documentation for --window-bytes, pack.windowBytes
Documentation/config.txt | 5 +++
Documentation/git-pack-objects.txt | 8 +++++
Documentation/git-repack.txt | 8 +++++
builtin-pack-objects.c | 58 +++++++++++++++++++++++++++++++----
delta.h | 7 ++++
diff-delta.c | 10 ++++++
git-repack.sh | 3 +-
7 files changed, 91 insertions(+), 8 deletions(-)
-bcd
^ permalink raw reply
* [PATCH 3/5] Add pack-objects window memory usage limit
From: Brian Downing @ 2007-07-12 3:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brian Downing
In-Reply-To: <11842100581060-git-send-email-bdowning@lavos.net>
This adds an option (--window-bytes=N) and configuration variable
(pack.windowBytes = 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.
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
builtin-pack-objects.c | 56 ++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 132ce96..6e441b7 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-bytes=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,21 @@ 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 - count;
+ if (tail > idx) {
+ tail += window + 1;
+ tail %= 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 +1514,8 @@ static void find_deltas(struct object_entry **list, int window, int depth)
next:
idx++;
+ if (count < window)
+ count++;
if (idx >= window)
idx = 0;
} while (i > 0);
@@ -1523,6 +1554,10 @@ static int git_pack_config(const char *k, const char *v)
window = git_config_int(k, v);
return 0;
}
+ if(!strcmp(k, "pack.windowbytes")) {
+ window_memory_limit = git_config_int(k, v);
+ return 0;
+ }
if(!strcmp(k, "pack.depth")) {
depth = git_config_int(k, v);
return 0;
@@ -1699,6 +1734,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
usage(pack_usage);
continue;
}
+ if (!prefixcmp(arg, "--window-bytes=")) {
+ char *end;
+ window_memory_limit = strtoul(arg+15, &end, 0);
+ if (!arg[15] || *end)
+ 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 4/5] Add --window-bytes option to git-repack
From: Brian Downing @ 2007-07-12 3:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brian Downing
In-Reply-To: <11842100581060-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..4cff812 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-bytes=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-bytes=*) extra="$extra $1" ;;
--depth=*) extra="$extra $1" ;;
*) usage ;;
esac
--
1.5.2.GIT
^ permalink raw reply related
* [PATCH 5/5] Add documentation for --window-bytes, pack.windowBytes
From: Brian Downing @ 2007-07-12 3:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Brian Downing
In-Reply-To: <11842100581060-git-send-email-bdowning@lavos.net>
Signed-off-by: Brian Downing <bdowning@lavos.net>
---
Documentation/config.txt | 5 +++++
Documentation/git-pack-objects.txt | 8 ++++++++
Documentation/git-repack.txt | 8 ++++++++
3 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index aeece84..83c7dc1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -592,6 +592,11 @@ 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.windowBytes::
+ This option provides an additional limit on top of `pack.window`;
+ the window size will dynamically scale down so as to not take
+ up more than N bytes in memory.
+
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..21ed198 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -85,6 +85,14 @@ base-name::
times to get to the necessary object.
The default value for --window is 10 and --depth is 50.
+--window-bytes=[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.
+
--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..805d930 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -68,6 +68,14 @@ 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-bytes=[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.
+
--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
* Re: [PATCH 3/5] Add pack-objects window memory usage limit
From: Nicolas Pitre @ 2007-07-12 4:25 UTC (permalink / raw)
To: Brian Downing; +Cc: git, Junio C Hamano
In-Reply-To: <11842100582887-git-send-email-bdowning@lavos.net>
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.
-- tail = 2 - 4 = -2 (or a big uint32_t value)
-- tail > idx is true
-- tail += window + 1 -> -2 + 10 + 1 = 9
-- tail %= window is useless
-- you free entry 9 instead of entry 8.
Instead, you should do:
tail = idx - count;
if (tail > idx)
tail += window;
or even:
tail = (idx + window - count) % window;
> next:
> idx++;
> + if (count < window)
> + count++;
And of course you want:
if (count + 1 < window)
count++;
So not to count the new entry when the window gets full.
Nicolas
^ permalink raw reply
* Re: [PATCH 5/5] Add documentation for --window-bytes, pack.windowBytes
From: Nicolas Pitre @ 2007-07-12 4:35 UTC (permalink / raw)
To: Brian Downing; +Cc: git, Junio C Hamano
In-Reply-To: <1184210058514-git-send-email-bdowning@lavos.net>
On Wed, 11 Jul 2007, Brian Downing wrote:
> Signed-off-by: Brian Downing <bdowning@lavos.net>
> ---
> Documentation/config.txt | 5 +++++
> Documentation/git-pack-objects.txt | 8 ++++++++
> Documentation/git-repack.txt | 8 ++++++++
> 3 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index aeece84..83c7dc1 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -592,6 +592,11 @@ 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.windowBytes::
> + This option provides an additional limit on top of `pack.window`;
> + the window size will dynamically scale down so as to not take
> + up more than N bytes in memory.
> +
This doesn't say what the default (unlimited) is.
> 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..21ed198 100644
> --- a/Documentation/git-pack-objects.txt
> +++ b/Documentation/git-pack-objects.txt
> @@ -85,6 +85,14 @@ base-name::
> times to get to the necessary object.
> The default value for --window is 10 and --depth is 50.
>
> +--window-bytes=[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.
Ditto here.
Also it is a bit akward to specify a size in bytes when you probably
want to specify a limit which is in the megabyte range. I'd call them
--window_mem and pack.windowmemory, and allow for unit suffixes of 'k',
'm', or 'g' to be supported if not already.
Nicolas
^ permalink raw reply
* Re: [PATCH 0/5] Memory-limited pack-object window support
From: Nicolas Pitre @ 2007-07-12 4:38 UTC (permalink / raw)
To: Brian Downing; +Cc: git, Junio C Hamano
In-Reply-To: <11842100581060-git-send-email-bdowning@lavos.net>
On Wed, 11 Jul 2007, Brian Downing wrote:
> This patch series implements a memory limit on the window size for
> pack-objects and repack. Basically, the window size will temporarily
> grow smaller than the --window option specifies if the total memory
> usage of the window is over the specified limit.
Besides the small issues I've pointed out already, I think this is a
very good thing.
Nicolas
^ 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 5:27 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0707110731n4ffa25afoea5395a856869325@mail.gmail.com>
On 7/11/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 7/11/07, pradeep singh <pradeep.rautela@gmail.com> wrote:
[snip]
>
[...]
ok, i got the repo up. Thanks for the suggestion Alex.
Anyway i could not get gitweb running after running git-instaweb.
Any thoughts on how to setup a gitweb interface ?
Thanks
--
Pradeep
^ permalink raw reply
* Re: [PATCH 2/2] Make git-tag a builtin.
From: Junio C Hamano @ 2007-07-12 5:46 UTC (permalink / raw)
To: Carlos Rica; +Cc: git, Johannes Schindelin, Kristian Høgsberg
In-Reply-To: <46952755.2050307@gmail.com>
Carlos Rica <jasampler@gmail.com> writes:
> Mime-Version: 1.0
> Content-Type: text/plain; charset=windows-1252
Hmmmmmm.....
> This replaces the script "git-tag.sh" with "builtin-tag.c".
> It is based in a previous work on it from Kristian Høgsberg.
>
> There are some minor changes in the behaviour of "git tag" here:
> "git tag -v" now can get more than one tag to verify, like "git tag -d" did,
> "git tag" with no arguments prints all tags, more like "git branch" does, and
> the template for the edited message adds an empty line, like in "git commit".
These probably are good changes (except that "no argument" case
might be a bit annoying, but I guess it's Ok). Is there any
need for documentation updates with these changes?
> diff --git a/builtin-tag.c b/builtin-tag.c
> new file mode 100644
> index 0000000..1824379
> --- /dev/null
> +++ b/builtin-tag.c
> @@ -0,0 +1,430 @@
> +/*
> + * Builtin "git tag"
> + *
> + * Copyright (c) 2007 Kristian H??gsberg <krh@redhat.com>,
I do not know how the above would come out, but it surely was
not Høgsberg in the copy I received...
We probably should make sure our sources are in UTF-8; I suspect
builtin-branch.c is not.
> + * Carlos Rica <jasampler@gmail.com>
> + * Based on git-tag.sh and mktag.c by Linus Torvalds.
> + */
> +
> +#include "cache.h"
> +#include "builtin.h"
> +#include "refs.h"
> +#include "tag.h"
> +#include "run-command.h"
> +
> +static const char builtin_tag_usage[] =
> + "git-tag [-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg>] <tagname> [<head>]";
> +
> +static char signingkey[1000];
> +static int lines;
> +
> +static void launch_editor(const char *path, char **buffer, unsigned long *len)
> +{
> + const char *editor, *terminal;
> + struct child_process child;
> + const char *args[3];
> + int fd;
> +
> + editor = getenv("VISUAL");
> + if (!editor)
> + editor = getenv("EDITOR");
> +
> + terminal = getenv("TERM");
> + if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
> + fprintf(stderr,
> + "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
> + "Please supply the message using either -m or -F option.\n");
> + exit(1);
> + }
Ah, this is taken from git-commit.sh ;-) Does your "git tag"
support the -F option (builtin_tag_usage[] does not seem to
mention it)? I wonder what happens when this function migrates
to editor.c and a new program, other than git-tag and
git-commit, that is without -F nor -m options wants to call
this.
> + if (!editor)
> + editor = "vi";
> +
> + memset(&child, 0, sizeof(child));
> + child.argv = args;
> + args[0] = editor;
> + args[1] = path;
> + args[2] = NULL;
> +
> + if (run_command(&child))
> + die("could not launch editor %s.", editor);
This message is not quite true, isn't it? The editor might have
been launched but exited with non-zero status.
> + fd = open(path, O_RDONLY);
> + if (fd == -1)
> + die("could not read %s.", path);
And this is "could not open", with probably strerror(errno) to
be helpful.
> + if (read_pipe(fd, buffer, len))
> + die("could not read message file '%s': %s",
> + path, strerror(errno));
> + close(fd);
> +}
> +
> +#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
> +
> +static int show_reference(const char *refname, const unsigned char *sha1,
> + int flag, void *cb_data)
> +{
> ...
> +}
Ok.
> +static int list_tags(const char *pattern)
> +{
> ...
> +}
Ok.
> +static int delete_tags(const char **argv)
> +{
> + const char **p;
> + char ref[PATH_MAX];
> + int had_error = 0;
> + unsigned char sha1[20];
> +
> + for (p = argv; *p; p++) {
> + if (snprintf(ref, sizeof ref, "refs/tags/%s", *p) > sizeof ref)
> + die("tag name '%s' too long.", *p);
"sizeof foo" may be proper ANSI C, but somehow many people seem
to find "sizeof(foo)" much easier to read, including me.
I think the overflow check is wrong. snprintf() does not write
more than sizeof(ref) including the training NUL, but when its
output is truncated, it returns the number of bytes that would
have been written to the buffer, excluding the NUL termination.
What this means is that if it returns sizeof(ref), it also is a
case of overflow. Further, if C library is not C99, e.g. older
glibc (say 2.0.6), it could return negative status indicating an
error condition upon truncated output.
I suspect you inherited these problems from builtin-branch.c,
but we'd better fix them there as well.
> + if (!resolve_ref(ref, sha1, 1, NULL)) {
> + fprintf(stderr, "tag '%s' not found.\n", *p);
> + had_error = 1;
> + continue;
> + }
We might want to call error("tag '%s' not found.") instead for
later libification...
> ...
> +}
> +
> +static int run_verify_tag_command(unsigned char *sha1)
> +{
> + int ret;
> + const char *argv_verify_tag[] = {"git-verify-tag",
> + "-v", "SHA1_HEX", NULL};
> + argv_verify_tag[2] = sha1_to_hex(sha1);
> +
> + ret = run_command_v_opt(argv_verify_tag, 0);
> +
> + if (ret <= -10000)
> + die("unable to run %s\n", argv_verify_tag[0]);
> + return -ret;
> +}
I wonder why you need to differentiate between ERR_RUN_COMMAND_*
and non-zero exit status... Also do you need to "return -ret",
instead of not negating? In C programs error returns are often
negative and finish_command() follows that convention.
> +static int verify_tags(const char **argv)
> +{
> ...
> +}
Ok.
> +static int do_sign(char *buffer, size_t size, size_t max)
> +{
> + struct child_process gpg;
> + const char *args[4];
> + char *bracket;
> + int len;
> +
> + if (signingkey[0] == '\0') {
> + strlcpy(signingkey, git_committer_info(1), sizeof signingkey);
> + bracket = strchr(signingkey, '>');
> + if (bracket)
> + bracket[1] = '\0';
> + }
Hmph. Silently truncate with strlcpy instead of erroring out on
insanely long committer info?
> + memset(&gpg, 0, sizeof(gpg));
> + gpg.argv = args;
> + gpg.in = -1;
> + gpg.out = -1;
> + args[0] = "gpg";
> + args[1] = "-bsau";
> + args[2] = signingkey;
> + args[3] = NULL;
> +
> + if (start_command(&gpg))
> + die("could not run gpg.");
> +
> + write_or_die(gpg.in, buffer, size);
> + close(gpg.in);
> + gpg.close_in = 0;
> + len = read_in_full(gpg.out, buffer + size, max - size);
Bi-di pipes makes me nervous, but this case should be Ok. It
could try writing "---BEGIN PGP SIGNATURE---" before reading
from you and get stuck because you are not reading from it but
that is only in theory.
I wonder why our read_in_full and write_in_full do not return
ssize_t although that's how they count things internally. At
least you could make do_sign() to return ssize_t, and change the
die() to "return error()" to have the caller deal with it, which
would be easier for people other than "git tag -s" to call this
if they later want to.
What happens when gpg gave more than you expect? We get
truncated signature and I do not see any code to notice the
breakage...
> +
> + finish_command(&gpg);
> +
> + return size + len;
> +}
> +
> +static const char tag_template[] =
> + "\n"
> + "#\n"
> + "# Write a tag message\n"
> + "#\n";
> +
> +static int git_tag_config(const char *var, const char *value)
> +{
> ...
> +}
Ok, except for the same strlcpy comment above.
> +#define MAX_SIGNATURE_LENGTH 1024
> +/* message must be NULL or allocated, it will be reallocated and freed */
> +static void create_tag(const unsigned char *object, const char *tag,
> + char *message, int sign, unsigned char *result)
> +{
> + enum object_type type;
> + char header_buf[1024], *buffer;
> + int header_len, max_size;
> + unsigned long size;
> +
> + type = sha1_object_info(object, NULL);
> + if (type <= 0)
> + die("bad object type.");
> +
> + header_len = snprintf(header_buf, sizeof header_buf,
> + "object %s\n"
> + "type %s\n"
> + "tag %s\n"
> + "tagger %s\n\n",
> + sha1_to_hex(object),
> + typename(type),
> + tag,
> + git_committer_info(1));
> +
> + if (header_len >= sizeof header_buf)
> + die("tag header too big.");
This comparison '>=' is correct (i.e. not '>'), as opposed to
the one I commented way above.
> + if (!message) {
> ...
> + }
> + else {
> + buffer = message;
> + size = strlen(message);
> + }
> +
> + size = stripspace(buffer, size, 1);
> +
> + if (!message && !size)
> + die("no tag message?");
Why check 'message' here?
> ...
> + free(buffer);
> +}
The rest is Ok.
> +int cmd_tag(int argc, const char **argv, const char *prefix)
> +{
> ...
> + if (!strcmp(arg, "-F")) {
> + unsigned long len;
> + annotate = 1;
> + i++;
> + if (i == argc)
> + die("option -F needs an argument.");
> +
> + fd = open(argv[i], O_RDONLY);
> + if (fd < 0)
> + die("cannot open %s", argv[1]);
The shell script version relies on the magic "cat -" to read
from standard input upon "git tag -F -". It is understandable
that both of you and Dscho missed it, though.
> + len = 1024;
> + message = xmalloc(len);
> + if (read_pipe(fd, &message, &len))
> + die("cannot read %s", argv[1]);
> + message = xrealloc(message, len + 1);
> + message[len] = '\0';
> + continue;
> + }
We might be better off if read_pipe() is renamed to read_fd()
and made internally always NUL-terminate the buffer (but not
count that NUL as part of length). I dunno.
> + if (!strcmp(arg, "-u")) {
> + annotate = 1;
> + sign = 1;
> + i++;
> + if (i == argc)
> + die("option -u needs an argument.");
> + strlcpy(signingkey, argv[i], sizeof signingkey);
> + continue;
Extra space "if (expr". Return from strlcpy() not checked?
> + if (snprintf(ref, sizeof ref, "refs/tags/%s", tag) > sizeof ref)
> + die("tag '%s' too long.", tag);
This use of snprintf() and checking the overflow as error is
much nicer, don't you think, instead of silently ignoring
truncation?
> ...
> +}
The remainder of cmd_tag() looks Ok.
^ permalink raw reply
* Re: git-svn+cygwin failed fetch
From: Eric Wong @ 2007-07-12 5:48 UTC (permalink / raw)
To: Russ Dill; +Cc: git
In-Reply-To: <f9d2a5e10707110254j46d1123fuade955f17da0a8c5@mail.gmail.com>
Russ Dill <russ.dill@gmail.com> wrote:
> On 7/11/07, Eric Wong <normalperson@yhbt.net> wrote:
> >Russ Dill <russ.dill@gmail.com> wrote:
> >> [...]/src $ mkdir foo
> >> [...]/src $ cd foo
> >> [...]/src/foo $ git-svn init -t tags -b branches -T trunk
> >> https://www.[...].com/svn/foo/bar/bla
> >> Initialized empty Git repository in .git/
> >> Using higher level of URL: https://www.[...].com/svn/foo/bar/bla =>
> >> https://www.[...].com/svn/foo
> >>
> >> [...]/src/foo $ git-svn fetch
> >> config --get svn-remote.svn.url: command returned error: 1
> >>
> >> [...]/src/foo $ git config --get svn-remote.svn.url
> >> https://www.[...].com/svn/foo
> >
> >Sorry, I can't help here other than recommending a real UNIX with
> >fork + pipe + exec and all that fun stuff.
> >
> >git-svn relies heavily[1] on both input and output pipes of the
> >safer-but-made-for-UNIX fork + pipe + exec(@list) variety, so I suspect
> >this is just the tip of the iceberg for Windows incompatibilies with
> >git-svn...
>
> Its actually reading and writing quite a bit of stuff from the config
> file, so why this one simple command would fail eludes me. Especially
> since it wrote it there in the first place. If I comment out the
> command_oneline and hardcode the value I know it should return,
> git-fetch runs. Its actually been running for several hours now.
Wow. That's a pleasant surprise that anything in git-svn works at all
on cygwin. I was almost certain git-svn on Windows was a hopeless cause
from other chatter I had heard on the mailing list.
command_oneline() is used everywhere in that code, so I'm at a total loss
as to why it would fail in one place. Can you put a the following lines
right before where it was failing?
print "GIT_CONFIG: $ENV{GIT_CONFIG} | GIT_DIR: $ENV{GIT_DIR}\n";
system('cat', "$ENV{GIT_DIR}/config");
And tell me what it outputs?
--
Eric Wong
^ permalink raw reply
* Re: mtimes of working files
From: Eric Wong @ 2007-07-12 6:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0707111940080.4516@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi list,
>
> > > > How difficult is it to have script (or maybe existing git option)
> > > > that would make mtimes of all working files equal to time of last
> > > > commit ?
>
> Now I slowly get really curious. Does _anybody_ know a scenario where
> this makes sense?
>
> (No, Eric, there are enough corner cases where your example of a clustered
> webserver breaks down, so I am not fully convinced that this is a useful
> case.)
I'll have to admit that most of my git usage is via git-svn, so I
mostly deal with linear history and some branching, but no real merges
in a git sense.
This is what I whipped up the other night:
http://yhbt.net/git-set-file-times
-----------------------------------8<-----------------------------------------
#!/usr/bin/perl -w
use strict;
# sets mtime and atime of files to the latest commit time in git
#
# This is useful for serving static content (managed by git)
# from a cluster of identically configured HTTP servers. HTTP
# clients and content delivery networks can get consistent
# Last-Modified headers no matter which HTTP server in the
# cluster they hit. This should improve caching behavior.
#
# This does not take into account merges, but if you're updating
# every machine in the cluster from the same commit (A) to the
# same commit (B), the mtimes will be _consistent_ across all
# machines if not necesarily accurate.
#
# THIS IS NOT INTENDED TO OPTIMIZE BUILD SYSTEMS SUCH AS 'make'
# YOU HAVE BEEN WARNED!
my %ls = ();
my $commit_time;
$/ = "\0";
open FH, 'git ls-files -z|' or die $!;
while (<FH>) {
chomp;
$ls{$_} = $_;
}
close FH;
$/ = "\n";
open FH, "git log -r --name-only --no-color --pretty=raw -z @ARGV |" or die $!;
while (<FH>) {
chomp;
if (/^committer .*? (\d+) (?:[\-\+]\d+)$/) {
$commit_time = $1;
} elsif (s/\0\0commit [a-f0-9]{40}$//) {
my @files = delete @ls{split(/\0/, $_)};
@files = grep { defined $_ } @files;
next unless @files;
utime $commit_time, $commit_time, @files;
}
last unless %ls;
}
close FH;
-----------------------------------8<-----------------------------------------
--
Eric Wong
^ permalink raw reply
* [PATCH] apply delta depth bias to already deltified objects
From: Nicolas Pitre @ 2007-07-12 6:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
We already apply a bias on the initial delta attempt with max_size being
a function of the base object depth. This already has the effect of
favoring shallower deltas even if deeper deltas could be smaller, and
therefore creating a wider delta tree (see commits 4e8da195 and
c3b06a69).
This principle should also be applied to all delta attempts for the same
object and not only the first attempt. With this the criteria for the
best delta is not only its size but also its depth, so that a shallower
delta might be selected even if it is larger than a deeper one. Even if
some deltas get larger, they allow for wider delta trees making the
depth limit less quickly reached and therefore better deltas can be
subsequently found, keeping the resulting pack size equal or even
smaller. Runtime access to the pack should also benefit from shallower
deltas.
Testing on the GIT repo before this patch provided those numbers:
$ git repack -a -f --depth=10
Total 56534 (delta 39441), reused 0 (delta 0)
29.38user 0.33system 0:29.73elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+20996minor)pagefaults 0swaps
$ git verify-pack -v
chain length = 1: 3150 objects
chain length = 2: 2713 objects
chain length = 3: 2441 objects
chain length = 4: 2205 objects
chain length = 5: 2014 objects
chain length = 6: 1895 objects
chain length = 7: 1802 objects
chain length = 8: 1997 objects
chain length = 9: 2710 objects
chain length = 10: 18514 objects
Pack size is 18273039 bytes.
With this patch:
$ git repack -a -f --depth=10
Total 56534 (delta 39370), reused 0 (delta 0)
24.32user 0.35system 0:24.78elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+19142minor)pagefaults 0swaps
$ git verify-pack -v
chain length = 1: 4059 objects
chain length = 2: 4048 objects
chain length = 3: 3775 objects
chain length = 4: 3428 objects
chain length = 5: 3007 objects
chain length = 6: 2793 objects
chain length = 7: 2701 objects
chain length = 8: 2988 objects
chain length = 9: 3935 objects
chain length = 10: 8636 objects
Pack size is 16311008 bytes.
Summary: this patch makes for slightly faster repacks, a smaller pack,
and a much lower count of object hanging on the max delta depth.
Same repo again with the default depth of 50:
Before:
Total 56534 (delta 39545), reused 0 (delta 0)
23.01user 0.36system 0:23.40elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+21017minor)pagefaults 0swaps
chain length = 1: 2882 objects
chain length = 2: 2443 objects
chain length = 3: 2157 objects
[...]
chain length = 48: 505 objects
chain length = 49: 782 objects
chain length = 50: 1802 objects
Pack size: 14651260 bytes.
After:
Total 56534 (delta 39596), reused 0 (delta 0)
22.98user 0.20system 0:23.20elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+20560minor)pagefaults 0swaps
chain length = 1: 2985 objects
chain length = 2: 2625 objects
chain length = 3: 2282 objects
[...]
chain length = 48: 477 objects
chain length = 49: 564 objects
chain length = 50: 392 objects
Pack size: 14383165 bytes.
With the Linux kernel repo:
Before:
Total 531582 (delta 431824), reused 0 (delta 0)
276.66user 3.64system 4:45.35elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+149606minor)pagefaults 0swaps
chain length = 1: 40779 objects
chain length = 2: 35650 objects
chain length = 3: 30226 objects
chain length = 4: 25619 objects
chain length = 5: 21709 objects
[...]
chain length = 46: 2223 objects
chain length = 47: 3263 objects
chain length = 48: 3175 objects
chain length = 49: 2128 objects
chain length = 50: 6413 objects
Pack size: 155680720 bytes.
After:
Total 531582 (delta 432537), reused 0 (delta 0)
270.31user 3.15system 4:35.07elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+140517minor)pagefaults 0swaps
chain length = 1: 41555 objects
chain length = 2: 36919 objects
chain length = 3: 31217 objects
chain length = 4: 26762 objects
chain length = 5: 22348 objects
[...]
chain length = 46: 1842 objects
chain length = 47: 1800 objects
chain length = 48: 1483 objects
chain length = 49: 1216 objects
chain length = 50: 1310 objects
Pack size: 154421656 bytes.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
This apparently makes BRian's patological case worse (although better
than before his same-size-shallower patch), but I think that the
improvement in the general case is worth it. Even Brian's pack gets
smaller so...
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 54b9d26..1eb86ed 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1332,12 +1332,14 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
/* Now some size filtering heuristics. */
trg_size = trg_entry->size;
- max_size = trg_size/2 - 20;
+ if (!trg_entry->delta)
+ max_size = trg_size/2 - 20;
+ else
+ max_size = trg_entry->delta_size * max_depth /
+ (max_depth - trg_entry->depth + 1);
max_size = max_size * (max_depth - src_entry->depth) / max_depth;
if (max_size == 0)
return 0;
- if (trg_entry->delta && trg_entry->delta_size <= max_size)
- max_size = trg_entry->delta_size;
src_size = src_entry->size;
sizediff = src_size < trg_size ? trg_size - src_size : 0;
if (sizediff >= max_size)
^ permalink raw reply related
* Re: [PATCH] Work around a bad interaction between Tcl and cmd.exe with "^{tree}"
From: Shawn O. Pearce @ 2007-07-12 6:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4693AF6C.99DB933@eudaptics.com>
Johannes Sixt <J.Sixt@eudaptics.com> wrote:
> It seems that MSYS's wish does some quoting for Bourne shells, in
> particular, escape the first '{' of the "^{tree}" suffix, but then it uses
> cmd.exe to run "git rev-parse". However, cmd.exe does not remove the
> backslash, so that the resulting rev expression ends up in git's guts
> as unrecognizable garbage: rev-parse fails, and git-gui hickups in a way
> that it must be restarted.
Finally fixed in git-gui 0.7.5, which I just pushed out.
--
Shawn.
^ permalink raw reply
* What's new in git-gui
From: Shawn O. Pearce @ 2007-07-12 7:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
I just released git-gui 0.7.5 to the usual location:
gitweb: http://repo.or.cz/w/git-gui.git/
clone: git://repo.or.cz/git-gui.git
http://repo.or.cz/r/git-gui.git
Changes since git-gui 0.7.4 (currently in git.git maint):
Gerrit Pape (1):
git-gui: properly popup error if gitk should be started but is not installed
Shawn O. Pearce (9):
git-gui: Unlock the index when cancelling merge dialog
git-gui: Don't bind F5/M1-R in all windows
git-gui: Bind M1-P to push action
git-gui: Include a Push action on the left toolbar
git-gui: Ensure windows shortcuts always have .bat extension
git-gui: Skip nicknames when selecting author initials
git-gui: Correct ls-tree buffering problem in browser
git-gui: Don't linewrap within console windows
git-gui: Work around bad interaction between Tcl and cmd.exe on ^{tree}
-----------------
In addition to the above the git-gui 0.8.0 development line is
progressing along nicely. Most of the changes have been focused
around new branch management features, including deleting remote
branches, switching to a detached HEAD, and creating branches from
a detached HEAD.
There is also some limited support for a "bound branch" concept.
Users can now create (or recreate) a local branch whose name
matches the name of a tracking branch. During the creation (and
optional checkout) git-gui will fetch the tracking branch and then
fast-forward or reset the local branch to match it.
The master branch has the following changes since 0.7.5 and
gitgui-0.7.4-21-g03d2562 (currently in git.git master):
Junio C Hamano (1):
git-gui: use "blame -w -C -C" for "where did it come from, originally?"
Shawn O. Pearce (41):
Merge branch 'maint'
Merge branch 'maint'
git-gui: Start blame windows as tall as possible
git-gui: Correct resizing of remote branch delete dialog
Merge branch 'maint'
git-gui: Honor rerere.enabled configuration option
git-gui: New Git version check support routine
Merge branch 'maint'
git-gui: Teach class system to support [$this cmd] syntax
git-gui: Abstract the revision picker into a mega widget
git-gui: Refactor the delete branch dialog to use class system
git-gui: Optimize for newstyle refs/remotes layout
git-gui: Maintain remote and source ref for tracking branches
git-gui: Allow users to match remote branch names locally
git-gui: Fast-forward existing branch in branch create dialog
git-gui: Enhance choose_rev to handle hundreds of branches
git-gui: Sort tags descending by tagger date
git-gui: Option to default new branches to match tracking branches
git-gui: Automatically refresh tracking branches when needed
git-gui: Better handling of detached HEAD
git-gui: Refactor our ui_status_value update technique
git-gui: Refactor branch switch to support detached head
git-gui: Unabbreviate commit SHA-1s prior to display
git-gui: Default selection to first matching ref
git-gui: Allow double-click in checkout dialog to start checkout
git-gui: Extract blame viewer status bar into mega-widget
git-gui: Change the main window progress bar to use status_bar
git-gui: Show a progress meter for checking out files
git-gui: Always use absolute path to all git executables
git-gui: Correct gitk installation location
git-gui: Assume unfound commands are known by git wrapper
git-gui: Treat `git version` as `git --version`
git-gui: Perform our own magic shbang detection on Windows
git-gui: Teach console widget to use git_read
git-gui: Improve the Windows and Mac OS X shortcut creators
Merge branch 'maint'
git-gui: Paper bag fix for Cygwin shortcut creation
git-gui: Use sh.exe in Cygwin shortcuts
git-gui: Include a space in Cygwin shortcut command lines
Merge branch 'maint'
git-gui: Change prior tree SHA-1 verification to use git_read
--
Shawn.
^ permalink raw reply
* [PATCH] script to display a distribution of longest common hash prefixes
From: Nicolas Pitre @ 2007-07-12 7:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This script was originally posted on the git mailing list by
Randal L. Schwartz <merlyn@stonehenge.com>.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/contrib/stats/git-common-hash b/contrib/stats/git-common-hash
new file mode 100755
index 0000000..e27fd08
--- /dev/null
+++ b/contrib/stats/git-common-hash
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+# This script displays the distribution of longest common hash prefixes.
+# This can be used to determine the minimum prefix length to use
+# for object names to be unique.
+
+git rev-list --objects --all | sort | perl -lne '
+ substr($_, 40) = "";
+ # uncomment next line for a distribution of bits instead of hex chars
+ # $_ = unpack("B*",pack("H*",$_));
+ if (defined $p) {
+ ($p ^ $_) =~ /^(\0*)/;
+ $common = length $1;
+ if (defined $pcommon) {
+ $count[$pcommon > $common ? $pcommon : $common]++;
+ } else {
+ $count[$common]++; # first item
+ }
+ }
+ $p = $_;
+ $pcommon = $common;
+ END {
+ $count[$common]++; # last item
+ print "$_: $count[$_]" for 0..$#count;
+ }
+'
^ permalink raw reply related
* Re: finding the right remote branch for a commit
From: martin f krafft @ 2007-07-12 7:47 UTC (permalink / raw)
To: git discussion list
In-Reply-To: <Pine.LNX.4.64.0707112226170.4516@racer.site>
[-- Attachment #1: Type: text/plain, Size: 3188 bytes --]
also sprach Johannes Schindelin <Johannes.Schindelin@gmx.de> [2007.07.11.2326 +0200]:
> Okay, after discussing this for a bit on IRC, here is what I would
> do (I already said this on IRC, but the mailing list is really the
> better medium for this):
I agree. However, I find IRC to have its merits. For instance, all
of our discussion yesterday would have taken days over the list, but
on IRC I was able to interject when I did not understand something
or you could stop me when I was going down a garden path.
Of course, IRC isn't archived in the sense that lists are, which is
why I make an effort to send updates to the list, such as I did to
another thread yesterday: http://marc.info/?l=git&m=118418250002028&w=2
> I would actually rename .etc/ into gits/, because it is not
> a directory containing settings, but a directory containing
> repositories.
Yes and no. I already use ~/.etc/ to store my settings and symlink
into it, but I do like your idea too, actually. I have yet to go
and try it, and I shall report back then.
> Everytime I would work on, say, .vimrc, I would say
> "--git-dir=$HOME/gits/vim.git", or maybe even make an alias in
> $HOME/.gitconfig, which spares me that:
I wish there were a way to determine which repository a file belongs
to and then to automatically select the right repository. I guess
one can script that by iterating all repos and using git-ls-files,
possibly caching the result.
Anyway, I tried your approach and failed:
$ mkdir repo
$ GIT_DIR=repo git init
$ GIT_DIR=repo git config core.bare false
$ echo 1 >a; GIT_DIR=repo git add a; GIT_DIR=repo git commit -m.
fatal: add must be run in a work tree
nothing to commit (use "git add file1 file2" to include for commit)
I am probably doing something wrong, but what?
> Come to think of it, this is maybe what I would have done, but it appears
> to me that this is the _ideal_ use case for worktree:
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).
> - you have to say
>
> $ git --work-tree=$HOME --bare init
>
> which is a bit counterintuitive. After all, it is _not_ a bare
> repository. The whole purpose of worktree, as far as I understand, is
> to have a _detached_ repository, which would otherwise be called bare.
I said
GIT_DIR=repo git --work-tree `pwd` init
and that seems to do everything it should, it sets core.worktree to
. and core.bare to false.
> Those are serious bugs. Matthias, any idea how to fix them?
That would be splendid. I am operating off HEAD now, so feel free to
prod me for any testing.
--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
spamtraps: madduck.bogus@madduck.net
"a scientist once wrote that all truth passes through three stages:
first it is ridiculed, then violently opposed and eventually,
accepted as self-evident."
-- schopenhauer
[-- Attachment #2: Digital signature (GPG/PGP) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Installation failure caused by CDPATH environment variable
From: David Kastrup @ 2007-07-12 7:51 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0707111807470.4516@racer.site>
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.
After the first few dozen bug reports (which are just the tip of an
iceberg of people who say "git is crap and fails in strange ways") you
just recognize that you should just make that somebody else's problem.
I've done my fair amount of such educational tasks myself with
packages of mine. It just gets you annoyed more and more each time,
it is tedious, it gets people get a bad opinion of both your software
_and_ your person (try being friendly after the 20th actually
technically completely unnecessary such request, while basically
explaining that you won't fix it because it is a Darwinian trap for
stupid people and you want evolution to run its course).
A one-liner that makes this somebody else's problem (if at all) is
worth its weight in gold.
Trust me on that.
My personal lowlight in that area are dozens of university and company
document classes derived from some old AMSLaTeX class in prehistoric
times that redefined \endfigure in a way incompatible with quite a few
packages. Among them preview.sty written by me.
After about 5 bug reports about 5 different classes, explaining what
was wrong and why it broke quite a few packages, not just
preview-latex, and how people should fix it and how they should
complain to the people giving them the package... preview.sty just
patches the problematic definitions if it has a reasonable guess that
they are in place, and blows out a stern and verbose warning
explaining where to complain and why.
Of course, nobody ever does.
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.
--
David Kastrup
^ permalink raw reply
* Re: mtimes of working files
From: Andy Parkins @ 2007-07-12 7:57 UTC (permalink / raw)
To: git; +Cc: Jan Hudec, Johannes Schindelin
In-Reply-To: <20070711202615.GE3069@efreet.light.src>
On Wednesday 2007 July 11, Jan Hudec wrote:
> I first thought the idea had something to do with make, but it will
> actually promptly break most build tools, because change to earlier version
> is a change too, but they wouldn't detect it.
I've found git's mtime setting to be the best where make is concerned so I
would hate to see it changed. Even when switching branches or rebasing or
whatever, only the changed files get rebuilt. The only time you get an
unnecessary rebuild is if you do
git checkout branch1
git checkout branch2
git checkout branch1
But we can hardly expect git to be responsible for that.
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
^ permalink raw reply
* Re: [PATCH] git-merge: run commit hooks when making merge commits
From: Andy Parkins @ 2007-07-12 8:03 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Sam Vilain
In-Reply-To: <7vd4yy4opa.fsf@assigned-by-dhcp.cox.net>
On Wednesday 2007 July 11, Junio C Hamano wrote:
> The commit-msg hook I have no clue what people usually use it
> for in the real world, but a merge commit message tends to be
I use it for adding my Signed-Off-By automatically in my git repository. Of
course that's only valid for the likes of me because I can be sure that I
only ever commit my own patches, rather than integrating other people's.
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
^ permalink raw reply
* Teach read-tree 2-way merge to ignore intermediate symlinks
From: Junio C Hamano @ 2007-07-12 8:04 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, Daniel Barkalow
In-Reply-To: <20070704203541.GA13286@artemis.corp>
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>
---
Re: [BUG (or misfeature?)] git checkout and symlinks
Pierre Habouzit <madcoder@debian.org> writes:
> if in a branch [branch1] you track the file: dir1/file1.c
> and in the branch [branch2] you track elsewhere/file1.c and dir1 be
> symlink on elsewhere, then it's not possible to checkout the branch
> [branch1] if your previous checkout was [branch2]. You have to manually
> remove the symlink `dir1` else git complains that checkouting branch1
> would overwrite dir1/file1.c.
>
> I'm not sure how to fix this, and it's quite painful actually :)
We probably could add a path buffer to cache the last look-up
made by has_symlink_leading_path(), like the other caller
does, but this is to give the fix a wider exposure and testing
early.
t/t2007-checkout-symlink.sh | 50 +++++++++++++++++++++++++++++++++++++++++++
unpack-trees.c | 3 ++
2 files changed, 53 insertions(+), 0 deletions(-)
create mode 100755 t/t2007-checkout-symlink.sh
diff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh
new file mode 100755
index 0000000..0526fce
--- /dev/null
+++ b/t/t2007-checkout-symlink.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Junio C Hamano
+
+test_description='git checkout to switch between branches with symlink<->dir'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ mkdir frotz &&
+ echo hello >frotz/filfre &&
+ git add frotz/filfre &&
+ test_tick &&
+ git commit -m "master has file frotz/filfre" &&
+
+ git branch side &&
+
+ echo goodbye >nitfol &&
+ git add nitfol
+ test_tick &&
+ git commit -m "master adds file nitfol" &&
+
+ git checkout side &&
+
+ git rm --cached frotz/filfre &&
+ mv frotz xyzzy &&
+ ln -s xyzzy frotz &&
+ git add xyzzy/filfre frotz &&
+ test_tick &&
+ git commit -m "side moves frotz/ to xyzzy/ and adds frotz->xyzzy/"
+
+'
+
+test_expect_success 'switch from symlink to dir' '
+
+ git checkout master
+
+'
+
+rm -fr frotz xyzzy nitfol &&
+git checkout -f master || exit
+
+test_expect_success 'switch from dir to symlink' '
+
+ git checkout side
+
+'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index cac2411..89dd279 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -495,6 +495,9 @@ static void verify_absent(const char *path, const char *action,
if (o->index_only || o->reset || !o->update)
return;
+ if (has_symlink_leading_path(path, NULL))
+ return;
+
if (!lstat(path, &st)) {
int cnt;
^ permalink raw reply related
* git-log --follow?
From: Junio C Hamano @ 2007-07-12 8:31 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <7vzm22vyin.fsf@assigned-by-dhcp.cox.net>
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.
The "blame -C unpack-trees.c" output consists of this
distribution of origin:
464 read-tree.c
337 unpack-trees.c
74 builtin-read-tree.c
11 tree.c
8 tree.h
and most of the work by Daniel was done when the bulk of code
was still in read-tree.c. Naturally the log output of
unpack-trees.c does not have a single commit by him. "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.
I think this is just a testament that "following renames" is not
as useful in a real project as people seem to believe, not a
real complaint.
When 16da134 created unpack-trees.c, it initially moved only
very small part of builtin-read-tree.c to it. Later 076b0adc
made further code movements from builtin-read-tree.c to
unpack-trees.c.
An interesting thing is that builtin-read-tree.c immediately
before 16da134 is much similar to unpack-trees.c in 076b0adc
than unpack-trees.c in 16da134, exactly because of this stepwise
code movements. I do not think people can argue that "human
user knows he is renaming the file so recording the human
intention would have helped git a lot better" in this case, as
the human user who made 16da134 did not even intend to do a
rename.
^ permalink raw reply
* Re: Installation failure caused by CDPATH environment variable
From: Junio C Hamano @ 2007-07-12 8:34 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <86sl7u12m3.fsf@lola.quinscape.zz>
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.
^ permalink raw reply
* Re: git-svn and renames
From: Eric Wong @ 2007-07-12 9:06 UTC (permalink / raw)
To: Rogan Dawes; +Cc: Git Mailing List
In-Reply-To: <20070711082000.GA29371@muzzle>
Eric Wong <normalperson@yhbt.net> wrote:
> Rogan Dawes <lists@dawes.za.net> wrote:
> > Hi folks,
> >
> > I am trying to push my local changes to an imported SVN project back to
> > the SVN repo. For anyone who cares, this is the WebGoat repository which
> > you can find at http://dawes.za.net/rogan/webgoat/webgoat.git/
> >
> > I am trying to push it back to the primary GoogleCode repo. I have
> > successfully managed to push 20 or so commits, but I am now running up
> > against a problem with a renamed/moved file.
> >
> > I moved a file from a directory to a subdirectory, and made minimal
> > changes to this file so that it remained a valid Java class. i.e. I
> > changed the package, and a few other minor things. As can be seen at
> > <http://dawes.za.net/gitweb.cgi?p=rogan/webgoat/webgoat.git;a=commitdiff;h=486416188a3e49d60e1510166ac197e5e66cc4d2>,
> > git detects the rename with 93% similarity.
> >
> > However, when I try to push this change to the Google repo, git-svn dies
> > with the following error:
> >
> > $ git svn dcommit
> > RA layer request failed: PROPFIND request failed on '/svn/trunk/
> > webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/DefaultLessonAction.java':
> > PROPFIND of '/svn/trunk/
> > webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/DefaultLessonAction.java':
> > 400 Bad Request (https://webgoat.googlecode.com) at
> > /home/rdawes/bin/git-svn line 400
> >
> > [Yes, those paths have a space in them, however this does not seem to
> > have prevented me from committing the previous 20 or so changes.]
> >
> > I noted the following in the git-svn documentation, with regards to
> > handling renames. However, I am not renaming a directory, only a couple
> > of files.
>
> I've personally noted a rename issue with committing funky characters
> "#{}" in filenames (I was renaming to get rid of those funky characters of
> course). Haven't had proper time to look into it.
>
> Did any previous successful commits have renames in them? You may want
> to set similarity to 100% to disable rename detection.
SVN over HTTP is most definitely behaves differently from svn:// and
file:// wrt strange file names. I'll see if I can work on this again in
day or two, or if anybody wants to pick up where I left off, my
work-in-progress test case is below (it's currently failing, of course).
I'll have to look into how (if) svk handles it and also at the svn
command-line client. I'm fairly sure the command-line svn client
handles it somehow...
Right now I'm getting sleepy, my vision is blurring and I'm seeing
"SVN" as "S/M" through my tired eyes...
>From 4c0165488390ad0cafb3eea700d8c873576131c2 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Thu, 12 Jul 2007 01:48:14 -0700
Subject: [PATCH] git-svn: WIP test for commiting renames over DAV with funky file names
---
t/lib-git-svn.sh | 26 +++++++
t/t9115-git-svn-dcommit-funky-renames.sh | 37 +++++++++++
t/t9115/funky-names.dump | 105 ++++++++++++++++++++++++++++++
3 files changed, 168 insertions(+), 0 deletions(-)
create mode 100755 t/t9115-git-svn-dcommit-funky-renames.sh
create mode 100644 t/t9115/funky-names.dump
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index f6fe78c..60b26d4 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -48,3 +48,29 @@ svnrepo="file://$svnrepo"
poke() {
test-chmtime +1 "$1"
}
+
+SVN_HTTPD_MODULE_PATH=/usr/lib/apache2/modules
+SVN_HTTPD_PATH=/usr/sbin/apache2
+SVN_HTTPD_PORT=3443
+start_httpd () {
+mkdir $GIT_DIR/logs
+
+ cat > "$GIT_DIR/httpd.conf" <<EOF
+ServerName "git-svn test"
+ServerRoot "$GIT_DIR"
+DocumentRoot "$GIT_DIR"
+PidFile "$GIT_DIR/httpd.pid"
+Listen 127.0.0.1:$SVN_HTTPD_PORT
+LoadModule dav_module $SVN_HTTPD_MODULE_PATH/mod_dav.so
+LoadModule dav_svn_module $SVN_HTTPD_MODULE_PATH/mod_dav_svn.so
+<Location /svn>
+ DAV svn
+ SVNPath $rawsvnrepo
+</Location>
+EOF
+ $SVN_HTTPD_PATH -f $GIT_DIR/httpd.conf -k start
+}
+
+stop_httpd () {
+ $SVN_HTTPD_PATH -f $GIT_DIR/httpd.conf -k stop
+}
diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
new file mode 100755
index 0000000..b451ca2
--- /dev/null
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+
+
+test_description='git-svn dcommit can commit renames of files with ugly names'
+
+. ./lib-git-svn.sh
+
+test_expect_success 'load repository with strange names' "
+ svnadmin load -q $rawsvnrepo < ../t9115/funky-names.dump &&
+ start_httpd
+"
+
+test_expect_success 'init and fetch repository' "
+ git svn init http://127.0.0.1:$SVN_HTTPD_PORT/svn &&
+ git svn fetch &&
+ git reset --hard git-svn
+ "
+
+test_expect_success 'create file in existing ugly and empty dir' '
+ mkdir "#{bad_directory_name}" &&
+ echo hi > "#{bad_directory_name}/ foo" &&
+ git update-index --add "#{bad_directory_name}/ foo" &&
+ git commit -m "new file in ugly parent" &&
+ git svn dcommit
+ '
+
+test_expect_success 'rename ugly file' '
+ git mv "#{bad_directory_name}/ foo" "file name with feces" &&
+ git commit -m "rename ugly file" &&
+ git svn dcommit --rmdir
+ '
+
+stop_httpd
+
+test_done
diff --git a/t/t9115/funky-names.dump b/t/t9115/funky-names.dump
new file mode 100644
index 0000000..da0440a
--- /dev/null
+++ b/t/t9115/funky-names.dump
@@ -0,0 +1,105 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 819c44fe-2bcc-4066-88e4-985e2bc0b418
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2007-07-12T07:54:26.062914Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 152
+Content-length: 152
+
+K 7
+svn:log
+V 44
+what will those wacky people think of next?
+
+K 10
+svn:author
+V 12
+normalperson
+K 8
+svn:date
+V 27
+2007-07-12T08:00:05.011573Z
+PROPS-END
+
+Node-path: leading space
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path: leading space file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 5
+Text-content-md5: e4fa20c67542cdc21271e08d329397ab
+Content-length: 15
+
+PROPS-END
+ugly
+
+
+Node-path: #{bad_directory_name}
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path: #{cool_name}
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 18
+Text-content-md5: 87dac40ca337dfa3dcc8911388c3ddda
+Content-length: 28
+
+PROPS-END
+strange name here
+
+
+Node-path: dir name with spaces
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path: file name with spaces
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 7
+Text-content-md5: c1f10cfd640618484a2a475c11410fd3
+Content-length: 17
+
+PROPS-END
+spaces
+
+
+Node-path: regular_dir_name
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
--
Eric Wong
^ permalink raw reply related
* 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