Git development
 help / color / mirror / Atom feed
* [PATCH 03/10] merge: notice local merging of tags and keep it unwrapped
From: Junio C Hamano @ 2011-11-05  6:01 UTC (permalink / raw)
  To: git
In-Reply-To: <1320472900-6601-1-git-send-email-gitster@pobox.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I notice that Peff came up with the same patch independently.

 builtin/merge.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index dffd5ec..6a44b6d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -441,6 +441,11 @@ static void merge_name(const char *remote, struct strbuf *msg)
 				    sha1_to_hex(branch_head), remote);
 			goto cleanup;
 		}
+		if (!prefixcmp(found_ref, "refs/tags/")) {
+			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 		if (!prefixcmp(found_ref, "refs/remotes/")) {
 			strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of .\n",
 				    sha1_to_hex(branch_head), remote);
-- 
1.7.8.rc0.108.g71b5ec

^ permalink raw reply related

* [PATCH 01/10] Split GPG interface into its own helper library
From: Junio C Hamano @ 2011-11-05  6:01 UTC (permalink / raw)
  To: git
In-Reply-To: <1320472900-6601-1-git-send-email-gitster@pobox.com>

This mostly moves existing code from builtin/tag.c (for signing)
and builtin/verify-tag.c (for verifying) to a new gpg-interface.c
file to provide a more generic library interface.

 - sign_buffer() takes a payload strbuf, a signature strbuf, and a signing
   key, runs "gpg" to produce a detached signature for the payload, and
   appends it to the signature strbuf. The contents of a signed tag that
   concatenates the payload and the detached signature can be produced by
   giving the same strbuf as payload and signature strbuf.

 - verify_signed_buffer() takes a payload and a detached signature as
   <ptr, len> pairs, and runs "gpg --verify" to see if the payload matches
   the signature. It can optionally capture the output from GPG to allow
   the callers to pretty-print it in a way more suitable for their
   contexts.

"verify-tag" (aka "tag -v") used to save the whole tag contents as if it
is a detached signature, and fed gpg the payload part of the tag. It
relied on gpg to fail when the given tag is not signed but just is
annotated.  The updated run_gpg_verify() function detects the lack of
detached signature in the input, and errors out without bothering "gpg".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Unchanged from jc/signed-commit

 Makefile             |    2 +
 builtin/tag.c        |   76 ++-------------------------
 builtin/verify-tag.c |   35 ++-----------
 gpg-interface.c      |  138 ++++++++++++++++++++++++++++++++++++++++++++++++++
 gpg-interface.h      |   10 ++++
 tag.c                |    5 ++
 6 files changed, 166 insertions(+), 100 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h

diff --git a/Makefile b/Makefile
index 17404c4..e4b14af 100644
--- a/Makefile
+++ b/Makefile
@@ -528,6 +528,7 @@ LIB_H += exec_cmd.h
 LIB_H += fsck.h
 LIB_H += gettext.h
 LIB_H += git-compat-util.h
+LIB_H += gpg-interface.h
 LIB_H += graph.h
 LIB_H += grep.h
 LIB_H += hash.h
@@ -621,6 +622,7 @@ LIB_OBJS += entry.o
 LIB_OBJS += environment.o
 LIB_OBJS += exec_cmd.o
 LIB_OBJS += fsck.o
+LIB_OBJS += gpg-interface.o
 LIB_OBJS += graph.o
 LIB_OBJS += grep.o
 LIB_OBJS += hash.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95..cca1205 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -14,6 +14,7 @@
 #include "parse-options.h"
 #include "diff.h"
 #include "revision.h"
+#include "gpg-interface.h"
 
 static const char * const git_tag_usage[] = {
 	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
@@ -23,8 +24,6 @@ static const char * const git_tag_usage[] = {
 	NULL
 };
 
-static char signingkey[1000];
-
 struct tag_filter {
 	const char **patterns;
 	int lines;
@@ -208,60 +207,7 @@ static int verify_tag(const char *name, const char *ref,
 
 static int do_sign(struct strbuf *buffer)
 {
-	struct child_process gpg;
-	const char *args[4];
-	char *bracket;
-	int len;
-	int i, j;
-
-	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
-				sizeof(signingkey)) > sizeof(signingkey) - 1)
-			return error(_("committer info too long."));
-		bracket = strchr(signingkey, '>');
-		if (bracket)
-			bracket[1] = '\0';
-	}
-
-	/* When the username signingkey is bad, program could be terminated
-	 * because gpg exits without reading and then write gets SIGPIPE. */
-	signal(SIGPIPE, SIG_IGN);
-
-	memset(&gpg, 0, sizeof(gpg));
-	gpg.argv = args;
-	gpg.in = -1;
-	gpg.out = -1;
-	args[0] = "gpg";
-	args[1] = "-bsau";
-	args[2] = signingkey;
-	args[3] = NULL;
-
-	if (start_command(&gpg))
-		return error(_("could not run gpg."));
-
-	if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
-		close(gpg.in);
-		close(gpg.out);
-		finish_command(&gpg);
-		return error(_("gpg did not accept the tag data"));
-	}
-	close(gpg.in);
-	len = strbuf_read(buffer, gpg.out, 1024);
-	close(gpg.out);
-
-	if (finish_command(&gpg) || !len || len < 0)
-		return error(_("gpg failed to sign the tag"));
-
-	/* Strip CR from the line endings, in case we are on Windows. */
-	for (i = j = 0; i < buffer->len; i++)
-		if (buffer->buf[i] != '\r') {
-			if (i != j)
-				buffer->buf[j] = buffer->buf[i];
-			j++;
-		}
-	strbuf_setlen(buffer, j);
-
-	return 0;
+	return sign_buffer(buffer, buffer, get_signing_key());
 }
 
 static const char tag_template[] =
@@ -270,21 +216,11 @@ static const char tag_template[] =
 	"# Write a tag message\n"
 	"#\n");
 
-static void set_signingkey(const char *value)
-{
-	if (strlcpy(signingkey, value, sizeof(signingkey)) >= sizeof(signingkey))
-		die(_("signing key value too long (%.10s...)"), value);
-}
-
 static int git_tag_config(const char *var, const char *value, void *cb)
 {
-	if (!strcmp(var, "user.signingkey")) {
-		if (!value)
-			return config_error_nonbool(var);
-		set_signingkey(value);
-		return 0;
-	}
-
+	int status = git_gpg_config(var, value, cb);
+	if (status)
+		return status;
 	return git_default_config(var, value, cb);
 }
 
@@ -463,7 +399,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	if (keyid) {
 		sign = 1;
-		set_signingkey(keyid);
+		set_signing_key(keyid);
 	}
 	if (sign)
 		annotate = 1;
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 3134766..28c2174 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -11,6 +11,7 @@
 #include "run-command.h"
 #include <signal.h>
 #include "parse-options.h"
+#include "gpg-interface.h"
 
 static const char * const verify_tag_usage[] = {
 		"git verify-tag [-v|--verbose] <tag>...",
@@ -19,42 +20,16 @@ static const char * const verify_tag_usage[] = {
 
 static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 {
-	struct child_process gpg;
-	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
-	char path[PATH_MAX];
-	size_t len;
-	int fd, ret;
+	int len;
 
-	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
-	if (fd < 0)
-		return error("could not create temporary file '%s': %s",
-						path, strerror(errno));
-	if (write_in_full(fd, buf, size) < 0)
-		return error("failed writing temporary file '%s': %s",
-						path, strerror(errno));
-	close(fd);
-
-	/* find the length without signature */
 	len = parse_signature(buf, size);
 	if (verbose)
 		write_in_full(1, buf, len);
 
-	memset(&gpg, 0, sizeof(gpg));
-	gpg.argv = args_gpg;
-	gpg.in = -1;
-	args_gpg[2] = path;
-	if (start_command(&gpg)) {
-		unlink(path);
-		return error("could not run gpg.");
-	}
-
-	write_in_full(gpg.in, buf, len);
-	close(gpg.in);
-	ret = finish_command(&gpg);
+	if (size == len)
+		return error("no signature found");
 
-	unlink_or_warn(path);
-
-	return ret;
+	return verify_signed_buffer(buf, len, buf + len, size - len, NULL);
 }
 
 static int verify_tag(const char *name, int verbose)
diff --git a/gpg-interface.c b/gpg-interface.c
new file mode 100644
index 0000000..ff232c8
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,138 @@
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "gpg-interface.h"
+#include "sigchain.h"
+
+static char *configured_signing_key;
+
+void set_signing_key(const char *key)
+{
+	free(configured_signing_key);
+	configured_signing_key = xstrdup(key);
+}
+
+int git_gpg_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "user.signingkey")) {
+		if (!value)
+			return config_error_nonbool(var);
+		set_signing_key(value);
+	}
+	return 0;
+}
+
+const char *get_signing_key(void)
+{
+	if (configured_signing_key)
+		return configured_signing_key;
+	return git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+}
+
+/*
+ * Create a detached signature for the contents of "buffer" and append
+ * it after "signature"; "buffer" and "signature" can be the same
+ * strbuf instance, which would cause the detached signature appended
+ * at the end.
+ */
+int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
+{
+	struct child_process gpg;
+	const char *args[4];
+	ssize_t len;
+	size_t i, j, bottom;
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args;
+	gpg.in = -1;
+	gpg.out = -1;
+	args[0] = "gpg";
+	args[1] = "-bsau";
+	args[2] = signing_key;
+	args[3] = NULL;
+
+	if (start_command(&gpg))
+		return error(_("could not run gpg."));
+
+	/*
+	 * When the username signingkey is bad, program could be terminated
+	 * because gpg exits without reading and then write gets SIGPIPE.
+	 */
+	sigchain_push(SIGPIPE, SIG_IGN);
+
+	if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
+		close(gpg.in);
+		close(gpg.out);
+		finish_command(&gpg);
+		return error(_("gpg did not accept the data"));
+	}
+	close(gpg.in);
+
+	bottom = signature->len;
+	len = strbuf_read(signature, gpg.out, 1024);
+	close(gpg.out);
+
+	sigchain_pop(SIGPIPE);
+
+	if (finish_command(&gpg) || !len || len < 0)
+		return error(_("gpg failed to sign the data"));
+
+	/* Strip CR from the line endings, in case we are on Windows. */
+	for (i = j = bottom; i < signature->len; i++)
+		if (signature->buf[i] != '\r') {
+			if (i != j)
+				signature->buf[j] = signature->buf[i];
+			j++;
+		}
+	strbuf_setlen(signature, j);
+
+	return 0;
+}
+
+/*
+ * Run "gpg" to see if the payload matches the detached signature.
+ * gpg_output_to tells where the output from "gpg" should go:
+ *   < 0: /dev/null
+ *   = 0: standard error of the calling process
+ *   > 0: the specified file descriptor
+ */
+int verify_signed_buffer(const char *payload, size_t payload_size,
+			 const char *signature, size_t signature_size,
+			 struct strbuf *gpg_output)
+{
+	struct child_process gpg;
+	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+	char path[PATH_MAX];
+	int fd, ret;
+
+	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+	if (fd < 0)
+		return error("could not create temporary file '%s': %s",
+			     path, strerror(errno));
+	if (write_in_full(fd, signature, signature_size) < 0)
+		return error("failed writing detached signature to '%s': %s",
+			     path, strerror(errno));
+	close(fd);
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args_gpg;
+	gpg.in = -1;
+	if (gpg_output)
+		gpg.err = -1;
+	args_gpg[2] = path;
+	if (start_command(&gpg)) {
+		unlink(path);
+		return error("could not run gpg.");
+	}
+
+	write_in_full(gpg.in, payload, payload_size);
+	close(gpg.in);
+
+	if (gpg_output)
+		strbuf_read(gpg_output, gpg.err, 0);
+	ret = finish_command(&gpg);
+
+	unlink_or_warn(path);
+
+	return ret;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
new file mode 100644
index 0000000..b9c3608
--- /dev/null
+++ b/gpg-interface.h
@@ -0,0 +1,10 @@
+#ifndef GPG_INTERFACE_H
+#define GPG_INTERFACE_H
+
+extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);
+extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output);
+extern int git_gpg_config(const char *, const char *, void *);
+extern void set_signing_key(const char *);
+extern const char *get_signing_key(void);
+
+#endif
diff --git a/tag.c b/tag.c
index 7d38cc0..3aa186d 100644
--- a/tag.c
+++ b/tag.c
@@ -139,6 +139,11 @@ int parse_tag(struct tag *item)
 	return ret;
 }
 
+/*
+ * Look at a signed tag object, and return the offset where
+ * the embedded detached signature begins, or the end of the
+ * data when there is no such signature.
+ */
 size_t parse_signature(const char *buf, unsigned long size)
 {
 	char *eol;
-- 
1.7.8.rc0.108.g71b5ec

^ permalink raw reply related

* [PATCH 00/10] Pulling signed tag
From: Junio C Hamano @ 2011-11-05  6:01 UTC (permalink / raw)
  To: git

This is my fourth iteration to solve the "how can we allow authenticity of
work by contributors to be validated by the integrator and leave enough
information for later audit by third parties" topic. What is unusual is
that this is not a fourth re-roll of one approach re-re-re-polished, but
these four are all based on different design.

This round is based on Linus's "let the integrator pull signed tag from
the contributor" design.

The first patch is the same as the one at the bottom of the third
iteration which was jc/signed-commit topic. The rest are new.

 1. Split GPG interface into its own helper library

 2. fetch: do not store peeled tag object names in FETCH_HEAD
 3. merge: notice local merging of tags and keep it unwrapped
 4. fetch: allow "git fetch $there v1.0" to fetch a tag
 5. tests: distinguish merges of tags and commits

"git fetch" used to peel tags too early when storing in FETCH_HEAD (the
input to fmt-merge-msg), and "git merge" did the same when internally
preparing the list of what are merged to feed the internal fmt-merge-msg.

The above four stops doing so, whose effect can be seen in the changes to
the test vector in the fifth patch.

 6. refs DWIMmery: use the same rule for both "git fetch" and others

You can pull a tag with "git pull $there tags/for-linus", but this allows
you to say "git pull $there for-linus".

 7. fmt-merge-msg: avoid early returns
 8. fmt-merge-msg: package options into a structure
 9. fmt-merge-msg: Add contents of merged tag in the merge message

The first two of this segment are small clean-ups to make the third one
possible. When merging signed tag(s), the merge message is prepared with
the contents of the tag object for later independent audit at the end, and
also contains the output from the GPG verification process as the comment
to help the integrator verify it.

10. merge: force edit mode when merging a tag object

And in order to _show_ that comment, we would need to show it in the
editor before the commit happens, hence this conclusion patch.

 Makefile                                           |    2 +
 builtin.h                                          |    8 +-
 builtin/fetch.c                                    |    3 +-
 builtin/fmt-merge-msg.c                            |  126 +++++++++++++++----
 builtin/merge.c                                    |   28 +++-
 builtin/tag.c                                      |   76 +----------
 builtin/verify-tag.c                               |   35 +-----
 cache.h                                            |    2 +-
 gpg-interface.c                                    |  138 ++++++++++++++++++++
 gpg-interface.h                                    |   10 ++
 refs.c                                             |    7 -
 t/t4202-log.sh                                     |    4 +-
 t/t5510-fetch.sh                                   |    5 +-
 t/t5515/fetch.br-branches-default                  |    6 +-
 t/t5515/fetch.br-branches-default-merge            |    6 +-
 ...etch.br-branches-default-merge_branches-default |    6 +-
 t/t5515/fetch.br-branches-default-octopus          |    6 +-
 ...ch.br-branches-default-octopus_branches-default |    6 +-
 t/t5515/fetch.br-branches-default_branches-default |    6 +-
 t/t5515/fetch.br-branches-one                      |    6 +-
 t/t5515/fetch.br-branches-one-merge                |    6 +-
 t/t5515/fetch.br-branches-one-merge_branches-one   |    6 +-
 t/t5515/fetch.br-branches-one-octopus              |    6 +-
 t/t5515/fetch.br-branches-one-octopus_branches-one |    6 +-
 t/t5515/fetch.br-branches-one_branches-one         |    6 +-
 t/t5515/fetch.br-config-explicit                   |    6 +-
 t/t5515/fetch.br-config-explicit-merge             |    6 +-
 .../fetch.br-config-explicit-merge_config-explicit |    6 +-
 t/t5515/fetch.br-config-explicit-octopus           |    6 +-
 ...etch.br-config-explicit-octopus_config-explicit |    6 +-
 t/t5515/fetch.br-config-explicit_config-explicit   |    6 +-
 t/t5515/fetch.br-config-glob                       |    6 +-
 t/t5515/fetch.br-config-glob-merge                 |    6 +-
 t/t5515/fetch.br-config-glob-merge_config-glob     |    6 +-
 t/t5515/fetch.br-config-glob-octopus               |    6 +-
 t/t5515/fetch.br-config-glob-octopus_config-glob   |    6 +-
 t/t5515/fetch.br-config-glob_config-glob           |    6 +-
 t/t5515/fetch.br-remote-explicit                   |    6 +-
 t/t5515/fetch.br-remote-explicit-merge             |    6 +-
 .../fetch.br-remote-explicit-merge_remote-explicit |    6 +-
 t/t5515/fetch.br-remote-explicit-octopus           |    6 +-
 ...etch.br-remote-explicit-octopus_remote-explicit |    6 +-
 t/t5515/fetch.br-remote-explicit_remote-explicit   |    6 +-
 t/t5515/fetch.br-remote-glob                       |    6 +-
 t/t5515/fetch.br-remote-glob-merge                 |    6 +-
 t/t5515/fetch.br-remote-glob-merge_remote-glob     |    6 +-
 t/t5515/fetch.br-remote-glob-octopus               |    6 +-
 t/t5515/fetch.br-remote-glob-octopus_remote-glob   |    6 +-
 t/t5515/fetch.br-remote-glob_remote-glob           |    6 +-
 t/t5515/fetch.br-unconfig                          |    6 +-
 t/t5515/fetch.br-unconfig_--tags_.._.git           |    6 +-
 ...nfig_.._.git_one_tag_tag-one_tag_tag-three-file |    6 +-
 ...fig_.._.git_tag_tag-one-tree_tag_tag-three-file |    6 +-
 ...h.br-unconfig_.._.git_tag_tag-one_tag_tag-three |    6 +-
 t/t5515/fetch.br-unconfig_branches-default         |    6 +-
 t/t5515/fetch.br-unconfig_branches-one             |    6 +-
 t/t5515/fetch.br-unconfig_config-explicit          |    6 +-
 t/t5515/fetch.br-unconfig_config-glob              |    6 +-
 t/t5515/fetch.br-unconfig_remote-explicit          |    6 +-
 t/t5515/fetch.br-unconfig_remote-glob              |    6 +-
 t/t5515/fetch.master                               |    6 +-
 t/t5515/fetch.master_--tags_.._.git                |    6 +-
 ...ster_.._.git_one_tag_tag-one_tag_tag-three-file |    6 +-
 ...ter_.._.git_tag_tag-one-tree_tag_tag-three-file |    6 +-
 .../fetch.master_.._.git_tag_tag-one_tag_tag-three |    6 +-
 t/t5515/fetch.master_branches-default              |    6 +-
 t/t5515/fetch.master_branches-one                  |    6 +-
 t/t5515/fetch.master_config-explicit               |    6 +-
 t/t5515/fetch.master_config-glob                   |    6 +-
 t/t5515/fetch.master_remote-explicit               |    6 +-
 t/t5515/fetch.master_remote-glob                   |    6 +-
 t/t7600-merge.sh                                   |    6 +-
 t/t7604-merge-custom-message.sh                    |    2 +-
 t/t7608-merge-messages.sh                          |    4 +-
 tag.c                                              |    5 +
 75 files changed, 482 insertions(+), 327 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h

-- 
1.7.8.rc0.108.g71b5ec

^ permalink raw reply

* Re: [PATCH 0/4] fsck improvements
From: Junio C Hamano @ 2011-11-05  5:26 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8Ch0m5KQ--VWzsmyNq2R-uHsvSYofHP8L+qcvhRrqgoKQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> 2011/11/5 Junio C Hamano <gitster@pobox.com>:
>> I am not sure what purpose patch 2 serves, though. When we find a checksum
>> mismatch for an object in a packstream due to a single-bit error, we would
>> still be able to salvage other objects in other parts of the pack as long
>> as we have a good .idx file, and in such a case, wouldn't it be better if
>> we attempted to find as many corrupt objects that we know we cannot
>> recover as possible and tell the user about them, so that they can be
>> skipped during the recovery process?
>
> It's the inconsistency in that for(;;) loop. If we are going to
> salvage as many objects as we could, should we do "continue;" instead
> of "break;" when unpack_entry() or check_sha1_signature() fails?

I do not think making things worse for the sake of "consistency" is a good
sell ;-). How hard would it be to make it also continue when these report
a corruption?

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-05  4:37 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: git, Shawn Pearce, James Bottomley, Jeff Garzik, Andrew Morton,
	linux-ide, LKML
In-Reply-To: <CA+55aFzKPPqwGOe5Ov0FHF1DHbKmNhm=ePvcaY5uqR7cwFhQGQ@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> However - exactly beause git apparently makes it do that "Merge commit
> " message, I suspect we've peeled things too early and too much. We've
> peeled it so early that once again something thinks it's a commit, not
> a tag.

And you are right.

I am working on a larger series that should sit on top of 89587fa (Split
GPG interface into its own helper library, 2011-09-07), which is the first
commit in jc/signed-commit topic.

-- >8 --
Subject: [PATCH] merge: notice local merging of tags and keep it unwrapped

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index f61b367..ce3b4f8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -428,6 +428,11 @@ static void merge_name(const char *remote, struct strbuf *msg)
 				    sha1_to_hex(branch_head), remote);
 			goto cleanup;
 		}
+		if (!prefixcmp(found_ref, "refs/tags/")) {
+			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 		if (!prefixcmp(found_ref, "refs/remotes/")) {
 			strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of .\n",
 				    sha1_to_hex(branch_head), remote);
-- 
1.7.8.rc0.108.g71b5ec


^ permalink raw reply related

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Jeff King @ 2011-11-05  3:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, git, Shawn Pearce, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <CA+55aFzKPPqwGOe5Ov0FHF1DHbKmNhm=ePvcaY5uqR7cwFhQGQ@mail.gmail.com>

On Fri, Nov 04, 2011 at 04:10:59PM -0700, Linus Torvalds wrote:

> I'm pretty sure people have already done "git merge v3.1" kind of
> things using local tags (where no peeling of FETCH_HEAD has been
> done). See
> 
>     git log --merges --grep 'Merge.*v[23]\.[0-9]'
> 
> for a ton of examples of this (and there's something odd going on: we
> have "Merge commit .." and "Merge tag ..", and I suspect the latter is
> people editing it to be correct by hand, but I dunno).

It looks like fmt-merge-msg looks in FETCH_HEAD to see if each line is
marked as "branch" or "tag". So I get "Merge tag ..." with:

  git pull . tag v1.0

but I get "Merge commit ..." with:

  git merge v1.0

When "git merge" is run, it actually creates a fake FETCH_HEAD in memory
and feeds it to fmt-merge-msg. But that process doesn't seem to bother
looking at tags. I think we just need this:

---
diff --git a/builtin/merge.c b/builtin/merge.c
index dffd5ec..6a44b6d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -439,10 +439,15 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		if (!prefixcmp(found_ref, "refs/heads/")) {
 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
 				    sha1_to_hex(branch_head), remote);
 			goto cleanup;
 		}
+		if (!prefixcmp(found_ref, "refs/tags/")) {
+			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 		if (!prefixcmp(found_ref, "refs/remotes/")) {
 			strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of .\n",
 				    sha1_to_hex(branch_head), remote);
 			goto cleanup;
 		}

where the result of merge_name is just fed to fmt-merge-msg eventually.

-Peff

^ permalink raw reply related

* Re: [PATCH 4/4] fsck: print progress
From: Nguyen Thai Ngoc Duy @ 2011-11-05  3:26 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111104201416.GA26591@sigill.intra.peff.net>

2011/11/5 Jeff King <peff@peff.net>:
> Thanks, this is a good place to put a progress meter, too. If you're
> feeling like pushing this topic further, "git prune" might be a good
> place for a progress meter, too. It does a similar traversal[1] for
> reachability, and makes "git gc" appear to hang at the end (we have nice
> progress meters for packing, but it takes something like 25 seconds to
> run "git prune" at the end, during which we are silent).
>
> -Peff
>
> [1] I wonder why fsck doesn't use mark_reachable from reachable.c.

Thanks. I'll have a look.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec
From: Nguyen Thai Ngoc Duy @ 2011-11-05  3:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwrbfa6j6.fsf@alter.siamese.dyndns.org>

2011/11/5 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Pathspec in "git log -p <pathspec>" is used for both commit pruning
>> and diff generation. If --full-diff is given, then diff pathspec is
>> reset to generate complete diff.
>>
>> This patch gives more control to diff generation.  The first pathspec in
>> "git log -p -- <pathspec> -- <pathspec>" is used as commit pruning
>> as usual. The second one is used for diff generation. So --full-diff
>> now is essentially "git log -p -- <pathspec> --".
>
> I agree that giving more control to diff generation is a good idea, and
> this certainly is better than the previous round that nobody reviewed
> before you rerolled this round.
>
> But I have doubts about declaring "--full-diff is equivalent to giving the
> 'output' pathspec that is empty".

The equivalent command should be "git log -p -- <pathspec> -- :/" (in
case you're in subdir). What doubts specifically?

> Have you thought about the interaction between this and -M/-C options?

No, yes I may have problems there.

> This is not a new "problem" (and it is probably not a problem to begin
> with).  If you had "--" in the working tree and you wanted to treat it as
> a path, you said either one of these:
>
>    $ git log HEAD ./--
>    $ git log HEAD -- --
>
> The latter is what your patch breaks, I suspect.
>
> Also it forces existing scripts and programs that knew "everything in this
> array is a pathspec" and added "--" before adding the contents of the
> array to form a command line to drive "git log" family of commands to be
> updated.

We could only recognize the second "--" when a new argument is given
so it won't break existing use. But I'll need to look at -C/-M first.
Smells like --follow problem.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 0/4] fsck improvements
From: Nguyen Thai Ngoc Duy @ 2011-11-05  3:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1utnd8yf.fsf@alter.siamese.dyndns.org>

2011/11/5 Junio C Hamano <gitster@pobox.com>:
> I am not sure what purpose patch 2 serves, though. When we find a checksum
> mismatch for an object in a packstream due to a single-bit error, we would
> still be able to salvage other objects in other parts of the pack as long
> as we have a good .idx file, and in such a case, wouldn't it be better if
> we attempted to find as many corrupt objects that we know we cannot
> recover as possible and tell the user about them, so that they can be
> skipped during the recovery process?

It's the inconsistency in that for(;;) loop. If we are going to
salvage as many objects as we could, should we do "continue;" instead
of "break;" when unpack_entry() or check_sha1_signature() fails?
-- 
Duy

^ permalink raw reply

* Re: share object storage for multiple clones of different repositories
From: Junio C Hamano @ 2011-11-05  2:26 UTC (permalink / raw)
  To: Gelonida N; +Cc: git
In-Reply-To: <j91rcq$1uo$1@dough.gmane.org>

Gelonida N <gelonida@gmail.com> writes:

> SHARED_STORAGE=$HOME/shared_storage
> mkdir $SHARED_STORAGE
>
> git clone remotehost1:repo1
> cd repo1
> rsync -av .git/objects $SHARED_REPO

Up to this part it is probably OK.  Repeat that for all your local
repositories to collect all objects in $HOME/shared_storage.

After doing that, do this in all of your local repositories:

	rm -rf .git/objects
        mkdir -p .git/objects/info
        echo $HOME/shared/storage >.git/objects/info/alternates

The reason why nobody should follow your original recipe is because any
"git gc"/"git repack" in any of your local repositories would break others
with that approach.

^ permalink raw reply

* Re: [PATCHv2] Add options to specify snapshot file name, prefix
From: Thomas Guyot-Sionnest @ 2011-11-05  0:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3fwi350ou.fsf@localhost.localdomain>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11-11-04 12:10 PM, Jakub Narebski wrote:
> Thomas Guyot-Sionnest <dermoth@aei.ca> writes:
> 
>> commit b629275 implemented "smart" snapshot names and prefixes. I have
>> scripts that used to rely on the old behaviour which allowed in some
>> cases to have fixed prefix, and would require modifications to work with
>> newer Gitweb.
> 
> If scripts use 'wget' or 'curl' you can always change the name of
> saved file:
> 
>   wget -O <file> ...
>   curl -o <file> ...
> 
> If downloaded snapshot is compressed tarfile, you can use
> --strip-components=1 to strip prefix.

I actually didn't care about the filename (in my script I pipe straight
to tar, and in a python app I have that will use snapshots I have I
planned on "opening" the url as a stream directly with the tarfile lib);
it was just as easy to add both anyway.

I thought I had looked up the tar man page for an option that strips the
path, clearly I missed that one. :)

>> This patch adds two parameters for overriding the snapshot name and
>> prefix, sn and sp respectively. For example, to get a snapshot
>> named "myproject.[suffix]" with no prefix one can add this query string:
>>   ?sn=myproject;sp=
> 
> Would you need support for expandable parameters in both (a la
> 'action' feature)?

I'm not sure what you mean... I never tinkered with gitweb.pl directly
before.

> [...] 
>> @@ -6684,11 +6686,19 @@ sub git_snapshot {
>>  	}
>>  
>>  	my ($name, $prefix) = snapshot_name($project, $hash);
>> +	if (defined($input_params{'snapshot_name'})) {
>> +		$name = $input_params{'snapshot_name'};
>> +	}
>> +	if (defined($input_params{'snapshot_prefix'})) {
>> +		$prefix = $input_params{'snapshot_prefix'};
>> +	} else {
>> +		$prefix .= '/';
>> +	}
>>  	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
>>  	my $cmd = quote_command(
>>  		git_cmd(), 'archive',
>>  		"--format=$known_snapshot_formats{$format}{'format'}",
>> -		"--prefix=$prefix/", $hash);
>> +		"--prefix=$prefix", $hash);
>>  	if (exists $known_snapshot_formats{$format}{'compressor'}) {
>>  		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
>>  	}
> 
> I wonder if you really want to allow prefix which do not end in '/'

I kind of agree, yet considering its lack of "front-end" visibility it
made me think of plumbing commands like git-checkout-index (which I use
sometimes to replace the "missing" git export) where prefix is nothing
more than an appended string to file names.

And now I remember, this is also exactly how git-archive works.

> (which would be suprising, isn't it), or just allow empty prefix too.
>
>
> For example
> 
>   @@ -6684,11 +6686,19 @@ sub git_snapshot {
>    	}
>    
>    	my ($name, $prefix) = snapshot_name($project, $hash);
>   +	if (defined($input_params{'snapshot_name'})) {
>   +		$name = $input_params{'snapshot_name'};
>   +	}
>   +	if (defined($input_params{'snapshot_prefix'})) {
>   +		$prefix = $input_params{'snapshot_prefix'};
>   +	}
>    	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
>    	my $cmd = quote_command(
>    		git_cmd(), 'archive',
>    		"--format=$known_snapshot_formats{$format}{'format'}",
>   -		"--prefix=$prefix/", $hash);
>   +		($prefix eq "" ? () : "--prefix=$prefix"), $hash);
>    	if (exists $known_snapshot_formats{$format}{'compressor'}) {
>    		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
>    	}
> 

You still have to add the /, i.e.:

>   +		($prefix eq "" ? () : "--prefix=$prefix/"), $hash);

And personally, I think git-archive is the one that should add a / - it
has much more visibility to end-users than this obscure query-string.

- -- 
Thomas
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk60iMAACgkQ6dZ+Kt5BchbOFwCgmDuUVd9vI+ZC8JDQPS+SC7e2
FpIAnAkf6BrmAcvY/3GA1wjQvR9s50c6
=0pb2
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH 1/2] git-gui: make config gui.warndetachedcommit a boolean
From: Pat Thoyts @ 2011-11-05  0:48 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Heiko Voigt, git
In-Reply-To: <CAKPyHN24RwNjJanaMQ0AjXc2iorFw=taCmcS-iuNt0vQ46_TOg@mail.gmail.com>

Bert Wesarg <bert.wesarg@googlemail.com> writes:

>On Sat, Oct 22, 2011 at 21:39, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>> ---
>>  lib/commit.tcl |    2 +-
>>  lib/option.tcl |    1 +
>>  2 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib/commit.tcl b/lib/commit.tcl
>> index 372bed9..e27e148 100644
>> --- a/lib/commit.tcl
>> +++ b/lib/commit.tcl
>> @@ -263,7 +263,7 @@ proc commit_commitmsg {curHEAD msg_p} {
>>        global is_detached repo_config
>>        global pch_error
>>
>> -       if {$is_detached && $repo_config(gui.warndetachedcommit)} {
>> +       if {$is_detached && [is_config_true gui.warndetachedcommit]} {
>>                set msg [mc "You are about to commit on a detached head.\
>>  This is a potentially dangerous thing to do because if you switch\
>>  to another branch you will loose your changes and it can be difficult\
>> diff --git a/lib/option.tcl b/lib/option.tcl
>> index 719103a..f7f866b 100644
>> --- a/lib/option.tcl
>> +++ b/lib/option.tcl
>> @@ -156,6 +156,7 @@ proc do_options {} {
>>                {i-0..99 gui.commitmsgwidth {mc "Commit Message Text Width"}}
>>                {t gui.newbranchtemplate {mc "New Branch Name Template"}}
>>                {c gui.encoding {mc "Default File Contents Encoding"}}
>> +               {b gui.warndetachedcommit {mc "Warn before commiting to a detached head"}}
>>                {s gui.stageuntracked {mc "Staging of untracked files"} {list "yes" "no" "ask"}}
>>                } {
>>                set type [lindex $option 0]
>
>Pat,
>
>if you're interessted in this patch, please fix the typo in the second
>hunk, mentioning 'commiting'.
>
>Also shouldn't this variable be called gui.warndetachedhead?
>
>Thanks.
>
>Bert

I've applied this with the spelling correction and also gitdir already
does [file join] on its args so squashed a slight modification there
too.

The flag is for warnings about commits. Calling 'warndetachedhead'
sounds like something that would warn you when you changed to a detached
head - not for when you are about to commit to one so I think the name
is fine.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-04 23:45 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7vaa8bbni3.fsf@alter.siamese.dyndns.org>

On Fri, Nov 4, 2011 at 2:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> If we really wanted to go this route, the attached single-liner should be
> sufficient for the DWIMmery.

My first reaction on reading the patch was "maybe it would be safer to
put the tags case after 'branches', so that the behavior in the
presense of ambiguity would stay the same as with the old case", but
thinking it through I think it's more important to be consistent with
the other lookups, which all prefer tags over branches when ambiguous.

So Ack, patch looks sane to me, and clearly makes it easier to fetch
tags individually.

                  Linus

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-04 23:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Shawn Pearce, James Bottomley, Jeff Garzik, Andrew Morton,
	linux-ide, LKML
In-Reply-To: <7v39e3bn1n.fsf@alter.siamese.dyndns.org>

On Fri, Nov 4, 2011 at 2:22 PM, Junio C Hamano <junio@pobox.com> wrote:
>
> builtin/merge.c was updated to use want_commit() that uses peel_to_type()
> to commit to make sure we do not get fed anything funky, and also uses
> struct commit_list to pass around list of parents to be recorded, so we
> should be OK in this department.

I'm pretty sure people have already done "git merge v3.1" kind of
things using local tags (where no peeling of FETCH_HEAD has been
done). See

    git log --merges --grep 'Merge.*v[23]\.[0-9]'

for a ton of examples of this (and there's something odd going on: we
have "Merge commit .." and "Merge tag ..", and I suspect the latter is
people editing it to be correct by hand, but I dunno).

So this has always worked, methinks.

However - exactly beause git apparently makes it do that "Merge commit
" message, I suspect we've peeled things too early and too much. We've
peeled it so early that once again something thinks it's a commit, not
a tag.

So if anything, I suspect "git merge" not only peels, but peels too
much (or at the wrong point). We should probably peel as late as
possible.

But it's a small detail.

                  Linus

^ permalink raw reply

* share object storage for multiple clones of different repositories
From: Gelonida N @ 2011-11-04 23:10 UTC (permalink / raw)
  To: git

Hi,

(Thunderbird frozewhile sending my previous message so here a resend)


I wondered whether it is possible, that all of my git repository clones
share the same object storage whether they are cloned from they same
remote repository or not.

In my case this might save a lot of diskspace and accelerate cloning
as some huge files are part of several repositories' history and as I'd
like to clone the same repository multiple times in order to have
parallel working directories for parallel tests on different versions /
branches / tags

Further this might reduce clone times.



My goal would be to:
- reduce disk space
- reduce clone time, as objects would be taken from the existing shared
  object storage

If possible it would be great if also all new created shared objects
would end up in the new object storage.


If git doesn't support this natively, then would following approach work
for reducing the disk space (clone time would not be reduced though)



SHARED_STORAGE=$HOME/shared_storage
mkdir $SHARED_STORAGE

git clone remotehost1:repo1
cd repo1
rsync -av .git/objects $SHARED_REPO
rm -rf .git/objects
ln -s $SHARED_REPO/objects .git/


git clone remotehost2:repo2
cd repo2
rsync -av .git/objects $SHARED_REPO
rm -rf .git/objects
ln -s $SHARED_REPO/objects .git/

Thanks for any feedback or other suggestions.

^ permalink raw reply

* Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec
From: Junio C Hamano @ 2011-11-04 22:07 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320322556-32675-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This form requires specifying "--" twice. If a file name happens to be
> "--", it may be misunderstood as the second "--" marker. This is an
> unfortunate consequence for this syntax. Users can still use "./--" or
> similar to workaround this.

This is not a new "problem" (and it is probably not a problem to begin
with).  If you had "--" in the working tree and you wanted to treat it as
a path, you said either one of these:

    $ git log HEAD ./--
    $ git log HEAD -- --

The latter is what your patch breaks, I suspect.

Also it forces existing scripts and programs that knew "everything in this
array is a pathspec" and added "--" before adding the contents of the
array to form a command line to drive "git log" family of commands to be
updated.

^ permalink raw reply

* Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec
From: Junio C Hamano @ 2011-11-04 22:04 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320322556-32675-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Pathspec in "git log -p <pathspec>" is used for both commit pruning
> and diff generation. If --full-diff is given, then diff pathspec is
> reset to generate complete diff.
>
> This patch gives more control to diff generation.  The first pathspec in
> "git log -p -- <pathspec> -- <pathspec>" is used as commit pruning
> as usual. The second one is used for diff generation. So --full-diff
> now is essentially "git log -p -- <pathspec> --".

I agree that giving more control to diff generation is a good idea, and
this certainly is better than the previous round that nobody reviewed
before you rerolled this round.

But I have doubts about declaring "--full-diff is equivalent to giving the
'output' pathspec that is empty".

Have you thought about the interaction between this and -M/-C options?

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-04 21:22 UTC (permalink / raw)
  To: git
  Cc: Linus Torvalds, Shawn Pearce, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7vlirvbq47.fsf@alter.siamese.dyndns.org>

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

> However we would need to audit so that we do not accidentally record the
> tag object names in the "parent" headers in the merge commits, which is
> what I'll be doing next.

Just reporting my findings.

builtin/merge.c was updated to use want_commit() that uses peel_to_type()
to commit to make sure we do not get fed anything funky, and also uses
struct commit_list to pass around list of parents to be recorded, so we
should be OK in this department.

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-04 21:12 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7v62j1gitn.fsf@alter.siamese.dyndns.org>

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

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>>   [torvalds@i5 linux]$ git fetch
>> git://github.com/rustyrussell/linux.git
>> rusty@rustcorp.com.au-v3.1-8068-g5087a50
>>   fatal: Couldn't find remote ref rusty@rustcorp.com.au-v3.1-8068-g5087a50
>>
>> oops. Ok, so his tag naming is *really* akward. Whatever.
>
> It is not "Whatever".
>
>  $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v3.0
>  fatal: Couldn't find remote ref v3.0
>
> I do not think we ever DWIMmed fetch refspecs to prefix refs/tags/, so it
> is not the naming but fetching tags without saying "git fetch tag v3.0"
> (which IIRC was your invention long time ago). 

If we really wanted to go this route, the attached single-liner should be
sufficient for the DWIMmery.

Note that the DWIMmery rules for "git fetch" and local "git rev-parse" are
still different even after this patch.

"git log frotz" can DWIM to "refs/remotes/frotz/HEAD", but in the remote
access context, "git fetch frotz" to fetch what the other side happened to
have fetched from what it calls 'frotz' (which may not have any relation
to what we consider is 'frotz') the last time would not make much sense,
so the fetch rules table does not include "refs/remotes/%.*s/HEAD".

When the user really wants to, "git fetch $there remotes/frotz/HEAD" would
let her do so anyway, so this is not about safety or security; it merely
is about confusion avoidance and discouraging meaningless usage.

Specifically, it is _not_ about ambiguity avoidance. A name that would
become ambiguous if we use the same rules table for both fetch and local
rev-parse would be ambiguous locally at the remote side.

If we really wanted to, we could 

	#define ref_fetch_rules ref_rev_parse_rules
 
in cache.h and remove the array's declaration from cache.h and its
definition from refs.c to really unify the two, but I haven't thought
things through.

-- >8 --
Subject: [PATCH] fetch: allow "git fetch $there v1.0" to fetch a tag

You can already do so with "git fetch $there tags/v1.0" but if it is not
ambiguous there is no reason to force users to type more.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index e69ba26..670a7b3 100644
--- a/refs.c
+++ b/refs.c
@@ -1001,6 +1001,7 @@ const char *ref_rev_parse_rules[] = {
 const char *ref_fetch_rules[] = {
 	"%.*s",
 	"refs/%.*s",
+	"refs/tags/%.*s",
 	"refs/heads/%.*s",
 	NULL
 };
-- 
1.7.8.rc0.108.g71b5ec

^ permalink raw reply related

* Re: aliases causing “Permission denied” error in git v1.7
From: Jeff King @ 2011-11-04 20:34 UTC (permalink / raw)
  To: Алексей Данченков
  Cc: git
In-Reply-To: <CALUFZ3n9cpHw3r3rcGriDqvJ+UM83L3Q19m=0YeAy51LBJzosA@mail.gmail.com>

On Fri, Nov 04, 2011 at 12:09:07PM +0400, Алексей Данченков wrote:

> $ git co -b newbranch
> $ git co oldbranch
> 
> results in "fatal: cannot exec 'git-co': Permission denied" error.

Git will try commands in your PATH before aliases. So you are running
into a permissions problem with a "git-co" in your PATH.

Or more likely, there is an inaccessible directory of your PATH (and we
get the same error code from execve whether it is the directory or the
file itself which lacks permission). Try "git foobar" and I suspect you
will see the same thing (it doesn't matter that you have no foobar
alias; we never get past the "try to exec it" stage).

If you are on Linux, try:

  strace -f -e execve git foobar

to see the the problematic entry that is returning EACCES. Or look
through your $PATH manually.

-Peff

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-04 20:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Shawn Pearce, git, James Bottomley, Jeff Garzik, Andrew Morton,
	linux-ide, LKML
In-Reply-To: <CA+55aFzstE-+NzfSAWMEokB7-rYsZOcZe9Ez-LxPNOKnciJ3UQ@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> So I suspect we should just apply this patch, but I didn't check
> exacty what the failed tests are - except for the first one, that just
> compares against a canned response (and the canned response should
> just be changed).

After applying your patch and running

$ perl -pi -e 'if (/\ttag /) {
    s/754b754407bf032e9a2f9d5a9ad05ca79a6b228f/6c9dec2b923228c9ff994c6cfe4ae16c12408dc5/;
    s/0567da4d5edd2ff4bb292a465ba9e64dcad9536b/c61a82b60967180544e3c19f819ddbd0c9f89899/;
    s/6134ee8f857693b96ff1cc98d3e2fd62b199e5a8/525b7fb068d59950d185a8779dc957c77eed73ba/;
}' t/t5515/fetch.*

to unpeel the three tags used in the test 5515 that used to expect
FETCH_HEAD to have peeled tags to expect tag objects themselves instead,
all tests passes.

> although I suspect it was just a fairly mindless case of "make it a
> commit, because the merge needs the commit" - never mind that the
> merge would peel it anyway.

I am 100% sure the machinery that comes up with the tree (or half-merged
conflicted state) does not mind being fed tags. After all they need to
peel them down to commits for common ancestor discovery, and they need to
further peel them down to trees to perform three-way merges.

However we would need to audit so that we do not accidentally record the
tag object names in the "parent" headers in the merge commits, which is
what I'll be doing next.

^ permalink raw reply

* Re: [PATCH 4/4] fsck: print progress
From: Jeff King @ 2011-11-04 20:14 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320421670-518-5-git-send-email-pclouds@gmail.com>

On Fri, Nov 04, 2011 at 10:47:50PM +0700, Nguyen Thai Ngoc Duy wrote:

>  static int traverse_reachable(void)
>  {
> +	struct progress *progress = NULL;
> +	unsigned int nr = 0;
>  	int result = 0;
> +	if (show_progress)
> +		progress = start_progress_delay("Checking connectivity", 0, 0, 2);
>  	while (pending.nr) {
>  		struct object_array_entry *entry;
>  		struct object *obj;
> @@ -146,7 +152,9 @@ static int traverse_reachable(void)
>  		entry = pending.objects + --pending.nr;
>  		obj = entry->item;
>  		result |= traverse_one_object(obj);
> +		display_progress(progress, ++nr);
>  	}
> +	stop_progress(&progress);
>  	return !!result;
>  }

Thanks, this is a good place to put a progress meter, too. If you're
feeling like pushing this topic further, "git prune" might be a good
place for a progress meter, too. It does a similar traversal[1] for
reachability, and makes "git gc" appear to hang at the end (we have nice
progress meters for packing, but it takes something like 25 seconds to
run "git prune" at the end, during which we are silent).

-Peff

[1] I wonder why fsck doesn't use mark_reachable from reachable.c.

^ permalink raw reply

* qgrit: graphical git rebase -i helper
From: Peter Oberndorfer @ 2011-11-04 19:38 UTC (permalink / raw)
  To: git; +Cc: Phil Hord, Junio C Hamano

Hi,

i wrote a small tool to support users during git rebase -i.
by presenting commits in a graphical interface.
(usable with git 1.7.8.rc0 and up)

Lacking a better idea i named it "qgrit" :-)
(Qt git rebase --interactive tool)

Notable features:
* Reorder commits by drag drop and up/down buttons
* Select actions from a combo box
* Undo reordering function
* Shows full commit message, not only title line
* Works on mingw (auto detects git path)
* Easy setup as rebase -i editor: run qgrit, click "Install qgrit" button
 (this also detects too old git versions)

For more information, screenshots and source code see:
https://github.com/qgrit/qgrit/wiki

Ideas for improvements, bugreports welcome!

Have fun,
Greetings Peter

^ permalink raw reply

* How do I get a squashed diff for review
From: Alexander Usov @ 2011-11-04 19:15 UTC (permalink / raw)
  To: git

Hi,

I'm wondering if there is an easy way to get a squashed diff of the
changes done on the feature branch for review.
In the simple cases (where feature branch is linear) there is an
absolutely fantastic way to get a patch for review:
git diff master...feature

However if the feature branch happened to be long-lived and had
mainline merged into it it's not going to work -- the
resulting diff would contain changes from the merge. The way we are
doing things now is to merge master into it
once more and then diff, however this is somewhat cumbersome. Is there
easier way to do it?

And while we are on the topic -- is there a tool for git similar to "bzr qdiff"?
It's a simple graphical diff viewer with 2 nice features -- it shows
complete diff (of multiple files) in a single window and
has a checkbox to switch between diff-only & full-text modes.
I have seen difftool, but it seems to work on per-file basis, and
something like "vi <(git diff ...)" lacks the easy way to
switch into full-text mode.

--
Best regards,
  Alexander.

^ permalink raw reply

* Re: [PATCH] http-push: don't always prompt for password
From: Junio C Hamano @ 2011-11-04 19:06 UTC (permalink / raw)
  To: Jeff King; +Cc: Stefan Naewe, git
In-Reply-To: <20111104174303.GA22568@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Since we now only call init_curl_http_auth when we know we need auth, I
> think it would make more sense to just move the user_name asking there,
> too, like:
>
>   static void init_curl_http_auth(CURL *result)
>   {
>           struct strbuf up = STRBUF_INIT;
>
>           if (!user_name)
>                   user_name = xstrdup(git_getpass_with_description("Username", description);
>           if (!user_pass)
>                   user_pass = xstrdup(git_getpass_with_description("Password", description);
>
>           strbuf_addf(&up, "%s:%s", user_name, user_pass);
>           curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL));
>   }
>
> And then it's easy to swap out the asking for credential_fill() when it
> becomes available. But I admit I don't care that much now, as I'll just
> end up doing that refactoring later with my credential patches anyway.

Yeah, let's not over-churn this part for now as we know it will change
anyway.

Thanks both!

^ 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