Git development
 help / color / mirror / Atom feed
* Re: Current Issues #3
From: Linus Torvalds @ 2006-05-22 10:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8xoue9eo.fsf@assigned-by-dhcp.cox.net>



On Mon, 22 May 2006, Junio C Hamano wrote:
> 
> * Per branch configuration
> 
>   The [section "foo"] configuration syntax update by Linus, and
>   git-parse-remote update to use remote.stuff.{url,push,pull} by
>   Johannes are now both in the "master".  The stage is set to
>   discuss what to actually do with per-branch configuration.
> 
>   We will use the [branch "foo"] section for configuration about
>   local branch named "foo".  I do not think there is any
>   disagreement about this.
> 
>   The ideas floated so far (I am forgetting many of them
>   perhaps):
> 
>     1. "upstream" refers to the remote section to use when
>        running "git-{fetch,pull,push}" while on that branch.
> 
> 	[branch "master"]
> 		upstream = "origin"
> 
> 	[remote "origin"]
>         	url = "git://git.kernel.org/.../git.git"
> 		fetch = refs/heads/master:refs/remotes/origin/master
> 
>     2. "url/fetch/push" directly specifies what would usually be
>        taken from a remote section by "git-{fetch,pull,push}"
>        while on that branch.
> 
> 	[branch "foo"]
>         	url = "company.com.xz:myrepo"
> 		fetch = refs/heads/master:refs/remotes/origin/master
> 		push = refs/heads/master:refs/heads/origin

I'd _much_ prefer (1) over (2).

However, I wonder if we couldn't do even better. How about forgetting 
about the "branch" vs "remote" thing, and instead splitting it into 
_three_: "branch", "repository" and "remote branch".

Something like

	[repo "origin"]
		url = "git://git.kernel.org/.../git.git"

	[repo "gitk"]
		url = "git://git.kernel.org/.../gitk.git"

to describe two remote repositories (and NOTE! No branch descriptions 
within those. We're just describing the actual repository, so we might 
have things like "readonly" to indicate that we can't push to them, but if 
we do things like that, they would be "repo-wide" things that we 
describe for that repository),

Then, we can describe remote branches within those repositories:

	[remote "origin/master"]
		repo = origin
		branch = master

	[remote "origin/next"]
		repo = origin
		branch = next

	[remote "origin/pu"]
		repo = origin
		branch = pu

	[remote "gitk/master"]
		repo = gitk
		branch = master

now, here we're describing two things: the name of the remote is what we 
will then use for the ".git/remotes/<name>" thing to remember the last 
value, and we're describing where to get that data (which repo, and which 
branch).

NOTE! In the example above, I made the name of the remote always match the 
<repo>/<branch> format, but that would be just a convention. You could do

	[remote "linus"]
		repo = kernel
		branch = master

to describe the "linus" remote as the master branch of the "kernel" 
repository.

Finally, local branches:

	[branch "master"]
		source = origin/master

	[branch "origin"]
		readonly
		source = origin/master

	[branch "next"]
		readonly
		source = origin/next

	[branch "pu"]
		readonly
		rebase
		source = origin/pu

	[branch "gitk"]
		readonly
		source = gitk/master

This marks the things that just _track_ somebody elses branch as being 
readonly (so "master" and "origin" are really different: they're both 
branches, but one of them just tracks remotes/origin/master, while the 
other one can be committed to), and "pu" has been marked as not only being 
read-only, it also re-bases to its source.

I dunno. Does this sound too verbose and abstract?

Normally, you'd not have a lot of these. For example, for somebody who 
follows the kernel, you'd literally just have

	[branch "master"]
		source = linus

	[remote "linus"]
		repo = kernel
		branch = master

	[repo "kernel"]
		url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6

and you'd be done. The above would describe both the local "master" branch 
and the "remotes/linus" head, and give the relationship between them.

The git repo is actually much more complex, especially if you want to 
track all of the different branches Junio has, and if you want to also 
track the branches Paul has to gitk.

But with the above, you can fairly naturally do:

 - "git pull" 

	No arguments. fetch the remote described by the current branch, 
	and merge into current branch (we might decide to fetch all the 
	remotes associated with that repo, just because once we do this, 
	we might as well, but that's not that important to the end 
	result).

 - "git pull <repo>"

	fetch all remotes that use <repo>. IFF the current branch is 
	matched to one of those remotes, merge the changes into the 
	current branch. But if you happened to be on another unrelated 
	branch, nothing happens aside from the fetch.

 - "git pull <remote>"

	fetch just the named remote. IFF that remote is also the remote 
	for the current branch, do merge it into current. Again, we 
	_might_ decide to just do the whole repo.

 - "git pull <repo> <branchname>"

	fetch the named branch from the named repository and merge it into 
	current (no ifs, buts or maybes - now we've basically overridden 
	the default relationships, so now the <repo> is just a pure 
	shorthand for the location of the repository)

 - "git pull <repo> <src>:<dst>"

	same as now. fetch <repo> <src> into <dst>, and merge it into the 
	current branch (again, we've overridden any default relationships).

but maybe this is overdesigned. Comments?

			Linus

^ permalink raw reply

* Re: Current Issues #3
From: Martin Waitz @ 2006-05-22 10:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8xoue9eo.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

hoi :)

On Mon, May 22, 2006 at 01:44:15AM -0700, Junio C Hamano wrote:
>     1. "upstream" refers to the remote section to use when
>        running "git-{fetch,pull,push}" while on that branch.
> 
> 	[branch "master"]
> 		upstream = "origin"

what do you do for [branch "next"] here?

Does it make sense to regard all refs/remotes/*/<branchname> as
upstream and merge these into the current branch when pulling?

Perhaps a pull could even merge all newly fetched remote heads
into the corresponding branch, but for that we'd need to be
able to merge without using the working dir.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* detect write failure, even for stdout
From: Jim Meyering @ 2006-05-22 10:27 UTC (permalink / raw)
  To: git
In-Reply-To: <7v3bf3jl15.fsf@assigned-by-dhcp.cox.net>

git doesn't always detect write failures.  A write I/O error,
(e.g., hardware I/O error or simply disk full)
doesn't provoke nonzero exit status:

    $ ./git-cat-file -t HEAD > /dev/full && echo did not detect write failure
    did not detect write failure

This is perhaps more important than the other things
I've reported, since it can lead to porcelain being unable
to detect a real failure in the plumbing.

Here are two more:

    $ ./git-ls-tree HEAD > /dev/full && echo fail
    fail
    $ ./git-show > /dev/full && echo fail
    fail

If you were using gnulib, I'd suggest simply adding this line

    atexit (close_stdout);

near the beginning of each `main'.  Then you wouldn't have to
manually track down each and every place where a write to stdout
can occur -- not to mention the maintenance burden of keeping
things correct as the code evolves.

^ permalink raw reply

* Re: Current Issues #3
From: Sean @ 2006-05-22 11:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: junkio, git
In-Reply-To: <Pine.LNX.4.64.0605220216310.3697@g5.osdl.org>

On Mon, 22 May 2006 03:18:02 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> [...]
> 
> but maybe this is overdesigned. Comments?


It all looks good, especially your description of the git pull variations
which seem more natural than what exists now.

The only minor comment i'd make is that we shouldn't mix so many different
names for the same thing.  In your example you have  "remote" (singular)
sections with branch sections that contain "source" keys which map to those
remote sections, both corresponding to "refs/remotes" (plural).

There doesn't seem to be any need to stick with "source" as a key, so :

[remote "origin/master"]
	repo = origin
	branch = master

[branch "master"]
	remote = "origin/master"

Sean

^ permalink raw reply

* [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Martin Langhoff @ 2006-05-22 11:38 UTC (permalink / raw)
  To: git, junkio, Johannes.Schindelin, torvals, spyderous, smurf
  Cc: Martin Langhoff

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>

---
This is ugly, but while I work on cleaning up the leak
that seems to be somewhere in the commit() sub, we may
as well set up a workaround.

I am not 100% happy woth including this in git.git. 
In any case, I hope we can revert it soon.

---

 git-cvsimport.perl |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

64ea3c83d8cd176ee972055bd1d11f398655dad8
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index c0ae00b..c1923d1 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -29,7 +29,7 @@ use IPC::Open2;
 $SIG{'PIPE'}="IGNORE";
 $ENV{'TZ'}="UTC";
 
-our($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S);
+our($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L);
 my (%conv_author_name, %conv_author_email);
 
 sub usage() {
@@ -85,7 +85,7 @@ sub write_author_info($) {
 	close ($f);
 }
 
-getopts("hivmkuo:d:p:C:z:s:M:P:A:S:") or usage();
+getopts("hivmkuo:d:p:C:z:s:M:P:A:S:L:") or usage();
 usage if $opt_h;
 
 @ARGV <= 1 or usage();
@@ -716,6 +716,7 @@ my $commit = sub {
 	}
 };
 
+my $commitcount = 1;
 while(<CVS>) {
 	chomp;
 	if($state == 0 and /^-+$/) {
@@ -849,6 +850,9 @@ #	VERSION:1.96->1.96.2.1
 	} elsif($state == 9 and /^\s*$/) {
 		$state = 10;
 	} elsif(($state == 9 or $state == 10) and /^-+$/) {
+		if ($opt_L && $commitcount++ >= $opt_L) {
+			last;
+		}
 		&$commit();
 		$state = 1;
 	} elsif($state == 11 and /^-+$/) {
-- 
1.3.2.g82000

^ permalink raw reply related

* [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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox