Hi,
On Thu, Oct 15, 2009 at 11:36 AM, Shawn O. Pearce <spearce@xxxxxxxxxxx> wrote:
> 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.
> @@ -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.
> @@ -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. :)
> 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;
> }
> @@ -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.
> @@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html