* Re: [PATCH] Prevent megablobs from gunking up git packs
From: Junio C Hamano @ 2007-05-23 0:28 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Dana How, git
In-Reply-To: <200705230144.38290.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> No, I was thinking about separate _kept_ pack (so it would be not
> repacked unless -f option is given) containing _only_ the large blobs.
> The only difference between this and your proposal is that megablobs
> would be in their mergablobs pack, but not loose.
I am not sure about the "unless -f option is given" part, but a
single .kept pack that contains only problematic blobs would be
an interesting experiment.
(0) prepare object names of problematic blobs, in huge.txt, one
object name per line;
(1) prepare a single pack that has them:
$ N=$(git-pack-object --depth=0 --window=0 pack <huge.txt)
$ echo 'Huge blobs -- do not repack' >pack-$N.keep
$ mv pack-$N.* .git/object/pack/.
(2) repack the remainder, with the default depth/window:
$ git repack -a -d
$ git prune
^ permalink raw reply
* Re: Git branching & pulling
From: Junio C Hamano @ 2007-05-23 0:35 UTC (permalink / raw)
To: Wink Saville; +Cc: Josef Weidendorfer, Steven Grimm, git, Paolo Bonzini
In-Reply-To: <d4cf37a60705221722t2167a0e8x810689218b87fb39@mail.gmail.com>
"Wink Saville" <wink@saville.com> writes:
>> Creation of a branch from another local one never has created
>> "branch.x.remote" or "branch.x.merge" entries. I am not even sure
>> that setting "branch.x.remote" to "." is working in the current version.
>
> I tired to create the appropriate entries and it didn't work,
> but maybe operator error.
>
>> BTW: There was some old behavior of "git pull" to always pull the master
>> branch from remote "origin" without any further parameters. I suppose that
>> you did not want this to happen in your example above ?!
>
> I expected it to pull from its upstream (i.e. the branches parent).
While "I branched and that means I will always merge from that
branch" does not hold true for everybody, we do have an option
to make that easy to work in recent git. Look in git-branch
documentation and check autosetupmerge configuration option.
I notice that Paolo did not update Documentation/config.txt
when he add this feature with 0746d19a; care to send in a patch
to correct this?
^ permalink raw reply
* Re: [PATCH] Prevent megablobs from gunking up git packs
From: Nicolas Pitre @ 2007-05-23 1:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Dana How, git
In-Reply-To: <7v3b1o758u.fsf@assigned-by-dhcp.cox.net>
On Tue, 22 May 2007, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > No, I was thinking about separate _kept_ pack (so it would be not
> > repacked unless -f option is given) containing _only_ the large blobs.
> > The only difference between this and your proposal is that megablobs
> > would be in their mergablobs pack, but not loose.
>
> I am not sure about the "unless -f option is given" part, but a
> single .kept pack that contains only problematic blobs would be
> an interesting experiment.
>
> (0) prepare object names of problematic blobs, in huge.txt, one
> object name per line;
>
> (1) prepare a single pack that has them:
>
> $ N=$(git-pack-object --depth=0 --window=0 pack <huge.txt)
> $ echo 'Huge blobs -- do not repack' >pack-$N.keep
> $ mv pack-$N.* .git/object/pack/.
If you're going to keep this pack, I think it might be worth attempting
deltas between those blobs anyway. If they ever deltify you'll gain in
disk space. And if they don't, well, you wasted the CPU cycles only
once. Unless you know for sure they're unlikely to deltify well.
Nicolas
^ permalink raw reply
* [PATCH v2] Prevent megablobs from gunking up git packs
From: Dana How @ 2007-05-23 2:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, danahow
git stores data in loose blobs or in packfiles. The former has
essentially now become an exception mechanism, to store unusually
*young* blobs. Here we re-use all this "exception" machinery to
store unusually *large* blobs as well.
This patch implements the following:
1. git pack-objects takes a new --max-blob-size=N flag, with the
effect that only blobs less than N KB are written to the
packfiles(s). If an already packed blob violates this limit
(perhaps these are fast-import packs or max-blob-size was
reduced), a new loose object is made as needed so data is not lost.
2. git repack inspects repack.maxblobsize . If set, its
value is passed to git pack-objects on the command line.
--max-blob-size=N is also accepted by git repack.
3. No other git pack-objects caller uses this feature or sees any change.
During pack *creation* this avoids copying & deltifying megablobs.
Therefore, for 100GB repositories, git repack can run in hours
for the first time after git fast-import, and in minutes thereafter.
You can still include megablobs in your packs thus:
% git-repack -a [-d] {with repack.maxblobsize = 256}
% git-repack --max-blob-size=0 [--max-pack-size=2047]
The first step creates a tight packfile and index with all metadata
and reasonable blob data, and the second collects the megablobs,
which could be enhanced with .keep file manipulation, etc.
During pack *use* this feature helps tighten access to heavily-used
metadata since it can't be separated by large blobs, and allows all
such metadata to be accessed through the smallest number of index files.
Megablobs are accessible with some overhead as loose objects,
which can be corrected with a second packing step as shown above
at the cost of more index files and packing time.
Documentation has been updated and operation with pack-object's
--stdout is prevented. This patch is based on "next".
Signed-off-by: Dana L. How <danahow@gmail.com>
---
Documentation/config.txt | 5 ++++
Documentation/git-pack-objects.txt | 6 +++++
Documentation/git-repack.txt | 10 ++++++++
builtin-pack-objects.c | 41 +++++++++++++++++++++++++++++++----
cache.h | 2 +
git-repack.sh | 9 +++++++-
sha1_file.c | 17 +++++++++++++-
7 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 179cb17..70fd3b1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -599,6 +599,11 @@ remotes.<group>::
The list of remotes which are fetched by "git remote update
<group>". See gitlink:git-remote[1].
+repack.maxblobsize::
+ Prevent gitlink:git-repack[1] from including blobs larger than
+ the specified number in kB, unless overridden by --max-blob-size=N switch.
+ Defaults to zero which means no maximum size is in effect.
+
repack.usedeltabaseoffset::
Allow gitlink:git-repack[1] to create packs that uses
delta-base offset. Defaults to false.
diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index cfe127a..ebd36b9 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -85,6 +85,12 @@ base-name::
times to get to the necessary object.
The default value for --window is 10 and --depth is 50.
+--max-blob-size=<n>::
+ Maximum size of included blobs, expressed in kB.
+ If specified, affected blobs only existing in packfiles
+ may be written out as new loose objects to prevent their loss.
+ The default is unlimited.
+
--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 2847c9b..7daa697 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -65,6 +65,12 @@ OPTIONS
to be applied that many times to get to the necessary object.
The default value for --window is 10 and --depth is 50.
+--max-blob-size=<n>::
+ Maximum size of included blobs, expressed in kB.
+ If specified, affected blobs only existing in packfiles
+ may be written out as new loose objects to prevent their loss.
+ The default is unlimited.
+
--max-pack-size=<n>::
Maximum size of each output packfile, expressed in MiB.
If specified, multiple packfiles may be created.
@@ -84,6 +90,10 @@ be able to read (this includes repositories from which packs can
be copied out over http or rsync, and people who obtained packs
that way can try to use older git with it).
+The configuration variable `repack.MaxBlobSize` provides the
+default for the --max-blob-size option if set. The latter
+takes precedence.
+
Author
------
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 19b0aa1..38e2a2b 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -17,7 +17,7 @@
static const char pack_usage[] = "\
git-pack-objects [{ -q | --progress | --all-progress }] [--max-pack-size=N] \n\
- [--local] [--incremental] [--window=N] [--depth=N] \n\
+ [--local] [--incremental] [--window=N] [--depth=N] [--max-blob-size=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]";
@@ -75,6 +75,7 @@ static int num_preferred_base;
static struct progress progress_state;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
+static uint32_t max_blob_size;
/*
* The object names in objects array are hashed with this hashtable,
@@ -371,8 +372,6 @@ static unsigned long write_object(struct sha1file *f,
pack_size_limit - write_offset : 0;
/* no if no delta */
int usable_delta = !entry->delta ? 0 :
- /* yes if unlimited packfile */
- !pack_size_limit ? 1 :
/* no if base written to previous pack */
entry->delta->offset == (off_t)-1 ? 0 :
/* otherwise double-check written to this
@@ -408,7 +407,7 @@ static unsigned long write_object(struct sha1file *f,
buf = read_sha1_file(entry->sha1, &type, &size);
if (!buf)
die("unable to read %s", sha1_to_hex(entry->sha1));
- if (size != entry->size)
+ if (size != entry->size && type == obj_type)
die("object %s size inconsistency (%lu vs %lu)",
sha1_to_hex(entry->sha1), size, entry->size);
if (usable_delta) {
@@ -564,6 +563,25 @@ static off_t write_one(struct sha1file *f,
return 0;
}
+ /* refuse to include megablobs */
+ if (max_blob_size && e->size >= max_blob_size) {
+ if (e->in_pack) {
+ /* rewrite as loose object so git-repack doesn't lose data */
+ void *buf;
+ enum object_type type;
+ unsigned long size;
+ buf = read_sha1_file(e->sha1, &type, &size);
+ if (!buf)
+ die("unable to read %s", sha1_to_hex(e->sha1));
+ if (write_sha1_file_ignore_packs(buf, size, typename(type), NULL) < 0)
+ die("failed to write object");
+ free(buf);
+ }
+ e->offset = (off_t)-1; /* might eject a reused delta base if mbs decreases */
+ written++;
+ return offset;
+ }
+
e->offset = offset;
size = write_object(f, e, offset);
if (!size) {
@@ -1422,13 +1440,16 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
/* Now some size filtering heuristics. */
trg_size = trg_entry->size;
+ src_size = src_entry->size;
+ /* prevent use if later dropped from packfile */
+ if (max_blob_size && (trg_size >= max_blob_size || src_size >= max_blob_size))
+ return 0;
max_size = trg_size/2 - 20;
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-1;
- src_size = src_entry->size;
sizediff = src_size < trg_size ? trg_size - src_size : 0;
if (sizediff >= max_size)
return 0;
@@ -1735,6 +1756,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
incremental = 1;
continue;
}
+ if (!prefixcmp(arg, "--max-blob-size=")) {
+ char *end;
+ max_blob_size = strtoul(arg+16, &end, 0) * 1024;
+ if (!arg[16] || *end)
+ usage(pack_usage);
+ continue;
+ }
if (!prefixcmp(arg, "--compression=")) {
char *end;
int level = strtoul(arg+14, &end, 0);
@@ -1855,6 +1883,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (!pack_to_stdout && thin)
die("--thin cannot be used to build an indexable pack.");
+ if (pack_to_stdout && max_blob_size)
+ die("--max-blob-size cannot be used to build a pack for transfer.");
+
prepare_packed_git();
if (progress)
diff --git a/cache.h b/cache.h
index ec85d93..a415924 100644
--- a/cache.h
+++ b/cache.h
@@ -343,6 +343,8 @@ extern int sha1_object_info(const unsigned char *, unsigned long *);
extern void * read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size);
extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);
extern int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
+extern int write_sha1_file_ignore_packs(void *buf, unsigned long len, const char *type,
+ unsigned char *return_sha1);
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type);
diff --git a/git-repack.sh b/git-repack.sh
index 4ea6e5b..6b4e1af 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -8,7 +8,7 @@ SUBDIRECTORY_OK='Yes'
. git-sh-setup
no_update_info= all_into_one= remove_redundant=
-local= quiet= no_reuse= extra=
+local= quiet= no_reuse= extra= max_blob_size=
while case "$#" in 0) break ;; esac
do
case "$1" in
@@ -18,6 +18,7 @@ do
-q) quiet=-q ;;
-f) no_reuse=--no-reuse-object ;;
-l) local=--local ;;
+ --max-blob-size=*) extra="$extra $1" max_blob_size=t ;;
--max-pack-size=*) extra="$extra $1" ;;
--window=*) extra="$extra $1" ;;
--depth=*) extra="$extra $1" ;;
@@ -35,6 +36,12 @@ true)
extra="$extra --delta-base-offset" ;;
esac
+# handle blob limiting
+if [ -z "$max_blob_size" ]; then
+ mbs="`git config --int repack.maxblobsize`"
+ [ -n "$mbs" ] && extra="$extra --max-blob-size=$mbs"
+fi
+
PACKDIR="$GIT_OBJECT_DIRECTORY/pack"
PACKTMP="$GIT_OBJECT_DIRECTORY/.tmp-$$-pack"
rm -f "$PACKTMP"-*
diff --git a/sha1_file.c b/sha1_file.c
index 12d2ef2..1424756 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1979,7 +1979,8 @@ int hash_sha1_file(const void *buf, unsigned long len, const char *type,
return 0;
}
-int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
+static int write_sha1_file_core(void *buf, unsigned long len, const char *type,
+ int checkpacks, unsigned char *returnsha1)
{
int size, ret;
unsigned char *compressed;
@@ -1997,7 +1998,7 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
filename = sha1_file_name(sha1);
if (returnsha1)
hashcpy(returnsha1, sha1);
- if (has_sha1_file(sha1))
+ if (checkpacks && has_sha1_file(sha1))
return 0;
fd = open(filename, O_RDONLY);
if (fd >= 0) {
@@ -2062,6 +2063,18 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
return move_temp_to_file(tmpfile, filename);
}
+int write_sha1_file(void *buf, unsigned long len, const char *type,
+ unsigned char *returnsha1)
+{
+ return write_sha1_file_core(buf, len, type, 1, returnsha1);
+}
+
+int write_sha1_file_ignore_packs(void *buf, unsigned long len, const char *type,
+ unsigned char *returnsha1)
+{
+ return write_sha1_file_core(buf, len, type, 0, returnsha1);
+}
+
/*
* We need to unpack and recompress the object for writing
* it out to a different file.
--
1.5.2.762.gd8c6-dirty
^ permalink raw reply related
* Re: Git string manipulation functions wrong?
From: Kyle Moffett @ 2007-05-23 3:22 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Petr Baudis, Erik Mouw, git
In-Reply-To: <20070521145925.GA6474@diana.vm.bytemark.co.uk>
On Mon, 21 May 2007 16:59:25 +0200 Karl Hasselström <kha@treskal.com> wrote:
> On 2007-05-21 16:36:16 +0200, Petr Baudis wrote:
> > It's the opposite for me - we don't properly set the NUL byte for
> > smoe of our strncpy() calls, but I don't really see his problem with
> > snprintf(), we seem to handle its return value correctly everywhere
> > (except diff.c, but there the buffer sizes should be designed in
> > such a way that an overflow should be impossible).
>
> I think this kind of detailed case-by-case analysis defeats Timo's
> point, though: that the C library functions make it too easy to write
> bugs. If it's necessary to do non-trivial bounds checking etc. at
> every call site, it doesn't really matter if we currently do get them
> all right; at some point, we _are_ going to miss one. Instead of using
> our collective C-fu to get difficult calls right, we should be using
> it to construct string routines that have low enough overhead that
> it's lost in the noise, and are dead simple to use (and, of course,
> that can be cleanly bypassed in the 1% of cases where it's necessary).
That would be mostly true, except for the fact that without snprintf()
returning how many bytes _would_ have been written, it's much harder to
reliably allocate buffers for the result on the first pass. For
example, this is a trivial implementation of an function which returns
a freshly-allocated formatted string:
char *data;
unsigned long len;
len = snprintf(NULL, 0, some_fmt, arg1, arg2, arg3);
if (!len)
return NULL;
data = malloc(len+1);
if (!data)
return NULL;
data[len] = '\0';
snprintf(data, len, some_fmt, arg1, arg2, arg3);
return data;
You can't do that without a loop if it returns how many bytes were
actually written (although some braindead platforms do that already).
Here's a function which handles both use-cases in an optimal way:
char *data = NULL;
unsigned long datalen = 0, len;
do {
len = snprintf(data, datalen, some_fmt, arg1, arg2, arg3);
if (!datalen) {
datalen = len ? len : 16;
data = malloc(datalen);
if (!data)
return NULL;
} else if (len >= datalen) {
void *newmem;
datalen = (len > datalen)?(len + 1):(datalen +16);
newmem = realloc(data, datalen);
if (!newmem) {
free(data);
return NULL
}
}
} while (len >= datalen);
data[len] = '\0';
return data;
Hopefully, on a nice modern platform, the first iteration will have len
equal to the ideal actual required length and so it will hit the first
case and carefully allocate exactly enough bytes, then on the second
loop through it will fill in exactly the required bytes and return
success. On one of the abovementioned dain-bramaged systems, this will
loop until snprintf doesn't use all the space in the buffer,
incrementing by some fixed value each time (in this implementation,
16). It should be obvious that correctly-implemented systems will be
significantly more performant than ones without the useful "feature" of
POSIX-compliance. :-D
Cheers,
Kyle Moffett
^ permalink raw reply
* Re: Commit ID in exported Tar Ball
From: Shawn O. Pearce @ 2007-05-23 5:22 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, git, Frank Lichtenheld, Johan Herland,
Thomas Glanzmann, Michael Gernoth, Linus Torvalds
In-Reply-To: <46538065.9080705@lsrfire.ath.cx>
Ren?? Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> $Id$ (and $commit$) is reversible, @@COMMITID@@ is not. That means you
> can create a synthetic file byte for byte with @@COMMITID@@ (and its not
> yet implemented brethren), but you can't do that with $Id$ -- it's
> impossible to get rid of the dollar signs.
Yes, and that's one of the big problems with the $Id$ syntax so
commonly used by versioning systems. Most files you want to insert
that automatic id into want a clean id string, not something that
starts with $Id: and ends with $...
Since we are apparently supporting $Foo: ...$ to collapse back to
$Foo$ reusing that syntax for git-archive is actually probably a
bad idea. We should support the checkout filters in git-archive (as
much as possible anyway) but what this thread has been going on is
something quite different... so we probably want a different syntax.
Which is why I'm also in favor of the @@COMMITID@@ syntax...
> > (1) introduce "const unsigned char commit_in_focus[20]",
> > globally available to git suite, and clear it at the
> > beginning of main();
>
> Ugh. Requiring another global variable doesn't smell like good design.
I agree. We already have a lot of globals. We need another one like
we need a hole in the head. Especially a global like this one... ;-)
> Do we want git-archive specific one-way conversions that are capable of
> creating files like git.spec? Or is this just a shiny toy hypnotizing
> me? 8-)
But aren't shiny toys fun? ;-)
--
Shawn.
^ permalink raw reply
* Re: Rebase max-pack-size?
From: Junio C Hamano @ 2007-05-23 5:29 UTC (permalink / raw)
To: Dana How; +Cc: Git Mailing List
In-Reply-To: <4648848B.1030304@gmail.com>
Dana How <danahow@gmail.com> writes:
> Let me know if you want them re-sent inline and/or separately.
Recently Linus was very unhappy on the kernel list when somebody
forwarded bunch of patches in a single message, saying we do
reviews one patch per message (and we do not do attachments).
I haven't been very strict about that kind of formality, as I
haven't felt the need too much so far. Let's see how well we
can do this with this message, which has 5 patches.
----------------------------------------------------------------
>>From 781820f147bcb12dae576734585b27f42faca3ea Mon Sep 17 00:00:00 2001
> From: Dana L. How <danahow@gmail.com>
> Date: Sun, 13 May 2007 11:28:19 -0700
> Subject: [PATCH] Alter sha1close() 3rd argument to request flush only
>
> update=0 suppressed writing the final SHA-1 but was not used.
> Now final=0 suppresses SHA-1 finalization, SHA-1 writing,
> and closing -- in other words, only flush the buffer.
>
> Signed-off-by: Dana L. How <danahow@gmail.com>
What it does is understandable but it somehow feels funny that
"sha1close(file, hashresult, 0)" does _not_ close it (and does
not hash either for obvious reasons ;-). I would say we should
let it pass this round, but might want to separate the first
part out into a separate "update hash and flush" function if we
get more callers.
----------------------------------------------------------------
>>From 0c75fd66aa6a758e245d85b9304e85a38e977f94 Mon Sep 17 00:00:00 2001
> From: Dana L. How <danahow@gmail.com>
> Date: Sun, 13 May 2007 11:34:56 -0700
> Subject: [PATCH 1/4] git-repack --max-pack-size: new file statics and code restructuring
>
> Add "pack_size_limit", the limit specified by --max-pack-size,
> "written_list", the list of objects written to the current pack,
> and "nr_written", the number of objects in written_list.
> Put "base_name" at file scope again and add forward declarations.
> Move write_index_file() call from cnd_pack_objects() to
> write_pack_file() since only the latter will know how
> many times to call write_index_file().
>
> Signed-off-by: Dana L. How <danahow@gmail.com>
I would have split this part a bit differently.
This is mostly about restructuring the code so that
write_index_file() is called from write_pack_file(), which by
itself is a very good change (but then we might have been better
off passing basename as a parameter).
You are not using written_list nor limit yet but are introducing
them in this step, which feels not quite right. I usually compile stuff
with -Werror, and if I ever have to bisect the series, this would bomb out
for these unused variables. Not nice.
----------------------------------------------------------------
>>From 40ac2f294ec68cae27f8a9262db4cab84f1d2257 Mon Sep 17 00:00:00 2001
> From: Dana L. How <danahow@gmail.com>
> Date: Sun, 13 May 2007 12:06:18 -0700
> Subject: [PATCH 2/4] git-repack --max-pack-size: write_{object,one}() respect pack limit
>
> With --max-pack-size, generate the appropriate write limit
> for each object and check against it before each group of writes.
> Update delta usability rules to handle base being in a previously-
> written pack. Inline sha1write_compress() so we know the
> exact size of the written data when it needs to be compressed.
> Detect and return write "failure".
>
> Signed-off-by: Dana L. How <danahow@gmail.com>
> ---
> builtin-pack-objects.c | 131 ++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 104 insertions(+), 27 deletions(-)
Again, this is split somewhat wrongly, as you do not have a way
to set max size from the caller, but more importantly, even
though write_one and write_object knows to obey the limit
(perhaps somebody bisecting this series later may set the limit
under the debugger, to work around the lack of option parser),
write_pack_file() does not notice zero return from write_one();
I would have added a check there to do:
die("sorry, limit reached and we do not have code to split the pack yet.")
which obviously can be updated in the next patch.
----------------------------------------------------------------
>>From 9ec3af1f52f41d0737b6271fded5e94527eb0466 Mon Sep 17 00:00:00 2001
> From: Dana L. How <danahow@gmail.com>
> Date: Sun, 13 May 2007 12:09:16 -0700
> Subject: [PATCH 3/4] git-repack --max-pack-size: split packs as asked by write_{object,one}()
>
> Rewrite write_pack_file() to break to a new packfile
> whenever write_object/write_one request it, and
> correct the header's object count in the previous packfile.
> Change write_index_file() to write an index
> for just the objects in the most recent packfile.
>
> Signed-off-by: Dana L. How <danahow@gmail.com>
> ---
> builtin-pack-objects.c | 120 ++++++++++++++++++++++++++++-------------------
> 1 files changed, 71 insertions(+), 49 deletions(-)
>
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 3023aac..ce9eb2d 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> ...
> @@ -652,7 +663,26 @@ static void write_pack_file(void)
> die("unable to rename temporary index file: %s",
> strerror(errno));
> puts(sha1_to_hex(object_list_sha1));
> + }
> +
> + /* mark written objects as written to previous pack */
> + for (j = 0; j < nr_written; j++) {
> + written_list[j]->offset = (off_t)-1;
> + }
> + nr_remaining -= nr_written;
> + } while (nr_remaining && i < nr_objects);
> +
> + free(written_list);
> + if (do_progress)
> + stop_progress(&progress_state);
> + if (written != nr_result)
> + die("wrote %u objects while expecting %u", written, nr_result);
> + for (j = 0; i < nr_objects; i++) {
> + struct object_entry *e = objects + i;
> + j += !e->offset && !e->preferred_base;
> }
I am a bit confused by this loop. Don't you have to start with i=0
for this check to be meaningful?
> + if (j)
> + die("wrote %u objects as expected but %u unwritten", written, j);
> }
----------------------------------------------------------------
>>From 279d1f9e0e7265c6ccf8759bbe8c5539bf3089fa Mon Sep 17 00:00:00 2001
> From: Dana L. How <danahow@gmail.com>
> Date: Sun, 13 May 2007 12:47:09 -0700
> Subject: [PATCH 4/4] git-repack --max-pack-size: add option parsing to enable feature
>
> Add --max-pack-size parsing and usage messages.
> Upgrade git-repack.sh to handle multiple packfile names,
> and build packfiles in GIT_OBJECT_DIRECTORY not GIT_DIR.
> Update documentation.
>
> Signed-off-by: Dana L. How <danahow@gmail.com>
> ---
> Documentation/git-pack-objects.txt | 5 +++++
> Documentation/git-repack.txt | 5 +++++
> builtin-pack-objects.c | 9 ++++++++-
> git-repack.sh | 14 ++++++++------
> 4 files changed, 26 insertions(+), 7 deletions(-)
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index ce9eb2d..930b57a 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> ...
> @@ -1713,6 +1713,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> pack_compression_level = level;
> continue;
> }
> + if (!prefixcmp(arg, "--max-pack-size=")) {
> + char *end;
> + pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
> + if (!arg[16] || *end)
> + usage(pack_usage);
> + continue;
> + }
> if (!prefixcmp(arg, "--window=")) {
> char *end;
> window = strtoul(arg+9, &end, 0);
Hmmm. I was almost going to suggest to have this spelled in
bytes, with suffixes like k/m/g. However, because wanting to
limit a pack under 1.4MB does not make much sense these days,
and because having to spell "up to 2GB" as 2047 is not too much
trouble, I think this is Ok.
Shouldn't we have a safety to error out when --stdout and
--max-pack-size are both given? Currently it silently ignores
the limit, doesn't it?
> diff --git a/git-repack.sh b/git-repack.sh
> index 8bf66a4..4ea6e5b 100755
> --- a/git-repack.sh
> +++ b/git-repack.sh
> ...
> @@ -35,7 +36,7 @@ true)
> esac
>
> PACKDIR="$GIT_OBJECT_DIRECTORY/pack"
> -PACKTMP="$GIT_DIR/.tmp-$$-pack"
> +PACKTMP="$GIT_OBJECT_DIRECTORY/.tmp-$$-pack"
> rm -f "$PACKTMP"-*
> trap 'rm -f "$PACKTMP"-*' 0 1 2 3 15
>
Although this is a good change, this hunk does not belong to
this.
----------------------------------------------------------------
Overall everything looks good, except some minor details noted
above. Separation of the commits into logical steps does not
need to be fixed up (they are already in 'next'), but follow-up
patches might be needed.
And I have to agree with Linus; responding this way was more
cumbersome than it should have been.
^ permalink raw reply
* Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h
From: Jonas Fonseca @ 2007-05-23 5:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <7vwsz07b10.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote Tue, May 22, 2007:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
> > Jonas Fonseca, Tue, May 22, 2007 23:29:45 +0200:
> >> + if (!strcmp(arg, "-h"))
> >> + usage(usage_str);
> >
> > $ git rev-list --usage
> > usage: git-rev-list [OPTION] <commit-id>... [ -- paths... ]
> > limiting output:
> > --max-count=nr
> > --max-age=epoch
> > ...
> >
> > Why should cherry-pick be different?
>
> Good question. FYI
>
> $ git rev-list --huh?
>
> works equally well ;-)
Because it is different?
$ git revert --why-must-it-be-so-hard-to-learn-git-sometimes
fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'
Because, contrary to git-rev-list, git-revert/cherry-pick is considered
part of the porcelain? Because asking that question to every small UI
improvement is not very useful?
And yes I could spell out '--help', but it just seems weird that I need
to know the calling convention of git-revert (_and_ git-cherry-pick for
that matter) in order to get the usage string, because that was what I
wanted to know in the first place. Anyway, if you don't like it, please
just drop the patch. :)
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h
From: Junio C Hamano @ 2007-05-23 5:52 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Alex Riesen, git
In-Reply-To: <20070523053110.GA23971@diku.dk>
Jonas Fonseca <fonseca@diku.dk> writes:
> Junio C Hamano <junkio@cox.net> wrote Tue, May 22, 2007:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>> ...
>> > Why should cherry-pick be different?
>>
>> Good question. FYI
>>
>> $ git rev-list --huh?
>>
>> works equally well ;-)
>
> Because it is different?
>
> $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes
> fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'
>
> Because, contrary to git-rev-list, git-revert/cherry-pick is considered
> part of the porcelain?
No, I did not notice it until now but you are right. The
command line argument parser for these commands is done somewhat
sloppily, compared to others.
How about doing something like this instead?
-- >8 --
Fix command line parameter parser of revert/cherry-pick
The parser was inconsistently done, in that it did not look at
the last command line parameter to see if it could be an unknown
option, although it was designed to notice unknown options if
they were given in positions the command expects to find them
(i.e. everything except the last parameter, which ought to be
<commit-ish>). This prevented a very natural invocation
$ git cherry-pick --help
from issuing the usage help.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/builtin-revert.c b/builtin-revert.c
index ea2f15b..80c348c 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -45,8 +45,10 @@ static void parse_options(int argc, const char **argv)
if (argc < 2)
usage(usage_str);
- for (i = 1; i < argc - 1; i++) {
+ for (i = 1; i < argc; i++) {
arg = argv[i];
+ if (arg[0] != '-')
+ break;
if (!strcmp(arg, "-n") || !strcmp(arg, "--no-commit"))
no_commit = 1;
else if (!strcmp(arg, "-e") || !strcmp(arg, "--edit"))
@@ -59,7 +61,8 @@ static void parse_options(int argc, const char **argv)
else if (strcmp(arg, "-r"))
usage(usage_str);
}
-
+ if (i != argc - 1)
+ usage(usage_str);
arg = argv[argc - 1];
if (get_sha1(arg, sha1))
die ("Cannot find '%s'", arg);
^ permalink raw reply related
* Re: Rebase max-pack-size?
From: Dana How @ 2007-05-23 6:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, danahow
In-Reply-To: <7virak5cr5.fsf@assigned-by-dhcp.cox.net>
On 5/22/07, Junio C Hamano <junkio@cox.net> wrote:
> Dana How <danahow@gmail.com> writes:
> ----------------------------------------------------------------
> > Subject: [PATCH] Alter sha1close() 3rd argument to request flush only
> >
> > update=0 suppressed writing the final SHA-1 but was not used.
> > Now final=0 suppresses SHA-1 finalization, SHA-1 writing,
> > and closing -- in other words, only flush the buffer.
>
> What it does is understandable but it somehow feels funny that
> "sha1close(file, hashresult, 0)" does _not_ close it (and does
> not hash either for obvious reasons ;-). I would say we should
> let it pass this round, but might want to separate the first
> part out into a separate "update hash and flush" function if we
> get more callers.
OK, see the list at the end.
> ----------------------------------------------------------------
> > Subject: [PATCH 1/4] git-repack --max-pack-size: new file statics and code restructuring
> >
> > Add "pack_size_limit", the limit specified by --max-pack-size,
> > "written_list", the list of objects written to the current pack,
> > and "nr_written", the number of objects in written_list.
> > Put "base_name" at file scope again and add forward declarations.
> > Move write_index_file() call from cnd_pack_objects() to
> > write_pack_file() since only the latter will know how
> > many times to call write_index_file().
>
> I would have split this part a bit differently.
>
> This is mostly about restructuring the code so that
> write_index_file() is called from write_pack_file(), which by
> itself is a very good change (but then we might have been better
> off passing basename as a parameter).
That would have worked too. I made base_name file scope
b/c that's how it was before NP's immediatley previous patches.
> You are not using written_list nor limit yet but are introducing
> them in this step, which feels not quite right. I usually compile stuff
> with -Werror, and if I ever have to bisect the series, this would bomb
> out for these unused variables. Not nice.
Very good point -- I did not think of that.
I will make sure in the future
each patch in a patchset has no warnings.
> ----------------------------------------------------------------
> > Subject: [PATCH 2/4] git-repack --max-pack-size: write_{object,one}() respect pack limit
> >
> > With --max-pack-size, generate the appropriate write limit
> > for each object and check against it before each group of writes.
> > Update delta usability rules to handle base being in a previously-
> > written pack. Inline sha1write_compress() so we know the
> > exact size of the written data when it needs to be compressed.
> > Detect and return write "failure".
>
> Again, this is split somewhat wrongly, as you do not have a way
> to set max size from the caller, but more importantly, even
> though write_one and write_object knows to obey the limit
> (perhaps somebody bisecting this series later may set the limit
> under the debugger, to work around the lack of option parser),
> write_pack_file() does not notice zero return from write_one();
> I would have added a check there to do:
>
> die("sorry, limit reached and we do not have code to split the pack yet.")
>
> which obviously can be updated in the next patch.
This makes sense to me. In a previous version of the patchset,
I had some temporary code like you say (just some extra
arguments, not any checking or die() calls) which was
immediately changed/replaced in the next patch.
Shawn or Nicolas didn't like that,
so I migrated to the split I used here.
I didn't really intend bisecting this patchset to do
anything useful *with respect to max-pack-size functionality*,
but bisecting it would be useful to detect if these patches
had broken something else. But I can rethink that in
the future.
> ----------------------------------------------------------------
> > Subject: [PATCH 3/4] git-repack --max-pack-size: split packs as asked by write_{object,one}()
> >
> > Rewrite write_pack_file() to break to a new packfile
> > whenever write_object/write_one request it, and
> > correct the header's object count in the previous packfile.
> > Change write_index_file() to write an index
> > for just the objects in the most recent packfile.
> >
> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> > --- a/builtin-pack-objects.c
> > +++ b/builtin-pack-objects.c
> > @@ -652,7 +663,26 @@ static void write_pack_file(void)
> > + } while (nr_remaining && i < nr_objects);
> > +
> > + for (j = 0; i < nr_objects; i++) {
> > + struct object_entry *e = objects + i;
> > + j += !e->offset && !e->preferred_base;
> > + }
> > + if (j)
> > + die("wrote %u objects as expected but %u unwritten", written, j);
> I am a bit confused by this loop. Don't you have to start with i=0
> for this check to be meaningful?
The previous do-while loop whose last line is shown
could end with i < nr_objects due to nr_remaining becoming
0. This means the objects [i ... nr_objects) have not been
inspected. They should all be either already written
or non-writable, which is what the two terms in the && expression
are testing. See list at end.
> ----------------------------------------------------------------
> > Subject: [PATCH 4/4] git-repack --max-pack-size: add option parsing to enable feature
> >
> > Add --max-pack-size parsing and usage messages.
> > Upgrade git-repack.sh to handle multiple packfile names,
> > and build packfiles in GIT_OBJECT_DIRECTORY not GIT_DIR.
> > Update documentation.
> >
> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> > --- a/builtin-pack-objects.c
> > +++ b/builtin-pack-objects.c
> > @@ -1713,6 +1713,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> > + if (!prefixcmp(arg, "--max-pack-size=")) {
> > + char *end;
> > + pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
> > + if (!arg[16] || *end)
> > + usage(pack_usage);
> > + continue;
> > + }
>
> Hmmm. I was almost going to suggest to have this spelled in
> bytes, with suffixes like k/m/g. However, because wanting to
> limit a pack under 1.4MB does not make much sense these days,
> and because having to spell "up to 2GB" as 2047 is not too much
> trouble, I think this is Ok.
I thought about suffixes too but I chose this way
b/c that's how git fast-import does it.
> Shouldn't we have a safety to error out when --stdout and
> --max-pack-size are both given? Currently it silently ignores
> the limit, doesn't it?
Yes & yes. Previously there was disagreement on this point.
One preference was to disallow the combination,
another was that it was useful and should be supported.
So I left the code in a state where a small follow-on patch
could make the decision. The current behavior is not
to disallow the combination, and it is well-defined
(you get a sequence of packs on stdout, concatenated,
whose headers indicate how many objects there are
in the current pack and all following).
I do not think --stdout && --max-pack-size is currently
useful, and a follow-on patch should complain.
See list at end.
> > diff --git a/git-repack.sh b/git-repack.sh
> > --- a/git-repack.sh
> > +++ b/git-repack.sh
> > @@ -35,7 +36,7 @@ true)
> > -PACKTMP="$GIT_DIR/.tmp-$$-pack"
> > +PACKTMP="$GIT_OBJECT_DIRECTORY/.tmp-$$-pack"
>
> Although this is a good change, this hunk does not belong to
> this.
OK.
If you decide not to keep this change here for historical clarity,
I can add it back later.
> Overall everything looks good, except some minor details noted
> above. Separation of the commits into logical steps does not
> need to be fixed up (they are already in 'next'), but follow-up
> patches might be needed.
OK, I will not edit and re-submit these patches.
But I will submit two follow-on patches:
Patch 1:
* Pull "sha1flush()" or similar out of sha1close() inside csum-file.c.
This will require some edits to callers.
Patch 2:
* Add a comment to "confusing loop" to explain what it's checking.
* Complain about --stdout && --max-pack-size combination.
This will have to happen tomorrow.
> And I have to agree with Linus; responding this way was more
> cumbersome than it should have been.
Understood.
Thanks,
--
Dana L. How danahow@gmail.com +1 650 804 5991 cell
^ permalink raw reply
* Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h
From: Jonas Fonseca @ 2007-05-23 6:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <7v8xbg5bno.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote Tue, May 22, 2007:
> Jonas Fonseca <fonseca@diku.dk> writes:
>
> > Junio C Hamano <junkio@cox.net> wrote Tue, May 22, 2007:
> >> Alex Riesen <raa.lkml@gmail.com> writes:
> >> ...
> >> > Why should cherry-pick be different?
> >>
> >> Good question. FYI
> >>
> >> $ git rev-list --huh?
> >>
> >> works equally well ;-)
> >
> > Because it is different?
> >
> > $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes
> > fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'
> >
> > Because, contrary to git-rev-list, git-revert/cherry-pick is considered
> > part of the porcelain?
>
> No, I did not notice it until now but you are right. The
> command line argument parser for these commands is done somewhat
> sloppily, compared to others.
>
> How about doing something like this instead?
FWIW, I like it. Sorry for my quick and dirty patch.
> -- >8 --
> Fix command line parameter parser of revert/cherry-pick
>
> The parser was inconsistently done, [...]in that it did not look at
> the last command line parameter to see if it could be an unknown
> option, although it was designed to notice unknown options if
> they were given in positions the command expects to find them
> (i.e. everything except the last parameter, which ought to be
> <commit-ish>). This prevented a very natural invocation
>
> $ git cherry-pick --help
>
> from issuing the usage help.
But --help is handled elsewhere, you meant -h ...
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h
From: Junio C Hamano @ 2007-05-23 7:04 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Alex Riesen, git
In-Reply-To: <20070523065750.GA25931@diku.dk>
Jonas Fonseca <fonseca@diku.dk> writes:
> But --help is handled elsewhere, you meant -h ...
Quite true. I meant --usage.
^ permalink raw reply
* Re: Rebase max-pack-size?
From: Junio C Hamano @ 2007-05-23 7:23 UTC (permalink / raw)
To: Dana How; +Cc: Git Mailing List
In-Reply-To: <56b7f5510705222333l6e285e67l1dc327322a2ab250@mail.gmail.com>
"Dana How" <danahow@gmail.com> writes:
> Patch 1:
> * Pull "sha1flush()" or similar out of sha1close() inside csum-file.c.
> This will require some edits to callers.
>
> Patch 2:
> * Add a comment to "confusing loop" to explain what it's checking.
> * Complain about --stdout && --max-pack-size combination.
>
> This will have to happen tomorrow.
>
>> And I have to agree with Linus; responding this way was more
>> cumbersome than it should have been.
> Understood.
>
> Thanks,
Although I mentioned it, as we have only one caller that does
not want to close the file, we probably do not need the first
patch right now.
Also thanks for the clarification on the (nr_result != nr_objects)
stuff -- it should have been obvious, as I was the guilty party
who originally introduced that condition (i.e. "thin" packs).
^ permalink raw reply
* [PATCH] make git-cvsimport work on ref-packed repositories
From: Stephan Springl @ 2007-05-23 7:13 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1742 bytes --]
Hi!
This helps us to use git-cvsimport on our ref-packed repositories.
Maybe you want to use this or a similar solution to be integrated in
stock git.
Thank you.
Stephan.
commit 83f8922f1ad385ef3493684838e11a34edbf68a7
Author: Stephan Springl <springl-git@bfw-online.de>
Date: Wed May 23 09:06:37 2007 +0200
Use git-for-each-ref to check whether the origin (or opt_o) branch exists.
This works in repositories that have their refs packed by
"git-pack-refs --all --prune" whereas testing the file
$git_dir/refs/heads/$opt_o does not.
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index ac74bc5..f68afe7 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -559,11 +559,6 @@ unless (-d $git_dir) {
$last_branch = $opt_o;
$orig_branch = "";
} else {
- -f "$git_dir/refs/heads/$opt_o"
- or die "Branch '$opt_o' does not exist.\n".
- "Either use the correct '-o branch' option,\n".
- "or import to a new repository.\n";
-
open(F, "git-symbolic-ref HEAD |") or
die "Cannot run git-symbolic-ref: $!\n";
chomp ($last_branch = <F>);
@@ -588,6 +583,11 @@ unless (-d $git_dir) {
$branch_date{$head} = $1;
}
close(H);
+ if (!exists $branch_date{$opt_o}) {
+ die "Branch '$opt_o' does not exist.\n".
+ "Either use the correct '-o branch' option,\n".
+ "or import to a new repository.\n";
+ }
}
-d $git_dir
--
Stephan Springl BFW Werner Völk GmbH
springl-git@bfw-online.de Energiemesstechnik & Service
+49 89 82917-452 Drosselgasse 5
82166 Gräfelfing/München
^ permalink raw reply related
* Re: git.or.cz IPv6
From: Petr Baudis @ 2007-05-23 8:23 UTC (permalink / raw)
To: Thomas Glanzmann; +Cc: GIT
In-Reply-To: <20070520094113.GB5015@cip.informatik.uni-erlangen.de>
Hi,
On Sun, May 20, 2007 at 11:41:13AM CEST, Thomas Glanzmann wrote:
> > I'd prefer to have AAAA records for IPv6-capable sites but if it
> > causes problems for more people, I will remove it - new technology
> > stuff is nice but the priority #1 is for the site to be reachable.
>
> btw. I still have trouble to access the site via IPv6.
hmm, I hoped it would get fixed, but it doesn't look so hopeful
anymore. :-( I've removed the AAAA records for now.
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
-- Samuel Beckett
^ permalink raw reply
* Re: Git branching & pulling
From: Josef Weidendorfer @ 2007-05-23 9:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Wink Saville, Steven Grimm, git, Paolo Bonzini
In-Reply-To: <7vwsz05qcq.fsf@assigned-by-dhcp.cox.net>
On Wednesday 23 May 2007, Junio C Hamano wrote:
> "Wink Saville" <wink@saville.com> writes:
>
> >> Creation of a branch from another local one never has created
> >> "branch.x.remote" or "branch.x.merge" entries. I am not even sure
> >> that setting "branch.x.remote" to "." is working in the current version.
> >
> > I tired to create the appropriate entries and it didn't work,
> > but maybe operator error.
> >
> >> BTW: There was some old behavior of "git pull" to always pull the master
> >> branch from remote "origin" without any further parameters. I suppose that
> >> you did not want this to happen in your example above ?!
> >
> > I expected it to pull from its upstream (i.e. the branches parent).
>
> While "I branched and that means I will always merge from that
> branch" does not hold true for everybody, we do have an option
> to make that easy to work in recent git. Look in git-branch
> documentation and check autosetupmerge configuration option.
Ah, I stand corrected: I just checked - git already allows to default
to even a local upstream for "git pull", not only a remote tracking
branch.
Very nice.
What about making "git rebase" without arguments default to
the stored upstream?
Josef
^ permalink raw reply
* [PATCH] Use git-for-each-ref to check whether the origin branch exists.
From: Johannes Schindelin @ 2007-05-23 11:13 UTC (permalink / raw)
To: Stephan Springl; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705230909310.25524@lar.bfw.de>
From: Stephan Springl <springl-git@bfw-online.de>
This works in repositories that have their refs packed by
"git-pack-refs --all --prune" whereas testing the file
$git_dir/refs/heads/$opt_o does not.
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
On Wed, 23 May 2007, Stephan Springl wrote:
> This helps us to use git-cvsimport on our ref-packed
> repositories.
Default for ref-packed repositories is to pack only the tags,
therefore you usually do not need this patch. However, it looks
obviously correct to me. A cursory test also showed that it does
not break anything.
> Maybe you want to use this or a similar solution to be
> integrated in stock git.
Unfortunately your patch is white-space corrupted (it has an extra
space on all lines starting with a space, it seems). Therefore I
redid it with this email.
It would be nice to follow Documentation/SubmittingPatches next
time. For example, I guess that you want to sign off on it...
git-cvsimport.perl | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index ac74bc5..f68afe7 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -559,11 +559,6 @@ unless (-d $git_dir) {
$last_branch = $opt_o;
$orig_branch = "";
} else {
- -f "$git_dir/refs/heads/$opt_o"
- or die "Branch '$opt_o' does not exist.\n".
- "Either use the correct '-o branch' option,\n".
- "or import to a new repository.\n";
-
open(F, "git-symbolic-ref HEAD |") or
die "Cannot run git-symbolic-ref: $!\n";
chomp ($last_branch = <F>);
@@ -588,6 +583,11 @@ unless (-d $git_dir) {
$branch_date{$head} = $1;
}
close(H);
+ if (!exists $branch_date{$opt_o}) {
+ die "Branch '$opt_o' does not exist.\n".
+ "Either use the correct '-o branch' option,\n".
+ "or import to a new repository.\n";
+ }
}
-d $git_dir
--
1.5.2.2527.ga2df
^ permalink raw reply related
* Re: [PATCH 1/3] Added generic string handling code.
From: Timo Sirainen @ 2007-05-23 10:49 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20070522134007.GK4489@pasky.or.cz>
[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]
On Tue, 2007-05-22 at 15:40 +0200, Petr Baudis wrote:
> On Sun, May 20, 2007 at 04:24:29AM CEST, Timo Sirainen wrote:
> > diff --git a/str.c b/str.c
> > new file mode 100644
> > index 0000000..d46e7f4
> > --- /dev/null
> > +++ b/str.c
> > @@ -0,0 +1,40 @@
> > +#include "str.h"
> > +
> > +void _str_append(struct string *str, const char *cstr)
>
> _ is reserved namespace.
I remember __ is, but was _ too? A lot of programs are using that. :)
> > +{
> > + unsigned int avail = str->size - str->len;
> > + unsigned int len = strlen(cstr);
> > +
> > + if (len >= avail) {
> > + len = avail - 1;
> > + str->overflowed = 1;
> > + }
> > + memcpy(str->buf + str->len, cstr, len);
> > + str->len += len;
> > + str->buf[str->len] = '\0';
>
> You can copy len + 1 and avoid this assignment.
Not if the string overflowed.
> > +}
> > +
> > +void _str_printfa(struct string *str, const char *fmt, ...)
>
> printfA?
"append". I think I got it originally from glib:
/* These aliases are included for compatibility. */
#define g_string_sprintf g_string_printf
#define g_string_sprintfa g_string_append_printf
If there's a chance that this string handling code would get used, I
could write another patch with a bit clearer names and support for
dynamically growing strings too. Something like:
STATIC_STRING(name, 1234);
sstr_append(name, "hello"); // or static_str_append()?
struct string *dyn = str_new(1024); // initial length
str_append(dyn, "hello");
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Use git-for-each-ref to check whether the origin branch exists.
From: Stephan Springl @ 2007-05-23 11:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705231153000.4113@racer.site>
On Wed, 23 May 2007, Johannes Schindelin wrote:
> From: Stephan Springl <springl-git@bfw-online.de>
> This works in repositories that have their refs packed by
> "git-pack-refs --all --prune" whereas testing the file
> $git_dir/refs/heads/$opt_o does not.
>
> Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Unfortunately your patch is white-space corrupted (it has an extra
> space on all lines starting with a space, it seems). Therefore I
> redid it with this email.
Sorry for that especially as we are very very strict with whitespace and
coding conventions in our own development ... arrg. Thanks anyway.
Stephan
^ permalink raw reply
* Re: [PATCH 1/3] Added generic string handling code.
From: Petr Baudis @ 2007-05-23 13:24 UTC (permalink / raw)
To: Timo Sirainen; +Cc: git
In-Reply-To: <1179917386.32181.1643.camel@hurina>
On Wed, May 23, 2007 at 12:49:46PM CEST, Timo Sirainen wrote:
> On Tue, 2007-05-22 at 15:40 +0200, Petr Baudis wrote:
> > On Sun, May 20, 2007 at 04:24:29AM CEST, Timo Sirainen wrote:
> > > diff --git a/str.c b/str.c
> > > new file mode 100644
> > > index 0000000..d46e7f4
> > > --- /dev/null
> > > +++ b/str.c
> > > @@ -0,0 +1,40 @@
> > > +#include "str.h"
> > > +
> > > +void _str_append(struct string *str, const char *cstr)
> >
> > _ is reserved namespace.
>
> I remember __ is, but was _ too? A lot of programs are using that. :)
C99 7.1.3 says that
-- All identifiers that begin with an underscore are always reserved
for use as identifiers with file scope in both the ordinary and tag name
spaces.
Hmm, this _is_ file scope, right?
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
-- Samuel Beckett
^ permalink raw reply
* Re: [PATCH 1/3] Added generic string handling code.
From: Timo Sirainen @ 2007-05-23 13:56 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20070523132429.GM4489@pasky.or.cz>
[-- Attachment #1: Type: text/plain, Size: 668 bytes --]
On Wed, 2007-05-23 at 15:24 +0200, Petr Baudis wrote:
> > > > +void _str_append(struct string *str, const char *cstr)
> > >
> > > _ is reserved namespace.
> >
> > I remember __ is, but was _ too? A lot of programs are using that. :)
>
> C99 7.1.3 says that
>
> -- All identifiers that begin with an underscore are always reserved
> for use as identifiers with file scope in both the ordinary and tag name
> spaces.
>
> Hmm, this _is_ file scope, right?
Right. I'll start changing my practices. Although grepping
under /usr/include shows that there are a lot of other software that
doesn't respect it either (X11, MySQL, OpenSSL at least).
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* HTTP trees trailing GIT trees
From: Panagiotis Issaris @ 2007-05-23 14:01 UTC (permalink / raw)
To: git
Hi all,
A few days ago I started noticing that my GIT tree cloned through http was
always trailing the one which was clone using the git protocol.
When pulling both clones to the lastest version I got this:
* Last one when accessed through http:
commit dedc2982f2f845357f28dff401fe5df8510c6a8f
Author: benoit <benoit <at> 9553f0bf-9b14-0410-a0b8-cfaf0461ba5b>
Date: Tue May 22 08:28:32 2007 +0000
* Last one when accessed through git:
commit 55d4b9a1d0bb75a085462d4f885301507d8fd082
Author: takis <takis <at> 9553f0bf-9b14-0410-a0b8-cfaf0461ba5b>
Date: Wed May 23 09:07:57 2007 +0000
I reported this to the person who had setup the repository:
http://article.gmane.org/gmane.comp.video.ffmpeg.devel/51151
But unfortunately, the problem seems to remain.
Is this a known problem, or might this be a bug or misconfiguration?
I am using git version 1.5.2.35.ga334 when using http and git version 1.5.1.3
when using the git protocol.
The repository is available through http as:
http://git.mplayerhq.hu/ffmpeg
Through the git protocol:
git://git.mplayerhq.hu/ffmpeg
And through gitweb:
http://git.mplayerhq.hu/
With friendly regards,
Takis
^ permalink raw reply
* Re: HTTP trees trailing GIT trees
From: Petr Baudis @ 2007-05-23 14:05 UTC (permalink / raw)
To: Panagiotis Issaris; +Cc: git
In-Reply-To: <loom.20070523T154909-285@post.gmane.org>
Hi,
On Wed, May 23, 2007 at 04:01:33PM CEST, Panagiotis Issaris wrote:
> I reported this to the person who had setup the repository:
> http://article.gmane.org/gmane.comp.video.ffmpeg.devel/51151
>
> But unfortunately, the problem seems to remain.
>
> Is this a known problem, or might this be a bug or misconfiguration?
did any push happenned since the post-update hook was enabled? It
takes effect only after the next push. So far,
http://git.mplayerhq.hu/ffmpeg/info/refs
and
http://git.mplayerhq.hu/ffmpeg/refs/heads/master
is still out-of-sync (keeping this in sync is what is the job of the
post-update hook, or git-update-server-info respectively).
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
-- Samuel Beckett
^ permalink raw reply
* Re: HTTP trees trailing GIT trees
From: Panagiotis Issaris @ 2007-05-23 14:15 UTC (permalink / raw)
To: git
In-Reply-To: <20070523140552.GN4489@pasky.or.cz>
Hi,
Petr Baudis <pasky <at> suse.cz> writes:
>
> Hi,
>
> On Wed, May 23, 2007 at 04:01:33PM CEST, Panagiotis Issaris wrote:
> > I reported this to the person who had setup the repository:
> > http://article.gmane.org/gmane.comp.video.ffmpeg.devel/51151
> >
> > But unfortunately, the problem seems to remain.
> >
> > Is this a known problem, or might this be a bug or misconfiguration?
>
> did any push happenned since the post-update hook was enabled? It
> takes effect only after the next push. So far,
>
> http://git.mplayerhq.hu/ffmpeg/info/refs
>
> and
>
> http://git.mplayerhq.hu/ffmpeg/refs/heads/master
>
> is still out-of-sync (keeping this in sync is what is the job of the
> post-update hook, or git-update-server-info respectively).
>
Yes, I'd think so, as Måns stated that he had enabled the hook on
2007-05-22 20:43:27. The last commit shown on http://git.mplayerhq.hu/
through gitweb occurred 25 minutes ago ("Wed, 23 May 2007 13:46:11 +0000").
Thanks for your fast reply.
With friendly regards,
Takis
^ permalink raw reply
* Re: HTTP trees trailing GIT trees
From: Panagiotis Issaris @ 2007-05-23 14:11 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20070523140552.GN4489@pasky.or.cz>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
Petr Baudis wrote:
> Hi,
>
> On Wed, May 23, 2007 at 04:01:33PM CEST, Panagiotis Issaris wrote:
>> I reported this to the person who had setup the repository:
>> http://article.gmane.org/gmane.comp.video.ffmpeg.devel/51151
>>
>> But unfortunately, the problem seems to remain.
>>
>> Is this a known problem, or might this be a bug or misconfiguration?
>
> did any push happenned since the post-update hook was enabled? It
> takes effect only after the next push. So far,
>
> http://git.mplayerhq.hu/ffmpeg/info/refs
>
> and
>
> http://git.mplayerhq.hu/ffmpeg/refs/heads/master
>
> is still out-of-sync (keeping this in sync is what is the job of the
> post-update hook, or git-update-server-info respectively).
Yes, I'd think so, as Måns stated that he had enabled the hook on
2007-05-22 20:43:27. The last commit shown on http://git.mplayerhq.hu/
through gitweb occurred 25 minutes ago ("Wed, 23 May 2007 13:46:11 +0000").
Thanks for your fast reply.
With friendly regards,
Takis
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGVEuA9kOxLuzz4CkRAkXTAJ9xqbkUO6qF88HwV7GSYnVkV+VRNgCfSr/D
ZPPYBfeD9RpqbXNKwmukObc=
=wkyo
-----END PGP SIGNATURE-----
^ 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