Git development
 help / color / mirror / Atom feed
* [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces
From: Tarmigan Casebolt @ 2009-12-30 18:59 UTC (permalink / raw)
  To: git; +Cc: spearce, gitster, Tarmigan Casebolt
In-Reply-To: <905315640912301009x491f957al839f66de7aba56ed@mail.gmail.com>

This should introduce no functional change in the tests or the amount
of test coverage.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
This is rebased on top of today's pu and the conflicts should be fixed.

 t/t5560-http-backend.sh          |  148 +-------------------------------------
 t/t5561-http-backend-noserver.sh |   52 +++++++++++++
 t/t556x_common                   |  119 ++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+), 147 deletions(-)
 create mode 100755 t/t5561-http-backend-noserver.sh
 create mode 100755 t/t556x_common

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 04a9896..dd844d3 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -12,16 +12,6 @@ LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5560'}
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-find_file() {
-	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	find $1 -type f |
-	sed -e 1q
-}
-
-config() {
-	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
-}
-
 GET() {
 	curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
 	tr '\015' Q <out |
@@ -52,143 +42,7 @@ log_div() {
 	echo "###" >>"$HTTPD_ROOT_PATH"/access.log
 }
 
-test_expect_success 'setup repository' '
-	echo content >file &&
-	git add file &&
-	git commit -m one &&
-
-	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git --bare init &&
-	 : >objects/info/alternates &&
-	 : >objects/info/http-alternates
-	) &&
-	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	git push public master:master &&
-
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git repack -a -d
-	) &&
-
-	echo other >file &&
-	git add file &&
-	git commit -m two &&
-	git push public master:master &&
-
-	LOOSE_URL=$(find_file objects/??) &&
-	PACK_URL=$(find_file objects/pack/*.pack) &&
-	IDX_URL=$(find_file objects/pack/*.idx)
-'
-
-get_static_files() {
-	GET HEAD "$1" &&
-	GET info/refs "$1" &&
-	GET objects/info/packs "$1" &&
-	GET objects/info/alternates "$1" &&
-	GET objects/info/http-alternates "$1" &&
-	GET $LOOSE_URL "$1" &&
-	GET $PACK_URL "$1" &&
-	GET $IDX_URL "$1"
-}
-
-SMART=smart
-test_expect_success 'direct refs/heads/master not found' '
-	log_div "refs/heads/master"
-	GET refs/heads/master "404 Not Found"
-'
-test_expect_success 'static file is ok' '
-	log_div "getanyfile default"
-	get_static_files "200 OK"
-'
-SMART=smart_noexport
-test_expect_success 'no export by default' '
-	log_div "no git-daemon-export-ok"
-	get_static_files "404 Not Found"
-'
-test_expect_success 'export if git-daemon-export-ok' '
-	log_div "git-daemon-export-ok"
-        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 touch git-daemon-export-ok
-	) &&
-        get_static_files "200 OK"
-'
-SMART=smart
-test_expect_success 'static file if http.getanyfile true is ok' '
-	log_div "getanyfile true"
-	config http.getanyfile true &&
-	get_static_files "200 OK"
-'
-test_expect_success 'static file if http.getanyfile false fails' '
-	log_div "getanyfile false"
-	config http.getanyfile false &&
-	get_static_files "403 Forbidden"
-'
-
-test_expect_success 'http.uploadpack default enabled' '
-	log_div "uploadpack default"
-	GET info/refs?service=git-upload-pack "200 OK"  &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack true' '
-	log_div "uploadpack true"
-	config http.uploadpack true &&
-	GET info/refs?service=git-upload-pack "200 OK" &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack false' '
-	log_div "uploadpack false"
-	config http.uploadpack false &&
-	GET info/refs?service=git-upload-pack "403 Forbidden" &&
-	POST git-upload-pack 0000 "403 Forbidden"
-'
-
-test_expect_success 'http.receivepack default disabled' '
-	log_div "receivepack default"
-	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-test_expect_success 'http.receivepack true' '
-	log_div "receivepack true"
-	config http.receivepack true &&
-	GET info/refs?service=git-receive-pack "200 OK" &&
-	POST git-receive-pack 0000 "200 OK"
-'
-test_expect_success 'http.receivepack false' '
-	log_div "receivepack false"
-	config http.receivepack false &&
-	GET info/refs?service=git-receive-pack "403 Forbidden" &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
-	git http-backend >act.out 2>act.err
-}
-
-path_info() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		test_must_fail run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
-test_expect_success 'http-backend blocks bad PATH_INFO' '
-	config http.getanyfile true &&
-
-	run_backend 0 /repo.git/HEAD &&
-
-	run_backend 1 /repo.git/../HEAD &&
-	run_backend 1 /../etc/passwd &&
-	run_backend 1 ../etc/passwd &&
-	run_backend 1 /etc//passwd &&
-	run_backend 1 /etc/./passwd &&
-	run_backend 1 /etc/.../passwd &&
-	run_backend 1 //domain/data.txt
-'
+. "$TEST_DIRECTORY"/t556x_common
 
 cat >exp <<EOF
 
diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
new file mode 100755
index 0000000..501b328
--- /dev/null
+++ b/t/t5561-http-backend-noserver.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='test git-http-backend-noserver'
+. ./test-lib.sh
+
+HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
+
+GET() {
+    return 0
+}
+
+POST() {
+    return 0
+}
+
+logdiv() {
+    return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+run_backend() {
+	REQUEST_METHOD=GET \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="$2" \
+	git http-backend >act.out 2>act.err
+}
+
+path_info() {
+	if test $1 = 0; then
+		run_backend "$2"
+	else
+		test_must_fail run_backend "$2" &&
+		echo "fatal: '$2': aliased" >exp.err &&
+		test_cmp exp.err act.err
+	fi
+}
+
+test_expect_success 'http-backend blocks bad PATH_INFO' '
+	config http.getanyfile true &&
+
+	run_backend 0 /repo.git/HEAD &&
+
+	run_backend 1 /repo.git/../HEAD &&
+	run_backend 1 /../etc/passwd &&
+	run_backend 1 ../etc/passwd &&
+	run_backend 1 /etc//passwd &&
+	run_backend 1 /etc/./passwd &&
+	run_backend 1 /etc/.../passwd &&
+	run_backend 1 //domain/data.txt
+'
+test_done
diff --git a/t/t556x_common b/t/t556x_common
new file mode 100755
index 0000000..1b4921c
--- /dev/null
+++ b/t/t556x_common
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+find_file() {
+	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	find $1 -type f |
+	sed -e 1q
+}
+
+config() {
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
+}
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 : >objects/info/alternates &&
+	 : >objects/info/http-alternates
+	) &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master &&
+
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git repack -a -d
+	) &&
+
+	echo other >file &&
+	git add file &&
+	git commit -m two &&
+	git push public master:master &&
+
+	LOOSE_URL=$(find_file objects/??) &&
+	PACK_URL=$(find_file objects/pack/*.pack) &&
+	IDX_URL=$(find_file objects/pack/*.idx)
+'
+
+get_static_files() {
+	GET HEAD "$1" &&
+	GET info/refs "$1" &&
+	GET objects/info/packs "$1" &&
+	GET objects/info/alternates "$1" &&
+	GET objects/info/http-alternates "$1" &&
+	GET $LOOSE_URL "$1" &&
+	GET $PACK_URL "$1" &&
+	GET $IDX_URL "$1"
+}
+
+SMART=smart
+test_expect_success 'direct refs/heads/master not found' '
+	log_div "refs/heads/master"
+	GET refs/heads/master "404 Not Found"
+'
+test_expect_success 'static file is ok' '
+	log_div "getanyfile default"
+	get_static_files "200 OK"
+'
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+	log_div "no git-daemon-export-ok"
+	get_static_files "404 Not Found"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+	log_div "git-daemon-export-ok"
+        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 touch git-daemon-export-ok
+	) &&
+        get_static_files "200 OK"
+'
+SMART=smart
+test_expect_success 'static file if http.getanyfile true is ok' '
+	log_div "getanyfile true"
+	config http.getanyfile true &&
+	get_static_files "200 OK"
+'
+test_expect_success 'static file if http.getanyfile false fails' '
+	log_div "getanyfile false"
+	config http.getanyfile false &&
+	get_static_files "403 Forbidden"
+'
+
+test_expect_success 'http.uploadpack default enabled' '
+	log_div "uploadpack default"
+	GET info/refs?service=git-upload-pack "200 OK"  &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack true' '
+	log_div "uploadpack true"
+	config http.uploadpack true &&
+	GET info/refs?service=git-upload-pack "200 OK" &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack false' '
+	log_div "uploadpack false"
+	config http.uploadpack false &&
+	GET info/refs?service=git-upload-pack "403 Forbidden" &&
+	POST git-upload-pack 0000 "403 Forbidden"
+'
+
+test_expect_success 'http.receivepack default disabled' '
+	log_div "receivepack default"
+	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
+test_expect_success 'http.receivepack true' '
+	log_div "receivepack true"
+	config http.receivepack true &&
+	GET info/refs?service=git-receive-pack "200 OK" &&
+	POST git-receive-pack 0000 "200 OK"
+'
+test_expect_success 'http.receivepack false' '
+	log_div "receivepack false"
+	config http.receivepack false &&
+	GET info/refs?service=git-receive-pack "403 Forbidden" &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
-- 
1.6.6

^ permalink raw reply related

* [PATCH RFC 2/2] Smart-http tests: Test http-backend without curl or a webserver
From: Tarmigan Casebolt @ 2009-12-30 18:59 UTC (permalink / raw)
  To: git; +Cc: spearce, gitster, Tarmigan Casebolt
In-Reply-To: <1262199542-73876-1-git-send-email-tarmigan+git@gmail.com>

This reuses many of the tests from t5560 but runs those tests without curl
or a webserver.  This will hopefully increase the testing coverage for
http-backend because it does not require users to set GIT_TEST_HTTPD.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
In addition to the rebase, I fixed a bunch of bugs in this version.  I'm
still leaving this as RFC as there may be some more.

 t/t5561-http-backend-noserver.sh |   49 ++++++++++++++++++++++---------------
 t/t556x_common                   |    3 ++
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
index 501b328..bda5029 100755
--- a/t/t5561-http-backend-noserver.sh
+++ b/t/t5561-http-backend-noserver.sh
@@ -5,40 +5,49 @@ test_description='test git-http-backend-noserver'
 
 HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
 
+run_backend() {
+        echo "$3"| \
+        QUERY_STRING="${2#*\?}" \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="${2%%\?*}" \
+	git http-backend >act.out 2>act.err
+}
+
 GET() {
-    return 0
+    REQUEST_METHOD="GET" \
+    run_backend 0 "/repo.git/$1" &&
+    grep "Status" act.out >act
+    if [ $? -eq 1 ];
+    then
+	printf "Status: 200 OK\r\n" > act
+    fi
+    printf "Status: $2\r\n" > exp &&
+    test_cmp exp act
 }
 
 POST() {
-    return 0
+    REQUEST_METHOD="POST" \
+    CONTENT_TYPE="application/x-$1-request" \
+    run_backend 0 "/repo.git/$1" "$2" &&
+    grep "Status" act.out >act
+    if [ $? -eq 1 ];
+    then
+	printf "Status: 200 OK\r\n" > act
+    fi
+    printf "Status: $3\r\n" > exp &&
+    test_cmp exp act
 }
 
-logdiv() {
+log_div() {
     return 0
 }
 
 . "$TEST_DIRECTORY"/t556x_common
 
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
-	git http-backend >act.out 2>act.err
-}
-
-path_info() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		test_must_fail run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
 test_expect_success 'http-backend blocks bad PATH_INFO' '
 	config http.getanyfile true &&
 
+	REQUEST_METHOD="GET" &&
 	run_backend 0 /repo.git/HEAD &&
 
 	run_backend 1 /repo.git/../HEAD &&
diff --git a/t/t556x_common b/t/t556x_common
index 1b4921c..be024e5 100755
--- a/t/t556x_common
+++ b/t/t556x_common
@@ -50,6 +50,7 @@ get_static_files() {
 }
 
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'direct refs/heads/master not found' '
 	log_div "refs/heads/master"
 	GET refs/heads/master "404 Not Found"
@@ -59,6 +60,7 @@ test_expect_success 'static file is ok' '
 	get_static_files "200 OK"
 '
 SMART=smart_noexport
+unset GIT_HTTP_EXPORT_ALL
 test_expect_success 'no export by default' '
 	log_div "no git-daemon-export-ok"
 	get_static_files "404 Not Found"
@@ -71,6 +73,7 @@ test_expect_success 'export if git-daemon-export-ok' '
         get_static_files "200 OK"
 '
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'static file if http.getanyfile true is ok' '
 	log_div "getanyfile true"
 	config http.getanyfile true &&
-- 
1.6.6

^ permalink raw reply related

* Re: [PATCH] bash completion: add space between branch name and status flags
From: Junio C Hamano @ 2009-12-30 19:27 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Nanako Shiraishi, git, Roman Fietze
In-Reply-To: <20091230145751.GE6914@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Nanako Shiraishi <nanako3@lavabit.com> wrote:
>> Junio, could you tell us what happened to this thread?
>> 
>> In response to a patch from Roman Fietze to outline a better way
>> to do it.  Nothing happened.
>
> Junio responded with a suggestion on how to improve the patch when
> GIT_PS1_SHOWDIRTYSTATE was not set, but Roman Fietze didn't send
> a revised patch, so it got dropped.
>
> Here is the revised patch, Junio, still think its a good idea?

Thanks for following up.

As I don't use $[wisu] myself, I don't have a very strong opinion either
way, but the user has spent cycles to compute them, so we'd better present
them in a way that is easier to read, I guess.

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index fbfa5f2..3c8b6df 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -163,10 +163,12 @@ __git_ps1 ()
>  			fi
>  		fi
>  
> +		local f="$w$i$s$u"
> +		f="${f:+ $f}$r"
>  		if [ -n "${1-}" ]; then
> -			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
> +			printf "$1" "$c${b##refs/heads/}$f"
>  		else
> -			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
> +			printf " (%s)" "$c${b##refs/heads/}$f"
>  		fi

I notice that printf argument look very similar.  Maybe we want to do
something like

    printf "${1:-" (%s)"}" ...

to avoid duplication?

^ permalink raw reply

* [PATCH/resend] CVS Server: Support reading base and roots from  environment
From: Phil Miller @ 2009-12-30 19:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Nanako Shiraishi
In-Reply-To: <7vocmwvmvr.fsf@alter.siamese.dyndns.org>

The Gitosis single-account Git/ssh hosting system runs git commands
through git-shell after confirming that the connecting user is
authorized to access the requested repository. This works well for
upload-pack and receive-pack, which take a repository argument through
git-shell. This doesn't work so well for `cvs server', which is passed
through literally, with no arguments. Allowing arguments risks
sneaking in `--export-all', so that restriction should be maintained.

Despite that, passing a repository root is necessary for per-user
access control by the hosting software, and passing a base path
improves usability without weakening security. Thus, git-cvsserver
needs to come up with these values at runtime by some other
means. Since git-shell preserves the environment for other purposes,
the environment can carry these arguments as well.

Thus, modify git-cvsserver to read $GIT_CVSSERVER_{BASE_PATH,ROOT} in
the absence of equivalent command line arguments.

Signed-off-by: Phil Miller <mille121@illinois.edu>
---
I believe this revision addresses all of your comments on the first submission.

Your comment about cramming multiple values into one environment variable made
me realize that more than one simply was unnecessary complexity, since gitosis
expects to authenticate access to a single repository anyway.

I've not documented what GIT_CVSSERVER_BASE_PATH is relative to, because it
behaves identically to the --base-path command line argument. Documenting
what that is relative to is a separate issue.

 Documentation/git-cvsserver.txt |   15 +++++++++++++++
 git-cvsserver.perl              |   22 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 99a7c14..fbab295 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -277,6 +277,21 @@ In `dbdriver` and `dbuser` you can use the following variables:
 	If no name can be determined, the
 	numeric uid is used.
 
+ENVIRONMENT
+-----------
+
+These variables obviate the need for command-line options in some
+circumstances, allowing easier restricted usage through git-shell.
+
+GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.
+
+GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The
+repository must still be configured to allow access through
+git-cvsserver, as described above.
+
+When these environment variables are set, the corresponding
+command-line arguments may not be used.
+
 Eclipse CVS Client Notes
 ------------------------
 
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 6dc45f5..f5b57b9 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -104,6 +104,7 @@ $log->info("--------------- STARTING -----------------");
 my $usage =
     "Usage: git cvsserver [options] [pserver|server] [<directory> ...]\n".
     "    --base-path <path>  : Prepend to requested CVSROOT\n".
+    "                          Can be read from GIT_CVSSERVER_BASE_PATH\n".
     "    --strict-paths      : Don't allow recursing into subdirectories\n".
     "    --export-all        : Don't check for gitcvs.enabled in config\n".
     "    --version, -V       : Print version information and exit\n".
@@ -111,7 +112,8 @@ my $usage =
     "\n".
     "<directory> ... is a list of allowed directories. If no directories\n".
     "are given, all are allowed. This is an additional restriction, gitcvs\n".
-    "access still needs to be enabled by the gitcvs.enabled config option.\n";
+    "access still needs to be enabled by the gitcvs.enabled config option.\n".
+    "Alternately, one directory may be specified in GIT_CVSSERVER_ROOT.\n";
 
 my @opts = ( 'help|h|H', 'version|V',
 	     'base-path=s', 'strict-paths', 'export-all' );
@@ -148,6 +150,24 @@ if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {
     die "--export-all can only be used together with an explicit whitelist\n";
 }
 
+# Environment handling for running under git-shell
+if (exists $ENV{GIT_CVSSERVER_BASE_PATH}) {
+    if ($state->{'base-path'}) {
+	die "Cannot specify base path both ways.\n";
+    }
+    my $base_path = $ENV{GIT_CVSSERVER_BASE_PATH};
+    $state->{'base-path'} = $base_path;
+    $log->debug("Picked up base path '$base_path' from environment.\n");
+}
+if (exists $ENV{GIT_CVSSERVER_ROOT}) {
+    if (@{$state->{allowed_roots}}) {
+	die "Cannot specify roots both ways: @ARGV\n";
+    }
+    my $allowed_root = $ENV{GIT_CVSSERVER_ROOT};
+    $state->{allowed_roots} = [ $allowed_root ];
+    $log->debug("Picked up allowed root '$allowed_root' from environment.\n");
+}
+
 # if we are called with a pserver argument,
 # deal with the authentication cat before entering the
 # main loop
-- 
debian.1.6.6_rc2.1.7.gc3ed7

^ permalink raw reply related

* Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line
From: Junio C Hamano @ 2009-12-30 19:46 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Sebastian Thiel, git
In-Reply-To: <20091230224122.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?

I didn't feel I had enough energy to read the commit log message after
seeing it was badly linewrapped and didn't have a sign-off, so I didn't
read it.

I've read it now; it is unclear from the proposed commit log message how
this fits in the larger picture.  Presumably this change is meant to be
useful when driving update-index through --stdin?  To see if I got the
intention right, let me try paraphrasing it...

    update-index: flush standard output after each action is reported

    A scripted Porcelain that runs "git update-index --stdin" might want
    to use a bidirectional pipe, while feeding one path at a time and
    reading the output from report() every time after feeding a path.

    Such a Porcelain would deadlock if the standard output is not flushed
    after report().

I don't know if the above is what Sebastian meant, though..

An obvious question, when phrased this way, is "what impact does this
change have for scripted Porcelains that don't use bi-di pipe?"  I think
the answer would be "The I/O overhead for flushing would increase", but I
don't know if it would be "... would increase but it is still negligible"
or "... would increase too much to make it noticeably or unusably slow
especially if it feeds hundreds of paths".  If it is the latter, this may
need to be controlled by another command line option.

Sebastian, care to redo the justification, make it a bit more readable,
and add your sign-off?

Thanks.

^ permalink raw reply

* Re: [PATCH v3] builtin-push: add --delete as syntactic sugar for :foo
From: Junio C Hamano @ 2009-12-30 19:49 UTC (permalink / raw)
  To: Jan Krüger; +Cc: Nanako Shiraishi, Git ML, Sverre Rabbelier
In-Reply-To: <20091230105244.67f5969e@perceptron>

Jan Krüger <jk@jk.gs> writes:

> Refspecs without a source side have been reported as confusing by many.
> As an alternative, this adds support for commands like:
>
>     git push origin --delete somebranch
>     git push origin --delete tag sometag
>
> Specifically, --delete will prepend a colon to all colon-less refspecs
> given on the command line, and will refuse to accept refspecs with
> colons to prevent undue confusion.
>
> Signed-off-by: Jan Krüger <jk@jk.gs>
> ---

Thanks.  From a cursory read, the patch looks good.  We would however want
to have a test that has test_must_fail to protect the error codepath from
getting broken in the future.

^ permalink raw reply

* Re: [PATCH/resend] CVS Server: Support reading base and roots from  environment
From: Junio C Hamano @ 2009-12-30 19:49 UTC (permalink / raw)
  To: Phil Miller; +Cc: Nanako Shiraishi, Git Mailing List
In-Reply-To: <81f018ac0912300912g25887523g1bdf29fd6e31d011@mail.gmail.com>

Phil Miller <mille121@illinois.edu> writes:

> I never resent with modifications as Junio requested. I've been busy
> with some end-of-year close-out stuff, and actually wanted to
> dramatically simplify part of the patch before the resend. I'll do
> that shortly.
> ...
> The documentation hasn't been updated, but the recent history makes as
> least some usage clear. I'll send more details in a follow-up.

Thanks Phil for status updates (and thanks Nana for prodding).

^ permalink raw reply

* Re: [PATCH v2] Let core.excludesfile default to ~/.gitexcludes.
From: Junio C Hamano @ 2009-12-30 19:49 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Nanako Shiraishi, git
In-Reply-To: <vpqhbr8ttwv.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> Junio, could you tell us what happened to this thread?
>
> I'm not Junio, but I can try ...
> ...
> So, I dropped the patch until we get closer to a consensus on what the
> default value should be.

Thanks for status updates.  Should we start trying to reach a consensus on
how to move forward, or is it not your itch?

It is not _too_ bad to treat ~/.gitconfig specially and support reading
from ~/.$SOMEGITTTYNAME/{excludes,attributes} files.  We can also add
support for ~/.$SOMEGITTYNAME/config and try reading from it as well, but
even if we did so I don't think we should stop reading from ~/.gitconfig.

I don't have strong preference myself either way.

^ permalink raw reply

* [PATCH v4] builtin-push: add --delete as syntactic sugar for :foo
From: Jan Krüger @ 2009-12-30 19:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Git ML, Sverre Rabbelier
In-Reply-To: <7vvdfoz1hi.fsf@alter.siamese.dyndns.org>

Refspecs without a source side have been reported as confusing by many.
As an alternative, this adds support for commands like:

    git push origin --delete somebranch
    git push origin --delete tag sometag

Specifically, --delete will prepend a colon to all colon-less refspecs
given on the command line, and will refuse to accept refspecs with
colons to prevent undue confusion.

Signed-off-by: Jan Krüger <jk@jk.gs>
---
 Documentation/git-push.txt |    4 ++++
 builtin-push.c             |   26 +++++++++++++++++++++++---
 t/t5516-fetch-push.sh      |   26 ++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 3 deletions(-)

Junio C Hamano <gitster@pobox.com> wrote:

> Thanks.  From a cursory read, the patch looks good.  We would however
> want to have a test that has test_must_fail to protect the error
> codepath from getting broken in the future.

Here you are.

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 52c0538..e3eb1e8 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -91,6 +91,10 @@ nor in any Push line of the corresponding remotes file---see below).
 	will be tab-separated and sent to stdout instead of stderr.  The full
 	symbolic names of the refs will be given.
 
+--delete::
+	All listed refs are deleted from the remote repository. This is
+	the same as prefixing all refs with a colon.
+
 --tags::
 	All refs under `$GIT_DIR/refs/tags` are pushed, in
 	addition to refspecs explicitly listed on the command
diff --git a/builtin-push.c b/builtin-push.c
index dcfb53f..f7661d2 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -15,6 +15,7 @@ static const char * const push_usage[] = {
 };
 
 static int thin;
+static int deleterefs;
 static const char *receivepack;
 
 static const char **refspec;
@@ -39,11 +40,24 @@ static void set_refspecs(const char **refs, int nr)
 			if (nr <= ++i)
 				die("tag shorthand without <tag>");
 			len = strlen(refs[i]) + 11;
-			tag = xmalloc(len);
-			strcpy(tag, "refs/tags/");
+			if (deleterefs) {
+				tag = xmalloc(len+1);
+				strcpy(tag, ":refs/tags/");
+			} else {
+				tag = xmalloc(len);
+				strcpy(tag, "refs/tags/");
+			}
 			strcat(tag, refs[i]);
 			ref = tag;
-		}
+		} else if (deleterefs && !strchr(ref, ':')) {
+			char *delref;
+			int len = strlen(ref)+1;
+			delref = xmalloc(len);
+			strcpy(delref, ":");
+			strcat(delref, ref);
+			ref = delref;
+		} else if (deleterefs)
+			die("--delete only accepts plain target ref names");
 		add_refspec(ref);
 	}
 }
@@ -196,6 +210,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
+		OPT_BOOLEAN( 0, "delete", &deleterefs, "delete refs"),
 		OPT_BOOLEAN( 0 , "tags", &tags, "push tags (can't be used with --all or --mirror)"),
 		OPT_BIT('n' , "dry-run", &flags, "dry run", TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, "machine-readable output", TRANSPORT_PUSH_PORCELAIN),
@@ -209,6 +224,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
+	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
+		die("--delete is incompatible with --all, --mirror and --tags");
+	if (deleterefs && argc < 2)
+		die("--delete doesn't make sense without any refs");
+
 	if (tags)
 		add_refspec("refs/tags/*");
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 516127b..0f04b2e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -547,6 +547,32 @@ test_expect_success 'allow deleting an invalid remote ref' '
 
 '
 
+test_expect_success 'allow deleting a ref using --delete' '
+	mk_test heads/master &&
+	(cd testrepo && git config receive.denyDeleteCurrent warn) &&
+	git push testrepo --delete master &&
+	(cd testrepo && test_must_fail git rev-parse --verify refs/heads/master)
+'
+
+test_expect_success 'allow deleting a tag using --delete' '
+	mk_test heads/master &&
+	git tag -a -m dummy_message deltag heads/master &&
+	git push testrepo --tags &&
+	(cd testrepo && git rev-parse --verify -q refs/tags/deltag) &&
+	git push testrepo --delete tag deltag &&
+	(cd testrepo && test_must_fail git rev-parse --verify refs/tags/deltag)
+'
+
+test_expect_success 'push --delete without args aborts' '
+	mk_test heads/master &&
+	test_must_fail git push testrepo --delete
+'
+
+test_expect_success 'push --delete refuses src:dest refspecs' '
+	mk_test heads/master &&
+	test_must_fail git push testrepo --delete master:foo
+'
+
 test_expect_success 'warn on push to HEAD of non-bare repository' '
 	mk_test heads/master
 	(cd testrepo &&
-- 
1.6.6.60.gc2ff1

^ permalink raw reply related

* Re: [PATCH/resend] CVS Server: Support reading base and roots from  environment
From: Phil Miller @ 2009-12-30 20:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Git Mailing List
In-Reply-To: <7vpr5wz1hb.fsf@alter.siamese.dyndns.org>

The Gitosis single-account Git/ssh hosting system runs git commands
through git-shell after confirming that the connecting user is
authorized to access the requested repository. This works well for
upload-pack and receive-pack, which take a repository argument through
git-shell. This doesn't work so well for `cvs server', which is passed
through literally, with no arguments. Allowing arguments risks
sneaking in `--export-all', so that restriction should be maintained.

Despite that, passing a repository root is necessary for per-user
access control by the hosting software, and passing a base path
improves usability without weakening security. Thus, git-cvsserver
needs to come up with these values at runtime by some other
means. Since git-shell preserves the environment for other purposes,
the environment can carry these arguments as well.

Thus, modify git-cvsserver to read $GIT_CVSSERVER_{BASE_PATH,ROOT} in
the absence of equivalent command line arguments.

Signed-off-by: Phil Miller <mille121@illinois.edu>
---
I believe this revision addresses all of your comments on the first submission.

Your comment about cramming multiple values into one environment variable made
me realize that more than one simply was unnecessary complexity, since gitosis
expects to authenticate access to a single repository anyway.

I've not documented what GIT_CVSSERVER_BASE_PATH is relative to, because it
behaves identically to the --base-path command line argument. Documenting
what that is relative to is a separate issue.

 Documentation/git-cvsserver.txt |   15 +++++++++++++++
 git-cvsserver.perl              |   22 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 99a7c14..fbab295 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -277,6 +277,21 @@ In `dbdriver` and `dbuser` you can use the
following variables:
 	If no name can be determined, the
 	numeric uid is used.

+ENVIRONMENT
+-----------
+
+These variables obviate the need for command-line options in some
+circumstances, allowing easier restricted usage through git-shell.
+
+GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.
+
+GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The
+repository must still be configured to allow access through
+git-cvsserver, as described above.
+
+When these environment variables are set, the corresponding
+command-line arguments may not be used.
+
 Eclipse CVS Client Notes
 ------------------------

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 6dc45f5..f5b57b9 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -104,6 +104,7 @@ $log->info("--------------- STARTING -----------------");
 my $usage =
     "Usage: git cvsserver [options] [pserver|server] [<directory> ...]\n".
     "    --base-path <path>  : Prepend to requested CVSROOT\n".
+    "                          Can be read from GIT_CVSSERVER_BASE_PATH\n".
     "    --strict-paths      : Don't allow recursing into subdirectories\n".
     "    --export-all        : Don't check for gitcvs.enabled in config\n".
     "    --version, -V       : Print version information and exit\n".
@@ -111,7 +112,8 @@ my $usage =
     "\n".
     "<directory> ... is a list of allowed directories. If no directories\n".
     "are given, all are allowed. This is an additional restriction, gitcvs\n".
-    "access still needs to be enabled by the gitcvs.enabled config option.\n";
+    "access still needs to be enabled by the gitcvs.enabled config option.\n".
+    "Alternately, one directory may be specified in GIT_CVSSERVER_ROOT.\n";

 my @opts = ( 'help|h|H', 'version|V',
 	     'base-path=s', 'strict-paths', 'export-all' );
@@ -148,6 +150,24 @@ if ($state->{'export-all'} &&
!@{$state->{allowed_roots}}) {
     die "--export-all can only be used together with an explicit whitelist\n";
 }

+# Environment handling for running under git-shell
+if (exists $ENV{GIT_CVSSERVER_BASE_PATH}) {
+    if ($state->{'base-path'}) {
+	die "Cannot specify base path both ways.\n";
+    }
+    my $base_path = $ENV{GIT_CVSSERVER_BASE_PATH};
+    $state->{'base-path'} = $base_path;
+    $log->debug("Picked up base path '$base_path' from environment.\n");
+}
+if (exists $ENV{GIT_CVSSERVER_ROOT}) {
+    if (@{$state->{allowed_roots}}) {
+	die "Cannot specify roots both ways: @ARGV\n";
+    }
+    my $allowed_root = $ENV{GIT_CVSSERVER_ROOT};
+    $state->{allowed_roots} = [ $allowed_root ];
+    $log->debug("Picked up allowed root '$allowed_root' from environment.\n");
+}
+
 # if we are called with a pserver argument,
 # deal with the authentication cat before entering the
 # main loop
-- 
debian.1.6.6_rc2.1.7.gc3ed7

^ permalink raw reply related

* Re: Question about 'branch -d' safety
From: Nicolas Sebrecht @ 2009-12-30 21:08 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Nicolas Sebrecht, git
In-Reply-To: <20091230121238.6117@nanako3.lavabit.com>

The 30/12/09, Nanako Shiraishi wrote:
> Quoting Nicolas Sebrecht <nicolas.s.dev@gmx.fr>
> 
> > But even with it, we would hit some foreign workflow. Think: Bob
> > directly push to Alice and Alice does the same to Bob. I don't use this
> > kind of workflow myself but I consider them to be sensible enough to
> > have our attention.
> 
> Here is what I think about your scenario.
> 
> Bob directly pushes to Alice and Alice does the same to Bob, both to their refs/remotes/<other person>/ tracking branches

We can't say. They both may have refs/remotes/<same_id> .

Bob:
  $ git branch -d a_branch

Now, Bob has the "I don't want to loose" commit known in
refs/remotes/<same_id> only.

Alice, some time later:
  $ git push -f <same_id> <foo>:<a_branch>

Bob "lose" his commit.


I admit it is a very uncommon use case and Bob can still use the reflog.

-- 
Nicolas Sebrecht

^ permalink raw reply

* __git_ps1 not showing submodule dirty state
From: Kevin Ballard @ 2009-12-30 21:46 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

Why does the __git_ps1 function in git-completion.bash explicitly ignore submodules when showing the GIT_PS1_SHOWDIRTYSTATE status? The most common issue with my current repository is not realizing when submodules need to be updated because I blindly trust my prompt to tell me when I have dirty state.

-Kevin Ballard

^ permalink raw reply

* Re: Need some help with git rebase
From: Sylvain Rabot @ 2009-12-30 22:24 UTC (permalink / raw)
  To: git
In-Reply-To: <4B38B3A7.6010900@steek.com>

So,

I tried again "git rebase --onto 12.72.1 master feature".

Situation :

                         a--a--a--a--a--a--a--a--a feature
                        /
--x--x--x--x--x--x--x--x--x--x--x--x master
   \
    o--o--o--o--o--o--o 12.72.1


Work flow :

$ git checkout 12.72.1
$ git rebase --onto 12.72.1 master feature

a lot of conflicts, resolving and git rebase --continue

And then, at the end of the rebase, without me doing anything, feature
branch is checked out and it seems that its HEAD has been reset to the
new 12.72.1 HEAD.

--x--x--x--x--x--x--x--x--x--x--x--x master
   \
    o--o--o--o--o--o--o--a--a--a--a--a--a--a--a--a 12.72.1 feature

Is that normal ? If it is, how do I do to avoid my feature branch to be
reset at 12.72.1's HEAD ?

Any enlightenment is very welcome.

Regards.

-- 
Sylvain Rabot <sylvain@abstraction.fr>

^ permalink raw reply

* Re: Need some help with git rebase
From: Wincent Colaiuta @ 2009-12-30 22:46 UTC (permalink / raw)
  To: sylvain; +Cc: git
In-Reply-To: <1262211866.7068.1.camel@kheops>

El 30/12/2009, a las 23:24, Sylvain Rabot escribió:

> So,
>
> I tried again "git rebase --onto 12.72.1 master feature".
>
> Situation :
>
>                         a--a--a--a--a--a--a--a--a feature
>                        /
> --x--x--x--x--x--x--x--x--x--x--x--x master
>   \
>    o--o--o--o--o--o--o 12.72.1
>
>
> Work flow :
>
> $ git checkout 12.72.1
> $ git rebase --onto 12.72.1 master feature
>
> a lot of conflicts, resolving and git rebase --continue
>
> And then, at the end of the rebase, without me doing anything, feature
> branch is checked out and it seems that its HEAD has been reset to the
> new 12.72.1 HEAD.
>
> --x--x--x--x--x--x--x--x--x--x--x--x master
>   \
>    o--o--o--o--o--o--o--a--a--a--a--a--a--a--a--a 12.72.1 feature
>
> Is that normal ? If it is, how do I do to avoid my feature branch to  
> be
> reset at 12.72.1's HEAD ?
>
> Any enlightenment is very welcome.

Look at the "git-rebase" man page, particularly the order of the  
arguments, what they mean, and the usage examples of "--onto":

   $ git rebase --onto 12.72.1 master feature

Means, "replay these changes on top of 12.72.1", where "these changes"  
refers to commits on branch "feature" with upstream "master", which is  
what "git rebase" did for you.

If you actually want the "feature" branch to continue pointing at the  
old feature branch rather than your newly rebased one, you could just  
look up the old SHA1 for it and update it with:

   $ git branch -f feature abcd1234

Where "abcd1234" is the hash of the old "feature" HEAD.

But I don't really know why you'd want to do that. The purpose of "git  
rebase" isn't to copy or cherry-pick commits from one branch onto  
another, but to actually _move_ (or transplant, or replay, if you  
prefer) those commits.

Maybe I misunderstood your intentions though.

Cheers,
Wincent

^ permalink raw reply

* Re: __git_ps1 not showing submodule dirty state
From: Johan Herland @ 2009-12-31  1:40 UTC (permalink / raw)
  To: git; +Cc: Kevin Ballard, Shawn O. Pearce, trast
In-Reply-To: <A98B75F4-B9BD-4CB4-9335-754C59ECB64F@sb.org>

On Wednesday 30 December 2009, Kevin Ballard wrote:
> Why does the __git_ps1 function in git-completion.bash explicitly ignore
>  submodules when showing the GIT_PS1_SHOWDIRTYSTATE status? The most
>  common issue with my current repository is not realizing when submodules
>  need to be updated because I blindly trust my prompt to tell me when I
>  have dirty state.

According to git blame, it has been there since GIT_PS1_SHOWDIRTYSTATE was 
introduced in 738a94a... by Thomas Rast (CCed), but the commit message does 
not say why submodules are explicitly ignored.

FWIW, I agree with Kevin, and would like changed submodules to be included 
in the status.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [PATCH v2] Smart-http documentation: add example of how to  execute from userdir
From: Tay Ray Chuan @ 2009-12-31  2:18 UTC (permalink / raw)
  To: Tarmigan Casebolt; +Cc: Junio C Hamano, git, Shawn O. Pearce
In-Reply-To: <1261975612-67101-1-git-send-email-tarmigan+git@gmail.com>

Hi,

On Mon, Dec 28, 2009 at 12:46 PM, Tarmigan Casebolt
<tarmigan+git@gmail.com> wrote:
> Looking at the
>        s/update/updates/
> suggestion again, I decided not to make that change because I think
> the original is grammical even if a little bit awkward.  The first
> 'updates' is the noun, so the verb should be 'update' without an 's'.

Indeed.

> I also tried rephrasing that sentence completely, but did not end up
> with something better. ?Suggestions welcome.

My attempt is found below.

> +In the following example, a repository at

I was thinking s/a repository at/the repository/.

> +'/home/$username/devel/foo/bar.git' will be accessible at
> +'http://$hostname/\~$username/cgi-bin/git/foo/bar.git'

You left out a full-stop for this sentence.

> +From UserDir on Apache 2.x::
> +       One way to configure 'git-http-backend' to execute and serve
> +       from a user directory (for example, on a shared hosting
> +       provider), is to have a symbolic link named 'git' that points
> +       from the cgi directory to the 'git-http-backend' executable in
> +       libexec.  The advantage of the symbolic link is that any updates
> +       to the installed version of 'git-http-backend' also update the
> +       version that is run in the userdir.

We could try

  The advantage is that this symbolic link always points to the latest
  installed version; one does not have to make any changes to the
  symbolic link when the installed 'git-http-backend' is updated.

I thought the corollary (after the ';') was a bit long-winded, but it
explains the advantage more clearly.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* [PATCH] bash completion: add space between branch name and status flags
From: Shawn O. Pearce @ 2009-12-31  3:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git, Roman Fietze
In-Reply-To: <7veimc1cv6.fsf@alter.siamese.dyndns.org>

Improve the readability of the bash prompt by adding a space between
the branch name and the status flags (dirty, stash, untracked).

While we are cleaning up this section of code, the two cases for
formatting the prompt are identical except for the format string,
so make them the same.

Suggested-by: Roman Fietze <roman.fietze@telemotive.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
   Junio C Hamano <gitster@pobox.com> wrote:
   > I notice that printf argument look very similar.  Maybe we want to do
   > something like
   > 
   >     printf "${1:-" (%s)"}" ...
   > 
   > to avoid duplication?
   
   Ack.

   Because its rather far from the original poster's patch, I've
   taken blame for it.
   
 contrib/completion/git-completion.bash |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fbfa5f2..9ed7df2 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -163,11 +163,9 @@ __git_ps1 ()
 			fi
 		fi
 
-		if [ -n "${1-}" ]; then
-			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
-		else
-			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
-		fi
+		local f="$w$i$s$u"
+		f="${f:+ $f}$r"
+		printf "${1:- (%s)}" "$c${b##refs/heads/}$f"
 	fi
 }
 
-- 
1.6.6.325.g6f5f

-- 
Shawn.

^ permalink raw reply related

* Re: Need some help with git rebase
From: Tay Ray Chuan @ 2009-12-31  5:01 UTC (permalink / raw)
  To: sylvain; +Cc: git
In-Reply-To: <1262211866.7068.1.camel@kheops>

Hi,

On Thu, Dec 31, 2009 at 6:24 AM, Sylvain Rabot <sylvain@abstraction.fr> wrote:
> And then, at the end of the rebase, without me doing anything, feature
> branch is checked out and it seems that its HEAD has been reset to the
> new 12.72.1 HEAD.
>
> --x--x--x--x--x--x--x--x--x--x--x--x master
>   \
>    o--o--o--o--o--o--o--a--a--a--a--a--a--a--a--a 12.72.1 feature
>
> Is that normal ? If it is, how do I do to avoid my feature branch to be
> reset at 12.72.1's HEAD ?

reading the manual, I think this is normal.

According to the third paragraph of the description of git-rebase, the
branch feature will be reset to 12.72.1.

You can just rename the branch back to 12.72.1, and retrieve the old
tip of feature by examining the reflog.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [Updated PATCH 1/2] Report exec errors from run-command
From: Tarmigan @ 2009-12-31  5:26 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1262170338-11574-2-git-send-email-ilari.liusvaara@elisanet.fi>

On Wed, Dec 30, 2009 at 5:52 AM, Ilari Liusvaara
<ilari.liusvaara@elisanet.fi> wrote:
> Previously run-command was unable to report errors happening in exec
> call. Change it to pass errno from failed exec to errno value at
> return.
>
> The errno value passing can be done by opening close-on-exec pipe and
> piping the error code through in case of failure. In case of success,
> close-on-exec closes the pipe on successful exec and parent process
> gets end of file on read.

I was testing pu and 'git diff' and 'git log' would hang forever.

Bisecting pointed to v1 of this patch.  But seeing that v2 was out, I
tried v2 of the patch, but the issue remains.

Tried on OSX and linux with the same results.

Here's a gdb backtrace on OSX at the point where I interrupted it:
(gdb) bt
#0  0x9923dbfe in read$UNIX2003 ()
#1  0x000b399b in start_command (cmd=0x10b300) at run-command.c:110
#2  0x00099452 in setup_pager () at pager.c:94
#3  0x0002196b in cmd_diff (argc=1, argv=0xbffff42c, prefix=0x0) at
builtin-diff.c:316
#4  0x00002a2a in run_builtin [inlined] () at git.c:257
#5  0x00002a2a in handle_internal_command (argc=1, argv=0xbffff42c) at git.c:401
#6  0x00002cab in main (argc=1, argv=0xbffff42c) at git.c:443

and on linux:
(gdb) bt
#0  0x0000003530a0d590 in __read_nocancel () from /lib64/libpthread.so.0
#1  0x0000000000494858 in start_command (cmd=0x71ed60) at run-command.c:93
#2  0x000000000047daf8 in setup_pager () at pager.c:94
#3  0x000000000041d83f in cmd_diff (argc=1, argv=0x7fff256f9fe0, prefix=0x0)
    at builtin-diff.c:316
#4  0x0000000000403ced in handle_internal_command (argc=1, argv=0x7fff256f9fe0)
    at git.c:257
#5  0x0000000000403f26 in main (argc=1, argv=0x7fff256f9fe0) at git.c:445

Thanks,
Tarmigan

^ permalink raw reply

* Re: [PATCH v2] cvsserver: make the output of 'update' more compatible with cvs.
From: Junio C Hamano @ 2009-12-31  6:47 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Sergei Organov, git
In-Reply-To: <20091230224115.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?

Well, since I don't use cvsserver myself, but this v2 was done with
improvements based on some review suggestions, I was waiting for a
response or two from people who know better and care more about cvs server
emulation than me, which unfortunately didn't happen.

Thanks for reminding; I can queue it to 'pu' or perhaps 'next' and see if
anybody screams by getting hit by an unintended side effects.

^ permalink raw reply

* Re: [PATCH] t9700-perl-git.sh: Fix a test failure on Cygwin
From: Junio C Hamano @ 2009-12-31  6:49 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Ramsay Jones, GIT Mailing-list
In-Reply-To: <20091230224023.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?

Hmph, I think we have it as 81f4026 (t9700-perl-git.sh: Fix a test failure
on Cygwin, 2009-11-19).

^ permalink raw reply

* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2009-12-31  7:01 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1262182304-19911-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Skip-worktree entries are not on disk. There is no reason to call
> external grep in such cases.
>
> A trace message is also added to aid debugging external grep cases.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin-grep.c |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-grep.c b/builtin-grep.c
> index d582232..d49c637 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -488,17 +488,34 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
>  	read_cache();
>  
>  #if !NO_EXTERNAL_GREP
> +	if (cached)
> +		external_grep_allowed = 0;
> +	if (external_grep_allowed) {
> +		for (nr = 0; nr < active_nr; nr++) {
> +			struct cache_entry *ce = active_cache[nr];
> +			if (!S_ISREG(ce->ce_mode))
> +				continue;
> +			if (!pathspec_matches(paths, ce->name, opt->max_depth))
> +				continue;
> +			if (ce_skip_worktree(ce)) {
> +				external_grep_allowed = 0;
> +				break;
> +			}
> +		}
> +	}
>  	/*
>  	 * Use the external "grep" command for the case where
>  	 * we grep through the checked-out files. It tends to
>  	 * be a lot more optimized
>  	 */
> -	if (!cached && external_grep_allowed) {
> +	if (external_grep_allowed) {
>  		hit = external_grep(opt, paths, cached);
>  		if (hit >= 0)
>  			return hit;
>  		hit = 0;
>  	}

This looks a bit wrong for a couple of reasons:

 - external_grep() is designed to return negative without running external
   grep when it shouldn't be used (see the beginning of the function for
   how it refuses to run when opt->extended is set and other conditions).
   The new logic seems to belong there, i.e. "in addition to the existing
   case we decline, if ce_skip_worktree() entry exists in the cache, we
   decline";

 - It should be a separate helper function, has_skip_worktree_entry(paths);
   I wouldn't be surprised if there are some other codepaths that want to
   check for the same condition and do things differently, not just grep.

Originally I thought S_ISREG() check and pathspec_matches() check were
also questionable, but they actually are good things to have, as we skip
symbolic links (or submodules) and we do want to use external grep if the
subtree we are grepping in do not have skip_worktree entries even when
some other parts of the index are marked as skip_worktree.

> +	else
> +		trace_printf("grep: external grep not used\n");
>  #endif
>  
>  	for (nr = 0; nr < active_nr; nr++) {
> -- 
> 1.6.6.315.g1a406

^ permalink raw reply

* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2009-12-31  7:09 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <7v637nzky0.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> This looks a bit wrong for a couple of reasons:
>
>  - external_grep() is designed to return negative without running external
>    grep when it shouldn't be used (see the beginning of the function for
>    how it refuses to run when opt->extended is set and other conditions).
>    The new logic seems to belong there, i.e. "in addition to the existing
>    case we decline, if ce_skip_worktree() entry exists in the cache, we
>    decline";

IOW, something like this instead of your patch.  You would want to tests
to demonstrate the original breakage, perhaps?

 builtin-grep.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 813fe97..25ee75d 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -357,6 +357,21 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq)
 		strbuf_setlen(sb, sb->len - 1);
 }
 
+static int has_skip_worktree_entry(struct grep_opt *opt, const char **paths)
+{
+	int nr;
+	for (nr = 0; nr < active_nr; nr++) {
+		struct cache_entry *ce = active_cache[nr];
+		if (!S_ISREG(ce->ce_mode))
+			continue;
+		if (!pathspec_matches(paths, ce->name, opt->max_depth))
+			continue;
+		if (ce_skip_worktree(ce))
+			return 1;
+	}
+	return 0;
+}
+
 static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 {
 	int i, nr, argc, hit, len, status;
@@ -365,7 +380,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 	char *argptr = randarg;
 	struct grep_pat *p;
 
-	if (opt->extended || (opt->relative && opt->prefix_length))
+	if (opt->extended || (opt->relative && opt->prefix_length)
+	    || has_skip_worktree_entry(opt, paths))
 		return -1;
 	len = nr = 0;
 	push_arg("grep");

^ permalink raw reply related

* Re: [PATCH] gitk: Use git-difftool for external diffs
From: David Aguilar @ 2009-12-31  7:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nanako Shiraishi, peff, sam, git, paulus, Markus Heidelberg,
	Jay Soffian
In-Reply-To: <7vy6kk52an.fsf@alter.siamese.dyndns.org>

On Tue, Dec 29, 2009 at 11:49:52PM -0800, Junio C Hamano wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
> > Junio, could you tell us what happened to this thread?
> 
> See http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133414
> 
> In short, the particular way to call difftool this patch implements was
> found to be inadequate to support existing external diff support by gitk
> and a small difftool update will happen first, followed by a patch to gitk
> to use the updated difftool, to avoid regression.


I started the first step:

http://thread.gmane.org/gmane.comp.version-control.git/135613
http://thread.gmane.org/gmane.comp.version-control.git/135613/focus=135612

The 2nd patch implements the the --gui option which Markus
pointed out would be needed to avoid issues such as calling
"vimdiff" from a console-less gitk:

http://article.gmane.org/gmane.comp.version-control.git/133386


I marked the --gui patch as "RFC" since it introduced a new
config variable and I want to make sure that we agreed on its
name.  I didn't get any feedback about that patch
(my fault-- we were in RC freeze and I forgot to CC: Markus).

If that looks like a good first step then we can do the next
step which would be to introduce the --extcmd= option as
mentioned here:

http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133386

I will try and prepare the changes for --extcmd= within
the next week assuming the existing --gui patch is ok.


On a related note, Jay Soffian recently submitted a
git-mergetool--lib patch adding support for "diffmerge".
It made it clear that there were parts of git-mergetool--lib
that could use some refactoring:

http://thread.gmane.org/gmane.comp.version-control.git/134906

Jay did mention that he'd give it a shot at the time, though
it does seems like the refactoring could wait until we see
how --extcmd= fits into the world.


Thank you for following up on this thread, Nanako.

-- 
		David

^ permalink raw reply

* Re: Need some help with git rebase
From: Sylvain Rabot @ 2009-12-31  9:02 UTC (permalink / raw)
  To: git
In-Reply-To: <CB5B49CA-0C66-4384-9B47-3675E517E203@wincent.com>

On Wed, Dec 30, 2009 at 23:46, Wincent Colaiuta <win@wincent.com> wrote:
>
> Look at the "git-rebase" man page, particularly the order of the arguments, what they mean, and the usage examples of "--onto":
>
>  $ git rebase --onto 12.72.1 master feature
>
> Means, "replay these changes on top of 12.72.1", where "these changes" refers to commits on branch "feature" with upstream "master", which is what "git rebase" did for you.
>
> If you actually want the "feature" branch to continue pointing at the old feature branch rather than your newly rebased one, you could just look up the old SHA1 for it and update it with:
>
>  $ git branch -f feature abcd1234
>
> Where "abcd1234" is the hash of the old "feature" HEAD.
>
> But I don't really know why you'd want to do that. The purpose of "git rebase" isn't to copy or cherry-pick commits from one branch onto another, but to actually _move_ (or transplant, or replay, if you prefer) those commits.
>
> Maybe I misunderstood your intentions though.
>
> Cheers,
> Wincent
>

In fact I want to backport the commits of the feature branch into 12.72.1.
I used git rebase because the drawings of the man page looked like
that I wanted to do and it does except for the part it resets the head
of my feature branch.

But the good behavior would be to cherry pick each commit of the
feature branch and apply them into 12.72.1, right ?

Thanks for your answer.

Regards.

--
Sylvain

^ 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