Git development
 help / color / mirror / Atom feed
* Re: bug? in checkout with ambiguous refnames
From: Jeff King @ 2011-01-08 21:40 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Uwe Kleine-König, git, Junio C Hamano
In-Reply-To: <alpine.DEB.1.10.1101081449220.12031@debian>

On Sat, Jan 08, 2011 at 03:40:33PM -0500, Martin von Zweigbergk wrote:

> > Yeah, we generally resolve ambiguities in favor of the tag (and that
> > warning comes from deep within get_sha1_basic). So the real bug here is
> > that it still said "Switched to branch", which is totally wrong.
> > 
> > That being said, it probably would make more sense for "git checkout" to
> > prefer branches to tags.
> 
> What was the rationale for generally favoring tags?

I don't recall hearing any specific argument, but it has always been
that way from early on. I think it is from a vague sense of "tags are
more important than branch tips because they are about marking specific
points, not lines of development". But maybe other old-timers can say
more.

I don't necessarily buy that argument; my only reasoning is that we
should probably keep historic behavior.

> Why does that reasoning not apply to 'git checkout' too?

Because checkout has always been fundamentally about branches. It did
end up growing sane behavior for "git checkout tag" (i.e., a detached
HEAD), but branches are still the fundamental unit for most of its
arguments.

> Btw, what exactly does "generally" mean, i.e. which other commands
> don't favor tags? I know rebase is one example of a command that does
> not favor tags.

It means "we favor tags in resolve_ref, which is the underlying
machinery for most commands, so unless a command special-cases it, that
will be the behavior, and I am too lazy to exhaustively search for such
special cases".

> Slightly off topic, but why does 'git rev-parse --symbolic-full-name'
> not output anything when the input is ambiguous? 'git rev-parse'
> without any flags favors tags, so I would have expected to get
> something like refs/tags/$name back.

I dunno. I never tried it, but I would have expected to get the tag-name
back.

> The reason I'm asking is because I just happened to see in the rebase
> code the other day that it will rebase a detached head if the <branch>
> parameter is not "completely unqualified". For example 'git rebase
> master heads/topic' or 'git rebase master refs/heads/topic' will not
> update refs/heads/topic. I was trying to fix that by using 'git
> rev-parse --symbolic-full-name' to parse <branch>. That seemed to work
> fine until I saw this thread :-).

Heh. I think that would be an argument in favor of changing rev-parse's
behavior.

-Peff

^ permalink raw reply

* Re: bug? in checkout with ambiguous refnames
From: Martin von Zweigbergk @ 2011-01-08 20:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Uwe Kleine-König, git, Junio C Hamano
In-Reply-To: <20110107194909.GB6175@sigill.intra.peff.net>

On Fri, 7 Jan 2011, Jeff King wrote:

> On Fri, Jan 07, 2011 at 11:46:50AM +0100, Uwe Kleine-K?nig wrote:
> 
> > 	ukl@octopus:~/gsrc/linux-2.6$ git diff; git diff --cached
> > 
> > 	ukl@octopus:~/gsrc/linux-2.6$ git checkout sgu/mxs-amba-uart
> > 	warning: refname 'sgu/mxs-amba-uart' is ambiguous.
> > 	Previous HEAD position was c13fb17... Merge commit '65e29a85a419f9a161ab0f09f9d69924e36d940e' into HEAD
> > 	Switched to branch 'sgu/mxs-amba-uart'
> > 
> > OK, it might be a bad idea to have this ambiguity, still ...
> > [...]
> > So working copy and cache are at refs/tags/sgu/mxs-amba-uart, HEAD
> > points to refs/heads/sgu/mxs-amba-uart
> 
> Yeah, we generally resolve ambiguities in favor of the tag (and that
> warning comes from deep within get_sha1_basic). So the real bug here is
> that it still said "Switched to branch", which is totally wrong.
> 
> That being said, it probably would make more sense for "git checkout" to
> prefer branches to tags.

What was the rationale for generally favoring tags? Why does that
reasoning not apply to 'git checkout' too? At least without knowing
the answer to that question, I think I would prefer to have checkout
behave the same as the other commands do. Maybe a bit academic, but it
seems like it would be nice if e.g. 'git checkout $anything && git
show $anything' would show the HEAD commit and if 'git checkout
$anything && git diff HEAD $anything' was always empty.

Btw, what exactly does "generally" mean, i.e. which other commands
don't favor tags? I know rebase is one example of a command that does
not favor tags.

Slightly off topic, but why does 'git rev-parse --symbolic-full-name'
not output anything when the input is ambiguous? 'git rev-parse'
without any flags favors tags, so I would have expected to get
something like refs/tags/$name back.

The reason I'm asking is because I just happened to see in the rebase
code the other day that it will rebase a detached head if the <branch>
parameter is not "completely unqualified". For example 'git rebase
master heads/topic' or 'git rebase master refs/heads/topic' will not
update refs/heads/topic. I was trying to fix that by using 'git
rev-parse --symbolic-full-name' to parse <branch>. That seemed to work
fine until I saw this thread :-).

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2011, #01; Tue, 4)
From: Johannes Sixt @ 2011-01-08 19:47 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, git
In-Reply-To: <4D28AF82.4040102@ramsay1.demon.co.uk>

On Samstag, 8. Januar 2011, Ramsay Jones wrote:
> Junio C Hamano wrote:
> > * rj/test-fixes (2010-12-14) 4 commits
> >  - t4135-*.sh: Skip the "backslash" tests on cygwin
> >  - t3032-*.sh: Do not strip CR from line-endings while grepping on MinGW
> >  - t3032-*.sh: Pass the -b (--binary) option to sed on cygwin
> >  - t6038-*.sh: Pass the -b (--binary) option to sed on cygwin
> >
> > I don't think people on different vintage of Cygwin agreed they are good
> > workarounds---please correct me if I am mistaken.
>
> No, it was different vintages of MinGW not Cygwin. Well, to be more
> precise, it is the different versions of sed that are installed in MinGW by
> the msysGit installer. ;-)
>
> I used msysGit-fullinstall-1.6.4-preview20090729.exe to install msysGit,
> 18 months ago, and my version of sed is quite old. However, these patches
> (which were done mainly for the benefit of cygwin) were written assuming
> the more recent sed version installed by a more recent msysGit installer.
> (judging by commit ca02ad34.) In other words, the sed version on cygwin
> is new enough to know about the -b (--binary) option and so is the more
> recent msysGit installers (but I don't know exactly which version).
>
> I can use my patch #14, which you didn't pick up, to run the above tests
> on my old installation. (Johannes was the only other laggard identified
> and he claims to be upgrading soon! :-D Yeah, I should too.)
>
> So, unless Johannes can think of something I've missed, I think all of
> these commits are good to go...

I've upgraded meanwhile ;-) and have been using this branch since it was 
published the first time. From my POV, these series should go in.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Introduce new configuation option to override committer information
From: Stephen Kelly @ 2011-01-08 19:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1294473809-11850-1-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

> Currently, there is no way to set committer information on a
> per-repository basis. The 'user.name' and 'user.email' configuration
> options set both author and committer information. To solve this,
> introduce 'user.committername' and 'user.committeremail' configuration
> options to override committer name and email respectively.
> 
> Reported-by: Stephen Kelly <steveire@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.co

<snip>

Thanks for working on this. I wanted to try it out, but it no longer applies 
cleanly since 4c28e4ada03f5440251545cf91e0d81bce9b010d and after trivial 
merging, I can't build.

Can you update the patch?

Thanks,

Steve.

^ permalink raw reply

* Re: No way to resolve git am conflicts.
From: Stephen Kelly @ 2011-01-08 19:21 UTC (permalink / raw)
  To: git
In-Reply-To: <20110108163633.GB28898@burratino>

Jonathan Nieder wrote:

> Stephen Kelly wrote:
> 
>> git am the.patch
> [...]
>> Applying: Introduce new configuation option to override committer
>> information
>> error: patch failed: builtin/commit.c:1352
>> error: builtin/commit.c: patch does not apply
>> Patch failed at 0001 Introduce new configuation option to override
>> committer information
>> When you have resolved this problem run "git am --resolved".
>> If you would prefer to skip this patch, instead run "git am --skip".
>> To restore the original branch and stop patching run "git am --abort".
>> $ git diff
> [...]
>> As git status doesn't tell me what the conflict is, I can't resolve it.
> 
> Have you tried "git am -3" or "git am --reject" (after "git am
> --abort")?

git am -3 worked, thanks.

> 
> I agree that the hints printed are suboptimal in this case.  Please
> feel free to make them better if you have time for it. :)

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2011, #01; Tue, 4)
From: Ramsay Jones @ 2011-01-08 18:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <7vipy4dy8y.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> 
> * rj/test-fixes (2010-12-14) 4 commits
>  - t4135-*.sh: Skip the "backslash" tests on cygwin
>  - t3032-*.sh: Do not strip CR from line-endings while grepping on MinGW
>  - t3032-*.sh: Pass the -b (--binary) option to sed on cygwin
>  - t6038-*.sh: Pass the -b (--binary) option to sed on cygwin
> 
> I don't think people on different vintage of Cygwin agreed they are good
> workarounds---please correct me if I am mistaken.

No, it was different vintages of MinGW not Cygwin. Well, to be more precise,
it is the different versions of sed that are installed in MinGW by the
msysGit installer. ;-)

I used msysGit-fullinstall-1.6.4-preview20090729.exe to install msysGit,
18 months ago, and my version of sed is quite old. However, these patches
(which were done mainly for the benefit of cygwin) were written assuming
the more recent sed version installed by a more recent msysGit installer.
(judging by commit ca02ad34.) In other words, the sed version on cygwin
is new enough to know about the -b (--binary) option and so is the more
recent msysGit installers (but I don't know exactly which version).

I can use my patch #14, which you didn't pick up, to run the above tests
on my old installation. (Johannes was the only other laggard identified
and he claims to be upgrading soon! :-D Yeah, I should too.)

So, unless Johannes can think of something I've missed, I think all of
these commits are good to go...

ATB,
Ramsay Jones

^ permalink raw reply

* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
From: Jonathan Nieder @ 2011-01-08 18:22 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: git
In-Reply-To: <20110108164825.GC28898@burratino>

Jonathan Nieder wrote:

> From a quick grep, it seems you are right:
> 
>  $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | sum
>  65126     1
>  $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | sum
>  64807     1
>  $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | sum
>  13350     1
>  $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | sum
>  07810     1

sum does something totally different than I expected.  With [1]
comes the more reasonable

 $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | addup
 485
 $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | addup
 2620
 $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | addup
 594
 $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | addup
 3133

So the code bloat and use of backticks are less dire than I feared.

[1] 
	addup () {
		sum=0
		while read term
		do
			: $((sum = $sum + $term))
		done
		echo $sum
	}

^ permalink raw reply

* Re: git-archive and core.eol
From: Erik Faye-Lund @ 2011-01-08 17:28 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List, msysGit, eyvind.bernhardsen
In-Reply-To: <4D28683B.4020400@lsrfire.ath.cx>

On Sat, Jan 8, 2011 at 2:35 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 15.12.2010 23:32, schrieb Erik Faye-Lund:
>> I recently tried the following on Windows:
>>
>> $ git init
>> Initialized empty Git repository in c:/Users/kusma/test/.git/
>> $ echo "foo
>> bar" > test.txt
>> $ git -c core.autocrlf=true add test.txt
>> warning: LF will be replaced by CRLF in test.txt.
>> The file will have its original line endings in your working directory.
>> $ git commit -m.
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>  create mode 100644 test.txt
>> $ git -c core.autocrlf=true -c core.eol=lf archive --format=tar HEAD > test.tar
>> $ tar xvf test.tar
>> $ od -c test.txt
>> 0000000   f   o   o  \r  \n   b   a   r  \r  \n
>> 0000012
>>
>> Just to be sure, I checked this:
>>
>> $ git show HEAD:test.txt | od -c
>> 0000000   f   o   o  \n   b   a   r  \n
>> 0000010
>>
>> Yep, the file has LF in the repo, as expected... the warning from
>> git-add is a bit confusing, but OK.
>>
>> Hmm, so git-archive writes CRLF even if I said I wanted LF. But then I
>> tried this on Linux:
>>
>> $ git init
>> Initialized empty Git repository in /home/kusma/src/test/.git/
>> $ echo "foo
>> bar" > test.txt
>> $ git add test.txt
>> $ git commit -m.
>> [master (root-commit) c6f195e] .
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>  create mode 100644 test.txt
>> $ git -c core.autocrlf=true -c core.eol=crlf archive --format=tar HEAD
>>> test.tar
>> $ tar xvf test.tar
>> test.txt
>> $ od -c test.txt
>> 0000000   f   o   o  \r  \n   b   a   r  \r  \n
>> 0000012
>>
>> This leaves me a bit puzzled. On Linux, I can override the default
>> new-line style CRLF for git-archive, but I can't override it to LF on
>> Windows?
>>
>> I expected it to work because sha1_file_to_archive calls
>> convert_to_working_tree. I've tried stepping through the code, but I
>> don't quite understand where it goes wrong. Or even how the code is
>> supposed to work :P
>>
>> Does anyone have any clue what's going on? I'm running with the
>> current master, git version 1.7.3.3.585.g74f6e.
>
> I can't seem to replicate this (1.7.4-rc1); see below for the test script
> I tried to come up with.  It should test all combinations of the relevant
> config variables and the text attribute.  I cheated by simply setting the
> expectations to match the results on Linux; I didn't check if these are
> indeed the correct results.  The test passes for me on MinGW, too, though.
>
> Did I miss a variable or are some of the expectations wrong?
>
> Thanks,
> René
>


Really? I haven't looked through what the test actually does (I'm out
sick right now, and don't have many working brain-cells), but every
single test fails for me:

not ok - 1 setup
#	
#		printf "1\\n2\\n" >lf &&
#		printf "1\\r\\n2\\r\\n" >crlf &&
#	
#		echo "*.txt text" >.gitattributes &&
#		git add .gitattributes &&
#	
#		mkdir autocrlf_false &&
#		cp lf crlf autocrlf_false/ &&
#		cp lf autocrlf_false/lf.txt &&
#		cp crlf autocrlf_false/crlf.txt &&
#		git -c core.autocrlf=false add autocrlf_false/ &&
#	
#		mkdir autocrlf_true &&
#		cp lf crlf autocrlf_true/ &&
#		cp lf autocrlf_true/lf.txt &&
#		cp crlf autocrlf_true/crlf.txt &&
#		git -c core.autocrlf=true add autocrlf_true/ &&
#	
#		git commit -m.
#	
not ok - 2 archive with autocrlf=false eol=crlf
#	
#			git -c core.autocrlf=false -c core.eol=crlf archive HEAD
>autocrlf_false-eol_crlf.tar &&
#			(mkdir autocrlf_false-eol_crlf.d && cd autocrlf_false-eol_crlf.d
&& "tar" xf -) <autocrlf_false-eol_crlf.tar
#		
not ok - 3 archive with autocrlf=true eol=crlf
#	
#			git -c core.autocrlf=true -c core.eol=crlf archive HEAD
>autocrlf_true-eol_crlf.tar &&
#			(mkdir autocrlf_true-eol_crlf.d && cd autocrlf_true-eol_crlf.d &&
"tar" xf -) <autocrlf_true-eol_crlf.tar
#		
not ok - 4 archive with autocrlf=false eol=lf
#	
#			git -c core.autocrlf=false -c core.eol=lf archive HEAD
>autocrlf_false-eol_lf.tar &&
#			(mkdir autocrlf_false-eol_lf.d && cd autocrlf_false-eol_lf.d &&
"tar" xf -) <autocrlf_false-eol_lf.tar
#		
not ok - 5 archive with autocrlf=true eol=lf
#	
#			git -c core.autocrlf=true -c core.eol=lf archive HEAD
>autocrlf_true-eol_lf.tar &&
#			(mkdir autocrlf_true-eol_lf.d && cd autocrlf_true-eol_lf.d &&
"tar" xf -) <autocrlf_true-eol_lf.tar
#		
not ok - 6 add autocrlf=false, archive autocrlf=false eol=crlf: crlf => crlf
#	test_cmp crlf autocrlf_false-eol_crlf.d/autocrlf_false/crlf
not ok - 7 add autocrlf=false, archive autocrlf=false eol=crlf: lf => lf
#	test_cmp lf autocrlf_false-eol_crlf.d/autocrlf_false/lf
not ok - 8 add autocrlf=false, archive autocrlf=false eol=lf: crlf => crlf
#	test_cmp crlf autocrlf_false-eol_lf.d/autocrlf_false/crlf
not ok - 9 add autocrlf=false, archive autocrlf=false eol=lf: lf => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_false/lf
not ok - 10 add autocrlf=false, archive autocrlf=true eol=crlf: crlf => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_false/crlf
not ok - 11 add autocrlf=false, archive autocrlf=true eol=crlf: lf => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_false/lf
not ok - 12 add autocrlf=false, archive autocrlf=true eol=lf: crlf => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_false/crlf
not ok - 13 add autocrlf=false, archive autocrlf=true eol=lf: lf => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_false/lf
not ok - 14 add autocrlf=true, archive autocrlf=false eol=crlf: crlf => lf
#	test_cmp lf autocrlf_false-eol_crlf.d/autocrlf_true/crlf
not ok - 15 add autocrlf=true, archive autocrlf=false eol=crlf: lf => lf
#	test_cmp lf autocrlf_false-eol_crlf.d/autocrlf_true/lf
not ok - 16 add autocrlf=true, archive autocrlf=false eol=lf: crlf => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_true/crlf
not ok - 17 add autocrlf=true, archive autocrlf=false eol=lf: lf => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_true/lf
not ok - 18 add autocrlf=true, archive autocrlf=true eol=crlf: crlf => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_true/crlf
not ok - 19 add autocrlf=true, archive autocrlf=true eol=crlf: lf => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_true/lf
not ok - 20 add autocrlf=true, archive autocrlf=true eol=lf: crlf => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_true/crlf
not ok - 21 add autocrlf=true, archive autocrlf=true eol=lf: lf => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_true/lf
not ok - 22 add autocrlf=false, archive autocrlf=false eol=crlf:
crlf.txt => crlf
#	test_cmp crlf autocrlf_false-eol_crlf.d/autocrlf_false/crlf.txt
not ok - 23 add autocrlf=false, archive autocrlf=false eol=crlf: lf.txt => crlf
#	test_cmp crlf autocrlf_false-eol_crlf.d/autocrlf_false/lf.txt
not ok - 24 add autocrlf=false, archive autocrlf=false eol=lf: crlf.txt => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_false/crlf.txt
not ok - 25 add autocrlf=false, archive autocrlf=false eol=lf: lf.txt => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_false/lf.txt
not ok - 26 add autocrlf=false, archive autocrlf=true eol=crlf: crlf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_false/crlf.txt
not ok - 27 add autocrlf=false, archive autocrlf=true eol=crlf: lf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_false/lf.txt
not ok - 28 add autocrlf=false, archive autocrlf=true eol=lf: crlf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_false/crlf.txt
not ok - 29 add autocrlf=false, archive autocrlf=true eol=lf: lf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_false/lf.txt
not ok - 30 add autocrlf=true, archive autocrlf=false eol=crlf: crlf.txt => crlf
#	test_cmp crlf autocrlf_false-eol_crlf.d/autocrlf_true/crlf.txt
not ok - 31 add autocrlf=true, archive autocrlf=false eol=crlf: lf.txt => crlf
#	test_cmp crlf autocrlf_false-eol_crlf.d/autocrlf_true/lf.txt
not ok - 32 add autocrlf=true, archive autocrlf=false eol=lf: crlf.txt => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_true/crlf.txt
not ok - 33 add autocrlf=true, archive autocrlf=false eol=lf: lf.txt => lf
#	test_cmp lf autocrlf_false-eol_lf.d/autocrlf_true/lf.txt
not ok - 34 add autocrlf=true, archive autocrlf=true eol=crlf: crlf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_true/crlf.txt
not ok - 35 add autocrlf=true, archive autocrlf=true eol=crlf: lf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_crlf.d/autocrlf_true/lf.txt
not ok - 36 add autocrlf=true, archive autocrlf=true eol=lf: crlf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_true/crlf.txt
not ok - 37 add autocrlf=true, archive autocrlf=true eol=lf: lf.txt => crlf
#	test_cmp crlf autocrlf_true-eol_lf.d/autocrlf_true/lf.txt
# failed 37 among 37 test(s)
1..37

$ git --version
git version 1.7.4.rc1.3196.gfd693

(This is the current 'devel'-branch from
git://repo.or.cz/git/mingw/4msysgit.git)

^ permalink raw reply

* Re: Resumable clone/Gittorrent (again)
From: Luke Kenneth Casson Leighton @ 2011-01-08 17:21 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Nicolas Pitre, Git Mailing List
In-Reply-To: <AANLkTimgn2_BWYjbGKbGoeGJ=erKundX4umfy=s16dB1@mail.gmail.com>

On Sat, Jan 8, 2011 at 2:17 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Fri, Jan 7, 2011 at 10:59 PM, Luke Kenneth Casson Leighton
> <luke.leighton@gmail.com> wrote:
>>  bottom line: my take on this is (sorry to say, nguyen) that i don't
>> believe bittorrent "pieces" map well to the chains concept, unless the
>> chains are perfectly fixed identical sizes [which they could well be?
>> am i mistaken about this?]
>
> there are a few characteristics of bittorrent pieces that i see:
> verifiable, resumable, uniquely identifiable across peers and
> reasonbly small in count.
>
> The fixed size helps peers uniquely identify any pieces by splitting
> the whole transfer equally and indexing them in 1-dimension.

 ok - you haven't answered the question: are the chains perfectly
fixed identical sizes?

 if so they can be slotted into the bittorrent protocol by simply
pre-selecting the size to match.  with the downside that if there are
a million such "chains" you now pretty much overwhelm the peers with
the amount of processing, network traffic and memory requirements to
maintain the "pieces" map.

 if not then you now need to modify the bittorrent protocol to cope
with variable-length block sizes: the protocol only allows for the
last block to be of variable-length.

 also, it's worth pointing out that the entire code-base of every
single bittorrent client that you will ever be able to find revolves
around the concept of reassembly of files from "pieces".

 bottom line: the bittorrent protocol and the available bittorrent
source code libraries, both of which save you a great deal of time in
getting something up-and-running, is _not_ the right fit for the
concept of placing the proposed "chains" into bittorrent "pieces".

 translation: if you wish to pursue the "chains" concept, either a
heavily-modified bittorrent protocol and client _or_ an entirely new
peer-to-peer protocol is far more appropriate.

 orrr, doing what i initially suggested, which is to leave the
bittorrent protocol as-is and to open one .torrent per "chain".
especially if these "chains" vary considerably in size (k to gb)

l.

^ permalink raw reply

* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
From: Jonathan Nieder @ 2011-01-08 16:48 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: git
In-Reply-To: <20110108162353.GB4786@gmx.de>

Ralf Wildenhues wrote:

> But git makes heavy use of "no quoting needed on RHS of assignment"
> anyway, so it seems like this would be a good move nonetheless.

No disagreement there.

> And the
> testsuite uses backticks a lot,

>From a quick grep, it seems you are right:

 $ git grep -c -F -e '`' -- 't/*.sh' | cut -d: -f2 | sum
 65126     1
 $ git grep -c -F -e '$(' -- 't/*.sh' | cut -d: -f2 | sum
 64807     1
 $ git grep -c -F -e '`' -- '*.sh' | cut -d: -f2 | sum
 13350     1
 $ git grep -c -F -e '$(' -- '*.sh' | cut -d: -f2 | sum
 07810     1

Documentation/CodingGuidelines 

 - We prefer $( ... ) for command substitution; unlike ``, it
   properly nests.  It should have been the way Bourne spelled
   it from day one, but unfortunately isn't.

> it seems a move away from that should be
> done more uniformly?

I don't see why. :)  In fact, I personally would not be happy at all
to see such a high-churn patch as that, while using the $( ... )
form in new code and as part of clarifications to other parts of the
same lines would seem to me to be a welcome thing.

Having said all that, I have no strong investment in this.  Feel
free to do what works best for you.

Thanks,
Jonathan

^ permalink raw reply

* Re: No way to resolve git am conflicts.
From: Jonathan Nieder @ 2011-01-08 16:36 UTC (permalink / raw)
  To: Stephen Kelly; +Cc: git
In-Reply-To: <ig9nqq$4ib$1@dough.gmane.org>

Stephen Kelly wrote:

> git am the.patch
[...]
> Applying: Introduce new configuation option to override committer 
> information
> error: patch failed: builtin/commit.c:1352
> error: builtin/commit.c: patch does not apply
> Patch failed at 0001 Introduce new configuation option to override committer 
> information
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
> $ git diff
[...]
> As git status doesn't tell me what the conflict is, I can't resolve it.

Have you tried "git am -3" or "git am --reject" (after "git am
--abort")?

I agree that the hints printed are suboptimal in this case.  Please
feel free to make them better if you have time for it. :)

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: Robin Rosenberg @ 2011-01-08 16:35 UTC (permalink / raw)
  To: David Chanters; +Cc: git
In-Reply-To: <AANLkTinmq=3kJmtSVutf7dHAQ0QL3fr9_E3hZ7gDe1JY@mail.gmail.com>


8 jan 2011 kl. 12:23 skrev David Chanters:

> Hi all,
> 
> [ Please Cc me on any replies as I am not subscribed to this list, thanks. ]
> 
> I am wondering if I can get git diff to create "CVS-style patches"?
> What do I mean by that?  Well, whenever I do:
> 
> git diff
> 
> I get patch headers in the form:
> 
> diff --git a/foo.c b/foo.c
> index 57b9527..a2d947b 100644
> --- a/foo.c
> +++ b/foo.c
> 
> This is fine for git, but if I then want to import the same patch into
> CVS I have to either edit the patch, or mess around with the -p option
> to patch(1).
> 
Adding -p1 is not what I'd call "mess around". It is pretty standard
use with patch since most patches has en extra directory level anyway. I
think that's why git has those a/ b/ prefixes by default.

Have you tries git cvsexportcommit? It is by far the easiest way of exporting
from git to CVS.

-- robin

^ permalink raw reply

* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
From: Ralf Wildenhues @ 2011-01-08 16:23 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git
In-Reply-To: <20110108161441.GA28898@burratino>

* Jonathan Nieder wrote on Sat, Jan 08, 2011 at 05:14:41PM CET:
> Ralf Wildenhues wrote:
> 
> > [Subject: Avoid unportable nested double- and backquotes in shell scripts]
> >
> > Some shells parse them wrongly, esp. pdksh.
> 
> How does it treat $( ) command substitutions?  (We use those more
> heavily and they are easier on the eyes anyway.)

Better (except for the usual problems when 'case ...)' comes into play).
But git makes heavy use of "no quoting needed on RHS of assignment"
anyway, so it seems like this would be a good move nonetheless.  And the
testsuite uses backticks a lot, it seems a move away from that should be
done more uniformly?

Anyway, I'll be happy to respin in whatever form is acceptable.

> > --- a/t/t9107-git-svn-migrate.sh
> > +++ b/t/t9107-git-svn-migrate.sh
> > @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
> >  		echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
> >  	done &&
> >  	git svn migrate --minimize &&
> > -	test -z "`git config -l | grep "^svn-remote\.git-svn\."`" &&
> > +	! git config -l | grep "^svn-remote\.git-svn\." &&
> 
> I thought I remembered portability problems with the
> 
> 	! a | b
> 
> construct but it seems I am wrong; t7810-grep.sh uses that
> construct without trouble, at least.

Some non-Posix-conforming shells have problems with that too, e.g.,
Solaris /bin/sh, but I figured git wouldn't cater to them as I also saw
other such uses in the tree.

Cheers,
Ralf

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: Andreas Schwab @ 2011-01-08 16:17 UTC (permalink / raw)
  To: David Chanters; +Cc: Tomas Carnecky, git
In-Reply-To: <AANLkTi=_YdTsvFboQAj447SUtcxrVhM18QkvSVdpvdMJ@mail.gmail.com>

David Chanters <david.chanters@googlemail.com> writes:

> Sorry about that, that's poor wording on my part.  I don't mean "CVS
> won't barf" -- I meant that GNU Patch can still apply the patch
> without the meta-data referencing git still.

So this has nothing at all to do with CVS.  And GNU patch will happily
ignore everything that it does not recognize.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] Avoid unportable nested double- and backquotes in shell scripts.
From: Jonathan Nieder @ 2011-01-08 16:14 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: git
In-Reply-To: <20110108090105.GB14536@gmx.de>

Ralf Wildenhues wrote:

> [Subject: Avoid unportable nested double- and backquotes in shell scripts]
>
> Some shells parse them wrongly, esp. pdksh.

How does it treat $( ) command substitutions?  (We use those more
heavily and they are easier on the eyes anyway.)

> --- a/t/t9107-git-svn-migrate.sh
> +++ b/t/t9107-git-svn-migrate.sh
> @@ -94,7 +94,7 @@ test_expect_success 'migrate --minimize on old inited layout' '
>  		echo "$svnrepo"$path > "$GIT_DIR"/svn/$ref/info/url ) || exit 1;
>  	done &&
>  	git svn migrate --minimize &&
> -	test -z "`git config -l | grep "^svn-remote\.git-svn\."`" &&
> +	! git config -l | grep "^svn-remote\.git-svn\." &&

I thought I remembered portability problems with the

	! a | b

construct but it seems I am wrong; t7810-grep.sh uses that
construct without trouble, at least.

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: Tomas Carnecky @ 2011-01-08 15:51 UTC (permalink / raw)
  To: David Chanters; +Cc: Andreas Schwab, git
In-Reply-To: <AANLkTi=_YdTsvFboQAj447SUtcxrVhM18QkvSVdpvdMJ@mail.gmail.com>

  On 1/8/11 4:35 PM, David Chanters wrote:
> Sorry about that, that's poor wording on my part.  I don't mean "CVS
> won't barf" -- I meant that GNU Patch can still apply the patch
> without the meta-data referencing git still.  I think though this
> "--no-prefix" option might just do it.

Update your gnu patch. Never versions know how to handle the git 
specific metadata (gnu patch simply ignores it).

And the a/, b/ prefix is *not* git specific. Pretty much all versions of 
a patch program know how to deal with that, you just need to use -p1 on 
the commandline.

tom

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: David Chanters @ 2011-01-08 15:35 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Tomas Carnecky, git
In-Reply-To: <m2vd1za0af.fsf@igel.home>

On 8 January 2011 15:21, Andreas Schwab <schwab@linux-m68k.org> wrote:
> David Chanters <david.chanters@googlemail.com> writes:
>
>> Well, any one of those goals, really.  I just want to generate a patch
>> that CVS won't barf on
>
> What does "CVS won't barf" mean in this context?  CVS does not operate
> on patches.

Sorry about that, that's poor wording on my part.  I don't mean "CVS
won't barf" -- I meant that GNU Patch can still apply the patch
without the meta-data referencing git still.  I think though this
"--no-prefix" option might just do it.

David

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: Tomas Carnecky @ 2011-01-08 15:26 UTC (permalink / raw)
  To: David Chanters; +Cc: git
In-Reply-To: <AANLkTinx9qM=fjH53UodY0G870Ne3wpFiFEgEGNXxOY7@mail.gmail.com>

  On 1/8/11 2:43 PM, David Chanters wrote:
> Would the presense of the other meta-data, such as the "diff --git"
> line as well as the "Index" line cause any problems when applying this
> patch in CVS (I suppose now, my question doesn't have to apply to CVS

I don't know, you tell me. I've never used CVS.

> at all, more likely it will apply to any file sets)?  I am not sure
> how GNU Patch uses this meta information, and I assume "git diff" adds
> it for a good reason.

gnu patch most likely doesn't use the index line and ignores all the git 
specific additions to the diff format.

tom

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: Andreas Schwab @ 2011-01-08 15:21 UTC (permalink / raw)
  To: David Chanters; +Cc: Tomas Carnecky, git
In-Reply-To: <AANLkTinx9qM=fjH53UodY0G870Ne3wpFiFEgEGNXxOY7@mail.gmail.com>

David Chanters <david.chanters@googlemail.com> writes:

> Well, any one of those goals, really.  I just want to generate a patch
> that CVS won't barf on

What does "CVS won't barf" mean in this context?  CVS does not operate
on patches.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* git-config does not check validity of variable names
From: Libor Pechacek @ 2011-01-08 14:46 UTC (permalink / raw)
  To: git

Hello,

I've noticed that git-config accepts variable names in the form "a=b" for its
"get" operation.  That means "git config a=b" does not write anything to its
output and exists with status 1.

According to the man page only alphanumeric characters and - are allowed in
variable names.  Would it make sense to spit out an error message when the user
supplies an invalid variable name like the above?

Libor
-- 
Libor Pechacek
SUSE L3 Team, Prague

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: David Chanters @ 2011-01-08 13:43 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: git
In-Reply-To: <4D284F57.2000808@dbservice.com>

Hi Tomas,

On 8 January 2011 11:49, Tomas Carnecky <tom@dbservice.com> wrote:
> What exactly do you need to change in the patch? Remove the index line? The
> '--git' string? Remove or change the a/, b/ prefix?

Well, any one of those goals, really.  I just want to generate a patch
that CVS won't barf on -- what that means if not needing to use the
"-p" option to patch because git outputs a/ b/ prefix lines.  Your
"--no-prefix" option does that much, which might be enough.

I did read, I think on this list, that the GNU Patch program
interprets these "diff --git" lines?  What exactly are they for, do
you know?  I mean, even if I just did:

git diff --no-prefix

Would the presense of the other meta-data, such as the "diff --git"
line as well as the "Index" line cause any problems when applying this
patch in CVS (I suppose now, my question doesn't have to apply to CVS
at all, more likely it will apply to any file sets)?  I am not sure
how GNU Patch uses this meta information, and I assume "git diff" adds
it for a good reason.

I hope this all makes sense?

David

^ permalink raw reply

* Re: git-archive and core.eol
From: René Scharfe @ 2011-01-08 13:35 UTC (permalink / raw)
  To: kusmabite; +Cc: Git Mailing List, msysGit, eyvind.bernhardsen
In-Reply-To: <AANLkTi=kfE88F7dY5F_xtbEuh9DyUcN+ymeXqLMWztGQ@mail.gmail.com>

Am 15.12.2010 23:32, schrieb Erik Faye-Lund:
> I recently tried the following on Windows:
> 
> $ git init
> Initialized empty Git repository in c:/Users/kusma/test/.git/
> $ echo "foo
> bar" > test.txt
> $ git -c core.autocrlf=true add test.txt
> warning: LF will be replaced by CRLF in test.txt.
> The file will have its original line endings in your working directory.
> $ git commit -m.
>  1 files changed, 2 insertions(+), 0 deletions(-)
>  create mode 100644 test.txt
> $ git -c core.autocrlf=true -c core.eol=lf archive --format=tar HEAD > test.tar
> $ tar xvf test.tar
> $ od -c test.txt
> 0000000   f   o   o  \r  \n   b   a   r  \r  \n
> 0000012
> 
> Just to be sure, I checked this:
> 
> $ git show HEAD:test.txt | od -c
> 0000000   f   o   o  \n   b   a   r  \n
> 0000010
> 
> Yep, the file has LF in the repo, as expected... the warning from
> git-add is a bit confusing, but OK.
> 
> Hmm, so git-archive writes CRLF even if I said I wanted LF. But then I
> tried this on Linux:
> 
> $ git init
> Initialized empty Git repository in /home/kusma/src/test/.git/
> $ echo "foo
> bar" > test.txt
> $ git add test.txt
> $ git commit -m.
> [master (root-commit) c6f195e] .
>  1 files changed, 2 insertions(+), 0 deletions(-)
>  create mode 100644 test.txt
> $ git -c core.autocrlf=true -c core.eol=crlf archive --format=tar HEAD
>> test.tar
> $ tar xvf test.tar
> test.txt
> $ od -c test.txt
> 0000000   f   o   o  \r  \n   b   a   r  \r  \n
> 0000012
> 
> This leaves me a bit puzzled. On Linux, I can override the default
> new-line style CRLF for git-archive, but I can't override it to LF on
> Windows?
> 
> I expected it to work because sha1_file_to_archive calls
> convert_to_working_tree. I've tried stepping through the code, but I
> don't quite understand where it goes wrong. Or even how the code is
> supposed to work :P
> 
> Does anyone have any clue what's going on? I'm running with the
> current master, git version 1.7.3.3.585.g74f6e.

I can't seem to replicate this (1.7.4-rc1); see below for the test script
I tried to come up with.  It should test all combinations of the relevant
config variables and the text attribute.  I cheated by simply setting the
expectations to match the results on Linux; I didn't check if these are
indeed the correct results.  The test passes for me on MinGW, too, though.

Did I miss a variable or are some of the expectations wrong?

Thanks,
René


 t/t5002-archive-eol.sh |   86 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/t/t5002-archive-eol.sh b/t/t5002-archive-eol.sh
new file mode 100755
index 0000000..50f80f7
--- /dev/null
+++ b/t/t5002-archive-eol.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+
+test_description='git archive EOL tests'
+
+. ./test-lib.sh
+
+prepare() {
+	tfile="autocrlf_$1-eol_$2.tar"
+	dir="autocrlf_$1-eol_$2.d"
+	test_expect_success "archive with autocrlf=$1 eol=$2" "
+		git -c core.autocrlf=$1 -c core.eol=$2 archive HEAD >$tfile &&
+		(mkdir $dir && cd $dir && \"$TAR\" xf -) <$tfile
+	"
+}
+
+expect_success() {
+	file="autocrlf_$2-eol_$3.d/autocrlf_$1/$4"
+	desc="add autocrlf=$1, archive autocrlf=$2 eol=$3"
+	test_expect_success "$desc: $4 => $5" "test_cmp $5 $file"
+}
+
+test_expect_success 'setup' '
+	printf "1\\n2\\n" >lf &&
+	printf "1\\r\\n2\\r\\n" >crlf &&
+
+	echo "*.txt text" >.gitattributes &&
+	git add .gitattributes &&
+
+	mkdir autocrlf_false &&
+	cp lf crlf autocrlf_false/ &&
+	cp lf autocrlf_false/lf.txt &&
+	cp crlf autocrlf_false/crlf.txt &&
+	git -c core.autocrlf=false add autocrlf_false/ &&
+
+	mkdir autocrlf_true &&
+	cp lf crlf autocrlf_true/ &&
+	cp lf autocrlf_true/lf.txt &&
+	cp crlf autocrlf_true/crlf.txt &&
+	git -c core.autocrlf=true add autocrlf_true/ &&
+
+	git commit -m.
+'
+
+#	core.autocrlf	core.eol
+prepare	false		crlf
+prepare	true		crlf
+prepare	false		lf
+prepare	true		lf
+
+#		core.autocrlf	core.eol	original	expect
+#		(add)	(archive)
+expect_success	false	false	crlf		crlf		crlf
+expect_success	false	false	crlf		lf		lf
+expect_success	false	false	lf		crlf		crlf
+expect_success	false	false	lf		lf		lf
+expect_success	false	true	crlf		crlf		crlf
+expect_success	false	true	crlf		lf		crlf
+expect_success	false	true	lf		crlf		crlf
+expect_success	false	true	lf		lf		crlf
+expect_success	true	false	crlf		crlf		lf
+expect_success	true	false	crlf		lf		lf
+expect_success	true	false	lf		crlf		lf
+expect_success	true	false	lf		lf		lf
+expect_success	true	true	crlf		crlf		crlf
+expect_success	true	true	crlf		lf		crlf
+expect_success	true	true	lf		crlf		crlf
+expect_success	true	true	lf		lf		crlf
+
+expect_success	false	false	crlf		crlf.txt	crlf
+expect_success	false	false	crlf		lf.txt		crlf
+expect_success	false	false	lf		crlf.txt	lf
+expect_success	false	false	lf		lf.txt		lf
+expect_success	false	true	crlf		crlf.txt	crlf
+expect_success	false	true	crlf		lf.txt		crlf
+expect_success	false	true	lf		crlf.txt	crlf
+expect_success	false	true	lf		lf.txt		crlf
+expect_success	true	false	crlf		crlf.txt	crlf
+expect_success	true	false	crlf		lf.txt		crlf
+expect_success	true	false	lf		crlf.txt	lf
+expect_success	true	false	lf		lf.txt		lf
+expect_success	true	true	crlf		crlf.txt	crlf
+expect_success	true	true	crlf		lf.txt		crlf
+expect_success	true	true	lf		crlf.txt	crlf
+expect_success	true	true	lf		lf.txt		crlf
+
+test_done

^ permalink raw reply related

* No way to resolve git am conflicts.
From: Stephen Kelly @ 2011-01-08 13:11 UTC (permalink / raw)
  To: git


kde-devel@bishop:~/dev/src/git{master}$ curl -o the.patch 
http://download.gmane.org/gmane.comp.version-control.git/164809/164810 && 
git am the.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  
Current
                                 Dload  Upload   Total   Spent    Left  
Speed
100 10438    0 10438    0     0  12046      0 --:--:-- --:--:-- --:--:-- 
13451
Applying: Introduce new configuation option to override committer 
information
error: patch failed: builtin/commit.c:1352
error: builtin/commit.c: patch does not apply
Patch failed at 0001 Introduce new configuation option to override committer 
information
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Qt( 4.7 ) KDE ( 4.6 ) 
kde-devel@bishop:~/dev/src/git{master}$ git diff
Qt( 4.7 ) KDE ( 4.6 ) 
kde-devel@bishop:~/dev/src/git{master}$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       p.patch
#       the.patch
nothing added to commit but untracked files present (use "git add" to track)
Qt( 4.7 ) KDE ( 4.6 ) 
kde-devel@bishop:~/dev/src/git{master}$ 

As git status doesn't tell me what the conflict is, I can't resolve it.

^ permalink raw reply

* Re: Creating CVS-style patch headers with git-diff
From: Tomas Carnecky @ 2011-01-08 11:49 UTC (permalink / raw)
  To: David Chanters; +Cc: git
In-Reply-To: <AANLkTinmq=3kJmtSVutf7dHAQ0QL3fr9_E3hZ7gDe1JY@mail.gmail.com>

  On 1/8/11 12:23 PM, David Chanters wrote:
> Hi all,
>
> [ Please Cc me on any replies as I am not subscribed to this list, thanks. ]
>
> I am wondering if I can get git diff to create "CVS-style patches"?
> What do I mean by that?  Well, whenever I do:
>
> git diff
>
> I get patch headers in the form:
>
> diff --git a/foo.c b/foo.c
> index 57b9527..a2d947b 100644
> --- a/foo.c
> +++ b/foo.c
>
> This is fine for git, but if I then want to import the same patch into
> CVS I have to either edit the patch, or mess around with the -p option
> to patch(1).
What exactly do you need to change in the patch? Remove the index line? 
The '--git' string? Remove or change the a/, b/ prefix?
> I have seen that git-diff has options to change the a/ b/ headers --
> can anyone shed some light on this as to what I can do?

Are you maybe looking for the --no-prefix option?

tom

^ permalink raw reply

* Re: [RFD] My thoughts about implementing gitweb output caching
From: Jakub Narebski @ 2011-01-08 11:44 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, J.H., John 'Warthog9' Hawley
In-Reply-To: <20110108002643.GD15495@burratino>

On Sat, 8 Jan 2011, Jonathan Nieder wrote:
> 
> [...] A few uninformed reactions.

Thanks for your comments.

> Jakub Narebski wrote:
> 
> > There was request to support installing gitweb modules in a separate
> > directory, but that would require changes to "gitweb: Prepare for
> > splitting gitweb" patch (but it is doable).  Is there wider interest
> > in supporting such feature?
> 
> If you are referring to my suggestion, I see no reason to wait on
> that.  The lib/ dir can be made configurable later.

You are right.  Thanks for sanity check.  Though I'd prefer that we get
it right from the first time.

For the record, the changes to support configurable gitweblibdir are:

gitweb/Makefile:
----------------

   gitweblibdir_SQ = $(subst ','\'',$(gitwebdir)/lib)#'

 to

   gitweblibdir ?= $(gitwebdir)/lib
   ...
   gitweblibdir_SQ = $(subst ','\'',$(gitweblibdir))#'


   GITWEB_REPLACE = \
   ...
   	-e 's|++GITWEBLIBDIR++|$(gitweblibdir)|g' \


gitweb/gitweb.perl:
-------------------

   # __DIR__ is taken from Dir::Self __DIR__ fragment
   sub __DIR__ () {
   	File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
   }
   use lib __DIR__ . '/lib';

 to

   use lib $ENV{'GITWEBLIBDIR'} || "++GITWEBLIBDIR++";

or something like that


t/gitweb-lib.sh:
----------------

   gitweb_run () {
   ...
   	GITWEBLIBDIR="$GIT_BUILD_DIR/gitweb/lib"
   	export GITWEBLIBDIR

or the change to gitweb config in gitweb_init()

> > Simplest solution is to use $cgi->self_url() (note that what J.H. v8
> > uses, i.e.: "$my_url?". $ENV{'QUERY_STRING'}, where $my_url is just
> > $cgi->url() is not enough - it doesn't take path_info into account).
> >
> > Alternate solution, which I used in my rewrite, is to come up with
> > "canonical" URL, e.g. href(-replay => 1, -full => 1, -path_info => 0);
> > with this solution using path_info vs query parameters or reordering
> > query parameters still gives the same key.
> 
> It is easy to miss dependencies on parts of the URL that are being
> fuzzed out.  For example, the <base href...> tag is only inserted with
> path_info.  Maybe it would be less risky to first use self_url(), then
> canonicalize it in a separate patch?

You are right; it is only in the latest version of my rewrite that
I remembered that <base href=...> must be added also when caching is
enabled (c.f. using always "text/html" content type when caching is
enabled).
 
> > J.H. patches up and including v7, and my rewrite up and including v6,
> > excluded error pages from caching.  I think that the original resoning
> > behind choosing to do it this way was that A.), each of specific error
> > pages is usually accessed only once, so caching them would only take up
> > space bloating cache, but what is more important B.) that you can't
> > cache errors from caching engine.
> 
> Perhaps there is a user experience reason?  If I receive an error page
> due to a problem with my repository, all else being equal, I would
> prefer that the next time I reload it is fixed.  By comparison, having
> to reload multiple times to forget an obsolete non-error response
> would be less aggravating and perhaps expected.

True, there are errors that are transient (e.g. load too high), there are
errors that need webmaster / admin intervention (e.g. no disk space, or
cache permissions problem), and there are errors that are not fixable
(e.g. page not found).
 
> But the benefit from caching e.g. a response from a broken link would
> outweigh that.

The problem is _only_ if there is progress info indicator for set.

The issue you mentioned might be solved if we are able to set cache
expiration time individually (perhaps with the hint from within gitweb),
e.g. by using the fact that we cache HTTP response which can include
'Expires:' or 'Cache-Control: max-age=' header... or by manipulating
mtime.

> > Second is if there is no stale data to serve (or data is too stale), but
> > we have progress indicator.  In this case the foreground process is
> > responsible for rendering progress indicator, and background process is
> > responsible for generating data.  In this case foreground process waits
> > for data to be generated (unless progress info subroutine exits), so
> > strictly spaking we don't need to detach background process in this
> > case.
> 
> What happens when the client gets tired of waiting and closes the
> connection?

Hmmm... I don't know what happens to worker in non-persistent (CGI),
wrapped persistent (mod_perl via ModPerl::Registry, PSGI via gitweb.psgi)
and persistent (FastCGI) environments.  Well, detaching also in the case
of background generation because of progress_info wouldn't hurt, and
could help...


Thanks again for your comments.

Hoping to hear from J,H. soon...
-- 
Jakub Narebski
Poland

^ 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