Git development
 help / color / mirror / Atom feed
* [PATCH v2 2/3] clean up some of the output from git push --porcelain
From: Larry D'Anna @ 2010-02-05 20:49 UTC (permalink / raw)
  To: git; +Cc: Larry D'Anna
In-Reply-To: <cover.1265402797.git.larry@elder-gods.org>

* don't emit long explanatory message about non-fast-forward updates.

* send "To dest" lines to standard out so whoever is reading standard out knows
  which ref updates went to which remotes.

* only send the "Everything up-to-date" line if verbose.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
 builtin-push.c |    2 +-
 transport.c    |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 0a27072..ff0b1c6 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -123,7 +123,7 @@ static int push_with_options(struct transport *transport, int flags)
 	if (!err)
 		return 0;
 
-	if (nonfastforward && advice_push_nonfastforward) {
+	if (!(flags & TRANSPORT_PUSH_PORCELAIN) && nonfastforward && advice_push_nonfastforward) {
 		fprintf(stderr, "To prevent you from losing history, non-fast-forward updates were rejected\n"
 				"Merge the remote changes before pushing again.  See the 'Note about\n"
 				"fast-forwards' section of 'git push --help' for details.\n");
diff --git a/transport.c b/transport.c
index 3846aac..00d986c 100644
--- a/transport.c
+++ b/transport.c
@@ -675,7 +675,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
 {
 	if (!count)
-		fprintf(stderr, "To %s\n", dest);
+		fprintf(porcelain ? stdout : stderr, "To %s\n", dest);
 
 	switch(ref->status) {
 	case REF_STATUS_NONE:
@@ -1070,7 +1070,7 @@ int transport_push(struct transport *transport,
 				update_tracking_ref(transport->remote, ref, verbose);
 		}
 
-		if (!quiet && !ret && !refs_pushed(remote_refs))
+		if (!quiet && (!porcelain || verbose) && !ret && !refs_pushed(remote_refs))
 			fprintf(stderr, "Everything up-to-date\n");
 		return ret;
 	}
-- 
1.7.0.rc1.33.g07cf0f.dirty

^ permalink raw reply related

* [PATCH v2 0/3]
From: Larry D'Anna @ 2010-02-05 20:49 UTC (permalink / raw)
  To: git; +Cc: Larry D'Anna
In-Reply-To: <7v1vgz5ta7.fsf@alter.siamese.dyndns.org>

I've fixed his series according to Jeff' and Junio's comments.

Larry D'Anna (3):
  fix an error message in git-push so it goes to stderr
  clean up some of the output from git push --porcelain
  make git push --dry-run --porcelain exit with status 0 even if
    updates will be rejected

 builtin-push.c      |    8 ++++----
 builtin-send-pack.c |    4 ++++
 send-pack.h         |    1 +
 transport.c         |   11 +++++++----
 4 files changed, 16 insertions(+), 8 deletions(-)

^ permalink raw reply

* [PATCH v2 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected
From: Larry D'Anna @ 2010-02-05 20:49 UTC (permalink / raw)
  To: git; +Cc: Larry D'Anna
In-Reply-To: <cover.1265402797.git.larry@elder-gods.org>

The script calling git push --dry-run --porcelain can see clearly from the
output that the updates will be rejected.  However, it will probably need to
distinguish this condition from the push failing for other reasons, such as the
remote not being reachable.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
 builtin-send-pack.c |    4 ++++
 send-pack.h         |    1 +
 transport.c         |    7 +++++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 76c7206..358f5e1 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -476,6 +476,10 @@ int send_pack(struct send_pack_args *args,
 
 	if (ret < 0)
 		return ret;
+
+	if (args->porcelain && args->dry_run)
+		return 0;
+
 	for (ref = remote_refs; ref; ref = ref->next) {
 		switch (ref->status) {
 		case REF_STATUS_NONE:
diff --git a/send-pack.h b/send-pack.h
index 28141ac..60b4ba6 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -4,6 +4,7 @@
 struct send_pack_args {
 	unsigned verbose:1,
 		quiet:1,
+		porcelain:1,
 		send_mirror:1,
 		force_update:1,
 		use_thin_pack:1,
diff --git a/transport.c b/transport.c
index 00d986c..b41e1dc 100644
--- a/transport.c
+++ b/transport.c
@@ -558,8 +558,10 @@ static int fetch_refs_via_pack(struct transport *transport,
 	return (refs ? 0 : -1);
 }
 
-static int push_had_errors(struct ref *ref)
+static int push_had_errors(struct ref *ref, int flags)
 {
+	if (flags & TRANSPORT_PUSH_DRY_RUN && flags & TRANSPORT_PUSH_PORCELAIN)
+		return 0;
 	for (; ref; ref = ref->next) {
 		switch (ref->status) {
 		case REF_STATUS_NONE:
@@ -791,6 +793,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	args.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);
 	args.quiet = !!(flags & TRANSPORT_PUSH_QUIET);
 	args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
+	args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
 
 	ret = send_pack(&args, data->fd, data->conn, remote_refs,
 			&data->extra_have);
@@ -1052,7 +1055,7 @@ int transport_push(struct transport *transport,
 			flags & TRANSPORT_PUSH_FORCE);
 
 		ret = transport->push_refs(transport, remote_refs, flags);
-		err = push_had_errors(remote_refs);
+		err = push_had_errors(remote_refs, flags);
 
 		ret |= err;
 
-- 
1.7.0.rc1.33.g07cf0f.dirty

^ permalink raw reply related

* Re: git gc / git repack not removing unused objects?
From: Nicolas Pitre @ 2010-02-05 20:51 UTC (permalink / raw)
  To: Jon Nelson; +Cc: git
In-Reply-To: <cccedfc61002051145q1ff673e7s3db3bd7290be25e1@mail.gmail.com>

On Fri, 5 Feb 2010, Jon Nelson wrote:

> [Using git 1.6.4.2]
> 
> In one repo I have (136G objects directory, fully packed) I'm having
> some trouble.
> I've run git-gc --prune=now, git repack -Adf, and so on a half-dozen
> times and each time I do so it gets bigger, not smaller.

Please tell us more.

> Setting that aside for the moment, however, I've run into a stranger problem.
> 
> So I use "git verify-pack -v > gvp.out" and "sort -k3nr < gvp.out |
> head -n 20" to find the top 20 largest blobs.
> So I have a blob, b32c3d8e8e24d8d3035cf52f606c2873315fe2b8, and now I
> want to know what tree (or trees) it is in, so I try this:
> 
> 
> for i in $( git branch -a | sed -e 's/\*//g' | grep -v branch ); do if
> git ls-tree -l -r -t $i | grep
> b32c3d8e8e24d8d3035cf52f606c2873315fe2b8 > /dev/null; then echo $i;
> fi; done
> 
> The results: no branch or tree appears to contain that blob.

What you did above is simply to list trees that are reachable from the 
_heads_ of your branches.  If the blob belongs to a commit which isn't 
the latest revision of any of your branches then you won't see it like 
that.

> So I tried a different approach:
> 
> for i in $( grep tree gvp.out  | awk '{ print $1 }' ); do if git
> ls-tree $i | grep b32c3d8e8e24d8d3035cf52f606c2873315fe2b8 >
> /dev/null; then echo $i; fi ; done
> 
> This time, I find (at least) one tree
> (d813af1537358496ca34958bbff08b87590607bf) with the blob.
> But which branches might that tree appear in? None.
> 
> For each branch, I ran "git ls-tree -l -r -t" and saved the output in
> a file (one per branch).
> Then I grepped each file for the tree (
> (d813af1537358496ca34958bbff08b87590607bf) - no luck.
> I grepped each file for the blob (b32...) - no luck.
> 
> The results seem to suggest that I have packed trees which reference
> blobs, but that the trees themselves are not referenced in any branch
> and therefore I would expect that they would be pruned.

NO.  If those trees and blobs are stil there then they do get 
referenced.  But not from the latest commit on any of your branches.  
You need to dig further down in history to find a commit that actually 
references that blob/tree.  One easy method is to do:

	git log --raw --all

and within the pager ('less' by default) simply search for "b32c3d8".


Nicolas

^ permalink raw reply

* [PATCH v2 4/6] receive-pack: Refactor how capabilities are shown to the client
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git
In-Reply-To: <1265403462-20572-1-git-send-email-spearce@spearce.org>

Moving capability advertisement into the packet_write call itself
makes it easier to add additional capabilities to the list, be
it optional by configuration, or always present in the protocol.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-receive-pack.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 78c0e69..325ec6e 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -31,7 +31,7 @@ static int prefer_ofs_delta = 1;
 static int auto_update_server_info;
 static int auto_gc = 1;
 static const char *head_name;
-static char *capabilities_to_send;
+static int sent_capabilities;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -105,19 +105,21 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 
 static int show_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
-	if (!capabilities_to_send)
+	if (sent_capabilities)
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
 	else
-		packet_write(1, "%s %s%c%s\n",
-			     sha1_to_hex(sha1), path, 0, capabilities_to_send);
-	capabilities_to_send = NULL;
+		packet_write(1, "%s %s%c%s%s\n",
+			     sha1_to_hex(sha1), path, 0,
+			     " report-status delete-refs",
+			     prefer_ofs_delta ? " ofs-delta" : "");
+	sent_capabilities = 1;
 	return 0;
 }
 
 static void write_head_info(void)
 {
 	for_each_ref(show_ref, NULL);
-	if (capabilities_to_send)
+	if (!sent_capabilities)
 		show_ref("capabilities^{}", null_sha1, 0, NULL);
 
 }
@@ -670,10 +672,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	else if (0 <= receive_unpack_limit)
 		unpack_limit = receive_unpack_limit;
 
-	capabilities_to_send = (prefer_ofs_delta) ?
-		" report-status delete-refs ofs-delta " :
-		" report-status delete-refs ";
-
 	if (advertise_refs || !stateless_rpc) {
 		add_alternate_refs();
 		write_head_info();
-- 
1.7.0.rc1.199.g9253ab

^ permalink raw reply related

* [PATCH v2 0/6] receive-pack hooks over sideband
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git

This is a respin of the patch I posted yesterday.  Same idea,
we want to make the messages printed by hooks available over the
sideband protocol so git:// and http:// smart clients are able to
present them, similar to how ssh:// works.

No real major changes from v1, the series is just broken apart to
make it easier to review, and Erik Faye-Lund's patch was integrated
to get a bidirectional async interface, rather than my hacked
.is_reader attempt in v1.

Erik Faye-Lund (1):
  run-command: support custom fd-set in async

Shawn O. Pearce (5):
  run-command: Allow stderr to be a caller supplied pipe
  send-pack: demultiplex a sideband stream with status data
  receive-pack: Refactor how capabilities are shown to the client
  receive-pack: Wrap status reports inside side-band-64k
  receive-pack: Send hook output over side band #2

 Documentation/technical/api-run-command.txt |   52 ++++++++++---
 builtin-fetch-pack.c                        |    7 +-
 builtin-receive-pack.c                      |  111 +++++++++++++++++++++------
 builtin-send-pack.c                         |   66 ++++++++++++----
 convert.c                                   |    5 +-
 remote-curl.c                               |    7 +-
 run-command.c                               |   91 +++++++++++++++++++---
 run-command.h                               |   11 ++-
 t/t5401-update-hooks.sh                     |   22 +++---
 upload-pack.c                               |    7 +-
 10 files changed, 290 insertions(+), 89 deletions(-)

^ permalink raw reply

* [PATCH v2 1/6] run-command: Allow stderr to be a caller supplied pipe
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git
In-Reply-To: <1265403462-20572-1-git-send-email-spearce@spearce.org>

Like .out, .err may now be set to a file descriptor > 0, which
is a writable pipe/socket/file that the child's stderr will be
redirected into.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/technical/api-run-command.txt |    2 +-
 run-command.c                               |    8 ++++++++
 run-command.h                               |    2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
index b26c281..a1280dd 100644
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -135,7 +135,7 @@ stderr as follows:
 
 	.in: The FD must be readable; it becomes child's stdin.
 	.out: The FD must be writable; it becomes child's stdout.
-	.err > 0 is not supported.
+	.err: The FD must be writable; it becomes child's stderr.
 
   The specified FD is closed by start_command(), even if it fails to
   run the sub-process!
diff --git a/run-command.c b/run-command.c
index cf2d8f7..bfd2312 100644
--- a/run-command.c
+++ b/run-command.c
@@ -94,6 +94,9 @@ fail_pipe:
 		else if (need_err) {
 			dup2(fderr[1], 2);
 			close_pair(fderr);
+		} else if (cmd->err > 1) {
+			dup2(cmd->err, 2);
+			close(cmd->err);
 		}
 
 		if (cmd->no_stdout)
@@ -156,6 +159,9 @@ fail_pipe:
 	} else if (need_err) {
 		s2 = dup(2);
 		dup2(fderr[1], 2);
+	} else if (cmd->err > 2) {
+		s2 = dup(2);
+		dup2(cmd->err, 2);
 	}
 
 	if (cmd->no_stdout) {
@@ -228,6 +234,8 @@ fail_pipe:
 
 	if (need_err)
 		close(fderr[1]);
+	else if (cmd->err)
+		close(cmd->err);
 
 	return 0;
 }
diff --git a/run-command.h b/run-command.h
index fb34209..a29171a 100644
--- a/run-command.h
+++ b/run-command.h
@@ -18,7 +18,7 @@ struct child_process {
 	 * - Specify > 0 to set a channel to a particular FD as follows:
 	 *     .in: a readable FD, becomes child's stdin
 	 *     .out: a writable FD, becomes child's stdout/stderr
-	 *     .err > 0 not supported
+	 *     .err: a writable FD, becomes child's stderr
 	 *   The specified FD is closed by start_command(), even in case
 	 *   of errors!
 	 */
-- 
1.7.0.rc1.199.g9253ab

^ permalink raw reply related

* [PATCH v2 2/6] run-command: support custom fd-set in async
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund
In-Reply-To: <1265403462-20572-1-git-send-email-spearce@spearce.org>

From: Erik Faye-Lund <kusmabite@gmail.com>

This patch adds the possibility to supply a set of non-0 file
descriptors for async process communication instead of the
default-created pipe.

Additionally, we now support bi-directional communiction with the
async procedure, by giving the async function both read and write
file descriptors.

To retain compatiblity and similar "API feel" with start_command,
we require start_async callers to set .out = -1 to get a readable
file descriptor.  If either of .in or .out is 0, we supply no file
descriptor to the async process.

[sp: Note: Erik started this patch, and a huge bulk of it is
     his work.  All bugs were introduced later by Shawn.]

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/technical/api-run-command.txt |   50 +++++++++++++---
 builtin-fetch-pack.c                        |    7 +-
 convert.c                                   |    5 +-
 remote-curl.c                               |    7 +-
 run-command.c                               |   83 ++++++++++++++++++++++----
 run-command.h                               |    9 ++-
 upload-pack.c                               |    7 +-
 7 files changed, 131 insertions(+), 37 deletions(-)

diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
index a1280dd..8994859 100644
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -64,8 +64,8 @@ The functions above do the following:
 `start_async`::
 
 	Run a function asynchronously. Takes a pointer to a `struct
-	async` that specifies the details and returns a pipe FD
-	from which the caller reads. See below for details.
+	async` that specifies the details and returns a set of pipe FDs
+	for communication with the function. See below for details.
 
 `finish_async`::
 
@@ -180,17 +180,47 @@ The caller:
    struct async variable;
 2. initializes .proc and .data;
 3. calls start_async();
-4. processes the data by reading from the fd in .out;
-5. closes .out;
+4. processes communicates with proc through .in and .out;
+5. closes .in and .out;
 6. calls finish_async().
 
+The members .in, .out are used to provide a set of fd's for
+communication between the caller and the callee as follows:
+
+. Specify 0 to have no file descriptor passed.  The callee will
+  receive -1 in the corresponding argument.
+
+. Specify < 0 to have a pipe allocated; start_async() replaces
+  with the pipe FD in the following way:
+
+	.in: Returns the writable pipe end into which the caller
+	writes; the readable end of the pipe becomes the function's
+	in argument.
+
+	.out: Returns the readable pipe end from which the caller
+	reads; the writable end of the pipe becomes the function's
+	out argument.
+
+  The caller of start_async() must close the returned FDs after it
+  has completed reading from/writing from them.
+
+. Specify a file descriptor > 0 to be used by the function:
+
+	.in: The FD must be readable; it becomes the function's in.
+	.out: The FD must be writable; it becomes the function's out.
+
+  The specified FD is closed by start_async(), even if it fails to
+  run the function.
+
 The function pointer in .proc has the following signature:
 
-	int proc(int fd, void *data);
+	int proc(int in, int out, void *data);
 
-. fd specifies a writable file descriptor to which the function must
-  write the data that it produces. The function *must* close this
-  descriptor before it returns.
+. in, out specifies a set of file descriptors to which the function
+  must read/write the data that it needs/produces.  The function
+  *must* close these descriptors before it returns.  A descriptor
+  may be -1 if the caller did not configure a descriptor for that
+  direction.
 
 . data is the value that the caller has specified in the .data member
   of struct async.
@@ -205,8 +235,8 @@ because this facility is implemented by a pipe to a forked process on
 UNIX, but by a thread in the same address space on Windows:
 
 . It cannot change the program's state (global variables, environment,
-  etc.) in a way that the caller notices; in other words, .out is the
-  only communication channel to the caller.
+  etc.) in a way that the caller notices; in other words, .in and .out
+  are the only communication channels to the caller.
 
 . It must not change the program's state that the caller of the
   facility also uses.
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 8ed4a6f..dbd8b7b 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -586,12 +586,12 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
 	return retval;
 }
 
-static int sideband_demux(int fd, void *data)
+static int sideband_demux(int in, int out, void *data)
 {
 	int *xd = data;
 
-	int ret = recv_sideband("fetch-pack", xd[0], fd);
-	close(fd);
+	int ret = recv_sideband("fetch-pack", xd[0], out);
+	close(out);
 	return ret;
 }
 
@@ -613,6 +613,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
 		 */
 		demux.proc = sideband_demux;
 		demux.data = xd;
+		demux.out = -1;
 		if (start_async(&demux))
 			die("fetch-pack: unable to fork off sideband"
 			    " demultiplexer");
diff --git a/convert.c b/convert.c
index 491e714..e70ee09 100644
--- a/convert.c
+++ b/convert.c
@@ -241,7 +241,7 @@ struct filter_params {
 	const char *cmd;
 };
 
-static int filter_buffer(int fd, void *data)
+static int filter_buffer(int in, int out, void *data)
 {
 	/*
 	 * Spawn cmd and feed the buffer contents through its stdin.
@@ -254,7 +254,7 @@ static int filter_buffer(int fd, void *data)
 	memset(&child_process, 0, sizeof(child_process));
 	child_process.argv = argv;
 	child_process.in = -1;
-	child_process.out = fd;
+	child_process.out = out;
 
 	if (start_command(&child_process))
 		return error("cannot fork to run external filter %s", params->cmd);
@@ -291,6 +291,7 @@ static int apply_filter(const char *path, const char *src, size_t len,
 	memset(&async, 0, sizeof(async));
 	async.proc = filter_buffer;
 	async.data = &params;
+	async.out = -1;
 	params.src = src;
 	params.size = len;
 	params.cmd = cmd;
diff --git a/remote-curl.c b/remote-curl.c
index 3edbf57..6bb3366 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -184,13 +184,13 @@ static struct discovery* discover_refs(const char *service)
 	return last;
 }
 
-static int write_discovery(int fd, void *data)
+static int write_discovery(int in, int out, void *data)
 {
 	struct discovery *heads = data;
 	int err = 0;
-	if (write_in_full(fd, heads->buf, heads->len) != heads->len)
+	if (write_in_full(out, heads->buf, heads->len) != heads->len)
 		err = 1;
-	close(fd);
+	close(out);
 	return err;
 }
 
@@ -202,6 +202,7 @@ static struct ref *parse_git_refs(struct discovery *heads)
 	memset(&async, 0, sizeof(async));
 	async.proc = write_discovery;
 	async.data = heads;
+	async.out = -1;
 
 	if (start_async(&async))
 		die("cannot start thread to parse advertised refs");
diff --git a/run-command.c b/run-command.c
index bfd2312..0d95340 100644
--- a/run-command.c
+++ b/run-command.c
@@ -327,17 +327,51 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
 static unsigned __stdcall run_thread(void *data)
 {
 	struct async *async = data;
-	return async->proc(async->fd_for_proc, async->data);
+	return async->proc(async->proc_in, async->proc_out, async->data);
 }
 #endif
 
 int start_async(struct async *async)
 {
-	int pipe_out[2];
+	int need_in, need_out;
+	int fdin[2], fdout[2];
+	int proc_in, proc_out;
 
-	if (pipe(pipe_out) < 0)
-		return error("cannot create pipe: %s", strerror(errno));
-	async->out = pipe_out[0];
+	need_in = async->in < 0;
+	if (need_in) {
+		if (pipe(fdin) < 0) {
+			if (async->out > 0)
+				close(async->out);
+			return error("cannot create pipe: %s", strerror(errno));
+		}
+		async->in = fdin[1];
+	}
+
+	need_out = async->out < 0;
+	if (need_out) {
+		if (pipe(fdout) < 0) {
+			if (need_in)
+				close_pair(fdin);
+			else if (async->in)
+				close(async->in);
+			return error("cannot create pipe: %s", strerror(errno));
+		}
+		async->out = fdout[0];
+	}
+
+	if (need_in)
+		proc_in = fdin[0];
+	else if (async->in)
+		proc_in = async->in;
+	else
+		proc_in = -1;
+
+	if (need_out)
+		proc_out = fdout[1];
+	else if (async->out)
+		proc_out = async->out;
+	else
+		proc_out = -1;
 
 #ifndef WIN32
 	/* Flush stdio before fork() to avoid cloning buffers */
@@ -346,24 +380,47 @@ int start_async(struct async *async)
 	async->pid = fork();
 	if (async->pid < 0) {
 		error("fork (async) failed: %s", strerror(errno));
-		close_pair(pipe_out);
-		return -1;
+		goto error;
 	}
 	if (!async->pid) {
-		close(pipe_out[0]);
-		exit(!!async->proc(pipe_out[1], async->data));
+		if (need_in)
+			close(fdin[1]);
+		if (need_out)
+			close(fdout[0]);
+		exit(!!async->proc(proc_in, proc_out, async->data));
 	}
-	close(pipe_out[1]);
+
+	if (need_in)
+		close(fdin[0]);
+	else if (async->in)
+		close(async->in);
+
+	if (need_out)
+		close(fdout[1]);
+	else if (async->out)
+		close(async->out);
 #else
-	async->fd_for_proc = pipe_out[1];
+	async->proc_in = proc_in;
+	async->proc_out = proc_out;
 	async->tid = (HANDLE) _beginthreadex(NULL, 0, run_thread, async, 0, NULL);
 	if (!async->tid) {
 		error("cannot create thread: %s", strerror(errno));
-		close_pair(pipe_out);
-		return -1;
+		goto error;
 	}
 #endif
 	return 0;
+
+error:
+	if (need_in)
+		close_pair(fdin);
+	else if (async->in)
+		close(async->in);
+
+	if (need_out)
+		close_pair(fdout);
+	else if (async->out)
+		close(async->out);
+	return -1;
 }
 
 int finish_async(struct async *async)
diff --git a/run-command.h b/run-command.h
index a29171a..65ccb1c 100644
--- a/run-command.h
+++ b/run-command.h
@@ -64,17 +64,20 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
  */
 struct async {
 	/*
-	 * proc writes to fd and closes it;
+	 * proc reads from in; closes it before return
+	 * proc writes to out; closes it before return
 	 * returns 0 on success, non-zero on failure
 	 */
-	int (*proc)(int fd, void *data);
+	int (*proc)(int in, int out, void *data);
 	void *data;
+	int in;		/* caller writes here and closes it */
 	int out;	/* caller reads from here and closes it */
 #ifndef WIN32
 	pid_t pid;
 #else
 	HANDLE tid;
-	int fd_for_proc;
+	int proc_in;
+	int proc_out;
 #endif
 };
 
diff --git a/upload-pack.c b/upload-pack.c
index df15181..dc464d7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -105,12 +105,12 @@ static void show_edge(struct commit *commit)
 	fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
 }
 
-static int do_rev_list(int fd, void *create_full_pack)
+static int do_rev_list(int in, int out, void *create_full_pack)
 {
 	int i;
 	struct rev_info revs;
 
-	pack_pipe = xfdopen(fd, "w");
+	pack_pipe = xfdopen(out, "w");
 	init_revisions(&revs, NULL);
 	revs.tag_objects = 1;
 	revs.tree_objects = 1;
@@ -162,8 +162,9 @@ static void create_pack_file(void)
 	int arg = 0;
 
 	if (shallow_nr) {
+		memset(&rev_list, 0, sizeof(rev_list));
 		rev_list.proc = do_rev_list;
-		rev_list.data = 0;
+		rev_list.out = -1;
 		if (start_async(&rev_list))
 			die("git upload-pack: unable to fork git-rev-list");
 		argv[arg++] = "pack-objects";
-- 
1.7.0.rc1.199.g9253ab

^ permalink raw reply related

* Re: git gc / git repack not removing unused objects?
From: Jon Nelson @ 2010-02-05 21:04 UTC (permalink / raw)
  Cc: git
In-Reply-To: <alpine.LFD.2.00.1002051539080.1681@xanadu.home>

On Fri, Feb 5, 2010 at 2:51 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Fri, 5 Feb 2010, Jon Nelson wrote:
>
>> [Using git 1.6.4.2]
>>
>> In one repo I have (136G objects directory, fully packed) I'm having
>> some trouble.
>> I've run git-gc --prune=now, git repack -Adf, and so on a half-dozen
>> times and each time I do so it gets bigger, not smaller.
>
> Please tell us more.

I'll tell you whatever I can -- as soon as I know what it is you want.

>> Setting that aside for the moment, however, I've run into a stranger problem.
>>
>> So I use "git verify-pack -v > gvp.out" and "sort -k3nr < gvp.out |
>> head -n 20" to find the top 20 largest blobs.
>> So I have a blob, b32c3d8e8e24d8d3035cf52f606c2873315fe2b8, and now I
>> want to know what tree (or trees) it is in, so I try this:
>>
>>
>> for i in $( git branch -a | sed -e 's/\*//g' | grep -v branch ); do if
>> git ls-tree -l -r -t $i | grep
>> b32c3d8e8e24d8d3035cf52f606c2873315fe2b8 > /dev/null; then echo $i;
>> fi; done
>>
>> The results: no branch or tree appears to contain that blob.
>
> What you did above is simply to list trees that are reachable from the
> _heads_ of your branches.  If the blob belongs to a commit which isn't
> the latest revision of any of your branches then you won't see it like
> that.
>
>> So I tried a different approach:
>>
>> for i in $( grep tree gvp.out  | awk '{ print $1 }' ); do if git
>> ls-tree $i | grep b32c3d8e8e24d8d3035cf52f606c2873315fe2b8 >
>> /dev/null; then echo $i; fi ; done
>>
>> This time, I find (at least) one tree
>> (d813af1537358496ca34958bbff08b87590607bf) with the blob.
>> But which branches might that tree appear in? None.
>>
>> For each branch, I ran "git ls-tree -l -r -t" and saved the output in
>> a file (one per branch).
>> Then I grepped each file for the tree (
>> (d813af1537358496ca34958bbff08b87590607bf) - no luck.
>> I grepped each file for the blob (b32...) - no luck.
>>
>> The results seem to suggest that I have packed trees which reference
>> blobs, but that the trees themselves are not referenced in any branch
>> and therefore I would expect that they would be pruned.
>
> NO.  If those trees and blobs are stil there then they do get
> referenced.  But not from the latest commit on any of your branches.
> You need to dig further down in history to find a commit that actually
> references that blob/tree.  One easy method is to do:
>
>        git log --raw --all
>
> and within the pager ('less' by default) simply search for "b32c3d8".

OK. I'm piping "git log --raw --all" to a file this very moment. It'll
take a while. However, one thing I did not mention is that there
*should* be a 1:1 correlation between branches and commits. As in,
every time I did a commit, the commit was on a new branch. I'll look
into this, as I've fiddled with the repo a bunch of different ways
lately. I suspect the answer will be found in the logs.

Thanks for the response!

-- 
Jon

^ permalink raw reply

* Re: [PATCH v2 2/3] clean up some of the output from git push --porcelain
From: Junio C Hamano @ 2010-02-05 21:07 UTC (permalink / raw)
  To: Larry D'Anna; +Cc: git
In-Reply-To: <ec0db70b8166ee4093a5a2400cd592e4263db20b.1265402797.git.larry@elder-gods.org>

Larry D'Anna <larry@elder-gods.org> writes:

> diff --git a/builtin-push.c b/builtin-push.c
> index 0a27072..ff0b1c6 100644
> --- a/builtin-push.c
> +++ b/builtin-push.c
> @@ -123,7 +123,7 @@ static int push_with_options(struct transport *transport, int flags)
>  	if (!err)
>  		return 0;
>  
> -	if (nonfastforward && advice_push_nonfastforward) {
> +	if (!(flags & TRANSPORT_PUSH_PORCELAIN) && nonfastforward && advice_push_nonfastforward) {

I suspect that it would be a much nicer solution if you turned advice_*
off in very early parts of the codepath after you read configuration and
command line options to see if --porcelain was asked for.  That way, we
don't have to add code check for PORCELAIN next time we add code for new
kinds of advice.  People who are adding new advice_* would grep for
existing one (e.g. "advice_push_nonfastforward") and will notice this
line, but if you had

	if (flags & TRANSPORT_PUSH_PORDCELAIN) {
        	advice_push_nonfastforward = 0;
	}

in early parts of the code, they will notice that as well, and then they
know where to add the corresponding change necessary.

^ permalink raw reply

* [PATCH v2 3/6] send-pack: demultiplex a sideband stream with status data
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git
In-Reply-To: <1265403462-20572-1-git-send-email-spearce@spearce.org>

If the server advertises side-band-64k capability, we request
it and pull the status report data out of side band #1, and let
side band #2 go to our stderr.  The latter channel be used by the
remote side to send our user messages.  This basically mirrors the
side-band-64k capability in upload-pack.

Servers may choose to use side band #2 to send error messages from
hook scripts that are meant for the push end user.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-send-pack.c |   66 ++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8fffdbf..2478e18 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -372,6 +372,14 @@ static void print_helper_status(struct ref *ref)
 	strbuf_release(&buf);
 }
 
+static int sideband_demux(int in, int out, void *data)
+{
+	int *fd = data;
+	int ret = recv_sideband("send-pack", fd[0], out);
+	close(out);
+	return ret;
+}
+
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
@@ -382,18 +390,22 @@ int send_pack(struct send_pack_args *args,
 	struct strbuf req_buf = STRBUF_INIT;
 	struct ref *ref;
 	int new_refs;
-	int ask_for_status_report = 0;
 	int allow_deleting_refs = 0;
-	int expect_status_report = 0;
+	int status_report = 0;
+	int use_sideband = 0;
+	unsigned cmds_sent = 0;
 	int ret;
+	struct async demux;
 
 	/* Does the other end support the reporting? */
 	if (server_supports("report-status"))
-		ask_for_status_report = 1;
+		status_report = 1;
 	if (server_supports("delete-refs"))
 		allow_deleting_refs = 1;
 	if (server_supports("ofs-delta"))
 		args->use_ofs_delta = 1;
+	if (server_supports("side-band-64k"))
+		use_sideband = 1;
 
 	if (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
@@ -456,28 +468,30 @@ int send_pack(struct send_pack_args *args,
 		if (!ref->deletion)
 			new_refs++;
 
-		if (!args->dry_run) {
+		if (args->dry_run) {
+			ref->status = REF_STATUS_OK;
+		} else {
 			char *old_hex = sha1_to_hex(ref->old_sha1);
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 
-			if (ask_for_status_report) {
-				packet_buf_write(&req_buf, "%s %s %s%c%s",
+			if (!cmds_sent && (status_report || use_sideband)) {
+				packet_buf_write(&req_buf, "%s %s %s%c%s%s",
 					old_hex, new_hex, ref->name, 0,
-					"report-status");
-				ask_for_status_report = 0;
-				expect_status_report = 1;
+					status_report ? " report-status" : "",
+					use_sideband ? " side-band-64k" : "");
 			}
 			else
 				packet_buf_write(&req_buf, "%s %s %s",
 					old_hex, new_hex, ref->name);
+			ref->status = status_report ?
+				REF_STATUS_EXPECTING_REPORT :
+				REF_STATUS_OK;
+			cmds_sent++;
 		}
-		ref->status = expect_status_report ?
-			REF_STATUS_EXPECTING_REPORT :
-			REF_STATUS_OK;
 	}
 
 	if (args->stateless_rpc) {
-		if (!args->dry_run) {
+		if (!args->dry_run && cmds_sent) {
 			packet_buf_flush(&req_buf);
 			send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
 		}
@@ -487,23 +501,43 @@ int send_pack(struct send_pack_args *args,
 	}
 	strbuf_release(&req_buf);
 
-	if (new_refs && !args->dry_run) {
+	if (use_sideband && cmds_sent) {
+		memset(&demux, 0, sizeof(demux));
+		demux.proc = sideband_demux;
+		demux.data = fd;
+		demux.out = -1;
+		if (start_async(&demux))
+			die("receive-pack: unable to fork off sideband demultiplexer");
+		in = demux.out;
+	}
+
+	if (new_refs && cmds_sent) {
 		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
 			for (ref = remote_refs; ref; ref = ref->next)
 				ref->status = REF_STATUS_NONE;
+			if (use_sideband)
+				finish_async(&demux);
 			return -1;
 		}
 	}
-	if (args->stateless_rpc && !args->dry_run)
+	if (args->stateless_rpc && cmds_sent)
 		packet_flush(out);
 
-	if (expect_status_report)
+	if (status_report && cmds_sent)
 		ret = receive_status(in, remote_refs);
 	else
 		ret = 0;
 	if (args->stateless_rpc)
 		packet_flush(out);
 
+	if (use_sideband && cmds_sent) {
+		if (finish_async(&demux)) {
+			error("error in sideband demultiplexer");
+			ret = -1;
+		}
+		close(demux.out);
+	}
+
 	if (ret < 0)
 		return ret;
 	for (ref = remote_refs; ref; ref = ref->next) {
-- 
1.7.0.rc1.199.g9253ab

^ permalink raw reply related

* [PATCH v2 6/6] receive-pack: Send hook output over side band #2
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git
In-Reply-To: <1265403462-20572-1-git-send-email-spearce@spearce.org>

If the client requests to enable side-band-64k capability we can
safely send any hook stdout or stderr data down side band #2,
so the client can present it to the user.

If side-band-64k isn't enabled, hooks continue to inherit stderr
from the parent receive-pack process.

When the side band channel is being used the push client will wind up
prefixing all server messages with "remote: ", just like fetch does,
so our test vector has to be updated with the new expected output.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-receive-pack.c  |   65 ++++++++++++++++++++++++++++++++++++++++++----
 t/t5401-update-hooks.sh |   22 ++++++++--------
 2 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index ff3f117..da1c26b 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -139,11 +139,25 @@ static struct command *commands;
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
+static int copy_to_sideband(int in, int out, void *arg)
+{
+	char data[128];
+	while (1) {
+		ssize_t sz = xread(in, data, sizeof(data));
+		if (sz <= 0)
+			break;
+		send_sideband(1, 2, data, sz, use_sideband);
+	}
+	close(in);
+	return 0;
+}
+
 static int run_receive_hook(const char *hook_name)
 {
 	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
 	struct command *cmd;
 	struct child_process proc;
+	struct async muxer;
 	const char *argv[2];
 	int have_input = 0, code;
 
@@ -163,9 +177,23 @@ static int run_receive_hook(const char *hook_name)
 	proc.in = -1;
 	proc.stdout_to_stderr = 1;
 
+	if (use_sideband) {
+		memset(&muxer, 0, sizeof(muxer));
+		muxer.proc = copy_to_sideband;
+		muxer.in = -1;
+		code = start_async(&muxer);
+		if (code)
+			return code;
+		proc.err = muxer.in;
+	}
+
 	code = start_command(&proc);
-	if (code)
+	if (code) {
+		if (use_sideband)
+			finish_async(&muxer);
 		return code;
+	}
+
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string) {
 			size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
@@ -177,6 +205,8 @@ static int run_receive_hook(const char *hook_name)
 		}
 	}
 	close(proc.in);
+	if (use_sideband)
+		finish_async(&muxer);
 	return finish_command(&proc);
 }
 
@@ -184,6 +214,8 @@ static int run_update_hook(struct command *cmd)
 {
 	static const char update_hook[] = "hooks/update";
 	const char *argv[5];
+	struct child_process proc;
+	int code;
 
 	if (access(update_hook, X_OK) < 0)
 		return 0;
@@ -194,8 +226,18 @@ static int run_update_hook(struct command *cmd)
 	argv[3] = sha1_to_hex(cmd->new_sha1);
 	argv[4] = NULL;
 
-	return run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |
-					RUN_COMMAND_STDOUT_TO_STDERR);
+	memset(&proc, 0, sizeof(proc));
+	proc.no_stdin = 1;
+	proc.stdout_to_stderr = 1;
+	proc.err = use_sideband ? -1 : 0;
+	proc.argv = argv;
+
+	code = start_command(&proc);
+	if (code)
+		return code;
+	if (use_sideband)
+		copy_to_sideband(proc.err, -1, NULL);
+	return finish_command(&proc);
 }
 
 static int is_ref_checked_out(const char *ref)
@@ -384,8 +426,9 @@ static char update_post_hook[] = "hooks/post-update";
 static void run_update_post_hook(struct command *cmd)
 {
 	struct command *cmd_p;
-	int argc, status;
+	int argc;
 	const char **argv;
+	struct child_process proc;
 
 	for (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
 		if (cmd_p->error_string)
@@ -407,8 +450,18 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
-			| RUN_COMMAND_STDOUT_TO_STDERR);
+
+	memset(&proc, 0, sizeof(proc));
+	proc.no_stdin = 1;
+	proc.stdout_to_stderr = 1;
+	proc.err = use_sideband ? -1 : 0;
+	proc.argv = argv;
+
+	if (!start_command(&proc)) {
+		if (use_sideband)
+			copy_to_sideband(proc.err, -1, NULL);
+		finish_command(&proc);
+	}
 }
 
 static void execute_commands(const char *unpacker_error)
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 64f66c9..c3cf397 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -118,19 +118,19 @@ test_expect_success 'send-pack produced no output' '
 '
 
 cat <<EOF >expect
-STDOUT pre-receive
-STDERR pre-receive
-STDOUT update refs/heads/master
-STDERR update refs/heads/master
-STDOUT update refs/heads/tofail
-STDERR update refs/heads/tofail
-STDOUT post-receive
-STDERR post-receive
-STDOUT post-update
-STDERR post-update
+remote: STDOUT pre-receive
+remote: STDERR pre-receive
+remote: STDOUT update refs/heads/master
+remote: STDERR update refs/heads/master
+remote: STDOUT update refs/heads/tofail
+remote: STDERR update refs/heads/tofail
+remote: STDOUT post-receive
+remote: STDERR post-receive
+remote: STDOUT post-update
+remote: STDERR post-update
 EOF
 test_expect_success 'send-pack stderr contains hook messages' '
-	grep ^STD send.err >actual &&
+	grep ^remote: send.err | sed "s/ *\$//" >actual &&
 	test_cmp - actual <expect
 '
 
-- 
1.7.0.rc1.199.g9253ab

^ permalink raw reply related

* [PATCH v2 5/6] receive-pack: Wrap status reports inside side-band-64k
From: Shawn O. Pearce @ 2010-02-05 20:57 UTC (permalink / raw)
  To: git
In-Reply-To: <1265403462-20572-1-git-send-email-spearce@spearce.org>

If the client requests the side-band-64k protocol capability we
now wrap the status report data inside of packets sent to band #1.
This permits us to later send additional progress or informational
messages down band #2.

If side-band-64k was enabled, we always send a final flush packet
to let the client know we are done transmitting.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-receive-pack.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 325ec6e..ff3f117 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -2,6 +2,7 @@
 #include "pack.h"
 #include "refs.h"
 #include "pkt-line.h"
+#include "sideband.h"
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "commit.h"
@@ -27,6 +28,7 @@ static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
 static int unpack_limit = 100;
 static int report_status;
+static int use_sideband;
 static int prefer_ofs_delta = 1;
 static int auto_update_server_info;
 static int auto_gc = 1;
@@ -110,7 +112,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
 	else
 		packet_write(1, "%s %s%c%s%s\n",
 			     sha1_to_hex(sha1), path, 0,
-			     " report-status delete-refs",
+			     " report-status delete-refs side-band-64k",
 			     prefer_ofs_delta ? " ofs-delta" : "");
 	sent_capabilities = 1;
 	return 0;
@@ -466,6 +468,8 @@ static void read_head_info(void)
 		if (reflen + 82 < len) {
 			if (strstr(refname + reflen + 1, "report-status"))
 				report_status = 1;
+			if (strstr(refname + reflen + 1, "side-band-64k"))
+				use_sideband = LARGE_PACKET_MAX;
 		}
 		cmd = xmalloc(sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
@@ -565,17 +569,25 @@ static const char *unpack(void)
 static void report(const char *unpack_status)
 {
 	struct command *cmd;
-	packet_write(1, "unpack %s\n",
-		     unpack_status ? unpack_status : "ok");
+	struct strbuf buf = STRBUF_INIT;
+
+	packet_buf_write(&buf, "unpack %s\n",
+			 unpack_status ? unpack_status : "ok");
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string)
-			packet_write(1, "ok %s\n",
-				     cmd->ref_name);
+			packet_buf_write(&buf, "ok %s\n",
+					 cmd->ref_name);
 		else
-			packet_write(1, "ng %s %s\n",
-				     cmd->ref_name, cmd->error_string);
+			packet_buf_write(&buf, "ng %s %s\n",
+					 cmd->ref_name, cmd->error_string);
 	}
-	packet_flush(1);
+	packet_buf_flush(&buf);
+
+	if (use_sideband)
+		send_sideband(1, 1, buf.buf, buf.len, use_sideband);
+	else
+		safe_write(1, buf.buf, buf.len);
+	strbuf_release(&buf);
 }
 
 static int delete_only(struct command *cmd)
@@ -705,5 +717,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		if (auto_update_server_info)
 			update_server_info(0);
 	}
+	if (use_sideband)
+		packet_flush(1);
 	return 0;
 }
-- 
1.7.0.rc1.199.g9253ab

^ permalink raw reply related

* Re: [PATCH v2 5/6] receive-pack: Wrap status reports inside side-band-64k
From: Junio C Hamano @ 2010-02-05 21:14 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1265403462-20572-6-git-send-email-spearce@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> If the client requests the side-band-64k protocol capability we
> now wrap the status report data inside of packets sent to band #1.
> This permits us to later send additional progress or informational
> messages down band #2.
>
> If side-band-64k was enabled, we always send a final flush packet
> to let the client know we are done transmitting.

Two questions.

 - Why does use_sideband, the variable with the same name as a boolean
   variable used by other parts of the system to decide whether we should
   or should not use the sideband communiocation, get a value other than 0
   or 1?  What is the benefit of using it to keep an actual value?  Does
   the benefit outweigh the confusion factor?

 - What happens if client wants only side-band, not 64k?  This is just
   theoretical and "we don't bother" is a perfectly acceptable answer.  I
   am just curious ;-).

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Giuseppe Bilotta @ 2010-02-05 21:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johan Herland, Jakub Narebski, git, Johannes Schindelin
In-Reply-To: <7veikzfysv.fsf@alter.siamese.dyndns.org>

On Fri, Feb 5, 2010 at 5:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> * format-patch
>
> To add notes to format-patch output, we might want to do something like:
>
>    $ git format-patch --notes-ref=commits --notes-ref=amlog -1
>
> and produce:
>
>    From 8bff7c5383ed833bd1df9c8d85c00a27af3e5b02 Mon Sep 17 00:00:00 2001
>    From: Andrew Myrick <amyrick@apple.com>
>    Date: Sat, 30 Jan 2010 03:14:22 +0000
>    Subject: [PATCH] git-svn: persistent memoization
>    X-Notes: pulled on Fri Feb 5 07:36:12 2010 -0800
>     from git://git.bogomips.org/git-svn.git/
>    X-Notes-amlog: <1264821262-28322-1-git-send-email-amyrick@apple.com>

+1 for header style note exporting. X-Git-Notes might be better.

> Points to notice:
>
>  - There is no point forcing users to spell "--notes-ref" parameter
>   starting from refs/notes/; we should DWIM if they are missing;

Agreed.

>  - We would want to allow more than one notes hierarchy specified. This
>   would affect format_note() function---take list of struct notes_tree,
>   perhaps;

I get the impression everybody wants this ;-)

>  - Allow callers of tell format_note() to add the name of the notes
>   hierarchy the note came from (or just always add it if it is not the
>   default "refs/notes/commits").

This is probably the most bothering issue: find, for each output
format that involves notes, the smart way of also outputting the
namespace it came from.

>  - For format-patch that produces a mbox output, the email header part may
>   be a better place to put notes (obeying the usual "indent by one space
>   to continue the line" convention).

How would you cope with multi-line notes? One X-Git-Notes header per line?

> * "log --format=%N" and "log --show-notes"
>
> Currently %N expands to the hardcoded "log --show-notes" default format.
> We can probably keep it that way.  When the user asked for a non default
> notes hierarchy (i.e. other than refs/notes/commits), we may want to
> adjust "Notes:" string to use "Notes-%s:" to show which hierarchy it came
> from, and concatenate them together.

We might want to do without the dash in standard log output: Notes:
and Notes <namespace>:

> For "log --show-notes" output, we also might want to move the notes to the
> header part like I illustrated above in format-patch output, instead of
> "start with unindented Notes: and indented body at the end".  I.e. instead
> of showing this:
>
>    $ git log --notes-ref=amlog -1 4d0cc22
>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070
>    Author: Junio C Hamano <gitster@pobox.com>
>    Date:   Thu Feb 4 11:10:44 2010 -0800
>
>        fast-import: count --max-pack-size in bytes
>
>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git
>        ...
>        Acked-by: Nicolas Pitre <nico@fluxnic.net>
>
>    Notes:
>        <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>
>
>
> show it like this:
>
>    $ git log --notes-ref=amlog -1 4d0cc22
>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070
>    Author: Junio C Hamano <gitster@pobox.com>
>    Date:   Thu Feb 4 11:10:44 2010 -0800
>    Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>
>
>        fast-import: count --max-pack-size in bytes
>
>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git
>        ...
>        Acked-by: Nicolas Pitre <nico@fluxnic.net>

The footer approach has the benefit of allowing multi-line notes to
just be printed the same way as multi-line commit messages, whereas
the header output would require one header line per commit line.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Larry D'Anna @ 2010-02-05 21:39 UTC (permalink / raw)
  To: git; +Cc: Larry D'Anna

This is useful because if you're preparing a patch series with a cover letter
you can easily put together one line to format and email the whole thing to
yourself.  You check to make sure everything is right, and then just change the
recipient address and run it again.

git send-email --to my@mydomain.org  master..HEAD --cover-letter \
    --cover-subject "this is my patch series" --cover-blurb "$(cat blurb.txt)"

check the results in my inbox

git send-email --to git@vger.kernel.org  master..HEAD --cover-letter \
    --cover-subject "this is my patch series" --cover-blurb "$(cat blurb.txt)"

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
 Documentation/git-format-patch.txt |    8 ++++++++
 builtin-log.c                      |   15 +++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 9674f9d..522c56f 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -176,6 +176,14 @@ will want to ensure that threading is disabled for `git send-email`.
 	containing the shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--cover-subject=<subject>
+	Instead of using *** SUBJECT HERE ***, specify the subject line of the
+	cover letter.
+
+--cover-blurb=<blurb>
+	Instead of using *** BLURB HERE ***, specify a blurb for the body of the
+	cover letter.
+
 --suffix=.<sfx>::
 	Instead of using `.patch` as the suffix for generated
 	filenames, use specified suffix.  A common alternative is
diff --git a/builtin-log.c b/builtin-log.c
index 8d16832..e7ae37e 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -452,6 +452,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 
 /* format-patch */
 
+static const char *cover_subject = "*** SUBJECT HERE ***";
+static const char *cover_blurb = "*** BLURB HERE ***";
 static const char *fmt_patch_suffix = ".patch";
 static int numbered = 0;
 static int auto_number = 1;
@@ -647,7 +649,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 {
 	const char *committer;
 	const char *subject_start = NULL;
-	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
 	const char *msg;
 	const char *extra_headers = rev->extra_headers;
 	struct shortlog log;
@@ -695,12 +696,15 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 		if (has_non_ascii(list[i]->buffer))
 			need_8bit_cte = 1;
 
-	msg = body;
 	pp_user_info(NULL, CMIT_FMT_EMAIL, &sb, committer, DATE_RFC2822,
 		     encoding);
+
+	msg = cover_subject;
 	pp_title_line(CMIT_FMT_EMAIL, &msg, &sb, subject_start, extra_headers,
 		      encoding, need_8bit_cte);
+	msg = cover_blurb;
 	pp_remainder(CMIT_FMT_EMAIL, &msg, &sb, 0);
+
 	printf("%s\n", sb.buf);
 
 	strbuf_release(&sb);
@@ -913,6 +917,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    "print patches to standard out"),
 		OPT_BOOLEAN(0, "cover-letter", &cover_letter,
 			    "generate a cover letter"),
+		OPT_STRING(0, "cover-subject", &cover_subject, "subject",
+				   "use <subject> in the subject line of the cover letter"),
+		OPT_STRING(0, "cover-blurb", &cover_blurb, "blurb",
+				   "use <blurb> in the body of the cover letter"),
 		OPT_BOOLEAN(0, "numbered-files", &numbered_files,
 			    "use simple number sequence for output file names"),
 		OPT_STRING(0, "suffix", &fmt_patch_suffix, "sfx",
@@ -1048,6 +1056,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
 		die("--check does not make sense");
 
+	if (strchr(cover_subject, '\n'))
+		die("--cover-subject can not contain newlines");
+
 	if (!use_patch_format &&
 		(!rev.diffopt.output_format ||
 		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
-- 
1.7.0.rc1.33.g07cf0f.dirty

^ permalink raw reply related

* Re: git gc / git repack not removing unused objects?
From: Nicolas Pitre @ 2010-02-05 21:45 UTC (permalink / raw)
  To: Jon Nelson; +Cc: git
In-Reply-To: <cccedfc61002051304t6030d3f7if4bb14709ee6c918@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1822 bytes --]

On Fri, 5 Feb 2010, Jon Nelson wrote:

> On Fri, Feb 5, 2010 at 2:51 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > On Fri, 5 Feb 2010, Jon Nelson wrote:
> >
> >> [Using git 1.6.4.2]
> >>
> >> In one repo I have (136G objects directory, fully packed) I'm having
> >> some trouble.
> >> I've run git-gc --prune=now, git repack -Adf, and so on a half-dozen
> >> times and each time I do so it gets bigger, not smaller.
> >
> > Please tell us more.
> 
> I'll tell you whatever I can -- as soon as I know what it is you want.

I want to know what you did and what results you obtained to lead you to 
believe that 'git-gc --prune=now' makes your repository bigger.

> > You need to dig further down in history to find a commit that actually
> > references that blob/tree.  One easy method is to do:
> >
> >        git log --raw --all
> >
> > and within the pager ('less' by default) simply search for "b32c3d8".
> 
> OK. I'm piping "git log --raw --all" to a file this very moment. It'll
> take a while. However, one thing I did not mention is that there
> *should* be a 1:1 correlation between branches and commits. As in,
> every time I did a commit, the commit was on a new branch. 

Ouch.  Why?  I'm curious.

> I'll look into this, as I've fiddled with the repo a bunch of 
> different ways lately. I suspect the answer will be found in the logs.

If the blob you're looking for is not in that log, then it must be 
referenced by one of the reflog entries.  The reflog is a log of all the 
commits your branches, and the HEAD pointer, ever pointed at.  Those 
entries are kept for a certain period (3 months by default).  The HEAD 
reflog will keep a reference of all branches you might have visited, 
even deleted ones.  So another log:

	git log --raw -g HEAD

should ultimately give you the blob reference.


Nicolas

^ permalink raw reply

* Re: [PATCH v2 5/6] receive-pack: Wrap status reports inside side-band-64k
From: Shawn O. Pearce @ 2010-02-05 21:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd40j1j2a.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > If the client requests the side-band-64k protocol capability we
> > now wrap the status report data inside of packets sent to band #1.
> > This permits us to later send additional progress or informational
> > messages down band #2.
> >
> > If side-band-64k was enabled, we always send a final flush packet
> > to let the client know we are done transmitting.
> 
> Two questions.
> 
>  - Why does use_sideband, the variable with the same name as a boolean
>    variable used by other parts of the system to decide whether we should
>    or should not use the sideband communiocation, get a value other than 0
>    or 1?  What is the benefit of using it to keep an actual value?  Does
>    the benefit outweigh the confusion factor?

I was following the existing convention of use_sideband is
maximum-packet-size in server code, and boolean in client code.

Well, I do diverage a bit in the client, in the client side
use_sideband = 2 in builtin-fetch-pack.c for side-band-64k and
use_sideband = 1 in builtin-send-pack.c for the same thing.
 
>  - What happens if client wants only side-band, not 64k?  This is just
>    theoretical and "we don't bother" is a perfectly acceptable answer.  I
>    am just curious ;-).

Why bother?  What client doesn't understand side-band-64k but would
understand this?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH v2 5/6] receive-pack: Wrap status reports inside side-band-64k
From: Junio C Hamano @ 2010-02-05 22:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20100205215344.GC27498@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> I was following the existing convention of use_sideband is
> maximum-packet-size in server code, and boolean in client code.

Ahh, I forgot about that -- we did it when we introduced different packet
size limit.

Now it all makes sense.  Thanks.

^ permalink raw reply

* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Wesley J. Landaker @ 2010-02-05 22:26 UTC (permalink / raw)
  To: Larry D'Anna; +Cc: git
In-Reply-To: <1265405973-5670-1-git-send-email-larry@elder-gods.org>

On Friday 05 February 2010 14:39:33 Larry D'Anna wrote:
> This is useful because if you're preparing a patch series with a cover
>  letter you can easily put together one line to format and email the
>  whole thing to yourself.  You check to make sure everything is right,
>  and then just change the recipient address and run it again.
> 
> git send-email --to my@mydomain.org  master..HEAD --cover-letter \
>     --cover-subject "this is my patch series" --cover-blurb "$(cat
>  blurb.txt)"

One (minor?) issue is that the cover blub would be limited to the maximum 
allowed length of the command-line arguments set by the shell or OS. Since 
you are just catting a file, maybe "--cover-blub-file" would be better?

Just a thought.

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Junio C Hamano @ 2010-02-05 22:31 UTC (permalink / raw)
  To: Johan Herland; +Cc: Giuseppe Bilotta, Jakub Narebski, git, Johannes Schindelin
In-Reply-To: <7veikzfysv.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> * "log --format=%N" and "log --show-notes"
> 
> Currently %N expands to the hardcoded "log --show-notes" default format.
> We can probably keep it that way.  When the user asked for a non default
> notes hierarchy (i.e. other than refs/notes/commits), we may want to
> adjust "Notes:" string to use "Notes-%s:" to show which hierarchy it came
> from, and concatenate them together.
>
> For "log --show-notes" output, we also might want to move the notes to the
> header part like I illustrated above in format-patch output, instead of
> "start with unindented Notes: and indented body at the end".  I.e. instead
> of showing this:

Giuseppe was wondering about multi-line thing, so the illustration should
be adjusted to match the "format-patch" example to show a multi-line note,
I think.  Here is what I meant.

Instead of showing:

    $ git log --notes-ref=amlog -1 4d0cc22
    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070
    Author: Junio C Hamano <gitster@pobox.com>
    Date:   Thu Feb 4 11:10:44 2010 -0800

        fast-import: count --max-pack-size in bytes

        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git
        ...

    Notes:
        pulled on Fri Feb 5 07:36:12 2010 -0800
        from git://git.bogomips.org/git-svn.git/
    Notes-amlog:
        <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>

which is what 1.6.6 added, showing it like this:

    $ git log --notes-ref=amlog -1 4d0cc22
    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070
    Author: Junio C Hamano <gitster@pobox.com>
    Date:   Thu Feb 4 11:10:44 2010 -0800
    Notes: pulled on Fri Feb 5 07:36:12 2010 -0800
     from git://git.bogomips.org/git-svn.git/
    Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>

        fast-import: count --max-pack-size in bytes

        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git
        ...

might be easier to see.  After all, notes are metainformation on commits,
and people who are interested will look at the header, and those who are
not will skim over the block of text at the beginning, knowing that is
where all the metainformation is.

But this is just "might", not "should---I strongly believe".

^ permalink raw reply

* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Junio C Hamano @ 2010-02-05 22:33 UTC (permalink / raw)
  To: Wesley J. Landaker; +Cc: Larry D'Anna, git
In-Reply-To: <201002051526.18205.wjl@icecavern.net>

"Wesley J. Landaker" <wjl@icecavern.net> writes:

> On Friday 05 February 2010 14:39:33 Larry D'Anna wrote:
>> This is useful because if you're preparing a patch series with a cover
>>  letter you can easily put together one line to format and email the
>>  whole thing to yourself.  You check to make sure everything is right,
>>  and then just change the recipient address and run it again.
>> 
>> git send-email --to my@mydomain.org  master..HEAD --cover-letter \
>>     --cover-subject "this is my patch series" --cover-blurb "$(cat
>>  blurb.txt)"
>
> One (minor?) issue is that the cover blub would be limited to the maximum 
> allowed length of the command-line arguments set by the shell or OS. Since 
> you are just catting a file, maybe "--cover-blub-file" would be better?
>
> Just a thought.

The placeholder in particular and the cover letter itself in general are
meant to be edited.  I do not see much point in forcing people to edit yet
another file and have them specify with an cover-blurb option.

Not very interested.

^ permalink raw reply

* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Wesley J. Landaker @ 2010-02-05 22:53 UTC (permalink / raw)
  To: Larry D'Anna; +Cc: Junio C Hamano, git
In-Reply-To: <7vfx5fwbws.fsf@alter.siamese.dyndns.org>

On Friday 05 February 2010 15:33:23 Junio C Hamano wrote:
> The placeholder in particular and the cover letter itself in general are
> meant to be edited.  I do not see much point in forcing people to edit
>  yet another file and have them specify with an cover-blurb option.
> 
> Not very interested.

The original use-case is also pretty close to just doing the following:

$ git format-patch master..HEAD --cover-letter 
$ vi 0000-cover-letter.patch
$ git send-email --to my@mydomain.org *.patch
$ git send-email --to git@vger.kernel.org *.patch

Isn't that just as easy as the proposed --cover-* options?

^ permalink raw reply

* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Larry D'Anna @ 2010-02-05 22:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Wesley J. Landaker, git
In-Reply-To: <7vfx5fwbws.fsf@alter.siamese.dyndns.org>

* Junio C Hamano (gitster@pobox.com) [100205 17:33]:

> The placeholder in particular and the cover letter itself in general are
> meant to be edited.  I do not see much point in forcing people to edit yet
> another file and have them specify with an cover-blurb option.
> 
> Not very interested.

Yes, they're meant to be edited, but if you look at the steps required to submit
a series with cover letter, it's clear it could be a bit streamlined:

1) make your branch

2) git format-patch --cover-letter

3) edit the cover letter

3) review the series, and realize you need to fix something, fix it.

4) git format-patch --cover-letter again

5) edit the cover letter, *again*.  hopefully you didn't overwrite the old one.

6) git send-email --to myself

7) one last look over it in my inbox

8) git send-email --to the list

The whole thing is a lot less annoying and error-prone if you can have
git-send-email call git-format-patch.  

Besides, you're not forcing anyone to edit an extra file.  If you leave out
--cover-subject or --cover-blurb it just behaves in exactly the same way it
always did.


        --larry

^ permalink raw reply

* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Larry D'Anna @ 2010-02-05 23:00 UTC (permalink / raw)
  To: Wesley J. Landaker; +Cc: Junio C Hamano, git
In-Reply-To: <201002051553.27315.wjl@icecavern.net>

* Wesley J. Landaker (wjl@icecavern.net) [100205 17:53]:
> On Friday 05 February 2010 15:33:23 Junio C Hamano wrote:
> > The placeholder in particular and the cover letter itself in general are
> > meant to be edited.  I do not see much point in forcing people to edit
> >  yet another file and have them specify with an cover-blurb option.
> > 
> > Not very interested.
> 
> The original use-case is also pretty close to just doing the following:
> 
> $ git format-patch master..HEAD --cover-letter 
> $ vi 0000-cover-letter.patch
> $ git send-email --to my@mydomain.org *.patch
> $ git send-email --to git@vger.kernel.org *.patch
> 
> Isn't that just as easy as the proposed --cover-* options?

Except when you decide you need to modify it after sending it to yourself.  

       --larry

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox