From: Junio C Hamano Date: Fri, 7 Jun 2013 23:15:32 +0000 (-0700) Subject: Merge branch 'js/transport-helper-error-reporting-fix' into fc/makefile X-Git-Tag: v1.8.4-rc0~162^2~3 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/81b4f18fb8f9356477157a3b9f0652d371cba835?ds=inline;hp=-c Merge branch 'js/transport-helper-error-reporting-fix' into fc/makefile * js/transport-helper-error-reporting-fix: git-remote-testgit: build it to run under $SHELL_PATH git-remote-testgit: further remove some bashisms git-remote-testgit: avoid process substitution t5801: "VAR=VAL shell_func args" is forbidden transport-helper: update remote helper namespace transport-helper: trivial code shuffle transport-helper: warn when refspec is not used transport-helper: clarify pushing without refspecs transport-helper: update refspec documentation transport-helper: clarify *:* refspec transport-helper: improve push messages transport-helper: mention helper name when it dies transport-helper: report errors properly Conflicts: t/t5801-remote-helpers.sh --- 81b4f18fb8f9356477157a3b9f0652d371cba835 diff --combined Documentation/gitremote-helpers.txt index da746419b3,4d26e37d36..0827f69139 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@@ -159,11 -159,11 +159,11 @@@ Miscellaneous capabilitie carried out. 'refspec' :: - This modifies the 'import' capability, allowing the produced - fast-import stream to modify refs in a private namespace - instead of writing to refs/heads or refs/remotes directly. + For remote helpers that implement 'import' or 'export', this capability + allows the refs to be constrained to a private namespace, instead of + writing to refs/heads or refs/remotes directly. It is recommended that all importers providing the 'import' - capability use this. + capability use this. It's mandatory for 'export'. + A helper advertising the capability `refspec refs/heads/*:refs/svn/origin/branches/*` @@@ -174,8 -174,8 +174,8 @@@ ref This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by - the list command. If a helper does not need a specific 'refspec' - capability then it should advertise `refspec *:*`. + the list command. If no 'refspec' capability is advertised, + there is an implied `refspec *:*`. 'bidi-import':: This modifies the 'import' capability. @@@ -202,10 -202,6 +202,10 @@@ marks specified in before processing any input. For details, read up on '--import-marks=' in linkgit:git-fast-export[1]. +'signed-tags':: + This modifies the 'export' capability, instructing Git to pass + '--signed-tags=verbatim' to linkgit:git-fast-export[1]. In the + absence of this capability, Git will use '--signed-tags=warn-strip'. diff --combined Makefile index ff0f527b52,5f424a7e19..e9ac1229e8 --- a/Makefile +++ b/Makefile @@@ -460,6 -460,7 +460,7 @@@ SCRIPT_SH += git-mergetool.s SCRIPT_SH += git-pull.sh SCRIPT_SH += git-quiltimport.sh SCRIPT_SH += git-rebase.sh + SCRIPT_SH += git-remote-testgit.sh SCRIPT_SH += git-repack.sh SCRIPT_SH += git-request-pull.sh SCRIPT_SH += git-stash.sh @@@ -487,17 -488,11 +488,17 @@@ SCRIPT_PERL += git-svn.per SCRIPT_PYTHON += git-remote-testpy.py SCRIPT_PYTHON += git-p4.py +NO_INSTALL += git-remote-testpy + # Generated files for scripts SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH)) SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL)) SCRIPT_PYTHON_GEN = $(patsubst %.py,%,$(SCRIPT_PYTHON)) +SCRIPT_SH_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_SH_GEN)) +SCRIPT_PERL_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PERL_GEN)) +SCRIPT_PYTHON_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PYTHON_GEN)) + # Individual rules to allow e.g. # "make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script" # from subdirectories like contrib/*/ @@@ -507,12 -502,12 +508,12 @@@ build-sh-script: $(SCRIPT_SH_GEN build-python-script: $(SCRIPT_PYTHON_GEN) .PHONY: install-perl-script install-sh-script install-python-script -install-sh-script: $(SCRIPT_SH_GEN) - $(INSTALL) $(SCRIPT_SH_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' -install-perl-script: $(SCRIPT_PERL_GEN) - $(INSTALL) $(SCRIPT_PERL_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' -install-python-script: $(SCRIPT_PYTHON_GEN) - $(INSTALL) $(SCRIPT_PYTHON_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' +install-sh-script: $(SCRIPT_SH_INS) + $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' +install-perl-script: $(SCRIPT_PERL_INS) + $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' +install-python-script: $(SCRIPT_PYTHON_INS) + $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' .PHONY: clean-perl-script clean-sh-script clean-python-script clean-sh-script: @@@ -522,9 -517,9 +523,9 @@@ clean-perl-script clean-python-script: $(RM) $(SCRIPT_PYTHON_GEN) -SCRIPTS = $(SCRIPT_SH_GEN) \ - $(SCRIPT_PERL_GEN) \ - $(SCRIPT_PYTHON_GEN) \ +SCRIPTS = $(SCRIPT_SH_INS) \ + $(SCRIPT_PERL_INS) \ + $(SCRIPT_PYTHON_INS) \ git-instaweb ETAGS_TARGET = TAGS @@@ -1651,7 -1646,7 +1652,7 @@@ please_set_SHELL_PATH_to_a_more_modern_ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell strip: $(PROGRAMS) git$X - $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X + $(STRIP) $(STRIP_OPTS) $^ ### Target-specific flags and dependencies @@@ -1711,9 -1706,9 +1712,9 @@@ version.sp version.s version.o: EXTRA_C $(BUILT_INS): git$X $(QUIET_BUILT_IN)$(RM) $@ && \ - ln git$X $@ 2>/dev/null || \ - ln -s git$X $@ 2>/dev/null || \ - cp git$X $@ + ln $< $@ 2>/dev/null || \ + ln -s $< $@ 2>/dev/null || \ + cp $< $@ common-cmds.h: ./generate-cmdlist.sh command-list.txt @@@ -1778,7 -1773,7 +1779,7 @@@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % -e ' x' \ -e '}' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ - $@.perl >$@+ && \ + $< >$@+ && \ chmod +x $@+ && \ mv $@+ $@ @@@ -1802,8 -1797,8 +1803,8 @@@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git endif # NO_PERL ifndef NO_PYTHON -$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS -$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py +$(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS +$(SCRIPT_PYTHON_GEN): % : %.py $(QUIET_GEN)$(RM) $@ $@+ && \ INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \ --no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \ @@@ -1811,11 -1806,11 +1812,11 @@@ sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \ -e 's|\(os\.getenv("GITPYTHONLIB"\)[^)]*)|\1,"@@INSTLIBDIR@@")|' \ -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \ - $@.py >$@+ && \ + $< >$@+ && \ chmod +x $@+ && \ mv $@+ $@ else # NO_PYTHON -$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh +$(SCRIPT_PYTHON_GEN): % : unimplemented.sh $(QUIET_GEN)$(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \ diff --combined git-remote-testgit.sh index 0000000000,b5289493e2..e83315f0e3 mode 000000,100755..100755 --- a/git-remote-testgit.sh +++ b/git-remote-testgit.sh @@@ -1,0 -1,115 +1,116 @@@ + #!/bin/sh + # Copyright (c) 2012 Felipe Contreras + + alias=$1 + url=$2 + + dir="$GIT_DIR/testgit/$alias" + prefix="refs/testgit/$alias" + + default_refspec="refs/heads/*:${prefix}/heads/*" + + refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}" + + test -z "$refspec" && prefix="refs" + + export GIT_DIR="$url/.git" + + mkdir -p "$dir" + + if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS" + then + gitmarks="$dir/git.marks" + testgitmarks="$dir/testgit.marks" + test -e "$gitmarks" || >"$gitmarks" + test -e "$testgitmarks" || >"$testgitmarks" + fi + + while read line + do + case $line in + capabilities) + echo 'import' + echo 'export' + test -n "$refspec" && echo "refspec $refspec" + if test -n "$gitmarks" + then + echo "*import-marks $gitmarks" + echo "*export-marks $gitmarks" + fi ++ test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags" + echo + ;; + list) + git for-each-ref --format='? %(refname)' 'refs/heads/' + head=$(git symbolic-ref HEAD) + echo "@$head HEAD" + echo + ;; + import*) + # read all import lines + while true + do + ref="${line#* }" + refs="$refs $ref" + read line + test "${line%% *}" != "import" && break + done + + if test -n "$gitmarks" + then + echo "feature import-marks=$gitmarks" + echo "feature export-marks=$gitmarks" + fi + + if test -n "$GIT_REMOTE_TESTGIT_FAILURE" + then + echo "feature done" + exit 1 + fi + + echo "feature done" + git fast-export \ + ${testgitmarks:+"--import-marks=$testgitmarks"} \ + ${testgitmarks:+"--export-marks=$testgitmarks"} \ + $refs | + sed -e "s#refs/heads/#${prefix}/heads/#g" + echo "done" + ;; + export) + if test -n "$GIT_REMOTE_TESTGIT_FAILURE" + then + # consume input so fast-export doesn't get SIGPIPE; + # git would also notice that case, but we want + # to make sure we are exercising the later + # error checks + while read line; do + test "done" = "$line" && break + done + exit 1 + fi + + before=$(git for-each-ref --format=' %(refname) %(objectname) ') + + git fast-import \ + ${testgitmarks:+"--import-marks=$testgitmarks"} \ + ${testgitmarks:+"--export-marks=$testgitmarks"} \ + --quiet + + # figure out which refs were updated + git for-each-ref --format='%(refname) %(objectname)' | + while read ref a + do + case "$before" in + *" $ref $a "*) + continue ;; # unchanged + esac + echo "ok $ref" + done + + echo + ;; + '') + exit + ;; + esac + done diff --combined t/t5801-remote-helpers.sh index dbb02e2afd,0a83db8875..61479c3b16 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@@ -6,13 -6,7 +6,8 @@@ test_description='Test remote-helper import and export commands' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-gpg.sh - if ! type "${BASH-bash}" >/dev/null 2>&1; then - skip_all='skipping remote-testgit tests, bash not available' - test_done - fi - compare_refs() { git --git-dir="$1/.git" rev-parse --verify $2 >expect && git --git-dir="$3/.git" rev-parse --verify $4 >actual && @@@ -101,39 -95,28 +96,28 @@@ test_expect_failure 'push new branch wi test_expect_success 'cloning without refspec' ' GIT_REMOTE_TESTGIT_REFSPEC="" \ - git clone "testgit::${PWD}/server" local2 && + git clone "testgit::${PWD}/server" local2 2>error && + grep "This remote helper should implement refspec capability" error && compare_refs local2 HEAD server HEAD ' test_expect_success 'pulling without refspecs' ' (cd local2 && git reset --hard && - GIT_REMOTE_TESTGIT_REFSPEC="" git pull) && + GIT_REMOTE_TESTGIT_REFSPEC="" git pull 2>../error) && + grep "This remote helper should implement refspec capability" error && compare_refs local2 HEAD server HEAD ' - test_expect_failure 'pushing without refspecs' ' + test_expect_success 'pushing without refspecs' ' test_when_finished "(cd local2 && git reset --hard origin)" && (cd local2 && echo content >>file && git commit -a -m ten && - GIT_REMOTE_TESTGIT_REFSPEC="" git push) && - compare_refs local2 HEAD server HEAD - ' - - test_expect_success 'pulling with straight refspec' ' - (cd local2 && - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) && - compare_refs local2 HEAD server HEAD - ' - - test_expect_failure 'pushing with straight refspec' ' - test_when_finished "(cd local2 && git reset --hard origin)" && - (cd local2 && - echo content >>file && - git commit -a -m eleven && - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) && - compare_refs local2 HEAD server HEAD + GIT_REMOTE_TESTGIT_REFSPEC="" && + export GIT_REMOTE_TESTGIT_REFSPEC && + test_must_fail git push 2>../error) && + grep "remote-helper doesn.t support push; refspec needed" error ' test_expect_success 'pulling without marks' ' @@@ -167,25 -150,38 +151,57 @@@ test_expect_success 'push ref with exis compare_refs local dup server dup ' +test_expect_success GPG 'push signed tag' ' + (cd local && + git checkout master && + git tag -s -m signed-tag signed-tag && + git push origin signed-tag + ) && + compare_refs local signed-tag^{} server signed-tag^{} && + test_must_fail compare_refs local signed-tag server signed-tag +' + +test_expect_success GPG 'push signed tag with signed-tags capability' ' + (cd local && + git checkout master && + git tag -s -m signed-tag signed-tag-2 && + GIT_REMOTE_TESTGIT_SIGNED_TAGS=1 git push origin signed-tag-2 + ) && + compare_refs local signed-tag-2 server signed-tag-2 +' + + test_expect_success 'push update refs' ' + (cd local && + git checkout -b update master && + echo update >>file && + git commit -a -m update && + git push origin update + git rev-parse --verify remotes/origin/update >expect && + git rev-parse --verify testgit/origin/heads/update >actual && + test_cmp expect actual + ) + ' + + test_expect_success 'proper failure checks for fetching' ' + (GIT_REMOTE_TESTGIT_FAILURE=1 && + export GIT_REMOTE_TESTGIT_FAILURE && + cd local && + test_must_fail git fetch 2> error && + cat error && + grep -q "Error while running fast-import" error + ) + ' + + test_expect_success 'proper failure checks for pushing' ' + (GIT_REMOTE_TESTGIT_FAILURE=1 && + export GIT_REMOTE_TESTGIT_FAILURE && + cd local && + test_must_fail git push --all 2> error && + cat error && + grep -q "Reading from helper .git-remote-testgit. failed" error + ) + ' + test_expect_success 'push messages' ' (cd local && git checkout -b new_branch master && diff --combined transport-helper.c index 522d79178e,92174095ed..f11d78a55a --- a/transport-helper.c +++ b/transport-helper.c @@@ -11,6 -11,7 +11,7 @@@ #include "thread-utils.h" #include "sigchain.h" #include "argv-array.h" + #include "refs.h" static int debug; @@@ -25,7 -26,6 +26,7 @@@ struct helper_data option : 1, push : 1, connect : 1, + signed_tags : 1, no_disconnect_req : 1; char *export_marks; char *import_marks; @@@ -47,7 -47,7 +48,7 @@@ static void sendline(struct helper_dat die_errno("Full write to remote helper failed"); } - static int recvline_fh(FILE *helper, struct strbuf *buffer) + static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name) { strbuf_reset(buffer); if (debug) @@@ -55,7 -55,7 +56,7 @@@ if (strbuf_getline(buffer, helper, '\n') == EOF) { if (debug) fprintf(stderr, "Debug: Remote helper quit.\n"); - exit(128); + die("Reading from helper 'git-remote-%s' failed", name); } if (debug) @@@ -65,7 -65,7 +66,7 @@@ static int recvline(struct helper_data *helper, struct strbuf *buffer) { - return recvline_fh(helper->out, buffer); + return recvline_fh(helper->out, buffer, helper->name); } static void xchgline(struct helper_data *helper, struct strbuf *buffer) @@@ -192,8 -192,6 +193,8 @@@ static struct child_process *get_helper refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec ")); } else if (!strcmp(capname, "connect")) { data->connect = 1; + } else if (!strcmp(capname, "signed-tags")) { + data->signed_tags = 1; } else if (!prefixcmp(capname, "export-marks ")) { struct strbuf arg = STRBUF_INIT; strbuf_addstr(&arg, "--export-marks="); @@@ -214,9 -212,12 +215,11 @@@ int i; data->refspec_nr = refspec_nr; data->refspecs = parse_fetch_refspec(refspec_nr, refspecs); - for (i = 0; i < refspec_nr; i++) { + for (i = 0; i < refspec_nr; i++) free((char *)refspecs[i]); - } free(refspecs); + } else if (data->import || data->bidi_import || data->export) { + warning("This remote helper should implement refspec capability."); } strbuf_release(&buf); if (debug) @@@ -412,11 -413,9 +415,11 @@@ static int get_exporter(struct transpor /* we need to duplicate helper->in because we want to use it after * fastexport is done with it. */ fastexport->out = dup(helper->in); - fastexport->argv = xcalloc(5 + revlist_args->nr, sizeof(*fastexport->argv)); + fastexport->argv = xcalloc(6 + revlist_args->nr, sizeof(*fastexport->argv)); fastexport->argv[argc++] = "fast-export"; fastexport->argv[argc++] = "--use-done-feature"; + fastexport->argv[argc++] = data->signed_tags ? + "--signed-tags=verbatim" : "--signed-tags=warn-strip"; if (data->export_marks) fastexport->argv[argc++] = data->export_marks; if (data->import_marks) @@@ -473,7 -472,7 +476,7 @@@ static int fetch_with_import(struct tra * were fetching. * * (If no "refspec" capability was specified, for historical - * reasons we default to *:*.) + * reasons we default to the equivalent of *:*.) * * Store the result in to_fetch[i].old_sha1. Callers such * as "git fetch" can use the value to write feedback to the @@@ -540,7 -539,7 +543,7 @@@ static int process_connect_service(stru goto exit; sendline(data, &cmdbuf); - recvline_fh(input, &cmdbuf); + recvline_fh(input, &cmdbuf, name); if (!strcmp(cmdbuf.buf, "")) { data->no_disconnect_req = 1; if (debug) @@@ -622,7 -621,7 +625,7 @@@ static int fetch(struct transport *tran return -1; } - static void push_update_ref_status(struct strbuf *buf, + static int push_update_ref_status(struct strbuf *buf, struct ref **ref, struct ref *remote_refs) { @@@ -688,7 -687,7 +691,7 @@@ *ref = find_ref_by_name(remote_refs, refname); if (!*ref) { warning("helper reported unexpected status of %s", refname); - return; + return 1; } if ((*ref)->status != REF_STATUS_NONE) { @@@ -697,11 -696,12 +700,12 @@@ * status reported by the remote helper if the latter is 'no match'. */ if (status == REF_STATUS_NONE) - return; + return 1; } (*ref)->status = status; (*ref)->remote_status = msg; + return 0; } static void push_update_refs_status(struct helper_data *data, @@@ -710,11 -710,24 +714,24 @@@ struct strbuf buf = STRBUF_INIT; struct ref *ref = remote_refs; for (;;) { + char *private; + recvline(data, &buf); if (!buf.len) break; - push_update_ref_status(&buf, &ref, remote_refs); + if (push_update_ref_status(&buf, &ref, remote_refs)) + continue; + + if (!data->refspecs) + continue; + + /* propagate back the update to the remote namespace */ + private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name); + if (!private) + continue; + update_ref("update by helper", private, ref->new_sha1, NULL, 0, 0); + free(private); } strbuf_release(&buf); } @@@ -789,6 -802,9 +806,9 @@@ static int push_refs_with_export(struc struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; + if (!data->refspecs) + die("remote-helper doesn't support push; refspec needed"); + helper = get_helper(transport); write_constant(helper->in, "export\n"); @@@ -799,8 -815,9 +819,9 @@@ char *private; unsigned char sha1[20]; - if (!data->refspecs) - continue; + if (ref->deletion) + die("remote-helpers do not support ref deletion"); + private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name); if (private && !get_sha1(private, sha1)) { strbuf_addf(&buf, "^%s", private); @@@ -809,13 -826,8 +830,8 @@@ } free(private); - if (ref->deletion) { - die("remote-helpers do not support ref deletion"); - } - if (ref->peer_ref) string_list_append(&revlist_args, ref->peer_ref->name); - } if (get_exporter(transport, &exporter, &revlist_args)) @@@ -996,7 -1008,7 +1012,7 @@@ struct unidirectional_transfer int src_is_sock; /* Is destination socket? */ int dest_is_sock; - /* Transfer state (TRANSFERING/FLUSHING/FINISHED) */ + /* Transfer state (TRANSFERRING/FLUSHING/FINISHED) */ int state; /* Buffer. */ char buf[BUFFERSIZE];