* [PATCH v3 0/3] Signed-commit
From: Junio C Hamano @ 2011-10-19 0:20 UTC (permalink / raw)
To: git
In-Reply-To: <7vaa9f3pk8.fsf@alter.siamese.dyndns.org>
This is another re-roll of the signed-commit topic.
- The first patch refactors where the current code invokes gpg for
signing and verification of tags;
- The second patch introduces signed commit objects;
- The third patch teaches "git log/show" to show the signature.
The internal API to drive "gpg" has been updated to better suit the
verification side, but I suspect that it needs another round of revamp to
slurp in the diagnostic message "gpg" produces to give us better control
of the formatting of the output. e.g.
$ git show --format="%h %s%n%gpgsign" -s HEAD
might want to say:
a030b7cf log: --show-signature
RSA key ID 96AFE6CB, Good signature from "Junio C Hamano <gitster@pobox.com>"
or something like that, but the series is not there yet.
Junio C Hamano (3):
Split GPG interface into its own helper library
commit: teach --gpg-sign option
log: --show-signature
Makefile | 2 +
builtin/commit-tree.c | 24 ++++++++-
builtin/commit.c | 12 ++++-
builtin/merge.c | 16 +++++-
builtin/tag.c | 76 ++-------------------------
builtin/verify-tag.c | 36 ++-----------
commit.c | 74 ++++++++++++++++++++++++++-
commit.h | 5 ++-
gpg-interface.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++
gpg-interface.h | 10 ++++
log-tree.c | 17 ++++++
notes-cache.c | 2 +-
notes-merge.c | 2 +-
revision.c | 2 +
revision.h | 1 +
tag.c | 5 ++
16 files changed, 310 insertions(+), 111 deletions(-)
create mode 100644 gpg-interface.c
create mode 100644 gpg-interface.h
--
1.7.7.388.g3a4b7
^ permalink raw reply
* [PATCH v3 2/3] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-19 0:20 UTC (permalink / raw)
To: git
In-Reply-To: <1318983645-18897-1-git-send-email-gitster@pobox.com>
And this uses the gpg-interface.[ch] to allow signing the commit, i.e.
$ git commit --gpg-sign -m foo
You need a passphrase to unlock the secret key for
user: "Junio C Hamano <gitster@pobox.com>"
4096-bit RSA key, ID 96AFE6CB, created 2011-10-03 (main key ID 713660A7)
[master 8457d13] foo
1 files changed, 1 insertions(+), 0 deletions(-)
The lines of GPG detached signature are placed in new header lines, after
the standard tree/parent/author/committer headers, instead of tucking the
signature block at the end of the commit log message text (similar to how
signed tag is done), for multiple reasons:
- The signature won't clutter output from "git log" and friends if it is
in the extra header. If we place it at the end of the log message, we
would need to teach "git log" and friends to strip the signature block
with an option.
- Teaching new versions of "git log" and "gitk" to optionally verify and
show signatures is cleaner if we structurally know where the signature
block is (instead of scanning in the commit log message).
- The signature needs to be stripped upon various commit rewriting
operations, e.g. rebase, filter-branch, etc. They all already ignore
unknown headers, but if we place signature in the log message, all of
these tools (and third-party tools) also need to learn how a signature
block would look like.
- When we added the optional encoding header, all the tools (both in tree
and third-party) that acts on the raw commit object should have been
fixed to ignore headers they do not understand, so it is not like that
new header would be more likely to break than extra text in the commit.
A commit made with the above sample sequence would look like this:
$ git cat-file commit HEAD
tree 3cd71d90e3db4136e5260ab54599791c4f883b9d
parent b87755351a47b09cb27d6913e6e0e17e6254a4d4
author Junio C Hamano <gitster@pobox.com> 1317862251 -0700
committer Junio C Hamano <gitster@pobox.com> 1317862251 -0700
gpgsig -----BEGIN PGP SIGNATURE-----
gpgsig Version: GnuPG v1.4.10 (GNU/Linux)
gpgsig
gpgsig iQIcBAABAgAGBQJOjPtrAAoJELC16IaWr+bL4TMP/RSe2Y/jYnCkds9unO5JEnfG
gpgsig ...
gpgsig =dt98
gpgsig -----END PGP SIGNATURE-----
foo
but "git log" (unless you ask for it with --pretty=raw) output is not
cluttered with the signature information.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* The header field was updated from "sig" to "gpgsig" so that later we
won't have to regret the choice when we introduce different kind of
signature mechanism.
builtin/commit-tree.c | 24 +++++++++++++++++++++---
builtin/commit.c | 12 ++++++++++--
builtin/merge.c | 16 ++++++++++++++--
commit.c | 40 +++++++++++++++++++++++++++++++++++++++-
commit.h | 2 +-
notes-cache.c | 2 +-
notes-merge.c | 2 +-
7 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index d083795..a17811f 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -8,8 +8,9 @@
#include "tree.h"
#include "builtin.h"
#include "utf8.h"
+#include "gpg-interface.h"
-static const char commit_tree_usage[] = "git commit-tree <sha1> [(-p <sha1>)...] < changelog";
+static const char commit_tree_usage[] = "git commit-tree [-S<signer>] <sha1> [(-p <sha1>)...] < changelog";
static void new_parent(struct commit *parent, struct commit_list **parents_p)
{
@@ -25,6 +26,14 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
commit_list_insert(parent, parents_p);
}
+static int commit_tree_config(const char *var, const char *value, void *cb)
+{
+ int status = git_gpg_config(var, value, NULL);
+ if (status)
+ return status;
+ return git_default_config(var, value, cb);
+}
+
int cmd_commit_tree(int argc, const char **argv, const char *prefix)
{
int i;
@@ -32,11 +41,19 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
unsigned char tree_sha1[20];
unsigned char commit_sha1[20];
struct strbuf buffer = STRBUF_INIT;
+ const char *sign_commit = NULL;
- git_config(git_default_config, NULL);
+ git_config(commit_tree_config, NULL);
if (argc < 2 || !strcmp(argv[1], "-h"))
usage(commit_tree_usage);
+
+ if (!memcmp(argv[1], "-S", 2)) {
+ sign_commit = argv[1] + 2;
+ argv++;
+ argc--;
+ }
+
if (get_sha1(argv[1], tree_sha1))
die("Not a valid object name %s", argv[1]);
@@ -56,7 +73,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
if (strbuf_read(&buffer, 0, 0) < 0)
die_errno("git commit-tree: failed to read");
- if (commit_tree(buffer.buf, tree_sha1, parents, commit_sha1, NULL)) {
+ if (commit_tree(buffer.buf, tree_sha1, parents, commit_sha1,
+ NULL, sign_commit)) {
strbuf_release(&buffer);
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index cbc9613..90cf7e8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -26,6 +26,7 @@
#include "unpack-trees.h"
#include "quote.h"
#include "submodule.h"
+#include "gpg-interface.h"
static const char * const builtin_commit_usage[] = {
"git commit [options] [--] <filepattern>...",
@@ -85,6 +86,8 @@ static int all, edit_flag, also, interactive, patch_interactive, only, amend, si
static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *sign_commit;
+
/*
* The default commit message cleanup mode will remove the lines
* beginning with # (shell comments) and leading and trailing
@@ -144,6 +147,8 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"),
+ { OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id",
+ "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
/* end commit message options */
OPT_GROUP("Commit contents options"),
@@ -1323,6 +1328,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
static int git_commit_config(const char *k, const char *v, void *cb)
{
struct wt_status *s = cb;
+ int status;
if (!strcmp(k, "commit.template"))
return git_config_pathname(&template_file, k, v);
@@ -1330,7 +1336,9 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
-
+ status = git_gpg_config(k, v, NULL);
+ if (status)
+ return status;
return git_status_config(k, v, s);
}
@@ -1481,7 +1489,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
if (commit_tree(sb.buf, active_cache_tree->sha1, parents, sha1,
- author_ident.buf)) {
+ author_ident.buf, sign_commit)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index ab4077f..53cff02 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -26,6 +26,7 @@
#include "merge-recursive.h"
#include "resolve-undo.h"
#include "remote.h"
+#include "gpg-interface.h"
#define DEFAULT_TWOHEAD (1<<0)
#define DEFAULT_OCTOPUS (1<<1)
@@ -63,6 +64,7 @@ static int allow_rerere_auto;
static int abort_current_merge;
static int show_progress = -1;
static int default_to_upstream;
+static const char *sign_commit;
static struct strategy all_strategy[] = {
{ "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -206,6 +208,8 @@ static struct option builtin_merge_options[] = {
OPT_BOOLEAN(0, "abort", &abort_current_merge,
"abort the current in-progress merge"),
OPT_SET_INT(0, "progress", &show_progress, "force progress reporting", 1),
+ { OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id",
+ "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_END()
};
@@ -525,6 +529,8 @@ static void parse_branch_merge_options(char *bmo)
static int git_merge_config(const char *k, const char *v, void *cb)
{
+ int status;
+
if (branch && !prefixcmp(k, "branch.") &&
!prefixcmp(k + 7, branch) &&
!strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
@@ -562,6 +568,10 @@ static int git_merge_config(const char *k, const char *v, void *cb)
default_to_upstream = git_config_bool(k, v);
return 0;
}
+
+ status = git_gpg_config(k, v, NULL);
+ if (status)
+ return status;
return git_diff_ui_config(k, v, cb);
}
@@ -870,7 +880,8 @@ static int merge_trivial(void)
parent->next->item = remoteheads->item;
parent->next->next = NULL;
run_prepare_commit_msg();
- commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);
+ commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL,
+ sign_commit);
finish(result_commit, "In-index merge");
drop_save();
return 0;
@@ -900,7 +911,8 @@ static int finish_automerge(struct commit_list *common,
free_commit_list(remoteheads);
strbuf_addch(&merge_msg, '\n');
run_prepare_commit_msg();
- commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);
+ commit_tree(merge_msg.buf, result_tree, parents, result_commit,
+ NULL, sign_commit);
strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
finish(result_commit, buf.buf);
strbuf_release(&buf);
diff --git a/commit.c b/commit.c
index 97b4327..4bff3cd 100644
--- a/commit.c
+++ b/commit.c
@@ -6,6 +6,7 @@
#include "diff.h"
#include "revision.h"
#include "notes.h"
+#include "gpg-interface.h"
int save_commit_buffer = 1;
@@ -814,6 +815,40 @@ struct commit_list *reduce_heads(struct commit_list *heads)
return result;
}
+static const char gpg_sig_header[] = "gpgsig ";
+static const int gpg_sig_header_len = sizeof(gpg_sig_header) - 1;
+
+static int do_sign_commit(struct strbuf *buf, const char *keyid)
+{
+ struct strbuf sig = STRBUF_INIT;
+ int inspos, copypos;
+
+ /* find the end of the header */
+ inspos = strstr(buf->buf, "\n\n") - buf->buf + 1;
+
+ if (!keyid || !*keyid)
+ keyid = get_signing_key();
+ if (sign_buffer(buf, &sig, keyid)) {
+ strbuf_release(&sig);
+ return -1;
+ }
+
+ for (copypos = 0; sig.buf[copypos]; ) {
+ const char *bol = sig.buf + copypos;
+ const char *eol = strchrnul(bol, '\n');
+ int len = (eol - bol) + !!*eol;
+
+ strbuf_insert(buf, inspos, gpg_sig_header, gpg_sig_header_len);
+ inspos += gpg_sig_header_len;
+ strbuf_insert(buf, inspos, bol, len);
+ inspos += len;
+ copypos += len;
+ }
+ strbuf_release(&sig);
+ return 0;
+}
+
+
static const char commit_utf8_warn[] =
"Warning: commit message does not conform to UTF-8.\n"
"You may want to amend it after fixing the message, or set the config\n"
@@ -821,7 +856,7 @@ static const char commit_utf8_warn[] =
int commit_tree(const char *msg, unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author)
+ const char *author, const char *sign_commit)
{
int result;
int encoding_is_utf8;
@@ -864,6 +899,9 @@ int commit_tree(const char *msg, unsigned char *tree,
if (encoding_is_utf8 && !is_utf8(buffer.buf))
fprintf(stderr, commit_utf8_warn);
+ if (sign_commit && do_sign_commit(&buffer, sign_commit))
+ return -1;
+
result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret);
strbuf_release(&buffer);
return result;
diff --git a/commit.h b/commit.h
index 12d100b8..8c2419b 100644
--- a/commit.h
+++ b/commit.h
@@ -175,6 +175,6 @@ struct commit_list *reduce_heads(struct commit_list *heads);
extern int commit_tree(const char *msg, unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author);
+ const char *author, const char *sign_commit);
#endif /* COMMIT_H */
diff --git a/notes-cache.c b/notes-cache.c
index 4c8984e..c36a960 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -56,7 +56,7 @@ int notes_cache_write(struct notes_cache *c)
if (write_notes_tree(&c->tree, tree_sha1))
return -1;
- if (commit_tree(c->validity, tree_sha1, NULL, commit_sha1, NULL) < 0)
+ if (commit_tree(c->validity, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0)
return -1;
if (update_ref("update notes cache", c->tree.ref, commit_sha1, NULL,
0, QUIET_ON_ERR) < 0)
diff --git a/notes-merge.c b/notes-merge.c
index e1aaf43..c29c434 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -546,7 +546,7 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
/* else: t->ref points to nothing, assume root/orphan commit */
}
- if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL))
+ if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL))
die("Failed to commit notes tree to database");
}
--
1.7.7.388.g3a4b7
^ permalink raw reply related
* [PATCH v3 1/3] Split GPG interface into its own helper library
From: Junio C Hamano @ 2011-10-19 0:20 UTC (permalink / raw)
To: git
In-Reply-To: <1318983645-18897-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.
"verify-tag" (aka "tag -v") saved 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>
---
Makefile | 2 +
builtin/tag.c | 76 ++-------------------------
builtin/verify-tag.c | 36 ++-----------
gpg-interface.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++
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 8d6d451..2183223 100644
--- a/Makefile
+++ b/Makefile
@@ -530,6 +530,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
@@ -620,6 +621,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 667515e..fb0d4a1 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..4bbcf77 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,17 @@ 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, 0);
}
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..62f3806
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,137 @@
+#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,
+ int gpg_output_to)
+{
+ 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_to < 0)
+ gpg.no_stderr = 1;
+ else
+ gpg.err = gpg_output_to;
+ 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);
+ 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..df58bb9
--- /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, int gpg_output_to);
+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.7.388.g3a4b7
^ permalink raw reply related
* [PATCH v3 3/3] log: --show-signature
From: Junio C Hamano @ 2011-10-19 0:20 UTC (permalink / raw)
To: git
In-Reply-To: <1318983645-18897-1-git-send-email-gitster@pobox.com>
This teaches the "log" family of commands to pass the GPG signature in the
commit objects to "gpg --verify" via the verify_signed_buffer() interface
used to verify signed tag objects. E.g.
$ git show --show-signature -s HEAD
would show GPG output in the header part of the output.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* We may want to capture good/bad/untrusted signature information from
GPG and teach userformat to show that, but this code is not there yet.
commit.c | 34 ++++++++++++++++++++++++++++++++++
commit.h | 3 +++
log-tree.c | 17 +++++++++++++++++
revision.c | 2 ++
revision.h | 1 +
5 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/commit.c b/commit.c
index 4bff3cd..93045a2 100644
--- a/commit.c
+++ b/commit.c
@@ -848,6 +848,40 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)
return 0;
}
+int parse_signed_commit(const unsigned char *sha1,
+ struct strbuf *payload, struct strbuf *signature)
+{
+ unsigned long size;
+ enum object_type type;
+ char *buffer = read_sha1_file(sha1, &type, &size);
+ int in_header, saw_signature = -1;
+ char *line;
+
+ if (!buffer || type != OBJ_COMMIT)
+ goto cleanup;
+
+ line = buffer;
+ in_header = 1;
+ saw_signature = 0;
+ while (*line) {
+ char *next = strchrnul(line, '\n');
+ if (*next)
+ next++;
+ if (in_header && !prefixcmp(line, gpg_sig_header)) {
+ const char *sig = line + gpg_sig_header_len;
+ strbuf_add(signature, sig, next - sig);
+ saw_signature = 1;
+ } else {
+ strbuf_add(payload, line, next - line);
+ }
+ if (*line == '\n')
+ in_header = 0;
+ line = next;
+ }
+ cleanup:
+ free(buffer);
+ return saw_signature;
+}
static const char commit_utf8_warn[] =
"Warning: commit message does not conform to UTF-8.\n"
diff --git a/commit.h b/commit.h
index 8c2419b..1885471 100644
--- a/commit.h
+++ b/commit.h
@@ -177,4 +177,7 @@ extern int commit_tree(const char *msg, unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
const char *author, const char *sign_commit);
+extern int parse_signed_commit(const unsigned char *sha1,
+ struct strbuf *message, struct strbuf *signature);
+
#endif /* COMMIT_H */
diff --git a/log-tree.c b/log-tree.c
index 24c295e..749bb65 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -8,6 +8,7 @@
#include "refs.h"
#include "string-list.h"
#include "color.h"
+#include "gpg-interface.h"
struct decoration name_decoration = { "object names" };
@@ -395,6 +396,19 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
*extra_headers_p = extra_headers;
}
+static void show_signature(struct rev_info *opt, struct commit *commit)
+{
+ struct strbuf payload = STRBUF_INIT;
+ struct strbuf signature = STRBUF_INIT;
+
+ if (parse_signed_commit(commit->object.sha1, &payload, &signature) >= 0) {
+ verify_signed_buffer(payload.buf, payload.len,
+ signature.buf, signature.len, 0);
+ }
+ strbuf_release(&payload);
+ strbuf_release(&signature);
+}
+
void show_log(struct rev_info *opt)
{
struct strbuf msgbuf = STRBUF_INIT;
@@ -502,6 +516,9 @@ void show_log(struct rev_info *opt)
}
}
+ if (opt->show_signature)
+ show_signature(opt, commit);
+
if (!commit->buffer)
return;
diff --git a/revision.c b/revision.c
index c46cfaa..860a312 100644
--- a/revision.c
+++ b/revision.c
@@ -1381,6 +1381,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->show_notes = 1;
revs->show_notes_given = 1;
revs->notes_opt.use_default_notes = 1;
+ } else if (!strcmp(arg, "--show-signature")) {
+ revs->show_signature = 1;
} else if (!prefixcmp(arg, "--show-notes=") ||
!prefixcmp(arg, "--notes=")) {
struct strbuf buf = STRBUF_INIT;
diff --git a/revision.h b/revision.h
index 3d64ada..198bb95 100644
--- a/revision.h
+++ b/revision.h
@@ -89,6 +89,7 @@ struct rev_info {
show_merge:1,
show_notes:1,
show_notes_given:1,
+ show_signature:1,
pretty_given:1,
abbrev_commit:1,
abbrev_commit_given:1,
--
1.7.7.388.g3a4b7
^ permalink raw reply related
* Re: [PATCHv6 0/5] Moving gitweb documentation to manpages
From: Junio C Hamano @ 2011-10-19 4:29 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <1318763255-23495-1-git-send-email-jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> The original patch adding manpage for /etc/gitweb.conf was sent by
> Drew Northup (as can be seen from shortlog); I have added manpage for
> gitweb itself, inspired by his patch. Unfortunately Drew doesn't
> currently have time to work on this patch (patch series), so that is
> why it is me resending this series (yet again).
>
> The difference with previous version
>
> [PATCHv5/RFC 0/6] Moving gitweb documentation to manpages
> http://thread.gmane.org/gmane.comp.version-control.git/183166
> http://marc.info/?l=git&m=131809875619259&w=2
>
> is that "Documentation: Preparation for gitweb manpages" (originally a
> first patch) got removed, and two main patches got improved. Other
> patches are unchanged from previous version.
>
>
> Pull request:
> ~~~~~~~~~~~~~
> The following changes since commit 288396994f077eec7e7db0017838a5afbfbf81e3:
>
> Sync with maint (2011-10-15 20:59:50 -0700)
>
> are available in the git repository at:
>
> git://repo.or.cz/git/jnareb-git.git gitweb/doc
I fetched it and compared with the result of applying this series; it
appears that the version fetched from there is full of random whitespace
issues, e.g. ('$' added by "cat -e")
diff --git b/Documentation/gitweb.txt a/Documentation/gitweb.txt$
index 605a085..353e37f 100644$
--- b/Documentation/gitweb.txt$
+++ a/Documentation/gitweb.txt$
@@ -370,12 +370,12 @@ commitdiff::$
view shows information about commit in more detail, the 'commitdiff'$
action shows changeset for given commit.$
$
-patch::$
+patch:: $
Returns the commit in plain text mail format, suitable for applying with$
linkgit:git-am[1].$
The series seems to have been rebased on a more recent 'master' and does
not match the "since commit..." stated above (not a major offense; just
mentioning as I noticed it).
In any case, I've queued the one from the mailing list. Thanks.
^ permalink raw reply related
* Re: What should "git fetch origin +next" should do?
From: Junio C Hamano @ 2011-10-19 4:31 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Jeff King, git
In-Reply-To: <4E9CA5E2.2020701@xiplink.com>
Marc Branchaud <marcnarc@xiplink.com> writes:
> In all cases if the command-line refspec has no RHS then git should try to
> figure out which local ref to update from the config, and it should die if it
> can't figure out a local ref to create or update. (As I said above, maybe
> allow "git fetch origin foo:" to let the user put the tip of origin's foo ref
> into FETCH_HEAD.)
I'd agree with everything you said in your message, except for the above
"it should die" part.
You are assuming that the user knows what his configured refspecs would
normally do and that is the whole reason why "git fetch origin next" that
would update the remote tracking branch specified for the upstream's next
might feel more natural than the current behaviour. I too think that is a
reasonable assumption.
With the same assumption, if you said "git fetch origin frotz" when you
know you are not usually tracking the remote 'frotz' branch, the command
just should store what is fetched in FETCH_HEAD (and nowhere else) without
dying. I do not think how it helps the user to die in that situation.
> All this gets a bit more complicated if the user has currently checked out
> the a ref that should be updated (regardless of the presence of a LHS +).
That "do not update the currently checked out branch" already exists and
is correctly handled by "git fetch", I think.
^ permalink raw reply
* Re: Git --reference bug(?)
From: Michael Haggerty @ 2011-10-19 5:01 UTC (permalink / raw)
To: Andrea Gelmini; +Cc: gitster, git
In-Reply-To: <CAK-xaQaUxJ5c_kN48g7-J9fosDv6awbAFQSFLpF2fA+hc-i-MA@mail.gmail.com>
On 10/19/2011 12:04 AM, Andrea Gelmini wrote:
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> /tmp/3.1 --reference /home/gelma/dev/kernel/linus/
> Cloning into /tmp/3.1...
> fatal: Reference has invalid format: 'refs/tags/3.1.1.1^{}'
> fatal: The remote end hung up unexpectedly
The upstream repo reports what look like non-reference references:
$ git ls-remote --tags
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git | head
5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11
c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11^{}
5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11-tree
c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11-tree^{}
26791a8bcf0e6d33f43aef7682bdb555236d56de refs/tags/v2.6.12
9ee1c939d1cb936b1f98e8d81aeffab57bae46ab refs/tags/v2.6.12^{}
9e734775f7c22d2f89943ad6c745571f1930105f refs/tags/v2.6.12-rc2
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 refs/tags/v2.6.12-rc2^{}
0397236d43e48e821cce5bbe6a80a1a56bb7cc3a refs/tags/v2.6.12-rc3
a2755a80f40e5794ddc20e00f781af9d6320fafb refs/tags/v2.6.12-rc3^{}
[...]
I've never seen this format before; is this the remote protocol for
peeled refs or maybe the behavior of an old version of git?
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-19 5:28 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <4E9609E3.1000300@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
>>> Nit: the seen_non_root_char variable can be replaced by an early "return
>>> 0" from the loop and "return 1" if the loop falls through.
>>
>> Hmm, I thought that would fail when you feed "refs/heads/master" to the
>> function.
>
> You're right. My brain must be scrambled from all of the rebasing that
> I have been doing ;-)
>
> How about adding
>
> /*
> * Accept strings that are either ALL_CAPS or include a '/'.
> */
>
> (I think the underscore is implied by the example, but the comment could
> be expanded if necessary to be explicit.)
I was trying to summarize this topic for Release Notes.
Possibly incompatible changes
-----------------------------
* Special refs such as "HEAD", "MERGE_HEAD", and "FETCH_HEAD" are now
restricted to names with only uppercase letters and underscore. All
other refs must live under refs/ namespace. Earlier, you could
confuse git by storing an object name in $GIT_DIR/tmp/junk and say
"git show tmp/junk", but this will no longer work.
But noticed that "git update-ref tmp/junk HEAD" does create such a ref
that won't be recognized, and "git check-ref-format tmp/junk" is happy.
I think we would need to restrict check_ref_format() so that these
commands (and possibly others, but I think that single function will cover
pretty much everything) also reject "tmp/junk" immediately below $GIT_DIR
as a bad string. Otherwise we cannot merge these fixups, which would mean
we would have to revert the "Clean up refname checks and normalization"
series, at least the part that started emitting the "warning", which is a
mess I would rather want to avoid.
Opinions on how to get us out of this mess?
^ permalink raw reply
* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-19 6:19 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <7v39epft32.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I was trying to summarize this topic for Release Notes.
>
> Possibly incompatible changes
> -----------------------------
>
> * Special refs such as "HEAD", "MERGE_HEAD", and "FETCH_HEAD" are now
> restricted to names with only uppercase letters and underscore. All
> other refs must live under refs/ namespace. Earlier, you could
> confuse git by storing an object name in $GIT_DIR/tmp/junk and say
> "git show tmp/junk", but this will no longer work.
>
> But noticed that "git update-ref tmp/junk HEAD" does create such a ref
> that won't be recognized, and "git check-ref-format tmp/junk" is happy.
>
> I think we would need to restrict check_ref_format() so that these
> commands (and possibly others, but I think that single function will cover
> pretty much everything) also reject "tmp/junk" immediately below $GIT_DIR
> as a bad string. Otherwise we cannot merge these fixups, which would mean
> we would have to revert the "Clean up refname checks and normalization"
> series, at least the part that started emitting the "warning", which is a
> mess I would rather want to avoid.
>
> Opinions on how to get us out of this mess?
Addendum.
I was digging this further and see fairly large conflicts between the bulk
of "clean up refname checks and normalization" topic and the logic to
avoid the additional warning by tightening the dwimmery.
check_refname_format() has always assumed that it is OK to be called at
any level of substring, and there are many code like this one (example is
from remote.c::get_fetch_map()):
for (rmp = &ref_map; *rmp; ) {
if ((*rmp)->peer_ref) {
if (check_refname_format((*rmp)->peer_ref->name + 5,
REFNAME_ALLOW_ONELEVEL)) {
struct ref *ignore = *rmp;
error("* Ignoring funny ref '%s' locally",
(*rmp)->peer_ref->name);
This code somehow _knows_ that peer_ref->name begins with "refs/" and that
is the reason it adds 5 to skip that known part. In this particular case,
I think we can simply drop the +5 and allow-onelevel, but there are other
instances of the calls to the function that feeds the rest of the refname
string, skipping leading substring (not necessarily "refs/"), assuming
that any component string is either valid or invalid no matter where it
appears in the full refname. I wouldn't be surprised if some callers do
not even have enough information to tell what the leading substring would
be in the full refname context (e.g. parsing of "master:master" refspec,
relying on the later dwimmery to add refs/heads/ in front, could just
verify that "master" is in good format with allow-onelevel).
The new restriction bolted on to that logic in jc/check-ref-format-fixup
series to work around the new warning in 629cd3a (resolve_ref(): emit
warnings for improperly-formatted references, 2011-09-15) is incompatible
with the assumption, as we would need to check full refname, and treat the
first refname component very differently from other components. It has to
be either "refs" in multi-component refname, or all caps in a one-level
one, but the callers of check_refname_format() are not designed to feed
the full refname to begin with.
I am tempted to revert 629cd3a (resolve_ref(): emit warnings for
improperly-formatted references, 2011-09-15) that started giving the
warnings, and drop the jc/check-ref-format-fixup topic [*1*] altogether,
but that is a short-sighted workaround I would rather want to avoid. It
essentially declares that the "Clean up refname checks" topic was a
failure and did not manage to really clean things up.
A possible alternative might be to leave check_refname_format() and its
callers as they are, introduce check_full_refname() function that knows
the new restriction on top of check-ref-format-fixup, and use that in
lock_ref_sha1(), lock_any_ref_for_update() and is_refname_available()
[*2*]. That way, we can keep the potentially useful "ill-formed contents
in the ref" warning and avoid possible confusion caused by random files
that are directly under $GIT_DIR, which would be far more preferable in
the longer term.
Anybody wants to give it a try?
[Footnote]
*1* The topic is misnamed; it is about fixing dwimmery, not checking.
*2* This currently does not seem to check the well-formedness of the ref
it is creating at all; it should check the "ref", but not "oldref" to
allow renaming a malformed ref to correct earlier mistakes.
^ permalink raw reply
* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19 6:21 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Andrea Gelmini, gitster, git
In-Reply-To: <4E9E59A7.7070307@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 10/19/2011 12:04 AM, Andrea Gelmini wrote:
>> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> /tmp/3.1 --reference /home/gelma/dev/kernel/linus/
>> Cloning into /tmp/3.1...
>> fatal: Reference has invalid format: 'refs/tags/3.1.1.1^{}'
>> fatal: The remote end hung up unexpectedly
>
> The upstream repo reports what look like non-reference references:
>
> $ git ls-remote --tags
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git | head
> 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11
> c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11^{}
> 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11-tree
> c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11-tree^{}
> 26791a8bcf0e6d33f43aef7682bdb555236d56de refs/tags/v2.6.12
> 9ee1c939d1cb936b1f98e8d81aeffab57bae46ab refs/tags/v2.6.12^{}
> 9e734775f7c22d2f89943ad6c745571f1930105f refs/tags/v2.6.12-rc2
> 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 refs/tags/v2.6.12-rc2^{}
> 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a refs/tags/v2.6.12-rc3
> a2755a80f40e5794ddc20e00f781af9d6320fafb refs/tags/v2.6.12-rc3^{}
> [...]
>
> I've never seen this format before; is this the remote protocol for
> peeled refs or maybe the behavior of an old version of git?
This should be very well documented and has been the output from fairly
early days of ls-remote.
^ permalink raw reply
* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19 6:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, Andrea Gelmini, git
In-Reply-To: <7vpqhtec2n.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a refs/tags/v2.6.12-rc3
>> a2755a80f40e5794ddc20e00f781af9d6320fafb refs/tags/v2.6.12-rc3^{}
>> [...]
>>
>> I've never seen this format before; is this the remote protocol for
>> peeled refs or maybe the behavior of an old version of git?
>
> This should be very well documented and has been the output from fairly
> early days of ls-remote.
I take the first half back. The ls-remote documentation seems to have
bit-rotten quite a bit.
IIRC, we started doing this when we introduced auto-following of the tag
objects. Even update-server-info knows about it so it way predates smart
HTTP and is a fairly old and established behaviour.
^ permalink raw reply
* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19 6:33 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Andrea Gelmini, git
In-Reply-To: <7vipnlebwb.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a refs/tags/v2.6.12-rc3
>>> a2755a80f40e5794ddc20e00f781af9d6320fafb refs/tags/v2.6.12-rc3^{}
>>> [...]
>>>
>>> I've never seen this format before; is this the remote protocol for
>>> peeled refs or maybe the behavior of an old version of git?
>>
>> This should be very well documented and has been the output from fairly
>> early days of ls-remote.
>
> I take the first half back. The ls-remote documentation seems to have
> bit-rotten quite a bit.
>
> IIRC, we started doing this when we introduced auto-following of the tag
> objects. Even update-server-info knows about it so it way predates smart
> HTTP and is a fairly old and established behaviour.
In addition, you must be careful about what is added with add_extra_ref();
they are often not refs but are placeholders for Git to know that the
history leading to it is complete, even though they do not exist as
refs. The values of real refs that exist in your alternate object store
are treated this way, because you know you do not have to fetch (if you
are initiating fetch-pack) or receive (if the other side is initiating
send-pack) histories leading to them from the other side.
^ permalink raw reply
* Re: [PATCH 2/2] daemon: report permission denied error to clients
From: Clemens Buchacher @ 2011-10-19 6:33 UTC (permalink / raw)
To: Junio C Hamano, Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <20111018204101.GB2072@ecki>
Clemens Buchacher <drizzd@aon.at> wrote:
>
> I guess if permission is denied for access over git://, then nobody
> can use the repository. So it's clearly a server side issue.
>
> This change probably makes more sense for local access and over
> ssh. I already have a similar patch brewing for that.
As far as security is concerned, we have to treat ssh the same as git://, unless the user has permission to execute arbitrary commands and not just git-upload-pack. But I can think of no way to figure that out on the server side.
^ permalink raw reply
* Re: [PATCH 1/2] t1300: put git invocations inside test function
From: Johannes Sixt @ 2011-10-19 6:41 UTC (permalink / raw)
To: Jeff King; +Cc: Carlos Martín Nieto, Junio C Hamano, git
In-Reply-To: <20111012182920.GA18948@sigill.intra.peff.net>
Am 10/12/2011 20:29, schrieb Jeff King:
> @@ -750,13 +759,6 @@ test_expect_success NOT_MINGW 'get --path copes with unset $HOME' '
> test_cmp expect result
> '
>
> -rm .git/config
> -
> -git config quote.leading " test"
> -git config quote.ending "test "
> -git config quote.semicolon "test;test"
> -git config quote.hash "test#test"
> -
> cat > expect << EOF
> [quote]
> leading = " test"
> @@ -764,8 +766,14 @@ cat > expect << EOF
> semicolon = "test;test"
> hash = "test#test"
> EOF
> -
> -test_expect_success 'quoting' 'cmp .git/config expect'
> +test_expect_success 'quoting' '
> + rm .git/config &&
> + git config quote.leading " test" &&
> + git config quote.ending "test " &&
> + git config quote.semicolon "test;test" &&
> + git config quote.hash "test#test" &&
> + test_cmp expect .git/config
> +'
This innocently looking hunk fails on Windows, because the preceding tests
are skipped, and .git/config does not exist. I was tempted to just change
this to 'rm -f'. But there are a few other instances of 'rm .git/config'
in this file that were *not* moved inside the test function.
How would you like this solved?
- Move this one out again
- Add -f to just this one
- Add -f everywhere
- a combination of the above?
-- Hannes
^ permalink raw reply
* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19 6:55 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Andrea Gelmini, git
In-Reply-To: <7v8vohebhx.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
> In addition, you must be careful about what is added with add_extra_ref();
> they are often not refs but are placeholders for Git to know that the
> history leading to it is complete, even though they do not exist as
> refs. The values of real refs that exist in your alternate object store
> are treated this way, because you know you do not have to fetch (if you
> are initiating fetch-pack) or receive (if the other side is initiating
> send-pack) histories leading to them from the other side.
I think a quick and simple rule would be that add_extra_ref() is to give
our history in the object database extra anchor points to avoid fetching
or receiving pushes of unnecessary history into our object database, when
we know our object database has certain histories that are not reachable
from our own refs available. The names given to add_extra_ref() would not
follow any normal refname rules (in fact, "refs/tags/v2.6.12-rc3^{}"
peeled notation was designed not to collide with real refs, so was ".have"
sent from receive-pack.c to send-pack.c even though the latter is not kept
track of with the refs mechanism).
They do not have to be shown when resolve_ref() is called. They only need
to appear in for_each_ref() so that revision walking machinery can use
them when we need to compute the set-difference of commits between what we
have and what the other side has.
Hope this clears things up a bit.
^ permalink raw reply
* Re: [PATCH 1/2] t1300: put git invocations inside test function
From: Junio C Hamano @ 2011-10-19 7:04 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Carlos Martín Nieto, git
In-Reply-To: <4E9E7115.60303@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> This innocently looking hunk fails on Windows, because the preceding tests
> are skipped, and .git/config does not exist. I was tempted to just change
> this to 'rm -f'. But there are a few other instances of 'rm .git/config'
> in this file that were *not* moved inside the test function.
>
> How would you like this solved?
>
> - Move this one out again
> - Add -f to just this one
> - Add -f everywhere
> - a combination of the above?
Probably "rm -f .git/config" to all tests would be the most appropriate,
as the desire to start from an empty config file is a sign that each test
wants to be as independent as possible from previous tests.
An alternative approach to achieve the independence would be to make each
test responsible for cleaning up after itself, with test_when_finished,
but that won't work for the first one, as the test framework would give
you the original one. Besides, that's trying to be cooperative when each
test can fail in an unexpected way (after all the purpose of tests is to
fail when things go wrong). In order to achieve isolation, each test
protecting themselves from others, as long as it can be easily and
reasonably done (e.g. a single "rm -f"), is probably far more preferrable
than each test trying to clean after itself, risking possible failure to
do so.
^ permalink raw reply
* [PATCH] t1300: attempting to remove a non-existent .git/config is not an error
From: Johannes Sixt @ 2011-10-19 7:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Carlos Martín Nieto, git
In-Reply-To: <7vzkgxcviz.fsf@alter.siamese.dyndns.org>
From: Johannes Sixt <j6t@kdbg.org>
Since some tests before test number 79 ("quoting") are skipped, .git/config
does not exist and 'rm .git/config' fails. Fix this particular case.
While at it, move other instance of 'rm .git/config' that occur in this
file inside the test function to document that the test cases want to
protect themselves from remnants of earlier tests.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
You may think that the justification of this change would be the
other way round: protect test cases, then incidentally this fixes
a failure on Windows. But for me it is as described: The motivation
is the fix for Windows, and without that, the "protect test cases"
part would not have happened. :-)
t/t1300-repo-config.sh | 19 +++++++------------
1 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index fba5ae0..51caff0 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -558,8 +558,6 @@ EOF
test_expect_success "section was removed properly" \
"test_cmp expect .git/config"
-rm .git/config
-
cat > expect << EOF
[gitcvs]
enabled = true
@@ -570,6 +568,7 @@ EOF
test_expect_success 'section ending' '
+ rm -f .git/config &&
git config gitcvs.enabled true &&
git config gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite &&
git config gitcvs.dbname %Ggitcvs2.%a.%m.sqlite &&
@@ -642,8 +641,6 @@ test_expect_success 'invalid bool (set)' '
test_must_fail git config --bool bool.nobool foobar'
-rm .git/config
-
cat > expect <<\EOF
[bool]
true1 = true
@@ -658,6 +655,7 @@ EOF
test_expect_success 'set --bool' '
+ rm -f .git/config &&
git config --bool bool.true1 01 &&
git config --bool bool.true2 -1 &&
git config --bool bool.true3 YeS &&
@@ -668,8 +666,6 @@ test_expect_success 'set --bool' '
git config --bool bool.false4 FALSE &&
cmp expect .git/config'
-rm .git/config
-
cat > expect <<\EOF
[int]
val1 = 1
@@ -679,13 +675,12 @@ EOF
test_expect_success 'set --int' '
+ rm -f .git/config &&
git config --int int.val1 01 &&
git config --int int.val2 -1 &&
git config --int int.val3 5m &&
cmp expect .git/config'
-rm .git/config
-
cat >expect <<\EOF
[bool]
true1 = true
@@ -699,6 +694,7 @@ cat >expect <<\EOF
EOF
test_expect_success 'get --bool-or-int' '
+ rm -f .git/config &&
(
echo "[bool]"
echo true1
@@ -718,7 +714,6 @@ test_expect_success 'get --bool-or-int' '
'
-rm .git/config
cat >expect <<\EOF
[bool]
true1 = true
@@ -732,6 +727,7 @@ cat >expect <<\EOF
EOF
test_expect_success 'set --bool-or-int' '
+ rm -f .git/config &&
git config --bool-or-int bool.true1 true &&
git config --bool-or-int bool.false1 false &&
git config --bool-or-int bool.true2 yes &&
@@ -742,8 +738,6 @@ test_expect_success 'set --bool-or-int' '
test_cmp expect .git/config
'
-rm .git/config
-
cat >expect <<\EOF
[path]
home = ~/
@@ -752,6 +746,7 @@ cat >expect <<\EOF
EOF
test_expect_success NOT_MINGW 'set --path' '
+ rm -f .git/config &&
git config --path path.home "~/" &&
git config --path path.normal "/dev/null" &&
git config --path path.trailingtilde "foo~" &&
@@ -800,7 +795,7 @@ cat > expect << EOF
hash = "test#test"
EOF
test_expect_success 'quoting' '
- rm .git/config &&
+ rm -f .git/config &&
git config quote.leading " test" &&
git config quote.ending "test " &&
git config quote.semicolon "test;test" &&
--
1.7.7.1507.g94722
^ permalink raw reply related
* Re: Compiling on Windows
From: Vincent van Ravesteijn @ 2011-10-19 7:49 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git
In-Reply-To: <CAH5451=7Em7sPzknVx8i2VBSAZxZwg1Awr8s3Nr2W=A6SDEZEw@mail.gmail.com>
Op 18-10-2011 6:08, Andrew Ardill schreef:
> Hi list, I have been searching for details on what is required to
> compile on Windows, but haven't found anything conclusive. Perhaps
> there is something on the wiki, but unfortunately it is down at the
> moment.
>
> Can anyone point me in the right direction? I would like to be able to
> compile and test topic branches, and perhaps even do some dev work on
> my windows machine.
I once wrote a little step-by-step tutorial on how to compile the native
Windows Git with MSVC (Express).
http://blog.vfrconsultancy.nl/#post0
Be aware that git runs without apparent problems, but that all
functionality written in shell scripts can't be used.
HTH,
Vincent
^ permalink raw reply
* Re: Problems after deleting submodule
From: Howard Miller @ 2011-10-19 10:23 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <4E9DD9E0.80808@web.de>
On 18 October 2011 20:56, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 18.10.2011 13:54, schrieb Howard Miller:
>> I included a submodule in my project then decided I didn't like
>> submodules and deleted it again. I followed the advice of delting
>> .gitmodules, the bit from .git/config and then git rm'ing the
>> submodule. Seemed to work. I then copied files with the same directory
>> name into where the submodule was. However, I can't add them.
>>
>> Doing git add /path/to/old/submodule - does nothing, files are not
>> staged, no error messages no nothing.
>> If I try to git rm /path/to/old/submodule - it just says 'did not
>> match any files'.
>>
>> It simply does not seem to want to add anything to the old submodule
>> location. I had a grep around and could not see any obvious references
>> in the repo.
>>
>> I'm a bit stuck... any suggestions for things I can try much appreciated.
>
> Looks like the gitlink entry of the submodule is still there. I assume
> git ls-files --stage | grep 160000
> still shows the submodule? That would make it impossible to add anything
> in the former submodule directory.
>
> When I delete .gitmodules and the .git/config entry and do a
> git rm sub/
> I get
> fatal: pathspec 'sub/' did not match any files
>
> If I omit the trailing slash it is:
> fatal: git rm: 'sub': Is a directory
>
> I have to do a
> rm -rf sub
> followed by
> git rm sub
> to get everything cleaned up.
>
> Does that help you?
Yes - that was exactly the problem. I'm slightly embarrassed I missed
that. For some reason the initial delete of the submodule left the
empty directory in place (could have been a mistake on my part). I
didn't notice when I copied the new files back but it was enough to
break it.
I can't help thinking there could be some warning when doing an 'add'
but I don't think I have my head around it enough :)
Anyway, thanks for the help (again) everyone!
^ permalink raw reply
* shallow&single-branch clone?
From: Norbert Nemec @ 2011-10-19 13:30 UTC (permalink / raw)
To: git
Hi there,
it seems like there is no way to clone a single branch with truncated
history.
Truncating history is done by 'git clone --depth 1', there is not way to
restrict 'clone' to a single branch (the --branch option still downloads
all branches and only then chooses something other than HEAD as active
branch).
The manual sequence
git init
git remote add -t master -f origin URL
git checkout
allows a clone of a single branch but offers no means to truncate history.
The least intrusive solution would be an additional option to clone,
perhaps '--branch-only'.
More user friendly, this options should be on by default when --depth is
set. After all: who would expect branches to be cloned when the history
is explicitely truncated?
Ideas?
Greetings,
Norbert Nemec
^ permalink raw reply
* Re: What should "git fetch origin +next" should do?
From: Marc Branchaud @ 2011-10-19 13:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v8vohfvqs.fsf@alter.siamese.dyndns.org>
On 11-10-19 12:31 AM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
>
>> In all cases if the command-line refspec has no RHS then git should try to
>> figure out which local ref to update from the config, and it should die if it
>> can't figure out a local ref to create or update. (As I said above, maybe
>> allow "git fetch origin foo:" to let the user put the tip of origin's foo ref
>> into FETCH_HEAD.)
>
> I'd agree with everything you said in your message, except for the above
> "it should die" part.
>
> You are assuming that the user knows what his configured refspecs would
> normally do and that is the whole reason why "git fetch origin next" that
> would update the remote tracking branch specified for the upstream's next
> might feel more natural than the current behaviour. I too think that is a
> reasonable assumption.
>
> With the same assumption, if you said "git fetch origin frotz" when you
> know you are not usually tracking the remote 'frotz' branch, the command
> just should store what is fetched in FETCH_HEAD (and nowhere else) without
> dying. I do not think how it helps the user to die in that situation.
Sounds reasonable to me.
In all cases, I'd expect fetch to tell me what it did.
>> All this gets a bit more complicated if the user has currently checked out
>> the a ref that should be updated (regardless of the presence of a LHS +).
>
> That "do not update the currently checked out branch" already exists and
> is correctly handled by "git fetch", I think.
Sweet!
(I'd also be happy if fetch updated the ref, and left the checked-out HEAD
detached, with attendant messages. But then I'm quite comfortable working
with detached HEADs.)
M.
^ permalink raw reply
* How to verify that lines were only moved, not edited?
From: Johannes Sixt @ 2011-10-19 14:34 UTC (permalink / raw)
To: git
I thought there was a way to use git-blame to find out whether a change
only shuffled lines, but otherwise did not modify them. I tried "git blame
-M -- the/file", but it does not work as expected, neither with a toy file
nor with a 5000+ lines file (with 55 lines moved).
git init
echo A > foo
echo B >> foo
git add foo
git commit -m initial
echo B > foo
echo A >> foo
git commit -a -m swapped
The results are:
$ git blame -M -s -- foo
^e3abca2 1) B
6189cb46 2) A
I would have expected:
^e3abca2 1) B
^e3abca2 2) A
Oh, look! This produces the expected result:
$ git blame -M1 -s -- foo
while this produces the same as with just -M:
$ git blame -M2 -s -- foo
But neither helps with my 5000+ lines file. Does it mean that the lines
were changed? But I'm sure they were just moved! Please help!
-- Hannes
^ permalink raw reply
* Re: git-gui: still smartcase (and pull into git.git)
From: Bert Wesarg @ 2011-10-19 14:59 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Git Mailing List, Andrew Ardill, Junio C Hamano
[ Cc'ing git@vger, because its not that private anymore ;-) ]
On Wed, Oct 19, 2011 at 15:06, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
>
> I've applied the last of your set of patches to the git-gui repository
> now. The search series with the regexp and smartcase search. I added
> themed entry fields using similar code to that I posted but also
> handling xp/vista and dealing with non-themed Tk.
>
> I had to add a trap to deal with partial regexp's. If you tried to
> enable regexp and then entered [Cc]opyrigth it would raise an error
> after opening the brace as its not a valid regexp. Handled by catching
> and returning {} which yields a pink background until the expression
> becomes valid again which I think works well.
Thanks for catching this.
>
> As I don't like the smartcase mode I've added a commit to only enable it
> if gui.search.smartcase is enabled. It looks like this was your
> intention anyway but the mode was always enabled. What I don't like
> about it is that you can't uncheck the Case checkbutton if there is a
> capital letter in the search box.
I checked my inspiration for this, it's a patch to the NEdit editor
for it's incremental searchbar, and you can't uncheck the Case
checkbutton there too, if an capital letter is entered. Other
smart-case implementation I know of, aren't incremental, for example
less with it's -i option. So duno what should happen. It does
definitive makes no sense to enter 'giT' and searching
case-insensitive, in my opinion.
I also have incorporated Andrew's suggestion to reset the case flag
when the user cleared the entry. Unfortunately, I can't send an
updated patch currently, maybe tomorrow, here is just the commit,
before fixing up the patch, which is now in your master.
commit af9e02860629f9da20a9434bfec74c115916f4e2
Author: Bert Wesarg <bert.wesarg@googlemail.com>
Date: Tue Oct 18 19:43:44 2011 +0200
searchbar: reset case flag after the user cleared the text
diff --git a/lib/search.tcl b/lib/search.tcl
index 58e6852..3388eb9 100644 lib/search.tcl
--- a/lib/search.tcl
+++ b/lib/search.tcl
@@ -155,10 +155,10 @@ method _incrsearch {} {
if {[catch {$ctext index anchor}]} {
$ctext mark set anchor [_get_new_anchor $this]
}
- if {[regexp {[[:upper:]]} $searchstring]} {
- set casesensitive 1
- }
if {$searchstring ne {}} {
+ if {[regexp {[[:upper:]]} $searchstring]} {
+ set casesensitive 1
+ }
set here [_do_search $this anchor mlen]
if {$here ne {}} {
$ctext see $here
@@ -169,6 +169,9 @@ method _incrsearch {} {
} else {
$w.ent configure -background lightpink
}
+ } else {
+ # reset case sensitivity, when the user cleared the entry field
+ set casesensitive $default_casesensitive
}
}
This should probably be now guarded by the new smartcase flag.
On the other hand, maybe we should change the 'gui.search.smartcase'
into 'gui.search.case' with 3 values case/nocase/smart?
>
> All pushed to master now at repo.or.cz. We should see it all end up in
> 1.7.8 I reckon.
But I saw that Juno has already pulled your gitgui-0.15.0 tag, which
was at 843d6597 at that time, and now you moved the tag :(
Bert
>
> Thanks,
> --
> 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 related
* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Michael Haggerty @ 2011-10-19 15:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <7vty75ec54.fsf@alter.siamese.dyndns.org>
On 10/19/2011 08:19 AM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I was trying to summarize this topic for Release Notes.
>>
>> Possibly incompatible changes
>> -----------------------------
>>
>> * Special refs such as "HEAD", "MERGE_HEAD", and "FETCH_HEAD" are now
>> restricted to names with only uppercase letters and underscore. All
>> other refs must live under refs/ namespace. Earlier, you could
>> confuse git by storing an object name in $GIT_DIR/tmp/junk and say
>> "git show tmp/junk", but this will no longer work.
>>
>> But noticed that "git update-ref tmp/junk HEAD" does create such a ref
>> that won't be recognized, and "git check-ref-format tmp/junk" is happy.
>>
>> I think we would need to restrict check_ref_format() so that these
>> commands (and possibly others, but I think that single function will cover
>> pretty much everything) also reject "tmp/junk" immediately below $GIT_DIR
>> as a bad string. Otherwise we cannot merge these fixups, which would mean
>> we would have to revert the "Clean up refname checks and normalization"
>> series, at least the part that started emitting the "warning", which is a
>> mess I would rather want to avoid.
>>
>> Opinions on how to get us out of this mess?
>
> Addendum.
>
> I was digging this further and see fairly large conflicts between the bulk
> of "clean up refname checks and normalization" topic and the logic to
> avoid the additional warning by tightening the dwimmery.
>
> check_refname_format() has always assumed that it is OK to be called at
> any level of substring, and there are many code like this one (example is
> from remote.c::get_fetch_map()):
>
> for (rmp = &ref_map; *rmp; ) {
> if ((*rmp)->peer_ref) {
> if (check_refname_format((*rmp)->peer_ref->name + 5,
> REFNAME_ALLOW_ONELEVEL)) {
> struct ref *ignore = *rmp;
> error("* Ignoring funny ref '%s' locally",
> (*rmp)->peer_ref->name);
>
> This code somehow _knows_ that peer_ref->name begins with "refs/" and that
> is the reason it adds 5 to skip that known part. In this particular case,
> I think we can simply drop the +5 and allow-onelevel, but there are other
> instances of the calls to the function that feeds the rest of the refname
> string, skipping leading substring (not necessarily "refs/"), assuming
> that any component string is either valid or invalid no matter where it
> appears in the full refname. I wouldn't be surprised if some callers do
> not even have enough information to tell what the leading substring would
> be in the full refname context (e.g. parsing of "master:master" refspec,
> relying on the later dwimmery to add refs/heads/ in front, could just
> verify that "master" is in good format with allow-onelevel).
>
> The new restriction bolted on to that logic in jc/check-ref-format-fixup
> series to work around the new warning in 629cd3a (resolve_ref(): emit
> warnings for improperly-formatted references, 2011-09-15) is incompatible
> with the assumption, as we would need to check full refname, and treat the
> first refname component very differently from other components. It has to
> be either "refs" in multi-component refname, or all caps in a one-level
> one, but the callers of check_refname_format() are not designed to feed
> the full refname to begin with.
>
> I am tempted to revert 629cd3a (resolve_ref(): emit warnings for
> improperly-formatted references, 2011-09-15) that started giving the
> warnings, and drop the jc/check-ref-format-fixup topic [*1*] altogether,
> but that is a short-sighted workaround I would rather want to avoid. It
> essentially declares that the "Clean up refname checks" topic was a
> failure and did not manage to really clean things up.
>
> A possible alternative might be to leave check_refname_format() and its
> callers as they are, introduce check_full_refname() function that knows
> the new restriction on top of check-ref-format-fixup, and use that in
> lock_ref_sha1(), lock_any_ref_for_update() and is_refname_available()
> [*2*]. That way, we can keep the potentially useful "ill-formed contents
> in the ref" warning and avoid possible confusion caused by random files
> that are directly under $GIT_DIR, which would be far more preferable in
> the longer term.
>
> Anybody wants to give it a try?
I think that the refs/-or-ALL_CAPS test fits most naturally in
check_refname_format(), controlled by an option flag.
I'm starting by building parts of the solution, namely something like:
* Add an option REFNAME_ALLOW_DWIM which causes check_refname_format()
to accept reference names that *could* be dwimmed into a valid refname.
Specifically, when this option is *not* specified, then the option must
start with "refs/" or be ALL_CAPS. When it *is* specified, the behavior
will be much like the current behavior when ALLOW_ONELEVEL is specified
(in fact, the new option might make ALLOW_ONELEVEL obsolete).
* Add a new function parse_refname_prefix(str, len, flags) which tries
to read a refname from the front of str (of length len, not necessarily
NUL-terminated) and returns the length of the part that could be used.
This function will support the same flag argument as
check_refname_format() (in fact the latter would become a trivial
wrapper of the new function). I expect that this function will be
useful for dwimmery.
Hopefully I'll have patches before the end of the (Berlin) day.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] t1300: attempting to remove a non-existent .git/config is not an error
From: Jeff King @ 2011-10-19 16:13 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Carlos Martín Nieto, git
In-Reply-To: <4E9E7E22.7010905@viscovery.net>
On Wed, Oct 19, 2011 at 09:37:06AM +0200, Johannes Sixt wrote:
> From: Johannes Sixt <j6t@kdbg.org>
>
> Since some tests before test number 79 ("quoting") are skipped, .git/config
> does not exist and 'rm .git/config' fails. Fix this particular case.
>
> While at it, move other instance of 'rm .git/config' that occur in this
> file inside the test function to document that the test cases want to
> protect themselves from remnants of earlier tests.
Thanks, this fix looks good to me.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox