* [PATCH] cvsimport: replace anonymous sub ref with a normal sub
From: Martin Langhoff @ 2006-05-22 12:45 UTC (permalink / raw)
To: git; +Cc: Martin Langhoff
commit() does not need to be an anonymous subreference. Keep it simple.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
git-cvsimport.perl | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
a0bbc1c2010ca46fc215453d5e4c4853c679f950
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index c1923d1..2ecfe14 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -563,7 +563,7 @@ my $state = 0;
my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
my(@old,@new,@skipped);
-my $commit = sub {
+sub commit {
my $pid;
while(@old) {
my @o2;
@@ -853,7 +853,7 @@ # VERSION:1.96->1.96.2.1
if ($opt_L && $commitcount++ >= $opt_L) {
last;
}
- &$commit();
+ commit();
$state = 1;
} elsif($state == 11 and /^-+$/) {
$state = 1;
@@ -863,7 +863,7 @@ # VERSION:1.96->1.96.2.1
print "* UNKNOWN LINE * $_\n";
}
}
-&$commit() if $branch and $state != 11;
+commit() if $branch and $state != 11;
unlink($git_index);
--
1.3.2.g82000
^ permalink raw reply related
* [PATCH] cvsimport: minor fixups
From: Martin Langhoff @ 2006-05-22 12:45 UTC (permalink / raw)
To: git; +Cc: Martin Langhoff
Cleanup @skipped after it's used. Close a fhandle.
Removing suspects one at a time.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
git-cvsimport.perl | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
93bef2832d30c9a04e95ff348b9ab8ab8cabee98
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 2ecfe14..176b787 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -650,6 +650,8 @@ sub commit {
"GIT_COMMITTER_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
"git-commit-tree", $tree,@par);
die "Cannot exec git-commit-tree: $!\n";
+
+ close OUT;
}
$pw->writer();
$pr->reader();
@@ -661,6 +663,7 @@ sub commit {
if (@skipped) {
$logmsg .= "\n\n\nSKIPPED:\n\t";
$logmsg .= join("\n\t", @skipped) . "\n";
+ @skipped = ();
}
print $pw "$logmsg\n"
--
1.3.2.g82000
^ permalink raw reply related
* Re: irc usage..
From: Martin Langhoff @ 2006-05-22 12:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: Donnie Berkholz, Yann Dirson, Git Mailing List, Matthias Urlichs,
Johannes Schindelin
In-Reply-To: <Pine.LNX.4.64.0605220203200.3697@g5.osdl.org>
On 5/22/06, Linus Torvalds <torvalds@osdl.org> wrote:
> On Mon, 22 May 2006, Martin Langhoff wrote:
> >
> > Or a slow leak in Perl? The 5.8.8 release notes do talk about some
> > leaks being fixed, but this 5.8.8 isn't making a difference.
> >
> > Working on it.
>
> Thanks. Looking at what I did convert, that horrid gentoo CVS tree is
> interesting. The resulting (partial) git history has 93413 commits and
> 850,000+ objects total, all in a totally linear history.
Ok, so there's 3 patches posted that should help narrow down the
problem. There's a new -L <imit> so that Donnie can get his stuff done
by running it in a while(true) loop. Not proud of it, but hey.
And there are two patches that I suspect may fix the leak. After
applying them, the cvsimport process grows up to ~13MB and then tapers
off, at least as far as my patience has gotten me. It's late on this
side of the globe so I'll look at the results tomorrow morning.
(BTW, I typo-ed Linus' address in the git-send-email invocation. Will
resend to him separately)
I'll also prep a patch as Linus suggests to do auto-repacking while
the import runs so we don't eat up the harddisk.
> git would basically cut down the disk usage for a live
> repo by a factor of 7 or so.
>
> _And_ I can do a "git log origin > /dev/null" in about 2.4 seconds. Take
> that, CVS.
Heh. Faster Gitticat, Kill Kill Kill!
martin
^ permalink raw reply
* Re: avoid atoi, when possible; int overflow -> heap corruption
From: Morten Welinder @ 2006-05-22 13:16 UTC (permalink / raw)
To: Jim Meyering; +Cc: Junio C Hamano, git
In-Reply-To: <871wumim28.fsf_-_@rho.meyering.net>
> There are about 20 uses of atoi, and most calls can return
> a usable result in spite of an invalid input -- just because
> atoi returns the same thing for "99" as "99-and-any-suffix".
> It would be better not to ignore invalid inputs.
atoi has undefined behaviour for "99-and-any-suffix". You might
get lucky and get back 99, but you might also get a random value
or a core dump.
Morten
^ permalink raw reply
* Re: avoid atoi, when possible; int overflow -> heap corruption
From: Jim Meyering @ 2006-05-22 13:31 UTC (permalink / raw)
To: Morten Welinder; +Cc: Junio C Hamano, git
In-Reply-To: <118833cc0605220616t75a182b1oa404d5efe8a1f5d9@mail.gmail.com>
"Morten Welinder" <mwelinder@gmail.com> wrote:
>> There are about 20 uses of atoi, and most calls can return
>> a usable result in spite of an invalid input -- just because
>> atoi returns the same thing for "99" as "99-and-any-suffix".
>> It would be better not to ignore invalid inputs.
>
> atoi has undefined behaviour for "99-and-any-suffix". You might
> get lucky and get back 99, but you might also get a random value
> or a core dump.
I've never heard of that.
POSIX says that atoi(str) is equivalent to:
(int) strtol(str, (char **)NULL, 10)
except that the handling of errors may differ.
If the value cannot be represented, the behavior is undefined.
Since strtol works fine with such a suffix, and since 99 can be
represented, I don't see why there would be any undefined behavior.
Do you know of an implementation for which `atoi ("99-and-any-suffix")'
does anything other than return 99?
^ permalink raw reply
* Re: avoid atoi, when possible; int overflow -> heap corruption
From: Jeff King @ 2006-05-22 13:37 UTC (permalink / raw)
To: Morten Welinder; +Cc: git
In-Reply-To: <118833cc0605220616t75a182b1oa404d5efe8a1f5d9@mail.gmail.com>
On Mon, May 22, 2006 at 09:16:50AM -0400, Morten Welinder wrote:
> atoi has undefined behaviour for "99-and-any-suffix". You might
> get lucky and get back 99, but you might also get a random value
> or a core dump.
Where do you get that from? The standard claims that it converts "the
initial portion of the string pointed to" (7.20.1.2). Furthermore, atoi
is equivalent to strtol with a base of 10 (with the exception of range
errors). From 7.20.1.4, paragraph 2:
The strtol [...] functions [...] decompose the input string into three
parts: an initial, possibly empty, sequence of white-space characters
[...], a subject sequence resembling an integer represented in some
radix determined by the value of base, and a final string of one or
more unrecognized characters...
If no conversion can be performed (i.e., you feed it garbage with no
number), zero is returned.
atoi does NOT handle range errors, however; the behavior is undefined in
that case. In practice, I expect most implementations do some sort of
wrapping.
-Peff
^ permalink raw reply
* Re: avoid atoi, when possible; int overflow -> heap corruption
From: Morten Welinder @ 2006-05-22 13:54 UTC (permalink / raw)
To: Morten Welinder, git
In-Reply-To: <20060522133746.GA12302@coredump.intra.peff.net>
My copy (which is admittedly a draft because I am cheap) does not
restrict undefined behaviour to _range_ errors, but simply says
"Except for the behavior on error, they are equivalent to [the strtol call]"
M.
^ permalink raw reply
* Re: [PATCH 2/3] tutorial: expanded discussion of commit history
From: J. Bruce Fields @ 2006-05-22 14:18 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <e4ruku$5uk$1@sea.gmane.org>
On Mon, May 22, 2006 at 11:01:20AM +0200, Jakub Narebski wrote:
> Junio C Hamano wrote:
> > I do not think being able to do diff with arbitrary stage is
> > often used in practice. By definition, you would want to do
> > diff with a stage during a conflicted merge, and most of the
> > time the default combined diff without any colon form should
> > give you the most useful results. Also, ":<path>" to mean the
> > entry in the index is often equivalent to "git diff --cached".
> >
> > IOW, these are obscure special purpose notation, and I do not
> > think tutorial is a good place to cover them.
>
> Hmmm... perhaps in tutorial-3.txt, covering merges and how to resolve
> conflicted merge, cherry picking, reverting and rebasing.
Even then I had the impression that stages were pretty much invisible to
users. So that should stay in core-tutorial.txt. Which could use some
revision (Junio had some ideas) but I'm personally more interested in
end-user documentation than developer documentation for now.
So I'd imagined future tutorial parts cannibalizing everyday.txt and the
howto's.
--b.
^ permalink raw reply
* [PATCH 0/2] tagsize < 8kb restriction
From: Björn Engelmann @ 2006-05-22 14:48 UTC (permalink / raw)
To: git
Hi,
I am currently working on an interface for source code quality assurance
tools to automatically scan newly commited code. Since it is the only
way to add data (scan-results) to an already-existing commit, I decided
to use tags for that.
Since the scan-results will most definitly exeed the 8kb-limit, I would
like to remove this artificial restriction.
I know I could also
git-hash-object -t tag -w
but prefer using the "official" way.
In order not to write duplicate code I used parts of index_pipe() in
sha1_file.c
What I found odd when writing the patch was that main() in mktag.c uses
xread() to read from stdin (which respects EAGAIN and EINTR return
values), but index_pipe() in sha1_file.c just uses read() for doing
merely the same thing. For unifying both routines i found that xread()
might be the better choice.
Removing the restriction was pretty straightforward but do you think
this would break something in other places ?
[PATCH 1/2]
remove the < 8kb restrinction from git-mktag
[PATCH 2/2]
add more informative error messages to git-mktag
Bj
^ permalink raw reply
* [PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag
From: Björn Engelmann @ 2006-05-22 14:48 UTC (permalink / raw)
To: git
Signed-off-by: Björn Engelmann <BjEngelmann@gmx.de>
---
5653af26c2cdb31e1f5646b8e563cc8fa27a8d43
cache.h | 1 +
mktag.c | 29 ++++++++++++++++-------------
sha1_file.c | 46 ++++++++++++++++++++++++++++++++++++----------
3 files changed, 53 insertions(+), 23 deletions(-)
5653af26c2cdb31e1f5646b8e563cc8fa27a8d43
diff --git a/cache.h b/cache.h
index 4b7a439..19e90eb 100644
--- a/cache.h
+++ b/cache.h
@@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en
extern int ce_modified(struct cache_entry *ce, struct stat *st, int);
extern int ce_path_match(const struct cache_entry *ce, const char
**pathspec);
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int
write_object, const char *type);
+extern int read_pipe(int fd, char** return_buf, unsigned long*
return_size);
extern int index_pipe(unsigned char *sha1, int fd, const char *type,
int write_object);
extern int index_path(unsigned char *sha1, const char *path, struct
stat *st, int write_object);
extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
diff --git a/mktag.c b/mktag.c
index 2328878..79c466c 100644
--- a/mktag.c
+++ b/mktag.c
@@ -45,7 +45,7 @@ static int verify_tag(char *buffer, unsi
unsigned char sha1[20];
const char *object, *type_line, *tag_line, *tagger_line;
- if (size < 64 || size > MAXSIZE-1)
+ if (size < 64)
return -1;
buffer[size] = 0;
@@ -105,8 +105,8 @@ static int verify_tag(char *buffer, unsi
int main(int argc, char **argv)
{
- unsigned long size;
- char buffer[MAXSIZE];
+ unsigned long size = 4096;
+ char *buffer = malloc(size);
unsigned char result_sha1[20];
if (argc != 1)
@@ -114,21 +114,24 @@ int main(int argc, char **argv)
setup_git_directory();
- // Read the signature
- size = 0;
- for (;;) {
- int ret = xread(0, buffer + size, MAXSIZE - size);
- if (ret <= 0)
- break;
- size += ret;
+ if (read_pipe(0, &buffer, &size)) {
+ free(buffer);
+ die("could not read from stdin");
}
-
+
// Verify it for some basic sanity: it needs to start with "object
<sha1>\ntype\ntagger "
- if (verify_tag(buffer, size) < 0)
+ if (verify_tag(buffer, size) < 0) {
+ free(buffer);
die("invalid tag signature file");
+ }
- if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
+ if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {
+ free(buffer);
die("unable to write tag file");
+ }
+
+ free(buffer);
+
printf("%s\n", sha1_to_hex(result_sha1));
return 0;
}
diff --git a/sha1_file.c b/sha1_file.c
index 2230010..ad02255 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1645,16 +1645,24 @@ int has_sha1_file(const unsigned char *s
return find_sha1_file(sha1, &st) ? 1 : 0;
}
-int index_pipe(unsigned char *sha1, int fd, const char *type, int
write_object)
+/*
+ * reads from fd as long as possible into a supplied buffer of size bytes.
+ * If neccessary the buffer's size is increased using realloc()
+ *
+ * returns 0 if anything went fine and -1 otherwise
+ *
+ * NOTE: both buf and size may change, but even when -1 is returned
+ * you still have to free() it yourself.
+ */
+int read_pipe(int fd, char** return_buf, unsigned long* return_size)
{
- unsigned long size = 4096;
- char *buf = malloc(size);
- int iret, ret;
+ char* buf = *return_buf;
+ unsigned long size = *return_size;
+ int iret;
unsigned long off = 0;
- unsigned char hdr[50];
- int hdrlen;
+
do {
- iret = read(fd, buf + off, size - off);
+ iret = xread(fd, buf + off, size - off);
if (iret > 0) {
off += iret;
if (off == size) {
@@ -1663,16 +1671,34 @@ int index_pipe(unsigned char *sha1, int
}
}
} while (iret > 0);
- if (iret < 0) {
+
+ *return_buf = buf;
+ *return_size = off;
+
+ if (iret < 0)
+ return -1;
+ return 0;
+}
+
+int index_pipe(unsigned char *sha1, int fd, const char *type, int
write_object)
+{
+ unsigned long size = 4096;
+ char *buf = malloc(size);
+ int ret;
+ unsigned char hdr[50];
+ int hdrlen;
+
+ if (read_pipe(fd, &buf, &size)) {
free(buf);
return -1;
}
+
if (!type)
type = blob_type;
if (write_object)
- ret = write_sha1_file(buf, off, type, sha1);
+ ret = write_sha1_file(buf, size, type, sha1);
else {
- write_sha1_file_prepare(buf, off, type, sha1, hdr, &hdrlen);
+ write_sha1_file_prepare(buf, size, type, sha1, hdr, &hdrlen);
ret = 0;
}
free(buf);
--
1.3.3.g5045-dirty
^ permalink raw reply related
* [PATCH 2/2] added more informative error messages to git-mktag
From: Björn Engelmann @ 2006-05-22 14:49 UTC (permalink / raw)
To: git
Signed-off-by: Björn Engelmann <BjEngelmann@gmx.de>
---
5045fd2336abbe57f191e8839759369ee1c7cc6e
mktag.c | 41 +++++++++++++++++++++++++++++++----------
1 files changed, 31 insertions(+), 10 deletions(-)
5045fd2336abbe57f191e8839759369ee1c7cc6e
diff --git a/mktag.c b/mktag.c
index 79c466c..9f4878c 100644
--- a/mktag.c
+++ b/mktag.c
@@ -45,42 +45,60 @@ static int verify_tag(char *buffer, unsi
unsigned char sha1[20];
const char *object, *type_line, *tag_line, *tagger_line;
- if (size < 64)
+ if (size < 64) {
+ printf("wanna fool me ? you obviously got the size wrong !\n");
return -1;
+ }
buffer[size] = 0;
/* Verify object line */
object = buffer;
- if (memcmp(object, "object ", 7))
+ if (memcmp(object, "object ", 7)) {
+ printf("char%i: does not start with \"object \"\n", 0);
return -1;
- if (get_sha1_hex(object + 7, sha1))
+ }
+ if (get_sha1_hex(object + 7, sha1)) {
+ printf("char%i: could not get SHA1 hash\n", 7);
return -1;
+ }
/* Verify type line */
type_line = object + 48;
- if (memcmp(type_line - 1, "\ntype ", 6))
+ if (memcmp(type_line - 1, "\ntype ", 6)) {
+ printf("char%i: could not find \"\\ntype \"\n", 47);
return -1;
+ }
/* Verify tag-line */
tag_line = strchr(type_line, '\n');
- if (!tag_line)
+ if (!tag_line) {
+ printf("char%i: could not find next \"\\n\"\n", (int)type_line
- (int)buffer);
return -1;
+ }
tag_line++;
- if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
+ if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n') {
+ printf("char%i: no \"tag \" found\n", (int)tag_line - (int)buffer);
return -1;
+ }
/* Get the actual type */
typelen = tag_line - type_line - strlen("type \n");
- if (typelen >= sizeof(type))
+ if (typelen >= sizeof(type)) {
+ printf("char%i: type too long\n", (int)type_line + strlen("type
\n") - (int)buffer);
return -1;
+ }
memcpy(type, type_line+5, typelen);
type[typelen] = 0;
/* Verify that the object matches */
- if (get_sha1_hex(object + 7, sha1))
+ if (get_sha1_hex(object + 7, sha1)) {
+ printf("char%i: could not get SHA1 hash but this is really odd
since i got it before !\n", 7);
return -1;
- if (verify_object(sha1, type))
+ }
+ if (verify_object(sha1, type)) {
+ printf("char%i: could not verify object %s\n", 7, sha1);
return -1;
+ }
/* Verify the tag-name: we don't allow control characters or spaces
in it */
tag_line += 4;
@@ -90,14 +108,17 @@ static int verify_tag(char *buffer, unsi
break;
if (c > ' ')
continue;
+ printf("char%i: could not verify tag name\n", (int)tag_line -
(int)buffer);
return -1;
}
/* Verify the tagger line */
tagger_line = tag_line;
- if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
+ if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n')) {
+ printf("char%i: could not find \"tagger\"\n", (int)tagger_line
- (int)buffer);
return -1;
+ }
/* The actual stuff afterwards we don't care about.. */
return 0;
--
1.3.3.g5045-dirty
^ permalink raw reply related
* Re: [PATCH 3/3] tutorial: add discussion of index file, object database
From: Jakub Narebski @ 2006-05-22 15:27 UTC (permalink / raw)
To: git
In-Reply-To: <1148255528.61d5d241.2@fieldses.org>
J. Bruce Fields wrote:
> +With the right arguments, git diff can also show us the difference
> +between the working directory and the last commit, or between the
> +index and the last commit:
> +
> +------------------------------------------------
> +$ git diff HEAD
[...]
> +$ git diff --cached
Junio C Hamano wrote:
> Also, ":<path>" to mean the
> entry in the index is often equivalent to "git diff --cached".
>
> IOW, these are obscure special purpose notation, and I do not
> think tutorial is a good place to cover them.
Hmm, wouldn't it be nice to have here :<path> mentioned as
alternative/extension to --cached, and <ref>:<path> including
HEAD:<path> as an extension of HEAD, and <path> as an extension
to not having arguments? By extension I mean adding path limiting
and some more advanced selection of things to diff.
--
Jakub Narebski
Warsaw, Poland
^ permalink raw reply
* Re: Segfaults with USE_CURL_MULTI
From: Pavel Roskin @ 2006-05-22 15:56 UTC (permalink / raw)
To: Florian Weimer; +Cc: git
In-Reply-To: <87fyj4y1lx.fsf@mid.deneb.enyo.de>
On Sat, 2006-05-20 at 20:47 +0200, Florian Weimer wrote:
> Is anybody else seeing segfaults on dumb HTTP pull with
> USE_CURL_MULTI?
_Everybody_ is seeing them! Just look for "segfaults" in the archives:
http://marc.theaimsgroup.com/?l=git&w=2&r=1&s=segfaults&q=b
This patch looks promising, but I'm yet to test it:
http://marc.theaimsgroup.com/?l=git&m=114816558325617&w=2
> For example, this crashes for me:
>
> $ git clone http://git.enyo.de/fw/debian/debfoster.git upstream
>
> GDB shows that this happens inside the call to curl_multi_perform.
Same for me:
http://marc.theaimsgroup.com/?l=git&m=114790994726570&w=2
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: synchronizing incremental git changes to cvs
From: Pavel Roskin @ 2006-05-22 16:29 UTC (permalink / raw)
To: Jim Meyering; +Cc: Johannes Schindelin, git
In-Reply-To: <873bf3jy2t.fsf@rho.meyering.net>
Hello, Jim!
On Sun, 2006-05-21 at 15:40 +0200, Jim Meyering wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Sun, 21 May 2006, Jim Meyering wrote:
> >
> >> Why am I interested? I want to switch the development of GNU coreutils
> >> from cvs to git.
I believe you have a very good reason to talk to decision makers in FSF.
Savannah is very poorly maintained, and I actually took one of my
projects (Orinoco driver) to SourceForge Subversion.
If losing a Linux driver is next to nothing, losing GNU coreutils is a
big deal for the GNU development site. You are likely to be heard if
you request git support.
> I would also like to continue making the repository
> >> available via cvs, for the sake of continuity. At worst, I can always
> >> cut the CVS cord, but that's a last resort.
Subversion is as easy as CVS for potential users, but it has a useful
"log" command if nothing else. It also have real changesets, which
means no more guesswork when moving changes back and forth.
> > If you only want to make a cvs repository available for tracking the
> > project, git-cvsserver is what you want. It is even faster than the
> > original cvs...
>
> That might work if I had sufficient access to the system hosting the
> public CVS repository. But there are restrictions (like no ssh access).
> Currently I rsync the master repo to an intermediate site, from which
> it is periodically pulled by savannah. Paranoia on both sides.
>
> If I end up leaving savannah, can someone propose a good site,
> i.e., secure, yet with git and rsync access?
Sorry, I don't know any free git hosters, but here's what you can do:
1) Pressure Savannah to support git
2) Use arch on Savannah
3) Move to Subversion on SourceForge, GNA.org or Berlios and use git-svn
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: synchronizing incremental git changes to cvs
From: Jakub Narebski @ 2006-05-22 17:05 UTC (permalink / raw)
To: git
In-Reply-To: <1148315362.29228.27.camel@dv>
Pavel Roskin wrote:
> Sorry, I don't know any free git hosters, but here's what you can do:
>
> 1) Pressure Savannah to support git
> 2) Use arch on Savannah
> 3) Move to Subversion on SourceForge, GNA.org or Berlios and use git-svn
I always thought that both Gna! and Savannah use Savane (SourceForge engine
fork), which has CVS and Subversion support, and from what I can see also
GNU Arch support. I'm quite sure that at Gna! you can choose between CVS,
SVN and Arch.
--
Jakub Narebski
Warsaw, Poland
^ permalink raw reply
* Re: irc usage..
From: Linus Torvalds @ 2006-05-22 17:27 UTC (permalink / raw)
To: Martin Langhoff
Cc: Donnie Berkholz, Yann Dirson, Git Mailing List, Matthias Urlichs,
Johannes Schindelin
In-Reply-To: <46a038f90605220554y569c11b9p24027772bd2ee79a@mail.gmail.com>
On Tue, 23 May 2006, Martin Langhoff wrote:
>
> And there are two patches that I suspect may fix the leak. After
> applying them, the cvsimport process grows up to ~13MB and then tapers
> off, at least as far as my patience has gotten me. It's late on this
> side of the globe so I'll look at the results tomorrow morning.
Ok, initial results are promising. git-cvsimport appears to be still
slowly growing, but it's at 40M (ie pretty tiny, considering that cvsps
grew to 800+MB on this archive) and growth seems to actually be slowing.
My conversion is only up to September 2002, but if it doesn't suddenly hit
some huge growth spurt, I wouldn't expect it to run out of memory. The CVS
server process itself is tiny, and doesn't seem to grow at all.
As to packing, it doing something like
while :
do
sleep 30
#
# repack roughly every 25600 objects
#
n=$(ls .git/objects/00 2> /dev/null | wc -l)
if [ $n -gt 100 ]; then
git repack -a
#
# Stupid sleep to make sure that nobody is still
# using any unpacked objects after the pack got
# generated
#
sleep 10
git prune-packed
fi
done
or similar (the above is totally untested - I've just done it by hand a
few times) should work. It's perfectly ok to repack the archive even while
the cvsimport script is adding more data and changing it.
Linus
^ permalink raw reply
* Re: irc usage..
From: Jakub Narebski @ 2006-05-22 17:51 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0605221013020.3697@g5.osdl.org>
Linus Torvalds wrote:
> git repack -a
> #
> # Stupid sleep to make sure that nobody is still
> # using any unpacked objects after the pack got
> # generated
> #
> sleep 10
> git prune-packed
Is it really necessary (on Linux at least)? Git boast it's atomicity...
--
Jakub Narebski
Warsaw, Poland
^ permalink raw reply
* Re: irc usage..
From: Linus Torvalds @ 2006-05-22 18:03 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <e4stna$o1g$1@sea.gmane.org>
On Mon, 22 May 2006, Jakub Narebski wrote:
>
> Linus Torvalds wrote:
>
> > git repack -a
> > #
> > # Stupid sleep to make sure that nobody is still
> > # using any unpacked objects after the pack got
> > # generated
> > #
> > sleep 10
> > git prune-packed
>
> Is it really necessary (on Linux at least)? Git boast it's atomicity...
I don't think it's necessary in practice.
But people _should_ realize that removing objects is very very special.
Whether it's done by "git prune-packed" or "git prune", that's a very
dangerous operations. "git prune" a lot more so than "git prune-packed",
of course (in fact, you should _never_ run "git prune" on a repository
that is active - you _will_ corrupt it)-
Doing "git prune-packed" _should_ be mostly safe on UNIX, since the
objects all exist in packs, and anybody who already opened an object will
keep the fd open, and not even notice that the name is gone. However,
there is at least one race:
object lookup "git repack -a -d"
============= ==================
- a process does its object
database setup. No new pack-file
yet.
- mv tmp-packfile active-packfile
- git prune-packed
- the process looks up the object,
and doesn't look in the pack-file
because it didn't see the pack-file.
So it tries to look up an object,
fails, and errors out.
It's not a fatal error (just re-try)
but it could break something like a
cvsimport
Now, in PRACTICE, I doubt you'd ever hit this. But the fact is, pruning
your repository (whether prune-packed or a full prune) is _the_ special
operation. It's something that removes a filesystem representation of an
object that is otherwise immutable.
Linus
^ permalink raw reply
* Re: irc usage..
From: Matthias Lederhofer @ 2006-05-22 19:03 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0605221055270.3697@g5.osdl.org>
> But people _should_ realize that removing objects is very very special.
Just a similar question: is there any reason not tu run git
repack/prune-packed as cron job? I would think of something like this
for every night:
- git prune-packed (remove objects packed last time)
- check how many objects git-count-objects counts, if it are not enough
abort
- git repack
git repack -a -d is probably a bad idea, I guess, because a program
could try to open them after they were deleted. Is there any way to
delete unnecessary packs (those which would repack -a -d delete)?
Making it possible to do a git repack -a and delete those packs the
next night?
^ permalink raw reply
* Re: irc usage..
From: Junio C Hamano @ 2006-05-22 19:09 UTC (permalink / raw)
To: Matthias Lederhofer; +Cc: git
In-Reply-To: <E1FiFgL-0003m6-Eb@moooo.ath.cx>
Matthias Lederhofer <matled@gmx.net> writes:
> ... Is there any way to
> delete unnecessary packs (those which would repack -a -d delete)?
> Making it possible to do a git repack -a and delete those packs the
> next night?
pack-redundant is supposed to figure it out, but I have never
used it myself so your mileage may vary.
^ permalink raw reply
* Re: irc usage..
From: Donnie Berkholz @ 2006-05-22 19:09 UTC (permalink / raw)
To: Martin Langhoff
Cc: Linus Torvalds, Yann Dirson, Git Mailing List, Matthias Urlichs,
Johannes Schindelin
In-Reply-To: <46a038f90605220554y569c11b9p24027772bd2ee79a@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]
Martin Langhoff wrote:
> On 5/22/06, Linus Torvalds <torvalds@osdl.org> wrote:
>> On Mon, 22 May 2006, Martin Langhoff wrote:
>> >
>> > Or a slow leak in Perl? The 5.8.8 release notes do talk about some
>> > leaks being fixed, but this 5.8.8 isn't making a difference.
>> >
>> > Working on it.
>>
>> Thanks. Looking at what I did convert, that horrid gentoo CVS tree is
>> interesting. The resulting (partial) git history has 93413 commits and
>> 850,000+ objects total, all in a totally linear history.
>
> Ok, so there's 3 patches posted that should help narrow down the
> problem. There's a new -L <imit> so that Donnie can get his stuff done
> by running it in a while(true) loop. Not proud of it, but hey.
>
> And there are two patches that I suspect may fix the leak. After
> applying them, the cvsimport process grows up to ~13MB and then tapers
> off, at least as far as my patience has gotten me. It's late on this
> side of the globe so I'll look at the results tomorrow morning.
OK, I started a new run without -L, and I'm watching it in top right
now. The cvsimport seems to be doing alright, but the cvs server process
sucks about another megabyte of virtual every 4-5 seconds. This is a bit
concerning since I don't have any swap. Shortly after it hit 670M, I got
"Cannot allocate memory" again. I've got a gig of RAM, and around 300M
was resident in various processes at the time.
So it seems the problem is in cvs itself. I will try another run with -L
now.
Thanks,
Donnie
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] tagsize < 8kb restriction
From: Junio C Hamano @ 2006-05-22 19:18 UTC (permalink / raw)
To: Björn Engelmann; +Cc: git
In-Reply-To: <4471CF23.1070807@gmx.de>
Björn Engelmann <BjEngelmann@gmx.de> writes:
> I am currently working on an interface for source code quality assurance
> tools to automatically scan newly commited code. Since it is the only
> way to add data (scan-results) to an already-existing commit, I decided
> to use tags for that.
>
> Since the scan-results will most definitly exeed the 8kb-limit, I would
> like to remove this artificial restriction.
Lifting the limit is good, but I am not sure if the use of tags
for that purpose is appropriate (or any git object for that
matter). I'll talk about that at the end.
> What I found odd when writing the patch was that main() in mktag.c uses
> xread() to read from stdin (which respects EAGAIN and EINTR return
> values), but index_pipe() in sha1_file.c just uses read() for doing
> merely the same thing. For unifying both routines i found that xread()
> might be the better choice.
Good.
> Removing the restriction was pretty straightforward but do you think
> this would break something in other places ?
I do not think so offhand.
Now, about the usage of such a long tag for your purpose.
As you noticed, commits and tags are the only types of objetcs
that can refer to other commits structurally. But there are
cases where you do not even need nor want structural reference.
For example, 'git cherry-pick' records the commit object name of
the cherry-picked commit in the commit message as part of the
text -- such a commit does not have structural reference to the
original commit, and we would not _want_ one. I have a strong
suspicion that your application does not need or want structural
reference to commits, and it might be better to merely mention
their object names as part of the text the application produces,
just like what 'git cherry-pick' does.
Presumably you will have one such tag per commit, and by default
'fetch' (both cg and git) tries to follow tags, which means
anybody who fetches new revision would automatically download
this QA data -- that is one implication of using a tag to store
this information. Without knowing the nature of it, I am not
sure if everybody who tracks the source wants such baggage. If
not, then use of a tag for this may not be appropriate.
Another question is if the QA data expected to be amended or
annotated later, after it is created.
If the answer is yes, then you probably would not want tags --
you can create a new tag that points at the same commit to
update the data, but then you have no structural relationships
given by git between such tags that point at the same commit.
You could infer their order by timestamp but that is about it.
I think you are better off creating a separate QA project that
adds one new file per commit on the main project, and have the
file identify the commit object on the main project (either
start your text file format for QA data with the commit object
name, or name each such QA data file after the commit object
name). Then your automated procedure could scan and add a new
file to the QA project every time a new commit is made to the
main project, and the data in the QA project can be amended or
annotated and the changes will be version controlled.
If the answer is no, then it is probably better to just use an
append-only log file that textually records which entry
corresponds to which commit in the project. If it is not
version controlled, and if it is not part of the main project, I
do not see much point in putting the data under git control and
in the same project.
^ permalink raw reply
* Re: [PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag
From: Junio C Hamano @ 2006-05-22 19:19 UTC (permalink / raw)
To: Björn Engelmann; +Cc: git
In-Reply-To: <4471CF5A.702@gmx.de>
Björn Engelmann <BjEngelmann@gmx.de> writes:
> @@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en
> extern int ce_modified(struct cache_entry *ce, struct stat *st, int);
> extern int ce_path_match(const struct cache_entry *ce, const char
> **pathspec);
> extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int
> write_object, const char *type);
> +extern int read_pipe(int fd, char** return_buf, unsigned long*
> return_size);
> extern int index_pipe(unsigned char *sha1, int fd, const char *type,
> int write_object);
I smell whitespace breakage around here...
> diff --git a/mktag.c b/mktag.c
> index 2328878..79c466c 100644
> --- a/mktag.c
> +++ b/mktag.c
>...
> @@ -114,21 +114,24 @@ int main(int argc, char **argv)
>
> setup_git_directory();
>
> - // Read the signature
> - size = 0;
> - for (;;) {
> - int ret = xread(0, buffer + size, MAXSIZE - size);
> - if (ret <= 0)
> - break;
> - size += ret;
> + if (read_pipe(0, &buffer, &size)) {
> + free(buffer);
> + die("could not read from stdin");
> }
> -
> +
Please do not introduce lines with trailing whitespaces.
> // Verify it for some basic sanity: it needs to start with "object
> <sha1>\ntype\ntagger "
> - if (verify_tag(buffer, size) < 0)
> + if (verify_tag(buffer, size) < 0) {
> + free(buffer);
> die("invalid tag signature file");
> + }
You seem to be striving for extra cleanliness, but I personally
consider it is not worth calling free() immediately before you
call die().
>
> - if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
> + if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {
> + free(buffer);
> die("unable to write tag file");
> + }
> +
> + free(buffer);
> +
> printf("%s\n", sha1_to_hex(result_sha1));
> return 0;
> }
A call to free() immediately before returning from main() might
look similar to the die() issue I mentioned above, but we might
extend it to do a lot more after writing the tag in the future,
so this one is very good to keep.
^ permalink raw reply
* Re: [PATCH 2/2] added more informative error messages to git-mktag
From: Junio C Hamano @ 2006-05-22 19:22 UTC (permalink / raw)
To: Björn Engelmann; +Cc: git
In-Reply-To: <4471CF91.9010202@gmx.de>
Björn Engelmann <BjEngelmann@gmx.de> writes:
> - if (size < 64)
> + if (size < 64) {
> + printf("wanna fool me ? you obviously got the size wrong !\n");
> return -1;
> + }
Please do this instead:
return error("wanna ...");
you can lose the braces and the message goes to the stderr.
> - if (memcmp(object, "object ", 7))
> + if (memcmp(object, "object ", 7)) {
> + printf("char%i: does not start with \"object \"\n", 0);
> return -1;
Although they may be synonyms, we tend to use %d for ints and it
is more conventional.
> tag_line++;
> - if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
> + if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n') {
> + printf("char%i: no \"tag \" found\n", (int)tag_line - (int)buffer);
> return -1;
> + }
If you have to cast, please do not cast pointers to ints and
take their difference, but take the difference and cast the
resulting ptrdiff_t to int, like this:
(int)(tag_line - buffer)
Or use "%td" instead of "%i" and lose the cast.
^ permalink raw reply
* [PATCH] Problem: 'trap...exit' causes error message when /bin/sh is ash.
From: Yakov Lerner @ 2006-05-22 19:34 UTC (permalink / raw)
To: git; +Cc: iler.ml
Problem: 'trap...exit' causes error message when /bin/sh is ash.
Fix: Change 'trap...exit' to 'trap...0' like in other scripts.
Signed-off-by: Yakov Lerner <iler.ml@gmail.com>
---
git-clone.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
954e49bc242cacd27e002f194d54a6895e64f88c
diff --git a/git-clone.sh b/git-clone.sh
index 227245c..d96894d 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -199,7 +199,7 @@ # Try using "humanish" part of source re
[ -e "$dir" ] && echo "$dir already exists." && usage
mkdir -p "$dir" &&
D=$(cd "$dir" && pwd) &&
-trap 'err=$?; cd ..; rm -r "$D"; exit $err' exit
+trap 'err=$?; cd ..; rm -r "$D"; exit $err' 0
case "$bare" in
yes) GIT_DIR="$D" ;;
*) GIT_DIR="$D/.git" ;;
@@ -407,5 +407,5 @@ Pull: refs/heads/$head_points_at:$origin
fi
rm -f "$GIT_DIR/CLONE_HEAD" "$GIT_DIR/REMOTE_HEAD"
-trap - exit
+trap - 0
--
1.3.GIT
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox