Re: [RFC PATCH v3 10/17] Move WebDAV HTTP push under remote-curl
From: Tay Ray Chuan <hidden>
Date: 2016-06-15 22:47:34
Subsystem:
the rest · Maintainer:
Linus Torvalds
Hi, On Thu, Oct 15, 2009 at 11:36 AM, Shawn O. Pearce [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt@@ -34,6 +34,10 @@ Commands are given by the caller on the helper's standard input, one per line. value of the ref. A space-separated list of attributes follows the name; unrecognized attributes are ignored. After the complete list, outputs a blank line. ++ +If 'push' is supported this may be called as 'list for-push' +to obtain the current refs prior to sending one or more 'push' +commands to the helper.
The new paragraph should have the same indentation as 'list'. It would have been great if you implemented this as a filter, such that 'list <attr>' lists the refs with the specified attribute <attr>, rather than hardcoding it.
quoted hunk ↗ jump to hunk
@@ -59,6 +63,22 @@ suitably updated. +When the push is complete, outputs one or more 'ok <dst>' or +'error <dst> <why>?' lines to indicate success or failure of +each pushed ref. The status report output is terminated by +a blank line. The option field <why> may be quoted in a C +style string if it contains an LF.
You should mention that this behaviour only occurs when the --helper-status option is used.
quoted hunk ↗ jump to hunk
@@ -106,6 +132,11 @@ OPTIONS +'option dry-run' \{'true'|'false'\}: + If true, pretend like the operation completed successfully, + but don't actually change any repository data. For most + helpers this only applies to the 'push', if supported. +
The 'like' after 'pretend' can be, like, removed. :)
quoted hunk ↗ jump to hunk
diff --git a/http-push.c b/http-push.c@@ -1941,9 +1946,14 @@ int main(int argc, char **argv) if (is_null_sha1(ref->peer_ref->new_sha1)) { if (delete_remote_branch(ref->name, 1) == -1) { - error("Could not remove %s", ref->name); + if (helper_status) + printf("error %s cannot remove\n", ref->name); + else + error("Could not remove %s", ref->name); rc = -4; }
I think error() calls should be left intact (as indicators to the user), even when --helper-status is specified. It wouldn't affect transport-helper.c's parsing of '(ok|error) <ref>' lines, since it reads stdout only. In other words, the above would read:
error("Could not remove %s", ref->name);
+ if (helper_status)
+ printf("error %s cannot remove\n", ref->name);
rc = -4;
}quoted hunk ↗ jump to hunk
@@ -1968,12 +1980,15 @@ int main(int argc, char **argv) * commits at the remote end and likely * we were not up to date to begin with. */ - error("remote '%s' is not an ancestor of\n" - "local '%s'.\n" - "Maybe you are not up-to-date and " - "need to pull first?", - ref->name, - ref->peer_ref->name); + if (helper_status) + printf("error %s non-fast forward\n", ref->name); + else + error("remote '%s' is not an ancestor of\n" + "local '%s'.\n" + "Maybe you are not up-to-date and " + "need to pull first?", + ref->name, + ref->peer_ref->name); rc = -2; continue; }
Same here.
quoted hunk ↗ jump to hunk
@@ -1987,14 +2002,20 @@ int main(int argc, char **argv) /* Lock remote branch ref */ ref_lock = lock_remote(ref->name, LOCK_TIME); if (ref_lock == NULL) { - fprintf(stderr, "Unable to lock remote branch %s\n", - ref->name); + if (helper_status) + printf("error %s lock error\n", ref->name); + else + fprintf(stderr, "Unable to lock remote branch %s\n", + ref->name); rc = 1; continue; }
Same here. Two more areas in http-push.c that should have status messages (generated on top of pu): (I'm not sure what ref should read when there's no match, but transport-helper.c should have no problems parsing it with 'null'.) -->8--
diff --git a/http-push.c b/http-push.c
index 9010ccc..e979feb 100644
--- a/http-push.c
+++ b/http-push.c@@ -1916,9 +1916,12 @@ int main(int argc, char **argv) /* Remove a remote branch if -d or -D was specified */ if (delete_branch) { - if (delete_remote_branch(refspec[0], force_delete) == -1) + if (delete_remote_branch(refspec[0], force_delete) == -1) { fprintf(stderr, "Unable to delete remote branch %s\n", refspec[0]); + if (helper_status) + printf("error %s cannot remove\n", refspec[0]); + } goto cleanup; }
@@ -1930,6 +1933,8 @@ int main(int argc, char **argv) } if (!remote_refs) { fprintf(stderr, "No refs in common and none specified; doing nothing.\n"); + if (helper_status) + printf("error null no match\n"); rc = 0; goto cleanup; } --
Cheers, Ray Chuan