* [PATCH v3 1/5] attr.c: avoid inappropriate access to strbuf "buf" member
From: Brandon Casey @ 2011-10-06 18:22 UTC (permalink / raw)
To: gitster; +Cc: git, peff, j.sixt, Brandon Casey
In-Reply-To: <VYN8m1JCy102-eaWWa-bsunEvt3zeXLJkVg7FZKZCtXT-Ww0vg7a8xA7NTvrZTiovKTnJ9Hlom0@cipher.nrlssc.navy.mil>
From: Brandon Casey <drafnel@gmail.com>
This code sequence performs a strcpy into the buf member of a strbuf
struct. The strcpy may move the position of the terminating nul of the
string and effectively change the length of string so that it does not
match the len member of the strbuf struct.
Currently, this sequence works since the strbuf was given a hint when it
was initialized to allocate enough space to accomodate the string that will
be strcpy'ed, but this is an implementation detail of strbufs, not a
guarantee.
So, lets rework this sequence so that the strbuf is only manipulated by
strbuf functions, and direct modification of its "buf" member is not
necessary.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
attr.c | 24 +++++++++++-------------
1 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/attr.c b/attr.c
index 33cb4e4..fe38fcc 100644
--- a/attr.c
+++ b/attr.c
@@ -552,7 +552,6 @@ static void prepare_attr_stack(const char *path)
{
struct attr_stack *elem, *info;
int dirlen, len;
- struct strbuf pathbuf;
const char *cp;
cp = strrchr(path, '/');
@@ -561,8 +560,6 @@ static void prepare_attr_stack(const char *path)
else
dirlen = cp - path;
- strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));
-
/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
@@ -607,27 +604,28 @@ static void prepare_attr_stack(const char *path)
* Read from parent directories and push them down
*/
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
- while (1) {
- char *cp;
+ struct strbuf pathbuf = STRBUF_INIT;
+ while (1) {
len = strlen(attr_stack->origin);
if (dirlen <= len)
break;
- strbuf_reset(&pathbuf);
- strbuf_add(&pathbuf, path, dirlen);
+ cp = memchr(path + len + 1, '/', dirlen - len - 1);
+ if (!cp)
+ cp = path + dirlen;
+ strbuf_add(&pathbuf, path, cp - path);
strbuf_addch(&pathbuf, '/');
- cp = strchr(pathbuf.buf + len + 1, '/');
- strcpy(cp + 1, GITATTRIBUTES_FILE);
+ strbuf_addstr(&pathbuf, GITATTRIBUTES_FILE);
elem = read_attr(pathbuf.buf, 0);
- *cp = '\0';
- elem->origin = strdup(pathbuf.buf);
+ strbuf_setlen(&pathbuf, cp - path);
+ elem->origin = strbuf_detach(&pathbuf, NULL);
elem->prev = attr_stack;
attr_stack = elem;
debug_push(elem);
}
- }
- strbuf_release(&pathbuf);
+ strbuf_release(&pathbuf);
+ }
/*
* Finally push the "info" one at the top of the stack.
--
1.7.7
^ permalink raw reply related
* [PATCH v3 2/5] cleanup: use internal memory allocation wrapper functions everywhere
From: Brandon Casey @ 2011-10-06 18:22 UTC (permalink / raw)
To: gitster; +Cc: git, peff, j.sixt, Brandon Casey
In-Reply-To: <VYN8m1JCy102-eaWWa-bsunEvt3zeXLJkVg7FZKZCtXT-Ww0vg7a8xA7NTvrZTiovKTnJ9Hlom0@cipher.nrlssc.navy.mil>
From: Brandon Casey <drafnel@gmail.com>
The "x"-prefixed versions of strdup, malloc, etc. will check whether the
allocation was successful and terminate the process otherwise.
A few uses of malloc were left alone since they already implemented a
graceful path of failure or were in a quasi external library like xdiff.
Additionally, the call to malloc in compat/win32/syslog.c was not modified
since the syslog() implemented there is a die handler and a call to the
x-wrappers within a die handler could result in recursion should memory
allocation fail. This will have to be addressed separately.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
attr.c | 2 +-
builtin/mv.c | 2 +-
compat/mingw.c | 2 +-
compat/qsort.c | 2 +-
remote.c | 2 +-
show-index.c | 2 +-
transport-helper.c | 4 ++--
7 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/attr.c b/attr.c
index fe38fcc..0793859 100644
--- a/attr.c
+++ b/attr.c
@@ -533,7 +533,7 @@ static void bootstrap_attr_stack(void)
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
elem = read_attr(GITATTRIBUTES_FILE, 1);
- elem->origin = strdup("");
+ elem->origin = xstrdup("");
elem->prev = attr_stack;
attr_stack = elem;
debug_push(elem);
diff --git a/builtin/mv.c b/builtin/mv.c
index 40f33ca..e9d191f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -29,7 +29,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
to_copy--;
if (to_copy != length || base_name) {
char *it = xmemdupz(result[i], to_copy);
- result[i] = base_name ? strdup(basename(it)) : it;
+ result[i] = base_name ? xstrdup(basename(it)) : it;
}
}
return get_pathspec(prefix, result);
diff --git a/compat/mingw.c b/compat/mingw.c
index 6ef0cc4..8947418 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1183,7 +1183,7 @@ static int WSAAPI getaddrinfo_stub(const char *node, const char *service,
}
ai->ai_addrlen = sizeof(struct sockaddr_in);
if (hints && (hints->ai_flags & AI_CANONNAME))
- ai->ai_canonname = h ? strdup(h->h_name) : NULL;
+ ai->ai_canonname = h ? xstrdup(h->h_name) : NULL;
else
ai->ai_canonname = NULL;
diff --git a/compat/qsort.c b/compat/qsort.c
index d93dce2..9574d53 100644
--- a/compat/qsort.c
+++ b/compat/qsort.c
@@ -55,7 +55,7 @@ void git_qsort(void *b, size_t n, size_t s,
msort_with_tmp(b, n, s, cmp, buf);
} else {
/* It's somewhat large, so malloc it. */
- char *tmp = malloc(size);
+ char *tmp = xmalloc(size);
msort_with_tmp(b, n, s, cmp, tmp);
free(tmp);
}
diff --git a/remote.c b/remote.c
index b8ecfa5..7840d2f 100644
--- a/remote.c
+++ b/remote.c
@@ -840,7 +840,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
refspec->dst, &ret))
return ret;
} else if (!strcmp(refspec->src, name))
- return strdup(refspec->dst);
+ return xstrdup(refspec->dst);
}
return NULL;
}
diff --git a/show-index.c b/show-index.c
index 4c0ac13..63f9da5 100644
--- a/show-index.c
+++ b/show-index.c
@@ -48,7 +48,7 @@ int main(int argc, char **argv)
unsigned char sha1[20];
uint32_t crc;
uint32_t off;
- } *entries = malloc(nr * sizeof(entries[0]));
+ } *entries = xmalloc(nr * sizeof(entries[0]));
for (i = 0; i < nr; i++)
if (fread(entries[i].sha1, 20, 1, stdin) != 1)
die("unable to read sha1 %u/%u", i, nr);
diff --git a/transport-helper.c b/transport-helper.c
index 4eab844..0713126 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -183,7 +183,7 @@ static struct child_process *get_helper(struct transport *transport)
ALLOC_GROW(refspecs,
refspec_nr + 1,
refspec_alloc);
- refspecs[refspec_nr++] = strdup(capname + strlen("refspec "));
+ refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec "));
} else if (!strcmp(capname, "connect")) {
data->connect = 1;
} else if (!prefixcmp(capname, "export-marks ")) {
@@ -445,7 +445,7 @@ static int fetch_with_import(struct transport *transport,
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
- private = strdup(posn->name);
+ private = xstrdup(posn->name);
read_ref(private, posn->old_sha1);
free(private);
}
--
1.7.7
^ permalink raw reply related
* [PATCH v3 0/5] reroll bc/attr-ignore-case
From: Brandon Casey @ 2011-10-06 18:22 UTC (permalink / raw)
To: gitster; +Cc: git, peff, j.sixt
This re-rolls the whole series so that it can replace your current
bc/attr-ignore-case. It includes the two v2 patches I submitted last
night and backs out the use of xmalloc in compat/win32/syslog.c that
Johannes Sixt pointed out as a recursion issue. Erik Faye-Lunde has a
patch to remove the existing calls to x-wrapper functions in the function.
Built on top of 5738c9c21e53356ab5020912116e7f82fd2d428f, the same base
as your current bc/attr-ignore-case.
-Brandon
Brandon Casey (4):
attr.c: avoid inappropriate access to strbuf "buf" member
cleanup: use internal memory allocation wrapper functions everywhere
builtin/mv.c: plug miniscule memory leak
attr.c: respect core.ignorecase when matching attribute patterns
Junio C Hamano (1):
attr: read core.attributesfile from git_default_core_config
attr.c | 46 ++++++++++++++-----------------------
builtin/check-attr.c | 2 +
builtin/mv.c | 6 ++++-
cache.h | 1 +
compat/mingw.c | 2 +-
compat/qsort.c | 2 +-
config.c | 3 ++
environment.c | 1 +
remote.c | 2 +-
show-index.c | 2 +-
t/t0003-attributes.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
transport-helper.c | 4 +-
12 files changed, 94 insertions(+), 37 deletions(-)
--
1.7.7
^ permalink raw reply
* Re: [PATCH/RFC jn/ident-from-etc-mailname] ident: do not retrieve default ident when unnecessary
From: Junio C Hamano @ 2011-10-06 18:19 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Sixt, git, Matt Kraai, Gerrit Pape, Ian Jackson,
Linus Torvalds
In-Reply-To: <20111006171719.GB6946@elie>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Avoid a getpwuid() call (which contacts the network if the password
> database is not local), read of /etc/mailname, gethostname() call, and
> reverse DNS lookup if the user has already chosen a name and email
> through configuration, the environment, or the command line.
Oh boy that is a hard to parse paragraph that took me three reads.
In any case, both the motivation of the patch and the change itself make
sense to me. Thanks.
^ permalink raw reply
* [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Sitaram Chamarty @ 2011-10-06 18:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <7v62k210pj.fsf@alter.siamese.dyndns.org>
This is useful if you forgot to restrict the diff to the paths you want
to see, or selecting precisely the ones you want is too much typing.
Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>
---
On Thu, Oct 06, 2011 at 10:36:40AM -0700, Junio C Hamano wrote:
> Thanks. It is clear from the subject and the patch text that you are
> changing "hit return to unconditionally launch" into "launch it if you
> want to", but can you give justification why a choice not to launch is
> needed in the log message?
OK; done.
git-difftool--helper.sh | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 8452890..0468446 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -38,15 +38,16 @@ launch_merge_tool () {
# $LOCAL and $REMOTE are temporary files so prompt
# the user with the real $MERGED name before launching $merge_tool.
+ ans=y
if should_prompt
then
printf "\nViewing: '$MERGED'\n"
if use_ext_cmd
then
- printf "Hit return to launch '%s': " \
+ printf "Launch '%s' [Y/n]: " \
"$GIT_DIFFTOOL_EXTCMD"
else
- printf "Hit return to launch '%s': " "$merge_tool"
+ printf "Launch '%s' [Y/n]: " "$merge_tool"
fi
read ans
fi
@@ -54,9 +55,9 @@ launch_merge_tool () {
if use_ext_cmd
then
export BASE
- eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
+ test "$ans" != "n" && eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
else
- run_merge_tool "$merge_tool"
+ test "$ans" != "n" && run_merge_tool "$merge_tool"
fi
}
--
1.7.6
^ permalink raw reply related
* Prompt for merge message?
From: Todd A. Jacobs @ 2011-10-06 17:49 UTC (permalink / raw)
To: git
I often find myself using "--no-ff -m foo" for merging short-lived
branches, because the merge commit usually needs to say something
about having finished a feature rather than referring to a branch that
will be deleted shortly anyway. However, it's a little annoying to
have to always write the commit message on the command-line,
especially in cases where a more expository multi-line message would
be useful.
Is there currently a way to get git to prompt for the merge message,
rather than using the default or requiring the -m flag? If not, isn't
this a common-enough use case to have that ability added to the merge
function?
^ permalink raw reply
* Re: [PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
From: Jay Soffian @ 2011-10-06 17:58 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Jay Soffian, nicolas.dichtel, Jeff King
In-Reply-To: <1317923315-54940-1-git-send-email-jaysoffian@gmail.com>
On Thu, Oct 6, 2011 at 1:48 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Note that do_recursive_merge() aborts if the merge cannot start, while
> try_merge_command() returns a non-zero value other than 1.
Maybe you want this on-top:
diff --git i/builtin/revert.c w/builtin/revert.c
index a95b255c86..7e4857530b 100644
--- i/builtin/revert.c
+++ w/builtin/revert.c
@@ -223,7 +223,7 @@ static void advise(const char *advice, ...)
va_end(params);
}
-static void print_advice(void)
+static void print_advice(int show_hint)
{
char *msg = getenv("GIT_CHERRY_PICK_HELP");
@@ -238,9 +238,11 @@ static void print_advice(void)
return;
}
- advise("after resolving the conflicts, mark the corrected paths");
- advise("with 'git add <paths>' or 'git rm <paths>'");
- advise("and commit the result with 'git commit'");
+ if (show_hint) {
+ advise("after resolving the conflicts, mark the corrected paths");
+ advise("with 'git add <paths>' or 'git rm <paths>'");
+ advise("and commit the result with 'git commit'");
+ }
}
static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -510,7 +512,7 @@ static int do_pick_commit(void)
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
msg.subject);
- print_advice();
+ print_advice(res == 1);
rerere(allow_rerere_auto);
} else {
if (!no_commit)
j.
^ permalink raw reply related
* [PATCH v2] mingw: avoid using strbuf in syslog
From: Erik Faye-Lund @ 2011-10-06 17:52 UTC (permalink / raw)
To: git; +Cc: drafnel, j.sixt, gitster
strbuf can call die, which again can call syslog from git-daemon.
Endless recursion is no fun; fix it by hand-rolling the logic. As
a side-effect malloc/realloc errors are changed into non-fatal
warnings; this is probably an improvement anyway.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Noticed-by: Johannes Sixt <j.sixt@viscovery.net>
---
compat/win32/syslog.c | 30 ++++++++++++++++++------------
1 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
index 42b95a9..d015e43 100644
--- a/compat/win32/syslog.c
+++ b/compat/win32/syslog.c
@@ -1,5 +1,4 @@
#include "../../git-compat-util.h"
-#include "../../strbuf.h"
static HANDLE ms_eventlog;
@@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)
void syslog(int priority, const char *fmt, ...)
{
- struct strbuf sb = STRBUF_INIT;
- struct strbuf_expand_dict_entry dict[] = {
- {"1", "% 1"},
- {NULL, NULL}
- };
WORD logtype;
- char *str;
+ char *str, *pos;
int str_len;
va_list ap;
@@ -39,11 +33,24 @@ void syslog(int priority, const char *fmt, ...)
}
str = malloc(str_len + 1);
+ if (!str) {
+ warning("malloc failed: '%s'", strerror(errno));
+ return;
+ }
+
va_start(ap, fmt);
vsnprintf(str, str_len + 1, fmt, ap);
va_end(ap);
- strbuf_expand(&sb, str, strbuf_expand_dict_cb, &dict);
- free(str);
+
+ while ((pos = strstr(str, "%1")) != NULL) {
+ str = realloc(str, ++str_len + 1);
+ if (!str) {
+ warning("realloc failed: '%s'", strerror(errno));
+ return;
+ }
+ memmove(pos + 2, pos + 1, strlen(pos));
+ pos[1] = ' ';
+ }
switch (priority) {
case LOG_EMERG:
@@ -66,7 +73,6 @@ void syslog(int priority, const char *fmt, ...)
}
ReportEventA(ms_eventlog, logtype, 0, 0, NULL, 1, 0,
- (const char **)&sb.buf, NULL);
-
- strbuf_release(&sb);
+ (const char **)&str, NULL);
+ free(str);
}
--
1.7.6.msysgit.0.579.ga3d6f
^ permalink raw reply related
* [PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
From: Jay Soffian @ 2011-10-06 17:48 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Jay Soffian, nicolas.dichtel, Jeff King
do_pick_commit() writes out CHERRY_PICK_HEAD before invoking merge (either
via do_recursive_merge() or try_merge_command()) on the assumption that if
the merge fails it is due to conflict. However, if the tree is dirty, the
merge may not even start, aborting before do_pick_commit() can remove
CHERRY_PICK_HEAD.
Instead, defer writing CHERRY_PICK_HEAD till after merge has returned.
At this point we know the merge has either succeeded or failed due
to conflict. In either case, we want CHERRY_PICK_HEAD to be written
so that it may be picked up by the subsequent invocation of commit.
Note that do_recursive_merge() aborts if the merge cannot start, while
try_merge_command() returns a non-zero value other than 1.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
builtin/revert.c | 10 ++++++++--
t/t3507-cherry-pick-conflict.sh | 15 +++++++++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 3117776c2c..a95b255c86 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -476,8 +476,6 @@ static int do_pick_commit(void)
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
- if (!no_commit)
- write_cherry_pick_head();
}
if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
@@ -498,6 +496,14 @@ static int do_pick_commit(void)
free_commit_list(remotes);
}
+ /* If the merge was clean or if it failed due to conflict, we write
+ * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
+ * However, if the merge did not even start, then we don't want to
+ * write it at all.
+ */
+ if (action == CHERRY_PICK && !no_commit && (res == 0 || res == 1))
+ write_cherry_pick_head();
+
if (res) {
error(action == REVERT
? _("could not revert %s... %s")
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 212ec54aaf..7601d0b0d6 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -77,6 +77,21 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '
test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
'
+test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '
+ pristine_detach initial &&
+ echo foo > foo &&
+ test_must_fail git cherry-pick base &&
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
+test_expect_success \
+'cherry-pick --strategy=resolve w/dirty tree does not set CHERRY_PICK_HEAD' '
+ pristine_detach initial &&
+ echo foo > foo &&
+ test_must_fail git cherry-pick --strategy=resolve base &&
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '
pristine_detach initial &&
(
--
1.7.7.6.g25c34
^ permalink raw reply related
* Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Junio C Hamano @ 2011-10-06 17:36 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <20111006125658.GB18709@sita-lt.atc.tcs.com>
Sitaram Chamarty <sitaramc@gmail.com> writes:
> Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>
> ---
>
> (re-rolled according to earlier discussion)
Thanks. It is clear from the subject and the patch text that you are
changing "hit return to unconditionally launch" into "launch it if you
want to", but can you give justification why a choice not to launch is
needed in the log message?
^ permalink raw reply
* Re: [PATCH] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
From: Jay Soffian @ 2011-10-06 17:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, nicolas.dichtel, Jeff King
In-Reply-To: <CAG+J_DxqpDAm-qPR-Udkr_b1gc=Y+LoKbsQdmiCi6ztNKz0_Mg@mail.gmail.com>
On Thu, Oct 6, 2011 at 1:28 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Thu, Oct 6, 2011 at 1:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I think do_recursive_merge() would die() when the merge cannot even start
>> (i.e. the local changes after the cherry-pick exits are the ones from the
>> time before such a failed cherry-pick started), but I suspect that the
>> other codepath uses try_merge_command() to drive strategies other than
>> recursive and it does not die() there in such a case. Can you make sure
>> this patch is sufficient in such a case as well?
>
> It's wrong to write out CHERRY_PICK_HEAD if we couldn't start the
> merge, but using cherry-pick with a strategy other than recursive
> seems broken in that case in other ways as well:
>
> $ git cherry-pick --strategy=resolve side
> error: Untracked working tree file 'bar' would be overwritten by merge.
> error: could not apply a77535c9ac... bar
> hint: after resolving the conflicts, mark the corrected paths
> hint: with 'git add <paths>' or 'git rm <paths>'
> hint: and commit the result with 'git commit'
>
> The "hint" advice is completely wrong.
For that matter, I don't see how to distinguish "the merge did not
even start" from "the merge had conflicts" when using
try_merge_command(). In the former case we do NOT want
CHERRY_PICK_HEAD left behind (nor to print the wrong advice above),
while in the latter case we do.
j.
^ permalink raw reply
* Re: [PATCH] mingw: avoid using strbuf in syslog
From: Erik Faye-Lund @ 2011-10-06 17:32 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, j.sixt, gitster
In-Reply-To: <CA+sFfMediaCnzFH6uKfVc_Bb+W+AA1nO+OdvB8vWjjrsD1kh9w@mail.gmail.com>
On Thu, Oct 6, 2011 at 7:27 PM, Brandon Casey <drafnel@gmail.com> wrote:
> On Thu, Oct 6, 2011 at 11:57 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> strbuf can call die, which again can call syslog from git-daemon.
>>
>> Endless recursion is no fun; fix it by hand-rolling the logic.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
>> Fixes the problem Brandon pointed out...
>
> Actually, Johannes should get that credit. He brought up the whole
> recursion issue.
>
OK, I didn't spot that one. But it's not in the commit message,
though. Perhaps it should be?
>> compat/win32/syslog.c | 26 ++++++++++++++------------
>> 1 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
>> index 42b95a9..3b8e2b7 100644
>> --- a/compat/win32/syslog.c
>> +++ b/compat/win32/syslog.c
>> @@ -1,5 +1,4 @@
>> #include "../../git-compat-util.h"
>> -#include "../../strbuf.h"
>>
>> static HANDLE ms_eventlog;
>>
>> @@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)
>>
>> void syslog(int priority, const char *fmt, ...)
>> {
>> - struct strbuf sb = STRBUF_INIT;
>> - struct strbuf_expand_dict_entry dict[] = {
>> - {"1", "% 1"},
>> - {NULL, NULL}
>> - };
>> WORD logtype;
>> - char *str;
>> + char *str, *pos;
>> int str_len;
>> va_list ap;
>>
>> @@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)
>> }
>>
>> str = malloc(str_len + 1);
>> + if (!str)
>> + return; /* no chance to report error */
>> +
>
> Just above this, warning() is used to at least print a message to
> stderr if vsnprintf() fails. It could be used here, and below when
> realloc fails.
>
Good point, so this should be squashed in:
---
diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
index 3b8e2b7..d015e43 100644
--- a/compat/win32/syslog.c
+++ b/compat/win32/syslog.c
@@ -33,8 +33,10 @@ void syslog(int priority, const char *fmt, ...)
}
str = malloc(str_len + 1);
- if (!str)
- return; /* no chance to report error */
+ if (!str) {
+ warning("malloc failed: '%s'", strerror(errno));
+ return;
+ }
va_start(ap, fmt);
vsnprintf(str, str_len + 1, fmt, ap);
@@ -42,8 +44,10 @@ void syslog(int priority, const char *fmt, ...)
while ((pos = strstr(str, "%1")) != NULL) {
str = realloc(str, ++str_len + 1);
- if (!str)
+ if (!str) {
+ warning("realloc failed: '%s'", strerror(errno));
return;
+ }
memmove(pos + 2, pos + 1, strlen(pos));
pos[1] = ' ';
}
^ permalink raw reply related
* Re: What's cooking in git.git (Oct 2011, #01; Tue, 4)
From: Pascal Obry @ 2011-10-06 17:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlisy119i.fsf@alter.siamese.dyndns.org>
Le 06/10/2011 19:24, Junio C Hamano a écrit :
> The impression I got from the discussion was quite different, which was
> that the patch was not even "good" by making certain things impossible to
> do by catering to a narrow corner case.
I have understood that this was not completely fixing all
slash/backslash issues (is it even possible?) but in no way there was
regressions pointed out. At least a specific error *was* fixed. But I
may have missed something, in that case sorry.
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net - http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B
^ permalink raw reply
* Re: [PATCH] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
From: Junio C Hamano @ 2011-10-06 17:28 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, nicolas.dichtel, Jeff King
In-Reply-To: <CAG+J_Dzh2Njjwykt-f4w_YqpftrJEpQfUW2OvSRs_MSPcNFQnw@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> For that matter, why does revert.c have it's own implementation of
> recursive instead of just calling try_merge_command("recursive", ...)?
I think the people who did this part wanted not to fork.
^ permalink raw reply
* Re: [PATCH] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
From: Jay Soffian @ 2011-10-06 17:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, nicolas.dichtel, Jeff King
In-Reply-To: <7vty7m12b2.fsf@alter.siamese.dyndns.org>
On Thu, Oct 6, 2011 at 1:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I think do_recursive_merge() would die() when the merge cannot even start
> (i.e. the local changes after the cherry-pick exits are the ones from the
> time before such a failed cherry-pick started), but I suspect that the
> other codepath uses try_merge_command() to drive strategies other than
> recursive and it does not die() there in such a case. Can you make sure
> this patch is sufficient in such a case as well?
It's wrong to write out CHERRY_PICK_HEAD if we couldn't start the
merge, but using cherry-pick with a strategy other than recursive
seems broken in that case in other ways as well:
$ git cherry-pick --strategy=resolve side
error: Untracked working tree file 'bar' would be overwritten by merge.
error: could not apply a77535c9ac... bar
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
The "hint" advice is completely wrong.
j.
^ permalink raw reply
* Re: [PATCH] mingw: avoid using strbuf in syslog
From: Brandon Casey @ 2011-10-06 17:27 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, j.sixt, gitster
In-Reply-To: <1317920244-6320-1-git-send-email-kusmabite@gmail.com>
On Thu, Oct 6, 2011 at 11:57 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> strbuf can call die, which again can call syslog from git-daemon.
>
> Endless recursion is no fun; fix it by hand-rolling the logic.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
> Fixes the problem Brandon pointed out...
Actually, Johannes should get that credit. He brought up the whole
recursion issue.
> compat/win32/syslog.c | 26 ++++++++++++++------------
> 1 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
> index 42b95a9..3b8e2b7 100644
> --- a/compat/win32/syslog.c
> +++ b/compat/win32/syslog.c
> @@ -1,5 +1,4 @@
> #include "../../git-compat-util.h"
> -#include "../../strbuf.h"
>
> static HANDLE ms_eventlog;
>
> @@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)
>
> void syslog(int priority, const char *fmt, ...)
> {
> - struct strbuf sb = STRBUF_INIT;
> - struct strbuf_expand_dict_entry dict[] = {
> - {"1", "% 1"},
> - {NULL, NULL}
> - };
> WORD logtype;
> - char *str;
> + char *str, *pos;
> int str_len;
> va_list ap;
>
> @@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)
> }
>
> str = malloc(str_len + 1);
> + if (!str)
> + return; /* no chance to report error */
> +
Just above this, warning() is used to at least print a message to
stderr if vsnprintf() fails. It could be used here, and below when
realloc fails.
> va_start(ap, fmt);
> vsnprintf(str, str_len + 1, fmt, ap);
> va_end(ap);
> - strbuf_expand(&sb, str, strbuf_expand_dict_cb, &dict);
> - free(str);
> +
> + while ((pos = strstr(str, "%1")) != NULL) {
> + str = realloc(str, ++str_len + 1);
> + if (!str)
> + return;
> + memmove(pos + 2, pos + 1, strlen(pos));
> + pos[1] = ' ';
> + }
>
> switch (priority) {
> case LOG_EMERG:
-Brandon
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2011, #01; Tue, 4)
From: Junio C Hamano @ 2011-10-06 17:27 UTC (permalink / raw)
To: Brad King; +Cc: git
In-Reply-To: <4E8DCF79.50200@kitware.com>
Brad King <brad.king@kitware.com> writes:
>> + push: teach --recurse-submodules the on-demand option
>> (this branch is tangled with fg/submodule-auto-push.)
>>
>> The second from the bottom one needs to be replaced with a properly
>> written commit log message.
>
> are a separate topic.
Thanks for reading the list carefully. What you described is exactly what
"tangled with" means.
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2011, #01; Tue, 4)
From: Junio C Hamano @ 2011-10-06 17:24 UTC (permalink / raw)
To: pascal; +Cc: git
In-Reply-To: <4E8DC01A.8060406@obry.net>
Pascal Obry <pascal@obry.net> writes:
>> Incomplete with respect to backslash processing in prefix_filename(), and
>> also loses the ability to escape glob specials.
>> Will discard.
>
> Sorry but this is letting best be enemy of good!
The impression I got from the discussion was quite different, which was
that the patch was not even "good" by making certain things impossible to
do by catering to a narrow corner case.
^ permalink raw reply
* "Use of uninitialized value" running "git svn clone"
From: Luiz-Otavio Zorzella @ 2011-10-06 17:23 UTC (permalink / raw)
To: git
I'm trying to convert a project (hosted in googlecode.com) from svn to
git, using the "git svn clone" command, and I'm getting an "Use of
uninitialized value" error. Here's the truncated output:
$ git svn clone https://test-libraries-for-java.googlecode.com/svn
--no-metadata -A ~/tmp/authors.txt -t tags -b branches -T trunk
test-libraries-for-java
r1 = c3adafa93a420f19b1bcfb6765fe0eb90aaa751c (refs/remotes/trunk)
A .classpath
A .project
A COPYING
A build.properties
A build.xml
W: +empty_dir: trunk/src
[...]
r10 = 8d5d7fdebdb7f822388fd3e4f4061abbfd1fb0cf (refs/remotes/trunk)
M test/com/google/common/testing/junit3/JUnitAssertsTest.java
r11 = 4c8a77660bf353ed55c9d583b39e263203c685a4 (refs/remotes/trunk)
Found possible branch point:
https://test-libraries-for-java.googlecode.com/svn/trunk =>
https://test-libraries-for-java.googlecode.com/svn/tags/release-1.0,
11
Use of uninitialized value $u in substitution (s///) at
/usr/lib/git-core/git-svn line 1731.
Use of uninitialized value $u in concatenation (.) or string at
/usr/lib/git-core/git-svn line 1731.
refs/remotes/trunk:
'https://test-libraries-for-java.googlecode.com/svn' not found in ''
For completeness, here's the authors.txt file I'm using:
$ cat ~/tmp/authors.txt
zorzella = Luiz-Otavio 'Z' Zorzella <zorzella@gmail.com>
(no author) = Luiz-Otavio 'Z' Zorzella <zorzella@gmail.com>
**************
Thanks in advance,
Z
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Matthieu Moy @ 2011-10-06 17:22 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Shawn Pearce, Junio C Hamano, git
In-Reply-To: <20111006171107.GA10973@elie>
Jonathan Nieder <jrnieder@gmail.com> writes:
> I probably missed some earlier discussion (so please forgive me this),
(same here)
> What happens if my old key is compromised and I want to throw away the
> signatures and replace them with signatures using my new key?
With the patch we're discussing, signatures are part of history, hence
can't be modified after the fact without rewritting them.
*But*, by design, unless sha1 itself is compromized (in which case Git
would need to change to another hash function, that would be no fun),
signing the tip of every branch is sufficient to sign the whole history.
So, your old signatures would remain there, and your new signature, for
new commits, would be added on top.
> How does this relate to the "push certificate" use case, which seemed
> to be mostly about authenticating published branch tips with
> signatures that are not necessarily important in the long term?
I'm wondering how this feature would fit in a typical flow, indeed.
Usually, I hack for a while, and when I'm happy enough, I push. But I
don't take the decision of what to push at commit time, so if the idea
is to sign only a few commits (i.e. the ones you push), then you should
decide this at commit time ("hmm, I should commit --gpg-sign this time
because I'm going to push this one").
If the idea is to sign every commit, then there should be a config
option so that we don't have to type it every time.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH WIP 0/3] git log --exclude
From: Junio C Hamano @ 2011-10-06 17:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy, git,
Jonathan Nieder
In-Reply-To: <20111006143441.GA21558@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The current protocol relies on certain repository properties on the
> remote end that narrow clone will violate. I don't see a way around that
> without a protocol extension to communicate the narrowness. What will
> that extension look like?
It would have to involve exchanging "I have/am interested in the paths
that match these pathspecs". I do not mind if our initial implementation
did not support anything other than fetching into a narrow from full and
pushing from a narrow to full. The second iteration could add fetching and
pushing between two narrows with the same set of narrowing pathspecs.
As to widening the area after a clone is made, I do not mind and if our
initial impementation only supported widening the area in a stupid way
(e.g. semi-equivalent of initial clone with the widened set of narrowing
pathspecs).
^ permalink raw reply
* Re: [PATCH] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
From: Jay Soffian @ 2011-10-06 17:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, nicolas.dichtel, Jeff King
In-Reply-To: <7vty7m12b2.fsf@alter.siamese.dyndns.org>
On Thu, Oct 6, 2011 at 1:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I think do_recursive_merge() would die() when the merge cannot even start
> (i.e. the local changes after the cherry-pick exits are the ones from the
> time before such a failed cherry-pick started), but I suspect that the
> other codepath uses try_merge_command() to drive strategies other than
> recursive and it does not die() there in such a case. Can you make sure
> this patch is sufficient in such a case as well?
Why does recursive die with a dirty tree but not other strategies?
For that matter, why does revert.c have it's own implementation of
recursive instead of just calling try_merge_command("recursive", ...)?
j.
^ permalink raw reply
* [PATCH/RFC jn/ident-from-etc-mailname] ident: do not retrieve default ident when unnecessary
From: Jonathan Nieder @ 2011-10-06 17:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, git, Matt Kraai, Gerrit Pape, Ian Jackson,
Linus Torvalds
In-Reply-To: <7vty7pga7y.fsf@alter.siamese.dyndns.org>
Avoid a getpwuid() call (which contacts the network if the password
database is not local), read of /etc/mailname, gethostname() call, and
reverse DNS lookup if the user has already chosen a name and email
through configuration, the environment, or the command line.
This should slightly speed up commands like "git commit". More
importantly, it improves error reporting when computation of the
default ident string does not go smoothly. For example, after
detecting a problem (e.g., "warning: cannot open /etc/mailname:
Permission denied") in retrieving the default committer identity:
touch /etc/mailname; # as root
chmod -r /etc/mailname; # as root
git commit -m 'test commit'
you can squelch the warning while waiting for your sysadmin to fix the
permissions problem.
echo '[user] email = me@example.com' >>~/.gitconfig
Inspired-by: Johannes Sixt <j6t@kdgb.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:
> --- a/ident.c
> +++ b/ident.c
> @@ -239,6 +239,10 @@ const char *fmt_ident(const char *name, const char *email,
> int warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);
> int name_addr_only = (flag & IDENT_NO_DATE);
>
> + if (name && !git_default_name[0])
> + strcpy(git_default_name, name);
> + if (email && !git_default_email[0])
> + strcpy(git_default_email, email);
That poisons the "git_default_foo" variables for the next fmt_ident
call. But something similar should work.
ident.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/ident.c b/ident.c
index edb43144..f619619b 100644
--- a/ident.c
+++ b/ident.c
@@ -117,19 +117,21 @@ static void copy_email(const struct passwd *pw)
sizeof(git_default_email) - len);
}
-static void setup_ident(void)
+static void setup_ident(const char **name, const char **emailp)
{
struct passwd *pw = NULL;
/* Get the name ("gecos") */
- if (!git_default_name[0]) {
+ if (!*name && !git_default_name[0]) {
pw = getpwuid(getuid());
if (!pw)
die("You don't exist. Go away!");
copy_gecos(pw, git_default_name, sizeof(git_default_name));
}
+ if (!*name)
+ *name = git_default_name;
- if (!git_default_email[0]) {
+ if (!*emailp && !git_default_email[0]) {
const char *email = getenv("EMAIL");
if (email && email[0]) {
@@ -144,6 +146,8 @@ static void setup_ident(void)
copy_email(pw);
}
}
+ if (!*emailp)
+ *emailp = git_default_email;
/* And set the default date */
if (!git_default_date[0])
@@ -239,11 +243,7 @@ const char *fmt_ident(const char *name, const char *email,
int warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);
int name_addr_only = (flag & IDENT_NO_DATE);
- setup_ident();
- if (!name)
- name = git_default_name;
- if (!email)
- email = git_default_email;
+ setup_ident(&name, &email);
if (!*name) {
struct passwd *pw;
--
1.7.7.rc1
^ permalink raw reply related
* Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
From: Brandon Casey @ 2011-10-06 17:17 UTC (permalink / raw)
To: kusmabite
Cc: Johannes Sixt, peff@peff.net, git@vger.kernel.org,
gitster@pobox.com, sunshine@sunshineco.com, bharrosh@panasas.com,
trast@student.ethz.ch
In-Reply-To: <CABPQNSak_jDbNQhzMoSN=NdWmyby3xJRwED54Ck5H1Y12HoGCQ@mail.gmail.com>
On Thu, Oct 6, 2011 at 11:50 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Oct 6, 2011 at 6:14 PM, Brandon Casey <drafnel@gmail.com> wrote:
>> [removed Alexey Shumkin from cc]
>>
>> On Thu, Oct 6, 2011 at 1:17 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Am 10/6/2011 4:00, schrieb Brandon Casey:
>>>> [resend without html bits added by "gmail offline"]
>>>> On Wed, Oct 5, 2011 at 7:53 PM, Brandon Casey <drafnel@gmail.com> wrote:
>>>>> On Thursday, September 15, 2011, Brandon Casey wrote:
>>>>>>
>>>>>> On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt <j.sixt@viscovery.net>
>>>>>>> There is a danger that the high-level die() routine (which is used by
>>>>>>> the
>>>>>>> x-wrappers) uses one of the low-level compat/ routines. IOW, in the case
>>>>>>> of errors, recursion might occur. Therefore, I would prefer that the
>>>>>>> compat/ routines do their own error reporting (preferably via return
>>>>>>> values and errno).
>>>>>>
>>>>>> Thanks. Will do.
>>>>>
>>>>> Hi Johannes,
>>>>> I have taken a closer look at the possibility of recursion with respect to
>>>>> die() and the functions in compat/. It appears the risk is only related to
>>>>> vsnprintf/snprintf at the moment. So as long as we avoid calling xmalloc et
>>>>> al from within snprintf.c, I think we should be safe from recursion.
>>>>> I'm inclined to keep the additions to mingw.c and win32/syslog.c since they
>>>>> both already use the x-wrappers or strbuf, even though they could easily be
>>>>> worked around. The other file that was touched is compat/qsort, which
>>>>> returns void and doesn't have a good alternative error handling path. So,
>>>>> I'm inclined to keep that one too.
>>>
>>> I'm fine with keeping the change to mingw.c (getaddrinfo related) and
>>> qsort: both are unlikely to be called from die().
>>>
>>> But syslog() *is* called from die() in git-daemon, and it would be better
>>> to back out the other offenders instead of adding to them.
>>
>> Ah. Yes, you're right. x-wrappers should not be used in syslog.c and
>> the use of strbuf's should be replaced.
>
> Good point. The patch for this looks something like this:
>
> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
> index 42b95a9..243538c 100644
> --- a/compat/win32/syslog.c
> +++ b/compat/win32/syslog.c
> @@ -1,5 +1,4 @@
> #include "../../git-compat-util.h"
> -#include "../../strbuf.h"
>
> static HANDLE ms_eventlog;
>
> @@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)
>
> void syslog(int priority, const char *fmt, ...)
> {
> - struct strbuf sb = STRBUF_INIT;
> - struct strbuf_expand_dict_entry dict[] = {
> - {"1", "% 1"},
> - {NULL, NULL}
> - };
> WORD logtype;
> - char *str;
> + char *str, *pos;
> int str_len;
> va_list ap;
>
> @@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)
> }
>
> str = malloc(str_len + 1);
> + if (!str)
> + return; /* no chance to report error */
> +
> va_start(ap, fmt);
> vsnprintf(str, str_len + 1, fmt, ap);
> va_end(ap);
> - strbuf_expand(&sb, str, strbuf_expand_dict_cb, &dict);
> - free(str);
> +
> + while ((pos = strstr(str, "%1")) != NULL) {
> + str = realloc(str, ++str_len + 1);
> + if (!str)
> + return;
> + memmove(pos + 2, pos + 1, strlen(pos));
> + pos[1] = ' ';
> + }
Is there any reason not to just use a different character than '%'
when replacing '%n'? Like '@'? Then the replacement could be done
in-place.
e.g. convert this:
fe80::3%1
into this:
fe80::3@1
I'm not usually on a windows platform, so maybe adding the space is
the common thing to do when trying to write an ipv6 address to the
event log using ReportEvent(). If not, then I think people would
probably figure out easily enough that '@n' referred to interface 'n'.
-Brandon
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Jonathan Nieder @ 2011-10-06 17:11 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <CAJo=hJvWbjEM9E5AjPHgmQ=eY8xf=Q=xtukeu2Ur7auUqeabDg@mail.gmail.com>
Shawn Pearce wrote:
> On Wed, Oct 5, 2011 at 17:56, Junio C Hamano <gitster@pobox.com> wrote:
>> 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)
[...]
> I like this approach better than the prior "push certificate" idea.
> The signature information is part of the history graph
I probably missed some earlier discussion (so please forgive me this),
but how is it intended to be used? Would projects
a. require as a matter of policy that all commits be signed
b. just sign releases as usual, but as commits in the history graph
instead of tags
c. sign the occasional especially interesting commit
What happens if my old key is compromised and I want to throw away the
signatures and replace them with signatures using my new key? How
does this relate to the "push certificate" use case, which seemed to
be mostly about authenticating published branch tips with signatures
that are not necessarily important in the long term?
In other words, something like this feature sounds like a sensible way
to commit the equivalent of a GPG-signed patch, but it doesn't seem
like a good fit for the "push certificate" use cases.
^ 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