Re: [PATCH] http-backend: respect GIT_NAMESPACE with dumb clients

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 28, 2013, 14:43 UTC
Message-ID: <7v38vfy48e.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1364446721-13351-1-git-send-email-jkoleszar@google.com>

John Koleszar <jkoleszar@google.com> writes:

Show 29 quoted lines

> diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
> index b5d7fbc..5a19d61 100755
> --- a/t/t5561-http-backend.sh
> +++ b/t/t5561-http-backend.sh
> @@ -134,6 +134,10 @@ POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
>  ###
>  GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
>  POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
> +
> +###  namespace test
> +###
> +GET  /smart_namespace/repo.git/info/refs HTTP/1.1 200
>  EOF
>  test_expect_success 'server request log matches test results' '
>  	sed -e "
> diff --git a/t/t556x_common b/t/t556x_common
> index 82926cf..cb9eb9d 100755
> --- a/t/t556x_common
> +++ b/t/t556x_common
> @@ -120,3 +120,19 @@ test_expect_success 'http.receivepack false' '
>  	GET info/refs?service=git-receive-pack "403 Forbidden" &&
>  	POST git-receive-pack 0000 "403 Forbidden"
>  '
> +test_expect_success 'backend respects namespaces' '
> +	log_div "namespace test"
> +	config http.uploadpack true &&
> +	config http.getanyfile true &&
> +
> +	GIT_NAMESPACE=ns && export GIT_NAMESPACE &&

When other people want to enhance this test suite later, their tests may not want the namespace contaminated with the environment variable. You would need to enclose from here to the end inside a subshell or something.

Show 7 quoted lines

> +	git push public master:master &&
> +	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> +		git for-each-ref | grep /$GIT_NAMESPACE/ >/dev/null
> +	) &&
> +
> +	git ls-remote public >exp &&  
> +	curl "$HTTPD_URL/smart_namespace/repo.git/info/refs" >act &&

Spell out "expect" and "actual".

For some unknwon reason, I am getting an HTTPD_URL at this point, causing it to fail with:

    curl: (3) <url> malformed

> +	test_cmp exp act &&
> +	(grep /ns/ exp && false || true)

What does that last line even mean?  Both

	false && false || true
        true && false || true

will yield true.  Leftover from your debugging session?