Git development
 help / color / mirror / Atom feed
* Re: ~/.git/config ?
From: Petr Baudis @ 2006-05-26 16:38 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: Anand Kumria, git
In-Reply-To: <20060526193325.d2a530a4.tihirvon@gmail.com>

Dear diary, on Fri, May 26, 2006 at 06:33:25PM CEST, I got a letter
where Timo Hirvonen <tihirvon@gmail.com> said that...
> I backup my $HOME using git, so there's a .git directory in ~.

Then it should be called ~/.gitconfig. :-)

> I don't think a global config file is really needed but it would be
> nice if .git/config would override the environment variables, not the
> other way around.

Then you have no other way to override .git/config e.g. when committing
patches submitted by other people.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

^ permalink raw reply

* Re: ~/.git/config ?
From: Jakub Narebski @ 2006-05-26 16:37 UTC (permalink / raw)
  To: git
In-Reply-To: <20060526193325.d2a530a4.tihirvon@gmail.com>

Timo Hirvonen wrote:

> Anand Kumria <wildfire@progsoc.uts.edu.au> wrote:
> 
>> Hi,
>> 
>> git is unable to construct a reasonable default email address in my
>> current environment.  So, I use GIT_AUTHOR_EMAIL and GIT_COMMITTER_EMAIL
>> to override things.
>> 
>> This has worked well but, now, I need to vary the email address for some
>> repositories.  Unfortunately the environment variables override
>> .git/config.
>> 
>> It would be good if things were like:
>>      - try to construct one automagically
>>      - use ~/.git/config (if available)
>>      - use .git/config
>>      - use environment variables
>> 
>> That way I could set my default email address in ~/.git/config and
>> override it as required for those repositories that need it.
> 
> I backup my $HOME using git, so there's a .git directory in ~.  I don't
> think a global config file is really needed but it would be nice if
> .git/config would override the environment variables, not the other way
> around.

Well, I'm not sure if environmental variables overriding wouldn't make
invocations like 'GIT_DIR=something git command' possible.

There are templates, also for config. Currently git lacks user (not
repository) config file, e.g. ~/.gitconfig (common for all repositories).

-- 
Jakub Narebski
Warsaw, Poland

^ permalink raw reply

* Re: ~/.git/config ?
From: Timo Hirvonen @ 2006-05-26 16:33 UTC (permalink / raw)
  To: Anand Kumria; +Cc: git
In-Reply-To: <20060526152837.GQ23852@progsoc.uts.edu.au>

Anand Kumria <wildfire@progsoc.uts.edu.au> wrote:

> Hi,
> 
> git is unable to construct a reasonable default email address in my
> current environment.  So, I use GIT_AUTHOR_EMAIL and GIT_COMMITTER_EMAIL
> to override things.
> 
> This has worked well but, now, I need to vary the email address for some
> repositories.  Unfortunately the environment variables override
> .git/config.
> 
> It would be good if things were like:
> 	- try to construct one automagically
> 	- use ~/.git/config (if available)
> 	- use .git/config
> 	- use environment variables
> 
> That way I could set my default email address in ~/.git/config and
> override it as required for those repositories that need it.

I backup my $HOME using git, so there's a .git directory in ~.  I don't
think a global config file is really needed but it would be nice if
.git/config would override the environment variables, not the other way
around.

-- 
http://onion.dynserv.net/~timo/

^ permalink raw reply

* git-apply can't apply patches to CRLF-files
From: Salikh Zakirov @ 2006-05-26 16:00 UTC (permalink / raw)
  To: git

Hello, 

git-apply can't apply the patch to file with windows-style CRLF line endings,
even if the patch was generated by git-format-patch.

Is this a bug or known deficiency?

The following script reproduces the problem
---------
#!/bin/sh
set -e
mkdir trash
cd trash
git init-db
echo "abc" > a
unix2dos a
git add a
git commit -m "a added" a
echo "cde" >> a
unix2dos a
git commit -m "a modified" a
git format-patch HEAD^
git reset --hard HEAD^
git am 0001*.txt
---------

The resulting output is
---------
$ ./test
defaulting to local storage area
a: done.
Committing initial tree 357c56061b96c1548b15168bc0d02e8d1a319e0b
a: done.
0001-a-modified.txt

Applying 'a modified'

error: patch failed: a:1
error: a: patch does not apply
Patch failed at 0001.
When you have resolved this problem run "git-am --resolved".
If you would prefer to skip this patch, instead run "git-am --skip".
---------

If I remove unix2dos calls and so the file has normal unix LF line endings,
then the result is correct as expected

---------
$ ./test
defaulting to local storage area
Committing initial tree 6afc8719a182fed19980da0e53d13fba1f94dd3f
0001-a-modified.txt

Applying 'a modified'

Wrote tree 49f5181a399bbcaac1da3bf693c466a281c4a255
Committed: 2b0a2936d0a65b3511882b8e88586ab054dd15b2
---------

^ permalink raw reply

* Re: Slow fetches of tags
From: Ralf Baechle @ 2006-05-26 15:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vd5e21zh9.fsf@assigned-by-dhcp.cox.net>

On Wed, May 24, 2006 at 09:48:34PM -0700, Junio C Hamano wrote:

> I think the right fix for this is to change upload-pack to
> traverse reachability chain from the "want" heads as it gets
> "have" from the downloader, and stop responding "continue" when
> all "want" heads can reach some "have" commits.  This would not
> prevent it from going down all the way to the root commit if
> what is wanted does not have anything to do with what the other
> end has (e.g. if you have only my main project branches, and you
> ask for html head for the first time), but it would have
> prevented Ralf's tree from getting "continue" after he asked
> only for v2.6.16.18 tag and said he has 2.6.16.18 commit and its
> ancestors.  It should not be too difficult to do this, but here
> is an alternative, client-side workaround.
> 
> -- >8 --
> [PATCH] fetch-pack: give up after getting too many "ack continue"

So I did test your patch.  In the big, slow repository it cuts down the
time for a

  git fetch git://www.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git master:v2.6.16-stable

from like 6min to about 7s.

Thanks!

  Ralf

^ permalink raw reply

* ~/.git/config ?
From: Anand Kumria @ 2006-05-26 15:28 UTC (permalink / raw)
  To: git

Hi,

git is unable to construct a reasonable default email address in my
current environment.  So, I use GIT_AUTHOR_EMAIL and GIT_COMMITTER_EMAIL
to override things.

This has worked well but, now, I need to vary the email address for some
repositories.  Unfortunately the environment variables override
.git/config.

It would be good if things were like:
	- try to construct one automagically
	- use ~/.git/config (if available)
	- use .git/config
	- use environment variables

That way I could set my default email address in ~/.git/config and
override it as required for those repositories that need it.

Thanks,
Anand

-- 
 `When any government, or any church for that matter, undertakes to say to
  its subjects, "This you may not read, this you must not see, this you are
  forbidden to know," the end result is tyranny and oppression no matter how
  holy the motives' -- Robert A Heinlein, "If this goes on --"

^ permalink raw reply

* (unknown)
From: Juergen Ruehle @ 2006-05-26 15:16 UTC (permalink / raw)
  To: git

subscribe git

^ permalink raw reply

* Re: t8001-annotate.sh fails on Mac OS X
From: Stefan Pfetzing @ 2006-05-26 13:51 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0605261534270.27610@wbgn013.biozentrum.uni-wuerzburg.de>

Hi Johannes,

2006/5/26, Johannes Schindelin <Johannes.Schindelin@gmx.de>:

> > I've been seeing the same failed test case for a long time now on
> > my own Mac OS X system.
>
> ... which is sort of funny, because I don't see it on my system. Running
> an iBook G3 with Mac OS X 10.2.8. "make test" runs through, and no, AFAICT
> I do not have any local modifications which could be responsible for that.

Hm, well thats strange, although I guess Shawn runs Tiger - as I do.
(10.4.6 currently).

I tried already with DarwinPorts perl and OSX sytem perl. Both did not
work as expected.

bye

Stefan
-- 
       http://www.dreamind.de/
Oroborus and Debian GNU/Linux Developer.

^ permalink raw reply

* Re: t8001-annotate.sh fails on Mac OS X
From: Johannes Schindelin @ 2006-05-26 13:36 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Stefan Pfetzing, Git Mailing List
In-Reply-To: <20060526011153.GA27720@spearce.org>

Hi,

On Thu, 25 May 2006, Shawn Pearce wrote:

> Stefan Pfetzing <stefan.pfetzing@gmail.com> wrote:
> > 
> > for some reason I could not yet figure out, t8001-annotate.sh fails at test 
> > 18.
> 
> I've been seeing the same failed test case for a long time now on
> my own Mac OS X system.

... which is sort of funny, because I don't see it on my system. Running 
an iBook G3 with Mac OS X 10.2.8. "make test" runs through, and no, AFAICT 
I do not have any local modifications which could be responsible for that.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
From: Herbert Xu @ 2006-05-26 12:33 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11486091792604-git-send-email-normalperson@yhbt.net>

On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
>
>  t/t6000lib.sh |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> cba907ce0b1c0927fb15cbb5dd91a4129ff9a950
> diff --git a/t/t6000lib.sh b/t/t6000lib.sh
> index c6752af..d402621 100755
> --- a/t/t6000lib.sh
> +++ b/t/t6000lib.sh
> @@ -69,7 +69,9 @@ on_committer_date()
>  {
>      _date=$1
>      shift 1
> -    GIT_COMMITTER_DATE=$_date "$@"
> +    export GIT_COMMITTER_DATE="$_date"
> +    "$@"
> +    unset GIT_COMMITTER_DATE

The original code looks correct to me.  So I think this too should
be fixed in dash instead.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
From: Herbert Xu @ 2006-05-26 12:23 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11486091793385-git-send-email-normalperson@yhbt.net>

On Thu, May 25, 2006 at 07:06:17PM -0700, Eric Wong wrote:
> None of the variables seem to conflict, so local was unnecessary.

BTW, dash supports (and has always supported) local which is a quite
useful feature.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] gitk: Replace "git-" commands with "git "
From: Paul Mackerras @ 2006-05-26 12:26 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: raa.lkml, git
In-Reply-To: <20060526145954.cea5613c.tihirvon@gmail.com>

OK, thanks.  I have applied your patch.  The ones that gitk uses that
aren't built-in now are git-rev-parse and git-repo-config, which
aren't performance-critical.

Paul.

^ permalink raw reply

* Re: [PATCH 2/4] tests: Remove heredoc usage inside quotes
From: Herbert Xu @ 2006-05-26 12:22 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11486091783808-git-send-email-normalperson@yhbt.net>

On Thu, May 25, 2006 at 07:06:16PM -0700, Eric Wong wrote:
> The use of heredoc inside quoted strings doesn't seem to be
> supported by dash.  pdksh seems to handle it fine, however.

This is a bug in dash and should be fixed there instead.
Thanks for drawing my attention to it.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] gitk: Replace "git-" commands with "git "
From: Timo Hirvonen @ 2006-05-26 11:59 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: raa.lkml, git
In-Reply-To: <17526.59159.484712.500414@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> wrote:

> Timo Hirvonen writes:
> 
> > Many commands are already built-in so I don't think it's a problem
> > anymore.
> 
> Which ones are built in now?

git-log$X git-whatchanged$X git-show$X \
git-count-objects$X git-diff$X git-push$X \
git-grep$X git-add$X git-rm$X git-rev-list$X \
git-check-ref-format$X \
git-init-db$X git-tar-tree$X git-upload-tar$X git-format-patch$X \
git-ls-files$X git-ls-tree$X \
git-read-tree$X git-commit-tree$X \
git-apply$X git-show-branch$X git-diff-files$X \
git-diff-index$X git-diff-stages$X git-diff-tree$X git-cat-file$X

-- 
http://onion.dynserv.net/~timo/

^ permalink raw reply

* Re: [PATCH] gitk: Replace "git-" commands with "git "
From: Paul Mackerras @ 2006-05-26 11:31 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: Alex Riesen, git
In-Reply-To: <20060524133455.f78b11a4.tihirvon@gmail.com>

Timo Hirvonen writes:

> Many commands are already built-in so I don't think it's a problem
> anymore.

Which ones are built in now?

Paul.

^ permalink raw reply

* Re: [RFC][PATCH] Allow transfer of any valid sha1
From: Junio C Hamano @ 2006-05-26 10:04 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Linus Torvalds, git
In-Reply-To: <m13bexetj1.fsf@ebiederm.dsl.xmission.com>

ebiederm@xmission.com (Eric W. Biederman) writes:

> diff --git a/fetch-pack.c b/fetch-pack.c
> index a3bcad0..c767d84 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -260,6 +260,27 @@ static void mark_recent_complete_commits
>  	}
>  }
>  
> +static struct ref **get_sha1_heads(struct ref **refs, int nr_heads, char **head)
> +{
> +	int i;
> +	for (i  = 0; i < nr_heads; i++) {
> +		struct ref *ref;
> +		unsigned char sha1[20];
> +		char *s = head[i];
> +		int len = strlen(s);
> +
> +		if (len != 40 || get_sha1_hex(s, sha1))
> +			continue;

So the new convention is fetch-pack can take ref name (as
before), or a bare 40-byte hexadecimal.  I think sane people
would not use ambiguous refname that says "deadbeef" five times,
and even if the do so they could disambiguate by explicitly
saying "refs/heads/" followed by "deadbeef" five times, so it
should be OK.

> +
> +		ref = xcalloc(1, sizeof(*ref) + len + 1);
> +		memcpy(ref->old_sha1, sha1, 20);
> +		memcpy(ref->name, s, len + 1);
> +		*refs = ref;
> +		refs = &ref->next;
> +	}
> +	return refs;
> +}
> +

This function takes the pointer to a location that holds a
pointer to a "struct ref" -- it is the location to store the
newly allocated ref structure, i.e. the next pointer of the last
element in the list.  When it returns, the location pointed at
by the pointer given to you points at the first element you
allocated, and it returns the next pointer of the last element
allocated by it.  That is the same calling convention as
connect.c::get_remote_heads().  So when calling this function to
append to a list you already have, you would give the next
pointer to the last element of the existing list.  But you do
not seem to do that.

I think the body of fetch_pack() should become something like:

	struct ref *ref, **tail;

        tail = get_remote_heads(fd[0], &ref, 0, NULL, 0);
	if (server_supports("multi_ack")) {
		...
	}
	tail = get_sha1_heads(tail, nr_match, match);
	if (everything_local(&ref, nr_match, match)) {
		...

> @@ -311,6 +332,8 @@ static int everything_local(struct ref *
>  	if (cutoff)
>  		mark_recent_complete_commits(cutoff);
>  
> +	filter_refs(refs, nr_match, match);
> +

I am not sure about this change.

In the original code we do not let get_remote_heads() to filter
the refs but call filter_refs() after the "mark all complete
remote refs as common" step for a reason.  Even though we may
not be fetching from some remote refs, we would want to take
advantage of the knowledge of what objects they have so that we
can mark as many objects as common as possible in the early
stage.  I suspect this change defeats that optimization.

So instead I would teach "mark all complete remote refs" loop
that not everything in refs list is a valid remote ref, and skip
what get_sha1_heads() injected, because these arbitrary ones we
got from the command line are not something we know exist on the
remote side.  Maybe something like this.

	/*
	 * Mark all complete remote refs as common refs.
	 * Don't mark them common yet; the server has to be told so first.
	 */
	for (ref = *refs; ref; ref = ref->next) {
		struct object *o;
                if (ref is SHA1 from the command line)
                	continue;
		o = deref_tag(lookup_object(ref->old_sha1), NULL, 0);
		if (!o || o->type != commit_type || !(o->flags & COMPLETE))
			continue;
		...

To implement "ref is SHA1 from the command line", I would add
another 1-bit field to "struct ref" and mark the new ones you
create in get_sha1_heads() as such (existing "force" field
could also become an 1-bit field -- we do not neeed a char).

> @@ -373,6 +394,7 @@ static int fetch_pack(int fd[2], int nr_
>  		packet_flush(fd[1]);
>  		die("no matching remote head");
>  	}
> +	get_sha1_heads(&ref, nr_match, match);

I talked about this one already...

> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index 187f088..2372df8 100755
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -105,6 +105,7 @@ canon_refs_list_for_fetch () {
>  		'') remote=HEAD ;;
>  		refs/heads/* | refs/tags/* | refs/remotes/*) ;;
>  		heads/* | tags/* | remotes/* ) remote="refs/$remote" ;;
> +		[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]) ;;

Yuck.  Don't we have $_x40 somewhere?

We never use uppercase so at least we could save 24 columns from
here ;-).

^ permalink raw reply

* Re: [RFC][PATCH] Allow transfer of any valid sha1
From: Eric W. Biederman @ 2006-05-26  8:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7virntsto6.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> ebiederm@xmission.com (Eric W. Biederman) writes:

>> - It feels really weird when everything else allows me to use sha1s
>>   for git-fetch to deny them.
>
> That is a real argument and I am not opposed to change
> fetch-pack to ask for an arbitrary SHA1 the caller obtained out
> of band.

Good this was the primary reason I kept pursuing the issue after
I figured out what it was.

>> Then there is the big hole in my plan to get better changelog information
>> that it appears that after Andrew pulls a branch he resolves some
>> merge conflicts.  If that is right I need to figure out how to address
>> that before I can improve git-quiltimport.sh.
>
> The last time I talked with Andrew, he is not doing a merge nor
> resolving merge conflicts.  He treats git primarily as a
> patchbomb distribution mechanism, and works on (a rough
> equivalent of) the output of format-patch from merge base
> between his base tree and individual subsystem tree.  After that
> things are normal quilt workflow outside git, whatever it is.

That sounds right.  I just know that there I had some strange
merge conflicts on the second git tree I pulled from.  Something
about a file being added twice.  It was one thing too many to
investigate this round.

Eric

^ permalink raw reply

* Re: [RFC][PATCH] Allow transfer of any valid sha1
From: Eric W. Biederman @ 2006-05-26  8:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7wpsu5c.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> So fixing fetch-pack is easy and pretty non-controversial.
>> The patch below handles that.
>
> I am at work so I cannot really spend time on this right now,
> but I am OK with letting it send arbitrary SHA1 the caller
> obtained out of band.  I do not know about your implementation,
> since I haven't really looked at it.

Agreed.  I'm not certain about my implementation yet either I
just know I was in the ball park.

I needed the conversation to understand what the limits were.

>> (The movement of filter_refs may actually be overkill)
>
> It may not just overkill but may actively be wrong, but again I
> haven't looked at it yet.
>
> Will take a look tonight.

Sure.  The code was all a work in progress so I don't expect to
have all of the details ironed out.  In particular I didn't
even look at the non fetch-pack case, and I didn't update the
documentation.

Eric

^ permalink raw reply

* Re: [PATCH 5/6] More accurately detect header lines in read_one_header_line
From: Eric W. Biederman @ 2006-05-26  8:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr72hns7h.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Junio C Hamano <junkio@cox.net> writes:
>
>> Was there a particular reason you needed this change?  That is,
>> did you have to parse mail-looking input that does not have a
>> blank line between runs of headers and the body of the message?

Yes.  I had patches that had a subject line followed by a blank line,
and the problem was that the old check thought the subject was a
header line, despite not even having a colon in it.

>> If so, I'd at least like to remove the || !isspace(colon[1])
>> from the test.  After all, I do not think RFC2822 requires a
>> whitespace after the colon there.
>
> In other words, something like this (tested):

Looks good to me, sorry for missing that one.

Eric

^ permalink raw reply

* Re: [PATCH 5/6] More accurately detect header lines in read_one_header_line
From: Junio C Hamano @ 2006-05-26  7:29 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: git
In-Reply-To: <m1mzd8iklr.fsf_-_@ebiederm.dsl.xmission.com>

ebiederm@xmission.com (Eric W. Biederman) writes:

> Only count lines of the form '^.*: ' and '^From ' as email
> header lines. 

I am having trouble with this patch.

> diff --git a/mailinfo.c b/mailinfo.c
> index 99989c2..c642ff4 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -385,20 +385,29 @@ static int read_one_header_line(char *li
>  {
>  	int ofs = 0;
>  	while (ofs < sz) {
> +		const char *colon;
>  		int peek, len;
>  		if (fgets(line + ofs, sz - ofs, in) == NULL)
> +			break;
>  		len = eatspace(line + ofs);
>  		if (len == 0)
> +			break;
> +		colon = strchr(line, ':');
> +		if (!colon || !isspace(colon[1])) {
> +			/* Readd the newline */
> +			line[ofs + len] = '\n';
> +			line[ofs + len + 1] = '\0';
> +			break;
>  		}

Because eatspace() eats the trailing space, although your commit
message say lines matching "^.*: " are headers, this does not
match the criteria:

        X-Spam-Checker-Version: SpamAssassin 3.1.1 (2006-03-10) on 
                gitster.siamese.dyndns.org
->      X-Spam-Level: 
        X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00
        	autolearn=ham version=3.1.1

Notice that the field body for this unstructured header
X-Spam-Level (an optional field) consists of a single
whitespace.  It will be gone because of eatspace() when your
check sees the line, so the header parsing stops prematurely.

Was there a particular reason you needed this change?  That is,
did you have to parse mail-looking input that does not have a
blank line between runs of headers and the body of the message?

If so, I'd at least like to remove the || !isspace(colon[1])
from the test.  After all, I do not think RFC2822 requires a
whitespace after the colon there.

^ permalink raw reply

* Re: git-format-patch possible regressions
From: Jakub Narebski @ 2006-05-26  6:26 UTC (permalink / raw)
  To: git
In-Reply-To: <7v4pzdqpit.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> So the way for qgit to use it would become something like this.
> Instead of giving a list of ranges like "a..b c..d e..f":
> 
>  * Run "format-patch a..b"; by reading from its stdout you know
>    what patches you got -- you count them.
> 
>  * Run "format-patch --start-number=6 c..d" (if you got 5 out of
>    a..b);
[...]

I still think that having _shortcut notation_ being different for very
different commands is not a bad idea.

If one is really concerned about consistency of rev-list options, we could
use ',' or something to separate separate lists of commits, e.g.

   git format-patch a..b , c..d , e..f

or

   git format-patch a..b --then c..d --then e..f

What do you think about the idea?

-- 
Jakub Narebski
Warsaw, Poland

^ permalink raw reply

* Re: git-format-patch possible regressions
From: Junio C Hamano @ 2006-05-26  6:16 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550605252309h2c4b74bcp50b095e09e6c133f@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> I was thinking, probably wrong, that the number prepended in file name
> is used also to disambiguate two patches with the same subject.

What Johannes and I were discussing was the other number -- the
total in the series.  IOW, y in "[PATCH x/y]".  OTOH, the number
used for disambiguation you care about is x, which is made
adjustable with --start-number patch.

So the way for qgit to use it would become something like this.
Instead of giving a list of ranges like "a..b c..d e..f":

 * Run "format-patch a..b"; by reading from its stdout you know
   what patches you got -- you count them.

 * Run "format-patch --start-number=6 c..d" (if you got 5 out of
   a..b);

 * Run "format-patch --start-number=n e..f" (now you know the drill).

Then the sequence out of c..d would start with a file 0006-xxxx.txt,
which is what you want for disambiguation.

^ permalink raw reply

* Re: git-format-patch possible regressions
From: Marco Costalba @ 2006-05-26  6:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <Pine.LNX.4.63.0605260125420.16816@wbgn013.biozentrum.uni-wuerzburg.de>

On 5/26/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 25 May 2006, Junio C Hamano wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > > Thinking about this again, it makes more sense not to imply --numbered:
> >
> > Yes, that makes sense.  That way you can say "Please start
> > naming the output files at 0032-xxxx.txt, because you gave me 31
> > patch series last time, but I do not want [PATCH x/y] on the
> > subject line, just [PATCH]".
> >
> > That brings up another issue.  Don't we need to have another
> > option --total-number that overrides the /y part above?
>
> I thought about that, too. Isn't the --numbered only useful for submitting
> a patch series via mail? And isn't it necessary to make certain that these
> patches really apply in that order? Isn't it then sensible to force the
> user to have a branch (at least a throw-away one) having exactly these
> patches, just to make sure that the patches really, really apply in that
> order?
>
> If all that is true, then --start-number && --numbered does not make sense
> at all.
>

I was thinking, probably wrong, that the number prepended in file name
is used also to disambiguate two patches with the same subject.


     Marco

^ permalink raw reply

* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Martin Langhoff @ 2006-05-26  6:02 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Martin Langhoff, Git Mailing List, Junio C Hamano,
	Johannes.Schindelin, spyderous, smurf
In-Reply-To: <Pine.LNX.4.64.0605252204590.5623@g5.osdl.org>

On 5/26/06, Linus Torvalds <torvalds@osdl.org> wrote:
> I'm doing it too, just for fun.

Well, it's good to not be so alone in our definition of fun ;-)

> Of course, since I'm doing this on a machine that basically has a laptop
> disk, the "just for fun" part is a bit sad. It's waiting for disk about
> 25% of the time ;/

Ouch.

> And it's slow as hell. I really wish we could do better on the CVS import
> front.

Me too. However, I don't think the perl part is so costly anymore.
It's down to waiting on IO. git-write-tree is also prominently there.
It takes a lot of memory in some writes -- I had thought it'd be
cheaper as it takes one tree object at the time...

I also have a trivial patch that I haven't posted yet, that runs cvsps
to a tempfile, and then reads the file. Serialising the tasks means
that we don't carry around cvsps' memory footprint during the import
itself.

...
> It's "git-rev-list --objects" that is the memory sucker for me, the
> packing itself doesn't seem to be too bad.


No, you're right, it's git-rev-list that gets called during the
repack. But it was pushing everything it could to swap. Once it didn't
fit in memory, it hit a brick wall :(

> The biggest cost seems to be git-write-tree, which is about 0.225 seconds
> for me on that tree on that machine. Which _should_ mean that we could do
> 4 commits a second, but that sure as hell ain't how it works out. It seems
> to do about 1.71 commits a second for me on that tree, which is pretty
> damn pitiful. Some cvs overhead, and probably some other git overhead too.

Well, we _have_ to fetch the file. I guess you are thinking of
extracting if frrom the RCS ,v file directly? One tihng that I found
that seemed to speed things up a bit was to declare TMPDIR to be a
directory in the same partition.

> (That's a 2GHz Merom, so the fact that you get ~6k commits per hour on
> your 2GHz Opteron is about the same speed - I suspect you're also at least
> partly limited by disk, our numbers seem to match pretty well).

Yup. This is _very_ diskbound.

> 200k commits at 6k commits per hour is about a day and a half (plus the
> occasional packing load). Taking that long to import a CVS archive is
> horrible. But I guess it _is_ several years of work, and I guess you
> really have to do it only once, but still.

And it's a huge CVS archive too.



martin

^ permalink raw reply

* Re: Clean up sha1 file writing
From: Junio C Hamano @ 2006-05-26  5:36 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <7virnv3qi6.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

>> I think "kompare" (the KDE diff tool) is nicer.
>
> I'd love to give it a whirl, but aptitude says it will consume
> 73.5MB diskspace to install it, with download size 22.4MB, which
> makes me go ... hmmmm (my machines are currently KDE free so the
> above counts slurping in the kdelibs essentials).  

It indeed is nicer ;-).  I got a good laugh watching it smoothly
move the inserted hunk as I scrolled down.

^ 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