* [PATCH v4 3/3] Smart-http tests: Test http-backend without curl or a webserver
From: Tarmigan Casebolt @ 2010-01-02 21:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Tarmigan Casebolt
In-Reply-To: <20100102214107.GO6914@spearce.org>
This reuses many of the tests from the old 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>
---
Oops forgot to commit the grep changes.
Only resending 3/3 of the series.
---
t/t5560-http-backend-noserver.sh | 26 ++++++++++++++++++++------
t/t556x_common | 3 +++
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index a9ba2d9..a63d3ec 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -6,18 +6,32 @@ test_description='test git-http-backend-noserver'
HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
run_backend() {
- REQUEST_METHOD=GET \
+ echo "$2"| \
+ QUERY_STRING="${1#*\?}" \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
- PATH_INFO="$1" \
+ PATH_INFO="${1%%\?*}" \
git http-backend >act.out 2>act.err
}
GET() {
- return 0
+ REQUEST_METHOD="GET" \
+ run_backend "/repo.git/$1" &&
+ if ! grep "Status" act.out >act; 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 "/repo.git/$1" "$2" &&
+ if ! grep "Status" act.out >act; then
+ printf "Status: 200 OK\r\n" > act
+ fi
+ printf "Status: $3\r\n" > exp &&
+ test_cmp exp act
}
log_div() {
@@ -28,9 +42,9 @@ log_div() {
expect_aliased() {
if test $1 = 0; then
- run_backend "$2"
+ REQUEST_METHOD=GET run_backend "$2"
else
- run_backend "$2" &&
+ REQUEST_METHOD=GET run_backend "$2" &&
echo "fatal: '$2': aliased" >exp.err &&
test_cmp exp.err act.err
fi
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.236.gc56f3
^ permalink raw reply related
* Re: [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver
From: Shawn O. Pearce @ 2010-01-02 21:41 UTC (permalink / raw)
To: Tarmigan; +Cc: Junio C Hamano, git
In-Reply-To: <905315641001021337v238515c8s34d820236dac698d@mail.gmail.com>
Tarmigan <tarmigan+git@gmail.com> wrote:
> On Sat, Jan 2, 2010 at 1:03 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> >> ?GET() {
> >> - ? ? return 0
> >> + ? ? REQUEST_METHOD="GET" \
> >> + ? ? run_backend "/repo.git/$1" &&
> >> + ? ? grep "Status" act.out >act
> >> + ? ? if [ $? -eq 1 ];
> >
> > I think this should be spelled as:
> >
> > ? ? ? ?if ! grep "Status" act.out >act; then
> > ? ? ? ? ? ? ? ?printf "Status: 200 OK\r\n" > act
> > ? ? ? ?fi
>
> OK. I had figured the "1" would mean "no matches not found" rather
> than some other error like "file does not exist". Not sure how
> portable that error value convention is for greps though, so I've
> changed it as you suggested.
Your v3 series still shows it the old way...
--
Shawn.
^ permalink raw reply
* [PATCH v4 3/3] Smart-http tests: Test http-backend without curl or a webserver
From: Tarmigan Casebolt @ 2010-01-02 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Tarmigan Casebolt
In-Reply-To: <20100102205928.GM6914@spearce.org>
This reuses many of the tests from the old 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>
---
As a side note, I am very impressed that git rebase was able to
apply this patch properly even after I renamed the tests earlier in
the series.
---
t/t5560-http-backend-noserver.sh | 30 ++++++++++++++++++++++++------
t/t556x_common | 3 +++
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index a9ba2d9..9013824 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -6,18 +6,36 @@ test_description='test git-http-backend-noserver'
HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
run_backend() {
- REQUEST_METHOD=GET \
+ echo "$2"| \
+ QUERY_STRING="${1#*\?}" \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
- PATH_INFO="$1" \
+ PATH_INFO="${1%%\?*}" \
git http-backend >act.out 2>act.err
}
GET() {
- return 0
+ REQUEST_METHOD="GET" \
+ run_backend "/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 "/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
}
log_div() {
@@ -28,9 +46,9 @@ log_div() {
expect_aliased() {
if test $1 = 0; then
- run_backend "$2"
+ REQUEST_METHOD=GET run_backend "$2"
else
- run_backend "$2" &&
+ REQUEST_METHOD=GET run_backend "$2" &&
echo "fatal: '$2': aliased" >exp.err &&
test_cmp exp.err act.err
fi
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.236.gc56f3
^ permalink raw reply related
* [PATCH v4 2/3] Smart-http tests: Break test t5560-http-backend into pieces
From: Tarmigan Casebolt @ 2010-01-02 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Tarmigan Casebolt
In-Reply-To: <20100102205928.GM6914@spearce.org>
This should introduce no functional change in the tests or the amount
of test coverage.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
t/t5560-http-backend-noserver.sh | 52 +++++++
t/t5560-http-backend.sh | 294 --------------------------------------
t/t5561-http-backend.sh | 149 +++++++++++++++++++
t/t556x_common | 119 +++++++++++++++
4 files changed, 320 insertions(+), 294 deletions(-)
create mode 100755 t/t5560-http-backend-noserver.sh
delete mode 100755 t/t5560-http-backend.sh
create mode 100755 t/t5561-http-backend.sh
create mode 100755 t/t556x_common
diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
new file mode 100755
index 0000000..a9ba2d9
--- /dev/null
+++ b/t/t5560-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"
+
+run_backend() {
+ REQUEST_METHOD=GET \
+ GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+ PATH_INFO="$1" \
+ git http-backend >act.out 2>act.err
+}
+
+GET() {
+ return 0
+}
+
+POST() {
+ return 0
+}
+
+log_div() {
+ return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+expect_aliased() {
+ if test $1 = 0; then
+ run_backend "$2"
+ else
+ 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 &&
+
+ expect_aliased 0 /repo.git/HEAD &&
+
+ expect_aliased 1 /repo.git/../HEAD &&
+ expect_aliased 1 /../etc/passwd &&
+ expect_aliased 1 ../etc/passwd &&
+ expect_aliased 1 /etc//passwd &&
+ expect_aliased 1 /etc/./passwd &&
+ expect_aliased 1 //domain/data.txt
+'
+
+test_done
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
deleted file mode 100755
index 0e4dc4b..0000000
--- a/t/t5560-http-backend.sh
+++ /dev/null
@@ -1,294 +0,0 @@
-#!/bin/sh
-
-test_description='test git-http-backend'
-. ./test-lib.sh
-
-if test -n "$NO_CURL"; then
- say 'skipping test, git built without http support'
- test_done
-fi
-
-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 |
- sed '
- s/Q$//
- 1q
- ' >act &&
- echo "HTTP/1.1 $2" >exp &&
- test_cmp exp act
-}
-
-POST() {
- curl --include --data "$2" \
- --header "Content-Type: application/x-$1-request" \
- "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
- tr '\015' Q <out |
- sed '
- s/Q$//
- 1q
- ' >act &&
- echo "HTTP/1.1 $3" >exp &&
- test_cmp exp act
-}
-
-log_div() {
- echo >>"$HTTPD_ROOT_PATH"/access.log
- echo "### $1" >>"$HTTPD_ROOT_PATH"/access.log
- 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="$1" \
- git http-backend >act.out 2>act.err
-}
-
-expect_aliased() {
- if test $1 = 0; then
- run_backend "$2"
- else
- 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 &&
-
- expect_aliased 0 /repo.git/HEAD &&
-
- expect_aliased 1 /repo.git/../HEAD &&
- expect_aliased 1 /../etc/passwd &&
- expect_aliased 1 ../etc/passwd &&
- expect_aliased 1 /etc//passwd &&
- expect_aliased 1 /etc/./passwd &&
- expect_aliased 1 //domain/data.txt
-'
-
-cat >exp <<EOF
-
-### refs/heads/master
-###
-GET /smart/repo.git/refs/heads/master HTTP/1.1 404 -
-
-### getanyfile default
-###
-GET /smart/repo.git/HEAD HTTP/1.1 200
-GET /smart/repo.git/info/refs HTTP/1.1 200
-GET /smart/repo.git/objects/info/packs HTTP/1.1 200
-GET /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
-GET /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
-GET /smart/repo.git/$LOOSE_URL HTTP/1.1 200
-GET /smart/repo.git/$PACK_URL HTTP/1.1 200
-GET /smart/repo.git/$IDX_URL HTTP/1.1 200
-
-### no git-daemon-export-ok
-###
-GET /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
-GET /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
-GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
-GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 -
-GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 -
-GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 -
-GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 -
-GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 -
-
-### git-daemon-export-ok
-###
-GET /smart_noexport/repo.git/HEAD HTTP/1.1 200
-GET /smart_noexport/repo.git/info/refs HTTP/1.1 200
-GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
-GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
-GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
-GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
-GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
-GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
-
-### getanyfile true
-###
-GET /smart/repo.git/HEAD HTTP/1.1 200
-GET /smart/repo.git/info/refs HTTP/1.1 200
-GET /smart/repo.git/objects/info/packs HTTP/1.1 200
-GET /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
-GET /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
-GET /smart/repo.git/$LOOSE_URL HTTP/1.1 200
-GET /smart/repo.git/$PACK_URL HTTP/1.1 200
-GET /smart/repo.git/$IDX_URL HTTP/1.1 200
-
-### getanyfile false
-###
-GET /smart/repo.git/HEAD HTTP/1.1 403 -
-GET /smart/repo.git/info/refs HTTP/1.1 403 -
-GET /smart/repo.git/objects/info/packs HTTP/1.1 403 -
-GET /smart/repo.git/objects/info/alternates HTTP/1.1 403 -
-GET /smart/repo.git/objects/info/http-alternates HTTP/1.1 403 -
-GET /smart/repo.git/$LOOSE_URL HTTP/1.1 403 -
-GET /smart/repo.git/$PACK_URL HTTP/1.1 403 -
-GET /smart/repo.git/$IDX_URL HTTP/1.1 403 -
-
-### uploadpack default
-###
-GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
-POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
-
-### uploadpack true
-###
-GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
-POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
-
-### uploadpack false
-###
-GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 403 -
-POST /smart/repo.git/git-upload-pack HTTP/1.1 403 -
-
-### receivepack default
-###
-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 -
-
-### receivepack true
-###
-GET /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
-POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
-
-### receivepack false
-###
-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 -
-EOF
-test_expect_success 'server request log matches test results' '
- sed -e "
- s/^.* \"//
- s/\"//
- s/ [1-9][0-9]*\$//
- s/^GET /GET /
- " >act <"$HTTPD_ROOT_PATH"/access.log &&
- test_cmp exp act
-'
-
-stop_httpd
-test_done
diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
new file mode 100755
index 0000000..8c6d0b2
--- /dev/null
+++ b/t/t5561-http-backend.sh
@@ -0,0 +1,149 @@
+#!/bin/sh
+
+test_description='test git-http-backend'
+. ./test-lib.sh
+
+if test -n "$NO_CURL"; then
+ say 'skipping test, git built without http support'
+ test_done
+fi
+
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5561'}
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+GET() {
+ curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
+ tr '\015' Q <out |
+ sed '
+ s/Q$//
+ 1q
+ ' >act &&
+ echo "HTTP/1.1 $2" >exp &&
+ test_cmp exp act
+}
+
+POST() {
+ curl --include --data "$2" \
+ --header "Content-Type: application/x-$1-request" \
+ "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
+ tr '\015' Q <out |
+ sed '
+ s/Q$//
+ 1q
+ ' >act &&
+ echo "HTTP/1.1 $3" >exp &&
+ test_cmp exp act
+}
+
+log_div() {
+ echo >>"$HTTPD_ROOT_PATH"/access.log
+ echo "### $1" >>"$HTTPD_ROOT_PATH"/access.log
+ echo "###" >>"$HTTPD_ROOT_PATH"/access.log
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+cat >exp <<EOF
+
+### refs/heads/master
+###
+GET /smart/repo.git/refs/heads/master HTTP/1.1 404 -
+
+### getanyfile default
+###
+GET /smart/repo.git/HEAD HTTP/1.1 200
+GET /smart/repo.git/info/refs HTTP/1.1 200
+GET /smart/repo.git/objects/info/packs HTTP/1.1 200
+GET /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET /smart/repo.git/$LOOSE_URL HTTP/1.1 200
+GET /smart/repo.git/$PACK_URL HTTP/1.1 200
+GET /smart/repo.git/$IDX_URL HTTP/1.1 200
+
+### no git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 -
+
+### git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 200
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
+
+### getanyfile true
+###
+GET /smart/repo.git/HEAD HTTP/1.1 200
+GET /smart/repo.git/info/refs HTTP/1.1 200
+GET /smart/repo.git/objects/info/packs HTTP/1.1 200
+GET /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET /smart/repo.git/$LOOSE_URL HTTP/1.1 200
+GET /smart/repo.git/$PACK_URL HTTP/1.1 200
+GET /smart/repo.git/$IDX_URL HTTP/1.1 200
+
+### getanyfile false
+###
+GET /smart/repo.git/HEAD HTTP/1.1 403 -
+GET /smart/repo.git/info/refs HTTP/1.1 403 -
+GET /smart/repo.git/objects/info/packs HTTP/1.1 403 -
+GET /smart/repo.git/objects/info/alternates HTTP/1.1 403 -
+GET /smart/repo.git/objects/info/http-alternates HTTP/1.1 403 -
+GET /smart/repo.git/$LOOSE_URL HTTP/1.1 403 -
+GET /smart/repo.git/$PACK_URL HTTP/1.1 403 -
+GET /smart/repo.git/$IDX_URL HTTP/1.1 403 -
+
+### uploadpack default
+###
+GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
+POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
+
+### uploadpack true
+###
+GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
+POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
+
+### uploadpack false
+###
+GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 403 -
+POST /smart/repo.git/git-upload-pack HTTP/1.1 403 -
+
+### receivepack default
+###
+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 -
+
+### receivepack true
+###
+GET /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
+POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
+
+### receivepack false
+###
+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 -
+EOF
+test_expect_success 'server request log matches test results' '
+ sed -e "
+ s/^.* \"//
+ s/\"//
+ s/ [1-9][0-9]*\$//
+ s/^GET /GET /
+ " >act <"$HTTPD_ROOT_PATH"/access.log &&
+ test_cmp exp act
+'
+
+stop_httpd
+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.236.gc56f3
^ permalink raw reply related
* [PATCH v4 1/3] Smart-http tests: Improve coverage in test t5560
From: Tarmigan Casebolt @ 2010-01-02 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Tarmigan Casebolt
In-Reply-To: <20100102205928.GM6914@spearce.org>
Commit 34b6cb8bb ("http-backend: Protect GIT_PROJECT_ROOT from /../
requests") added the path_info helper function to test t5560 but did
not use it. We should use it as it provides another level of error
checking.
The /etc/.../passwd case is one that is not special (and the test
fails for reasons other than being aliased), so we remove that test
case.
Also rename the function from 'path_info' to 'expect_aliased'.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
One thing to note is that
expect_aliased 0 /repo.git/HEAD
test still does not actually test any results back from http-backend,
but that's also how it was before as well.
---
t/t5560-http-backend.sh | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 04a9896..0e4dc4b 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -162,15 +162,15 @@ test_expect_success 'http.receivepack false' '
run_backend() {
REQUEST_METHOD=GET \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
- PATH_INFO="$2" \
+ PATH_INFO="$1" \
git http-backend >act.out 2>act.err
}
-path_info() {
+expect_aliased() {
if test $1 = 0; then
run_backend "$2"
else
- test_must_fail run_backend "$2" &&
+ run_backend "$2" &&
echo "fatal: '$2': aliased" >exp.err &&
test_cmp exp.err act.err
fi
@@ -179,15 +179,14 @@ path_info() {
test_expect_success 'http-backend blocks bad PATH_INFO' '
config http.getanyfile true &&
- run_backend 0 /repo.git/HEAD &&
+ expect_aliased 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
+ expect_aliased 1 /repo.git/../HEAD &&
+ expect_aliased 1 /../etc/passwd &&
+ expect_aliased 1 ../etc/passwd &&
+ expect_aliased 1 /etc//passwd &&
+ expect_aliased 1 /etc/./passwd &&
+ expect_aliased 1 //domain/data.txt
'
cat >exp <<EOF
--
1.6.6.236.gc56f3
^ permalink raw reply related
* Re: [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver
From: Tarmigan @ 2010-01-02 21:37 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20100102210318.GN6914@spearce.org>
On Sat, Jan 2, 2010 at 1:03 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
>> GET() {
>> - return 0
>> + REQUEST_METHOD="GET" \
>> + run_backend "/repo.git/$1" &&
>> + grep "Status" act.out >act
>> + if [ $? -eq 1 ];
>
> I think this should be spelled as:
>
> if ! grep "Status" act.out >act; then
> printf "Status: 200 OK\r\n" > act
> fi
OK. I had figured the "1" would mean "no matches not found" rather
than some other error like "file does not exist". Not sure how
portable that error value convention is for greps though, so I've
changed it as you suggested.
Thanks,
Tarmigan
^ permalink raw reply
* [PATCH v3] Smart-http documentation: add example of how to execute from userdir
From: Tarmigan Casebolt @ 2010-01-02 21:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Tarmigan Casebolt, Tay Ray Chuan, Shawn O. Pearce
In-Reply-To: <be6fef0d0912301818o678976ebqa3e339dc1ff1f2e8@mail.gmail.com>
Smart-http may be an attactive and easy way for people to setup git
hosting on shared servers whose primary web server configuration they
do not control. To facilite this, provide an example of how it may be
done.
cc: Tay Ray Chuan <rctay89@gmail.com>
cc: Shawn O. Pearce <spearce@spearce.org>
Editing-by: Tay Ray Chuan <rctay89@gmail.com>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Documentation/git-http-backend.txt | 32 ++++++++++++++++++++++++++++++--
1 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt
index 67aec06..0485033 100644
--- a/Documentation/git-http-backend.txt
+++ b/Documentation/git-http-backend.txt
@@ -60,8 +60,8 @@ automatically by the web server.
EXAMPLES
--------
-All of the following examples map 'http://$hostname/git/foo/bar.git'
-to '/var/www/git/foo/bar.git'.
+Unless otherwise noted, the following examples map
+'http://$hostname/git/foo/bar.git' to '/var/www/git/foo/bar.git'.
Apache 2.x::
Ensure mod_cgi, mod_alias, and mod_env are enabled, set
@@ -144,6 +144,34 @@ ScriptAliasMatch \
ScriptAlias /git/ /var/www/cgi-bin/gitweb.cgi/
----------------------------------------------------------------
+In the following example, the repository at
+'/home/$username/devel/foo/bar.git' will be accessible at
+'http://$hostname/~$username/cgi-bin/git/foo/bar.git'.
+
+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 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.
++
+----------------------------------------------------------------
+SetHandler cgi-script
+Options +SymLinksIfOwnerMatch
+SetEnv GIT_PROJECT_ROOT /home/$username/devel
+----------------------------------------------------------------
++
+Also add any needed authentication to the .htaccess file as discussed
+in earlier examples.
++
+If the web server does not follow any symbolic links, the
+'git-http-backend' executable may be copied into the cgi-bin directory
+and renamed to 'git' to acheive the same effect, but it will also need
+to be manually updated whenever a new version of 'git-http-backend' is
+installed.
ENVIRONMENT
-----------
--
1.6.6.236.gc56f3
^ permalink raw reply related
* Re: [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver
From: Shawn O. Pearce @ 2010-01-02 21:03 UTC (permalink / raw)
To: Tarmigan Casebolt; +Cc: Junio C Hamano, git
In-Reply-To: <1262465119-96115-3-git-send-email-tarmigan+git@gmail.com>
Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> GET() {
> - return 0
> + REQUEST_METHOD="GET" \
> + run_backend "/repo.git/$1" &&
> + grep "Status" act.out >act
> + if [ $? -eq 1 ];
I think this should be spelled as:
if ! grep "Status" act.out >act; then
printf "Status: 200 OK\r\n" > act
fi
--
Shawn.
^ permalink raw reply
* Re: [PATCH v3 2/3] Smart-http tests: Break test t5560-http-backend into pieces
From: Shawn O. Pearce @ 2010-01-02 20:59 UTC (permalink / raw)
To: Tarmigan Casebolt; +Cc: Junio C Hamano, git
In-Reply-To: <1262465119-96115-2-git-send-email-tarmigan+git@gmail.com>
Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> This should introduce no functional change in the tests or the amount
> of test coverage.
>
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
> ---
> t/t5560-http-backend.sh | 147 +-------------------------------------
> t/t5561-http-backend-noserver.sh | 52 +++++++++++++
These should be reversed. We'd want to find out the backend doesn't
work by itself before we see it fail under Apache.
Otherwise, Acked-by: Shawn O. Pearce <spearce@spearce.org>
--
Shawn.
^ permalink raw reply
* Re: [PATCH v3 RFC 1/3] Smart-http tests: Improve coverage in test t5560
From: Shawn O. Pearce @ 2010-01-02 20:54 UTC (permalink / raw)
To: Tarmigan Casebolt; +Cc: Junio C Hamano, git
In-Reply-To: <1262465119-96115-1-git-send-email-tarmigan+git@gmail.com>
Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> Commit 34b6cb8bb ("http-backend: Protect GIT_PROJECT_ROOT from /../
> requests") added the path_info helper function to test t5560 but did
> not use it. We should use it as it provides another level of error
> checking.
>
> The /etc/.../passwd case is one that is not special (and the test
> fails for reasons other than being aliased), so we remove that test
> case.
>
> Also rename the function from 'path_info' to 'expect_aliased'.
>
> cc: Shawn O. Pearce <spearce@spearce.org>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
--
Shawn.
^ permalink raw reply
* Re: Filename quoting / parsing problem
From: Andreas Gruenbacher @ 2010-01-02 20:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wcge4kr.fsf@alter.siamese.dyndns.org>
On Saturday 02 January 2010 07:37:08 pm Junio C Hamano wrote:
> Andreas Gruenbacher <agruen@suse.de> writes:
> > On Friday 01 January 2010 09:01:19 pm Junio C Hamano wrote:
> >> > Both "b file" and "c file " are parsed by "git apply" perfectly fine.
> >
> > Right, the "diff --git" lines are technically still parseable when the
> > file name stays the same. With renames, lines like "diff --git a/f a/f
> > b/f" or "diff --git a/f b/f b/f" are possible, but then there will also
> > be "renamed from" and "renamed to" headers which will disambiguate
> > things. Still, it doesn't seem like a good idea to allow such
> > ambiguities in the first place.
>
> You already realized that there is no ambiguity because "diff --git" lines
> are parsable and renames have explicit names. Why do you still maintain
> that we are allowing such "ambiguities" when there is none?
Don't get so aroused ...
Right now, git generates lines like "diff --git a/f a/f b/f b/f" in some
corner cases, and from such lines alone, it is not possible to tell what the
two file names are (either "a/f a/f" and "b/f b/f", or "a/f a/f b/f" and
"b/f"). I can only find that out by looking at the other header lines.
I would prefer a format which I can parse line by line without ambiguities in
the first place, because this would keep things much simpler and easier to
debug. (Think of other implementations of the extended diff format which may
not produce the exact same output as git.)
So I would be happy with either of this:
* Also quote spaces in the "diff --git" line so that I can always reliably
parse it, or
* Add an additional extended header line with the file name in case there
are no other header lines giving the file names away already (as for
renames, copies, or when there are "---" and "+++" lines).
After our discussion so far, option two would probably be easier for everyone:
you could add it without risking to break anything, and I could avoid parsing
the "diff --git" line altogether.
> >> Having said all that, I don't think we would mind a change to treat a
> >> pathname with trailing SP a bit specially (iow, quoting "c file " in the
> >> above failed attempt to reproduce the issue).
> >
> > I would prefer quoting file names which contain spaces anywhere,...
>
> The only reason I said I don't think we would mind changing the trailing
> SP case is because the reduced risk of getting our patches corrupted by
> MUA _might_ outweigh the benefit of not quoting to avoid an eyesore [*1*].
>
> But what you said would add to eyesore of quoted names (which you omitted
> from your quote) without any justification other than "I would prefer".
> The pros-and-cons in such a change is quite different; as we have already
> established that there is no ambiguity, "disambuguation" is not a "pro" in
> this comparison.
>
> [Footnote]
>
> *1* Strictly speaking, it is not just "an eyesore" that is an issue. Our
> diff output without renames are designed to be grokkable by other people's
> patch implementations (e.g. GNU patch), and the quoted pathnames are not
> understandable by them.
GNU patch doesn't look at "diff --git" lines or extended header lines at all
so far, so there are no compatibility issues yet. Quoting spaces in "---" and
"+++" lines would lead to problems with current GNU patch though. (So does
the quoting of several other characters like ", of course.)
> Even though our final version of quoted path format came from the GNU
> diff/patch maintainer (back then, at least):
>
> http://article.gmane.org/gmane.comp.version-control.git/10103
>
> I don't think it happened in the GNU land yet, and you would be the person
> to know about it ;-).
I'm working on it ...
Thanks,
Andreas
^ permalink raw reply
* [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver
From: Tarmigan Casebolt @ 2010-01-02 20:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O . Pearce, Tarmigan Casebolt
In-Reply-To: <905315641001021244o522ac092g7fdaaab83a2609e2@mail.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>
---
t/t5561-http-backend-noserver.sh | 30 ++++++++++++++++++++++++------
t/t556x_common | 3 +++
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
index a9ba2d9..9013824 100755
--- a/t/t5561-http-backend-noserver.sh
+++ b/t/t5561-http-backend-noserver.sh
@@ -6,18 +6,36 @@ test_description='test git-http-backend-noserver'
HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
run_backend() {
- REQUEST_METHOD=GET \
+ echo "$2"| \
+ QUERY_STRING="${1#*\?}" \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
- PATH_INFO="$1" \
+ PATH_INFO="${1%%\?*}" \
git http-backend >act.out 2>act.err
}
GET() {
- return 0
+ REQUEST_METHOD="GET" \
+ run_backend "/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 "/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
}
log_div() {
@@ -28,9 +46,9 @@ log_div() {
expect_aliased() {
if test $1 = 0; then
- run_backend "$2"
+ REQUEST_METHOD=GET run_backend "$2"
else
- run_backend "$2" &&
+ REQUEST_METHOD=GET run_backend "$2" &&
echo "fatal: '$2': aliased" >exp.err &&
test_cmp exp.err act.err
fi
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
* [PATCH v3 2/3] Smart-http tests: Break test t5560-http-backend into pieces
From: Tarmigan Casebolt @ 2010-01-02 20:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O . Pearce, Tarmigan Casebolt
In-Reply-To: <905315641001021244o522ac092g7fdaaab83a2609e2@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>
---
t/t5560-http-backend.sh | 147 +-------------------------------------
t/t5561-http-backend-noserver.sh | 52 +++++++++++++
t/t556x_common | 119 ++++++++++++++++++++++++++++++
3 files changed, 172 insertions(+), 146 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 0e4dc4b..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,142 +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="$1" \
- git http-backend >act.out 2>act.err
-}
-
-expect_aliased() {
- if test $1 = 0; then
- run_backend "$2"
- else
- 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 &&
-
- expect_aliased 0 /repo.git/HEAD &&
-
- expect_aliased 1 /repo.git/../HEAD &&
- expect_aliased 1 /../etc/passwd &&
- expect_aliased 1 ../etc/passwd &&
- expect_aliased 1 /etc//passwd &&
- expect_aliased 1 /etc/./passwd &&
- expect_aliased 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..a9ba2d9
--- /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"
+
+run_backend() {
+ REQUEST_METHOD=GET \
+ GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+ PATH_INFO="$1" \
+ git http-backend >act.out 2>act.err
+}
+
+GET() {
+ return 0
+}
+
+POST() {
+ return 0
+}
+
+log_div() {
+ return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+expect_aliased() {
+ if test $1 = 0; then
+ run_backend "$2"
+ else
+ 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 &&
+
+ expect_aliased 0 /repo.git/HEAD &&
+
+ expect_aliased 1 /repo.git/../HEAD &&
+ expect_aliased 1 /../etc/passwd &&
+ expect_aliased 1 ../etc/passwd &&
+ expect_aliased 1 /etc//passwd &&
+ expect_aliased 1 /etc/./passwd &&
+ expect_aliased 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 v3 RFC 1/3] Smart-http tests: Improve coverage in test t5560
From: Tarmigan Casebolt @ 2010-01-02 20:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O . Pearce, Tarmigan Casebolt
In-Reply-To: <905315641001021244o522ac092g7fdaaab83a2609e2@mail.gmail.com>
Commit 34b6cb8bb ("http-backend: Protect GIT_PROJECT_ROOT from /../
requests") added the path_info helper function to test t5560 but did
not use it. We should use it as it provides another level of error
checking.
The /etc/.../passwd case is one that is not special (and the test
fails for reasons other than being aliased), so we remove that test
case.
Also rename the function from 'path_info' to 'expect_aliased'.
cc: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
One thing to note is that
expect_aliased 0 /repo.git/HEAD
test still does not actually test any results back from http-backend,
but that's also how it was before as well.
---
t/t5560-http-backend.sh | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 04a9896..0e4dc4b 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -162,15 +162,15 @@ test_expect_success 'http.receivepack false' '
run_backend() {
REQUEST_METHOD=GET \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
- PATH_INFO="$2" \
+ PATH_INFO="$1" \
git http-backend >act.out 2>act.err
}
-path_info() {
+expect_aliased() {
if test $1 = 0; then
run_backend "$2"
else
- test_must_fail run_backend "$2" &&
+ run_backend "$2" &&
echo "fatal: '$2': aliased" >exp.err &&
test_cmp exp.err act.err
fi
@@ -179,15 +179,14 @@ path_info() {
test_expect_success 'http-backend blocks bad PATH_INFO' '
config http.getanyfile true &&
- run_backend 0 /repo.git/HEAD &&
+ expect_aliased 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
+ expect_aliased 1 /repo.git/../HEAD &&
+ expect_aliased 1 /../etc/passwd &&
+ expect_aliased 1 ../etc/passwd &&
+ expect_aliased 1 /etc//passwd &&
+ expect_aliased 1 /etc/./passwd &&
+ expect_aliased 1 //domain/data.txt
'
cat >exp <<EOF
--
1.6.6
^ permalink raw reply related
* Re: [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces
From: Tarmigan @ 2010-01-02 20:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O . Pearce, git
In-Reply-To: <7viqbmo173.fsf@alter.siamese.dyndns.org>
On Thu, Dec 31, 2009 at 9:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tarmigan <tarmigan+git@gmail.com> writes:
>
>> One reason it's labeled RFC is that I'm not very confident in my
>> ability to write portable shell script. It works for me with bash,
>> but I'm not completely confident that is would work on ksh or dash.
>> So it would be nice if you could specifically take a look at the new
>> POST() and GET() and see if you notice anything obviously wrong there.
>
> Looked Ok to me from a cursory reading, even though I wonder what the
> first argument to run_backend function is good for...
Thanks for looking. I used the run_backend that was introduced in
34b6cb8bb, but looking more closely, it seems that the "http-backend
blocks bad PATH_INFO" test wasn't actually checking anything. It
seems the path_info function was intended to be used, but never
actually was. I'm embarrassed to say that I was so focused on "no
changes to existing tests" that I wasn't thinking about the bigger
picture.
I have made a patch to address this, and have made it the first in the
series. I would like to have Shawn's ack on at least that first
patch, as I was trying to guess at his original intention with that
test.
Thanks,
Tarmigan
^ permalink raw reply
* [PATCH] Use warning function instead of fprintf(stderr, "Warning: ...").
From: Thiago Farina @ 2010-01-02 20:24 UTC (permalink / raw)
To: git
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
bisect.c | 2 +-
builtin-mv.c | 4 +---
http.c | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/bisect.c b/bisect.c
index dc18db8..d1f8c42 100644
--- a/bisect.c
+++ b/bisect.c
@@ -813,7 +813,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)
char *bad_hex = sha1_to_hex(current_bad_sha1);
char *good_hex = join_sha1_array_hex(&good_revs, ' ');
- fprintf(stderr, "Warning: the merge base between %s and [%s] "
+ warning("the merge base between %s and [%s] "
"must be skipped.\n"
"So we cannot be sure the first bad commit is "
"between %s and %s.\n"
diff --git a/builtin-mv.c b/builtin-mv.c
index f633d81..8ad7245 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -169,9 +169,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
* check both source and destination
*/
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
- fprintf(stderr, "Warning: %s;"
- " will overwrite!\n",
- bad);
+ warning("%s; will overwrite!\n", bad);
bad = NULL;
} else
bad = "Cannot overwrite";
diff --git a/http.c b/http.c
index ed6414a..afce7c3 100644
--- a/http.c
+++ b/http.c
@@ -1244,7 +1244,7 @@ int finish_http_object_request(struct http_object_request *freq)
process_http_object_request(freq);
if (freq->http_code == 416) {
- fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n");
+ warning("requested range invalid; we may already have all the data.\n");
} else if (freq->curl_result != CURLE_OK) {
if (stat(freq->tmpfile, &st) == 0)
if (st.st_size == 0)
--
1.6.6.rc3
^ permalink raw reply related
* Re: [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"
From: Junio C Hamano @ 2010-01-02 19:50 UTC (permalink / raw)
To: Christian Couder
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20100102053934.30066.85625.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> The purpose of this new option is to discard some of the last commits
> but to keep current changes in the work tree.
>
> The use case is when you work on something and commit that work. And
> then you work on something else that touches other files, but you don't
> commit it yet. Then you realize that what you commited when you worked
> on the first thing is not good or belongs to another branch.
>
> So you want to get rid of the previous commits (at least in the current
> branch) but you want to make sure that you keep the changes you have in
> the work tree. And you are pretty sure that your changes are independent
> from what you previously commited, so you don't want the reset to
> succeed if the previous commits changed a file that you also changed in
> your work tree.
>
> The table below shows what happens when running "git reset --option
> target" to reset the HEAD to another commit (as a special case "target"
> could be the same as HEAD) in the cases where "--merge" and "--keep"
> behave differently.
I think this new option is unrelated to "--merge"; iow, the only relation
to it is that it is an option to the same command "git reset", so it is
related but it is related the same way and to the degree as "--mixed" is.
Thinking about it even more, if the number of commits you are resetting
away is zero in your use case (i.e. target is HEAD), shouldn't this new
mode of operation degenerate to "--mixed"? So in that sense, it might
make sense to contrast it with "--mixed".
But let's try not to contrast it with anything else, and see how well it
stands on its own. The below is my attempt.
> working index HEAD target working index HEAD
> ----------------------------------------------------
> A B C D --keep (disallowed)
> A B C C --keep A C C
> B B C D --keep (disallowed)
> B B C C --keep B C C
Let's give an explanation of the above in terms of what this means to the
end users.
When you have changes to a path that the accumulated commits between
target..HEAD touch, you don't want to discard them. It doesn't matter
if the changes in the work tree has been partially added (A != B) or
fully added (A == B) to the index. In both cases, the operation is
disallowed, just like "checkout $another_branch" stops in such a case.
But if you have local modifications based on one version and dropping
the accumulated commits between target..HEAD does not involve that
path, we can safely "transplant" that change to the target.
Presented this way, a future direction (iow, I am not suggesting you to do
this before the current series solidifies) might be to allow users to do
something similar to "checkout -m $the_other_branch". IOW, instead of
disallowing "I have changed from C to B and switching to D" case, perform
a three-way merge to bring the work tree file to (D+(B-C)), transplanting
the local change you made on top of the target.
> The following table shows what happens on unmerged entries:
>
> working index HEAD target working index HEAD
> ----------------------------------------------------
> X U A B --keep (disallowed)
> X U A A --keep X A A
In a sense, this is consistent with the above; the local change attempted
happens to be an unmerged result.
But it is inconsistent with the intended use case you presented, which
leaves no room for unmerged entries to enter in the index to begin with.
It might be safer to error out on any unmerged entry in the index. I
dunno.
^ permalink raw reply
* Re: [RFC/PATCH 1/5] reset: make "reset --merge" discard work tree changes on unmerged entries
From: Junio C Hamano @ 2010-01-02 19:46 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.95746.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> From: Junio C Hamano <gitster@pobox.com>
>
> Commit 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) disallowed
> "git reset --merge" when there was unmerged entries. It acknowledged
> that this is not the best possible behavior, and that it would be better
> if unmerged entries were reset as if --hard (instead of --merge) has
> been used.
>
> Recently another commit (reset: use "unpack_trees()" directly instead of
> "git read-tree", 2009-12-30) changed the behavior of --merge to accept
> resetting unmerged entries if they are reset to a different state than
> HEAD, but it did not reset the changes in the work tree. So the behavior
> was kind of improved, but it was not yet as if --hard has been used.
It would be more honest if we said something like:
It changed a safer "I can't do as asked, please do it by hand"
into a more dangerous "I pretend that I did so but I didn't do the
full job; you need to fix up the result but I am not telling you
that you have to", which is a lot worse.
here instead (that is one reason why I said my fix-up was "squashable").
But that is a minor issue.
I have been thinking about two issues on this --merge change.
> - Updates merged_entry() and deleted_entry() so that they pay attention
> to cache entries with null sha1 (note that we _might_ want to use a
> new in-core ce->ce_flags instead of using the null-sha1 hack). They
> are previously unmerged entries, and the files in the work tree that
> correspond to them are resetted away by oneway_merge() to the version
> from the tree we are resetting to.
One is the use of ce_flags instead of relying on the 20-byte comparison I
said above, for both performance (minor) and future maintainability (much
bigger) concern. I have a feeling that we will regret later that we used
the null_sha1 trick here, when we want to express another "special" kind
of cache entry in unrelated situations. The use of null_sha1 hack was
expedient but I fell victim of the same mentality of declaring that this
is the _last_ such kind of special index entry and closing the door to
others who want to extend the system later with different kind of special
cache entry, which I often complain about myself to patches from other
people.
Another is that it _might_ make sense to use two-tree form of read-tree
machinery (but using a different version of unpack-trees.c::twoway_merge()
function), instead of the one-tree form of "we don't bother checking if
the index is consistent with HEAD and assume it is, and jump to the
target."
"git reset --merge $there" is about the situation where you started a
"mergy" operation (e.g. "git merge", "git am -3", "git rebase", ...) while
you had unrelated local changes in the work tree, and you want to go back
to the state before that operation $there (which is HEAD if the mergy
operation is "merge", but is different from HEAD if it was "am -3" and you
have successfully applied a patch or more already). Linus may know and
won't use "reset --merge" in a situation where it is not suitable, but not
everybody is Linus. Even though "reset --merge" may "correctly" work with
respect to the table you added to "git reset" documentation, it would do
something that may not "make sense" from the end-user's point of view when
used in a situation that it wasn't designed for. Using the two-tree form
allows the machinery to inspect the difference between HEAD and the index,
and detect cases where "reset --merge" was attempted when it shouldn't.
For example, if stage #2 of an unmerged path does not match HEAD, we know
there is something wrong.
This latter issue is much bigger and needs a lot more thought, and I don't
think it should block the series from going forward at all. But I think
it is worth keeping in the back of our heads.
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2010-01-02 19:45 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <fcaeb9bf1001021115j7b23264n42cfba7855c2253e@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> > +support_external_grep() {
>> > + case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
>> > + *"(default)"*) return 0;;
>> > + *"(ignored by this build)"*) return 1;;
>> > + *) test_expect_success 'External grep check is broken' 'false';;
>> > + esac
>> > +}
>>
>>
>> Heh, clever.
>>
>> git grep -h 2>&1 | grep 'allow calling of grep.*default' >/dev/null
>>
>> may be sufficient, though.
>
> Yes, until somebody changes help text in builtin-grep.c and all
> external grep tests become disable. I wanted to catch that case too.
Ok, that is a worthwhile thing to do.
Then please at least make the second grep "grep ext-grep", droping "-e --"
from it. We assume some implementation of external grep to lack "-e"
(e.g. Solaris).
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Nguyen Thai Ngoc Duy @ 2010-01-02 19:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtyv4cpna.fsf@alter.siamese.dyndns.org>
On 1/3/10, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
> > On Wed, Dec 30, 2009 at 11:09:52PM -0800, Junio C Hamano wrote:
> >> 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?
> >
> > Your patch works great. By the way I think we should move "cached"
> > check from grep_cache() into external_grep() too, for consistency.
>
>
> I think what I gave you is more consistent.
>
> "cached" is about "are we searching in the index or in the work tree?" and
> "external_grep()" is about "it often is faster to run external grep when
> we are searching in the work tree, so do that if the other constraints
> allow us to". An example of such a constraint is that we must be able to
> express the operation on the command line of a traditional grep, and use
> of git extended grep synatx makes it unfeasible, hence the function says
> "I can't" in such a case.
>
> You can change the definition of "external_grep()" to "try to use external
> grep somewhere, if the other constraints allow us to", and have the caller
> and the function do an "this time, please grep for this pattern in the
> index---I can't" exchange, but I don't see much point. We _know_ no
> external grep will ever be able to read from the index.
>
> We should simply drop "cached" argument from external_grep().
OK.
> > diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> > index abd14bf..f77970c 100755
> > --- a/t/t7002-grep.sh
> > +++ b/t/t7002-grep.sh
> > @@ -8,6 +8,14 @@ test_description='git grep various.
> >
> > . ./test-lib.sh
> >
> > +support_external_grep() {
> > + case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
> > + *"(default)"*) return 0;;
> > + *"(ignored by this build)"*) return 1;;
> > + *) test_expect_success 'External grep check is broken' 'false';;
> > + esac
> > +}
>
>
> Heh, clever.
>
> git grep -h 2>&1 | grep 'allow calling of grep.*default' >/dev/null
>
> may be sufficient, though.
>
Yes, until somebody changes help text in builtin-grep.c and all
external grep tests become disable. I wanted to catch that case too.
Or maybe just add another option --show-features to "git rev-parse"
(or extend git version string, is the version format fixed?) to list
all built-time features that a git build supports. Some comes to mind:
external-grep, iconv, ipv6, http (unlikely), threading... Much less
tricky to test.
--
Duy
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2010-01-02 18:44 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <20100102115041.GA32381@do>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Wed, Dec 30, 2009 at 11:09:52PM -0800, Junio C Hamano wrote:
>> 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?
>
> Your patch works great. By the way I think we should move "cached"
> check from grep_cache() into external_grep() too, for consistency.
I think what I gave you is more consistent.
"cached" is about "are we searching in the index or in the work tree?" and
"external_grep()" is about "it often is faster to run external grep when
we are searching in the work tree, so do that if the other constraints
allow us to". An example of such a constraint is that we must be able to
express the operation on the command line of a traditional grep, and use
of git extended grep synatx makes it unfeasible, hence the function says
"I can't" in such a case.
You can change the definition of "external_grep()" to "try to use external
grep somewhere, if the other constraints allow us to", and have the caller
and the function do an "this time, please grep for this pattern in the
index---I can't" exchange, but I don't see much point. We _know_ no
external grep will ever be able to read from the index.
We should simply drop "cached" argument from external_grep().
> I thought of tests when I wrote the patch, but it was hard to find a
> reliable way to detect if a git build supports external grep.
Ah, you are right. I forgot about that issue.
> diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
> index abd14bf..f77970c 100755
> --- a/t/t7002-grep.sh
> +++ b/t/t7002-grep.sh
> @@ -8,6 +8,14 @@ test_description='git grep various.
>
> . ./test-lib.sh
>
> +support_external_grep() {
> + case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
> + *"(default)"*) return 0;;
> + *"(ignored by this build)"*) return 1;;
> + *) test_expect_success 'External grep check is broken' 'false';;
> + esac
> +}
Heh, clever.
git grep -h 2>&1 | grep 'allow calling of grep.*default' >/dev/null
may be sufficient, though.
^ permalink raw reply
* Re: [PATCH] Reword -M, when in `git log`s documention, to suggest --follow
From: Junio C Hamano @ 2010-01-02 18:43 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
In-Reply-To: <1262412622-sup-7473@utwig>
Alex Vandiver <alex@chmrr.net> writes:
> most common usage model. It seems to me like the more correct fix
> would be to move the diff options to later in the file (after the
> options that are `git log`-specific), or to remove them entirely, and
> replace them with a pointer to git diff's options.
IIRC, the apporach we took earlier that people hated and resulted in the
current "include" approach was to say "It also takes many of options that
diff family of commands take". So reordering may work better but dropping
and referring would not, I suspect.
^ permalink raw reply
* Re: Filename quoting / parsing problem
From: Junio C Hamano @ 2010-01-02 18:37 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: git
In-Reply-To: <201001021236.26947.agruen@suse.de>
Andreas Gruenbacher <agruen@suse.de> writes:
> On Friday 01 January 2010 09:01:19 pm Junio C Hamano wrote:
>> > Both "b file" and "c file " are parsed by "git apply" perfectly fine.
>
> Right, the "diff --git" lines are technically still parseable when the file
> name stays the same. With renames, lines like "diff --git a/f a/f b/f" or
> "diff --git a/f b/f b/f" are possible, but then there will also be "renamed
> from" and "renamed to" headers which will disambiguate things. Still, it
> doesn't seem like a good idea to allow such ambiguities in the first place.
You already realized that there is no ambiguity because "diff --git" lines
are parsable and renames have explicit names. Why do you still maintain
that we are allowing such "ambiguities" when there is none?
>> Having said all that, I don't think we would mind a change to treat a
>> pathname with trailing SP a bit specially (iow, quoting "c file " in the
>> above failed attempt to reproduce the issue).
>
> I would prefer quoting file names which contain spaces anywhere,...
The only reason I said I don't think we would mind changing the trailing
SP case is because the reduced risk of getting our patches corrupted by
MUA _might_ outweigh the benefit of not quoting to avoid an eyesore [*1*].
But what you said would add to eyesore of quoted names (which you omitted
from your quote) without any justification other than "I would prefer".
The pros-and-cons in such a change is quite different; as we have already
established that there is no ambiguity, "disambuguation" is not a "pro" in
this comparison.
[Footnote]
*1* Strictly speaking, it is not just "an eyesore" that is an issue. Our
diff output without renames are designed to be grokkable by other people's
patch implementations (e.g. GNU patch), and the quoted pathnames are not
understandable by them. Even though our final version of quoted path
format came from the GNU diff/patch maintainer (back then, at least):
http://article.gmane.org/gmane.comp.version-control.git/10103
I don't think it happened in the GNU land yet, and you would be the person
to know about it ;-).
^ permalink raw reply
* Re: subtree merge tries to merge into wrong directory
From: Nils Adermann @ 2010-01-02 18:17 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git
In-Reply-To: <c10155f8a7a2dd451f0b74e323f3a989.squirrel@www.naderman.de>
naderman@naderman.de schrieb:
> So I finally got around to trying this. This was my first result:
>
> $ git merge -Xsubtree=lib/ezc/trunk/Reflection/ FETCH_HEAD
> fatal: entry not found in tree 60270661e0d2a5ee03b24609fac5c6d00d048988
>
Turns out that only happens because of the trailing slash.
lib/ezc/trunk/Reflection works reliably.
Nils
^ permalink raw reply
* Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Daniel Convissor @ 2010-01-02 17:14 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.76552.chriscool@tuxfamily.org>
On Sat, Jan 02, 2010 at 06:39:32AM +0100, Christian Couder wrote:
> +++ b/Documentation/git-reset.txt
...
> +--keep::
> + Resets the index to match the tree recorded by the named commit,
> + but keep changes in the working tree. Aborts if the reset would
> + change files that are already changes in the working tree.
Grammar suggestion for the last line there. Change "already changes" to
"already changed". Or better yet, use a different word, such as "already
modified", since "change" based words have been used so many times
already in the sentence.
Thanks,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 f: 718-854-0409
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox