* Re: [ANNOUNCE] GitTogether 2011 - Oct 24th/25th
From: Junio C Hamano @ 2011-09-07 21:00 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, Scott Chacon, git
In-Reply-To: <20110907193006.GB13364@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think that's reasonable, especially as we grow. However, one of the
> valuable things (for me, anyway) in previous GitTogethers is throwing
> all of these people together to some degree. I'm not terribly interested
> in day-to-day Gerrit issues, but sometimes the discussions start from
> some minor Gerrit annoyance, and we end up realizing that the right
> solution involves changes at a more fundamental layer, and all of git is
> better as a result. I'd hate to lose that developer/user interaction.
Same here, as I have been meaning to gauge interests from non-Gerrit
people on issues identified in Gerrit land (e.g. expand-refs).
> Maybe we can be segmented for part of the conference, and then bring
> everybody together for other parts. I dunno. I guess that involves
> predicting which parts will be useful for everybody to be together.
Also we would need to predict what parts we will have to begin with ;-).
^ permalink raw reply
* Re: Git without morning coffee
From: Michael Witten @ 2011-09-07 20:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <7vy5y02qf4.fsf@alter.siamese.dyndns.org>
On Wed, Sep 7, 2011 at 17:49, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> I think it would be great if at some point you could write a detailed
>> tutorial of how you maintain git...
>
> Is MaintNotes[*1*] taken together with Documentation/howto/maintain-git.txt
> insufficient?
>
> [Reference]
> *1* http://git-blame.blogspot.com/p/note-from-maintainer.html
I was unaware of the `howto'; I look forward to reading it.
^ permalink raw reply
* [PATCH 2/2] push -s: skeleton
From: Junio C Hamano @ 2011-09-07 20:57 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce
In-Reply-To: <7vfwk82hrt.fsf@alter.siamese.dyndns.org>
If a tag is GPG-signed, and if you trust the cryptographic robustness of
the SHA-1 and GPG, you can guarantee that all the history leading to the
signed commit is not tampered with. However, it would be both cumbersome
and cluttering to sign each and every commit. Especially if you strive to
keep your history clean by tweaking, rewriting and polishing your commits
before pushing the resulting history out, many commits you will create
locally end up not mattering at all, and it is a waste of time to sign
them.
A better alternative could be to sign a "push certificate" (for the lack
of better name) every time you push, asserting that what commits you are
pushing to update which refs. The basic workflow goes like this:
1. You push out your work with "git push -s";
2. "git push", as usual, learns where the remote refs are and which refs
are to be updated with this push. It prepares a text file in memory
that looks like this using this information:
Push-Certificate-Version: 1
Pusher: Junio C Hamano <gitster@pobox.com> 1315427886 -0700
Update: e83c51633... d4e58965f... refs/heads/master
Update: 5a144a288... 7931f38a2... refs/heads/next
An actual push certificate records full 40-char object name, but it is
ellided for brevity here.
The user then is asked to sign this push certificate using GPG. The
result is carried to the other side (i.e. receive-pack). In the
protocol exchange, this step comes immediately after the sender tells
what the result of the push should be, before it sends the pack data.
3. The receiving end will keep the signed push certificate in core,
receives the pack data and unpacks (or stores and runs index-pack)
as usual.
4. A new phase to record the push certificate is introduced in the
codepath after the receiving end runs receive_hook(). It is envisioned
that this phase:
a. parses the updated-to object names, and appends the push
certificate (still GPG signed) to a note attached to each of the
objects that will sit at the tip of the refs;
b. verifies that the push certificate is signed with a GPG key that is
authorized to push into this repository; and/or
c. invokes pre-push-verify-signature hook, feeds the push
certificate to it and asks it to veto the ref updates.
And here is a skeleton to implement it. It has all the necessary protocol
extensions implemented (although I do not know if we need separate
codepath for stateless RPC mode), but does not have subroutines to:
- Sign the certificate with GPG key;
- Parse the signed certificate to identify the updated-to objects, and
add the certificate as notes to them;
- Verify the certificate and find out what GPG key was used to sign it;
or
- Invoke and feed the certificate to pre-push-verify-hook.
all of which should be fairly trivial. The places that needs to implement
these are clearly marked with large comments, so I'll leave it up to other
people who are interested in the topic to fill in the blanks ;-)
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/push.c | 1 +
builtin/receive-pack.c | 54 +++++++++++++++++++++++++++++++++++++++-
builtin/send-pack.c | 65 +++++++++++++++++++++++++++++++++++++++++++++---
send-pack.h | 1 +
transport.c | 4 +++
transport.h | 4 +++
6 files changed, 124 insertions(+), 5 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 35cce53..2238f4e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -261,6 +261,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
TRANSPORT_PUSH_SET_UPSTREAM),
OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
+ OPT_BIT('s', "signed", &flags, "GPG sign the push", TRANSPORT_PUSH_SIGNED),
OPT_END()
};
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..307fc3b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -30,12 +30,14 @@ static int receive_unpack_limit = -1;
static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
+static int signed_push;
static int use_sideband;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
static int auto_gc = 1;
static const char *head_name;
static int sent_capabilities;
+static char *push_certificate;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -114,7 +116,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
else
packet_write(1, "%s %s%c%s%s\n",
sha1_to_hex(sha1), path, 0,
- " report-status delete-refs side-band-64k",
+ " report-status delete-refs side-band-64k signed-push",
prefer_ofs_delta ? " ofs-delta" : "");
sent_capabilities = 1;
return 0;
@@ -579,6 +581,31 @@ static void check_aliased_updates(struct command *commands)
string_list_clear(&ref_list, 0);
}
+static int record_signed_push(char *cert)
+{
+ /*
+ * This is the place for you to parse the signed push
+ * certificate, grab the commit object names the push updates
+ * refs to, and append the certificate to the notes to these
+ * commits.
+ *
+ * You could also feed the signed push certificate to GPG,
+ * verify the signer identity, and all the other fun stuff,
+ * including feeding it to "pre-push-verify-signature" hook.
+ *
+ * Here we just throw it to stderr to demonstrate that the
+ * codepath is being exercised.
+ */
+ char *cp, *ep;
+ for (cp = cert; *cp; cp = ep) {
+ ep = strchrnul(cp, '\n');
+ if (*ep == '\n')
+ ep++;
+ fprintf(stderr, "RSP: %.*s", (int)(ep - cp), cp);
+ }
+ return 0;
+}
+
static void execute_commands(struct command *commands, const char *unpacker_error)
{
struct command *cmd;
@@ -596,6 +623,12 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
return;
}
+ if (push_certificate && record_signed_push(push_certificate)) {
+ for (cmd = commands; cmd; cmd = cmd->next)
+ cmd->error_string = "n/a (push signature error)";
+ return;
+ }
+
check_aliased_updates(commands);
head_name = resolve_ref("HEAD", sha1, 0, NULL);
@@ -636,6 +669,8 @@ static struct command *read_head_info(void)
report_status = 1;
if (strstr(refname + reflen + 1, "side-band-64k"))
use_sideband = LARGE_PACKET_MAX;
+ if (strstr(refname + reflen + 1, "signed-push"))
+ signed_push = 1;
}
cmd = xcalloc(1, sizeof(struct command) + len - 80);
hashcpy(cmd->old_sha1, old_sha1);
@@ -731,6 +766,21 @@ static const char *unpack(void)
}
}
+static char *receive_push_certificate(void)
+{
+ struct strbuf cert = STRBUF_INIT;
+ for (;;) {
+ char line[1000];
+ int len;
+
+ len = packet_read_line(0, line, sizeof(line));
+ if (!len)
+ break;
+ strbuf_add(&cert, line, len);
+ }
+ return strbuf_detach(&cert, NULL);
+}
+
static void report(struct command *commands, const char *unpack_status)
{
struct command *cmd;
@@ -846,6 +896,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
if ((commands = read_head_info()) != NULL) {
const char *unpack_status = NULL;
+ if (signed_push)
+ push_certificate = receive_push_certificate();
if (!delete_only(commands))
unpack_status = unpack();
execute_commands(commands, unpack_status);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 87833f4..3193f34 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -237,6 +237,27 @@ static int sideband_demux(int in, int out, void *data)
return ret;
}
+static void sign_push_certificate(struct strbuf *cert)
+{
+ /*
+ * Here, take the contents of cert->buf, and have the user GPG
+ * sign it, and read it back in the strbuf.
+ *
+ * You may want to append some extra info to cert before giving
+ * it to GPG, possibly via a hook.
+ *
+ * Here we upcase them just to demonstrate that the codepath
+ * is being exercised.
+ */
+ char *cp;
+ for (cp = cert->buf; *cp; cp++) {
+ int ch = *cp;
+ if ('a' <= ch && ch <= 'z')
+ *cp = toupper(ch);
+ }
+ return;
+}
+
int send_pack(struct send_pack_args *args,
int fd[], struct child_process *conn,
struct ref *remote_refs,
@@ -250,9 +271,11 @@ int send_pack(struct send_pack_args *args,
int allow_deleting_refs = 0;
int status_report = 0;
int use_sideband = 0;
+ int signed_push = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
+ struct strbuf push_cert = STRBUF_INIT;
/* Does the other end support the reporting? */
if (server_supports("report-status"))
@@ -270,6 +293,19 @@ int send_pack(struct send_pack_args *args,
return 0;
}
+ if (args->signed_push) {
+ if (server_supports("signed-push"))
+ signed_push = !args->dry_run;
+ else
+ warning("The receiving side does not support signed-push");
+ }
+
+ if (signed_push) {
+ const char *committer_info = git_committer_info(0);
+ strbuf_addstr(&push_cert, "Push-Certificate-Version: 1\n");
+ strbuf_addf(&push_cert, "Pusher: %s\n", committer_info);
+ }
+
/*
* Finally, tell the other end!
*/
@@ -301,15 +337,19 @@ int send_pack(struct send_pack_args *args,
char *old_hex = sha1_to_hex(ref->old_sha1);
char *new_hex = sha1_to_hex(ref->new_sha1);
- if (!cmds_sent && (status_report || use_sideband)) {
- packet_buf_write(&req_buf, "%s %s %s%c%s%s",
+ if (!cmds_sent &&
+ (status_report || use_sideband || signed_push))
+ packet_buf_write(&req_buf, "%s %s %s%c%s%s%s",
old_hex, new_hex, ref->name, 0,
status_report ? " report-status" : "",
- use_sideband ? " side-band-64k" : "");
- }
+ use_sideband ? " side-band-64k" : "",
+ signed_push ? " signed-push" : "");
else
packet_buf_write(&req_buf, "%s %s %s",
old_hex, new_hex, ref->name);
+ if (signed_push && hashcmp(ref->old_sha1, ref->new_sha1))
+ strbuf_addf(&push_cert, "Update: %s %s %s\n",
+ old_hex, new_hex, ref->name);
ref->status = status_report ?
REF_STATUS_EXPECTING_REPORT :
REF_STATUS_OK;
@@ -326,6 +366,23 @@ int send_pack(struct send_pack_args *args,
safe_write(out, req_buf.buf, req_buf.len);
packet_flush(out);
}
+
+ if (signed_push) {
+ char *cp, *ep;
+
+ sign_push_certificate(&push_cert);
+ strbuf_reset(&req_buf);
+ for (cp = push_cert.buf; *cp; cp = ep) {
+ ep = strchrnul(cp, '\n');
+ if (*ep == '\n')
+ ep++;
+ packet_buf_write(&req_buf, "%.*s",
+ (int)(ep - cp), cp);
+ }
+ /* Do we need anything funky for stateless rpc? */
+ safe_write(out, req_buf.buf, req_buf.len);
+ packet_flush(out);
+ }
strbuf_release(&req_buf);
if (use_sideband && cmds_sent) {
diff --git a/send-pack.h b/send-pack.h
index 05d7ab1..754943e 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -11,6 +11,7 @@ struct send_pack_args {
use_thin_pack:1,
use_ofs_delta:1,
dry_run:1,
+ signed_push:1,
stateless_rpc:1;
};
diff --git a/transport.c b/transport.c
index fa279d5..7a7ffe4 100644
--- a/transport.c
+++ b/transport.c
@@ -476,6 +476,9 @@ static int set_git_option(struct git_transport_options *opts,
else
opts->depth = atoi(value);
return 0;
+ } else if (!strcmp(name, TRANS_OPT_SIGNED_PUSH)) {
+ opts->signed_push = !!value;
+ return 0;
}
return 1;
}
@@ -793,6 +796,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.progress = transport->progress;
args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
+ args.signed_push = !!(flags & TRANSPORT_PUSH_SIGNED);
ret = send_pack(&args, data->fd, data->conn, remote_refs,
&data->extra_have);
diff --git a/transport.h b/transport.h
index 059b330..d2fa478 100644
--- a/transport.h
+++ b/transport.h
@@ -8,6 +8,7 @@ struct git_transport_options {
unsigned thin : 1;
unsigned keep : 1;
unsigned followtags : 1;
+ unsigned signed_push : 1;
int depth;
const char *uploadpack;
const char *receivepack;
@@ -102,6 +103,7 @@ struct transport {
#define TRANSPORT_PUSH_PORCELAIN 16
#define TRANSPORT_PUSH_SET_UPSTREAM 32
#define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
+#define TRANSPORT_PUSH_SIGNED 128
#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
@@ -128,6 +130,8 @@ struct transport *transport_get(struct remote *, const char *);
/* Aggressively fetch annotated tags if possible */
#define TRANS_OPT_FOLLOWTAGS "followtags"
+#define TRANS_OPT_SIGNED_PUSH "signedpush"
+
/**
* Returns 0 if the option was used, non-zero otherwise. Prints a
* message to stderr if the option is not used.
--
1.7.7.rc0.186.g50963
^ permalink raw reply related
* [PATCH 1/2] send-pack: typofix error message
From: Junio C Hamano @ 2011-09-07 20:56 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce
The message identifies the process as receive-pack when it cannot fork the
sideband demultiplexer. We are actually a send-pack.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/send-pack.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index c1f6ddd..87833f4 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -334,7 +334,7 @@ int send_pack(struct send_pack_args *args,
demux.data = fd;
demux.out = -1;
if (start_async(&demux))
- die("receive-pack: unable to fork off sideband demultiplexer");
+ die("send-pack: unable to fork off sideband demultiplexer");
in = demux.out;
}
--
1.7.7.rc0.186.g50963
^ permalink raw reply related
* Re: [ANNOUNCE] GitTogether 2011 - Oct 24th/25th
From: Scott Chacon @ 2011-09-07 20:44 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, git
In-Reply-To: <20110907193006.GB13364@sigill.intra.peff.net>
Hey,
On Wed, Sep 7, 2011 at 7:30 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Sep 07, 2011 at 11:38:18AM -0700, Shawn O. Pearce wrote:
>
>> As we approach 50 people, does it makes sense to be able to break the
>> event down into 2 "tracks", and have 2 meeting spaces available? I
>> know a number of the folks on the attendee list are Gerrit Code Review
>> / Android sorts of shops and will want to discuss topics related to
>> that that aren't necessarily relevant to the GitHub users / Linux
>> kernel hacking folks that are also on the list. Being able to break
>> off some of those discussions might make the event more interesting
>> for everyone involved.
>
> I think that's reasonable, especially as we grow. However, one of the
> valuable things (for me, anyway) in previous GitTogethers is throwing
> all of these people together to some degree. I'm not terribly interested
> in day-to-day Gerrit issues, but sometimes the discussions start from
> some minor Gerrit annoyance, and we end up realizing that the right
> solution involves changes at a more fundamental layer, and all of git is
> better as a result. I'd hate to lose that developer/user interaction.
>
> Maybe we can be segmented for part of the conference, and then bring
> everybody together for other parts. I dunno. I guess that involves
> predicting which parts will be useful for everybody to be together.
Yeah, I think it would be best to have one track, but have breakout
periods where people can work on stuff together or have smaller
conversations, or even have smaller rooms that people can go into to
work for a while.
I really want to have one track though - I'm personally interested in
everything. I want to know how Gerrit is working these days and
different workflows and stuff. I think we have enough time where even
with 50 or 60 people we could have everyone who wanted to speak (which
isn't going to be everybody) have a chance without being too crazy.
Scott
^ permalink raw reply
* Re: The imporantance of including http credential caching in 1.7.7
From: Kyle Neath @ 2011-09-07 20:14 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4E6769E3.4070003@drmicha.warpmail.net>
Junio C Hamano <gitster@pobox.com> wrote:
> If this were a new, insignificant, and obscure feature in a piece of
> software with mere 20k users, it may be OK to release a new version with
> the feature in an uncooked shape.
For the sake of my paycheck, I should certainly hope not! I'm not at all
suggesting we merge what we have in. However, I do think this feature is
important enough to delay the release. I trust in the judgement of the core
members to know when something is ready for inclusion in master.
Michael J Gruber <git@drmicha.warpmail.net> wrote:
> So, it's been a year or more that you've been aware of the importance of
> this issue (from your/github's perspective), and we hear about it now,
> at the end of the rc phase.
I apologize if it sounds like that. I've been discussing this situation with
many people (including Jeff King) for a very long time now, and it was my
understanding that the credential caching was done and simply waiting for a
new release. This is the first I've heard that it will not be included in
1.7.7, so I'm voicing my opinion now. Admittedly, late in the game - and I
apologize for that.
I'd be happy to help in any capacity I can. Unfortunately I'm no C hacker, and
I've accepted that as a character flaw (it's something I'm working on). I'm
afraid I can't be of much help with the actual code. What I can provide is an
alternate viewpoint to the core team. A viewpoint of someone who's spent 3
years trying to make git easier for newcomers.
I urge the core team to think about what kind of opportunity we have here.
Credential caching isn't some minor feature. It's the last piece of the puzzle
that promotes Smart HTTP to a viable alternative over the git & ssh protocols.
Smart HTTP solves an huge collection of problems people have with using git
day to day. One URL to push privately and pull anonymously. No firewall
restrictions at universities or workplaces. Username & password
authentication. I get kind of turned on just thinking about it.
Kyle
^ permalink raw reply
* Re: [PATCH v4] gitk: Allow commit editing
From: Jeff King @ 2011-09-07 20:10 UTC (permalink / raw)
To: Michal Sojka; +Cc: git, paulus
In-Reply-To: <87fwknaveh.fsf@steelpick.2x.cz>
On Sat, Aug 27, 2011 at 02:31:02PM +0200, Michal Sojka wrote:
> Here is a new version with the micro-optimization.
>
> Another minor change is that this patch now applies to gitk repo
> (http://git.kernel.org/pub/scm/gitk/gitk.git) instead of the main git
> repo.
Thanks. This version fixes most of my complaints and looks reasonable
sane, as long as you accept the idea that starting an interactive rebase
behind the scenes is sane. I'm still not 100% convinced. But then, I am
not a big gitk user, either (I like visualizing with it, but I rarely
want to do anything interactive). So I'll leave Paul and anyone else to
argue or make a decision on that count.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] git svn dcommit: new option --interactive.
From: Frédéric Heitzmann @ 2011-09-07 20:02 UTC (permalink / raw)
To: Eric Wong; +Cc: git, gitster, jaysoffian
In-Reply-To: <20110906202601.GA11668@dcvr.yhbt.net>
Le 06/09/2011 22:26, Eric Wong a écrit :
> Frédéric Heitzmann<frederic.heitzmann@gmail.com> wrote:
>> Allow the user to check the patch set before it is commited to SNV. It is then
>> possible to accept/discard one patch, accept all, or quit.
>>
>> This interactive mode is similar with 'git send email' behaviour. However,
>> 'git svn dcommit' returns as soon as one patch is discarded.
>>
>> Part of the code was taken from git-send-email.perl
>> Thanks-to: Eric Wong<normalperson@yhbt.net> for the initial idea.
>> Signed-off-by: Frédéric Heitzmann<frederic.heitzmann@gmail.com>
> I agree with this feature, a few comments inline.
>
>> I would have preferred not duplicating the code snippets taken from
>> git-send-email ('ask' function, Term related code, ...) but I preferred not
>> to spoil Git.pm with it.
>> Any comment on a better way to factor perl code would be appreciated.
> We should put this into Git.pm at some point.
> (Somebody should refactor git-svn.perl into separate files too... :x)
>
>> Documentation/git-svn.txt | 8 +++++
>> git-svn.perl | 71 ++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 78 insertions(+), 1 deletions(-)
> Tests and feature should be the same patch
>> + return defined $default ? $default : undef
>> + unless defined $term->IN and defined fileno($term->IN) and
>> + defined $term->OUT and defined fileno($term->OUT);
> Things to make life easier for (mainly) C programmers:
>
> * Use C-style "&&" and "||" for conditionals. "and" and "or" are lower
> precedence and better used for control flow (see perlop(1) manpage).
>
> * Also, use parentheses for defined(foo) to disambiguate multiple
> conditions/statements.
>
My fault : I copied-pasted the 'ask' function from git-send-email.
Even if I rewrite it a litlle, it should not prevent anyone to mutalize
some code into Git.pm.
And, indeed, it will improve readability.
I wait a few days to see if anyone else has some comments and I send a
V2 patch serie.
Thanks for reviewing.
--
Fred
^ permalink raw reply
* Re: [PATCHv2 4/5] branch: introduce --list option
From: Jeff King @ 2011-09-07 19:56 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, git
In-Reply-To: <4E5A5290.4050005@drmicha.warpmail.net>
On Sun, Aug 28, 2011 at 04:37:04PM +0200, Michael J Gruber wrote:
> Currently, "-m -d" is forbidden", but "-m -v" is "-m", same for "-d -v".
> Do we want to keep it like that? Probably. I'll add the tests to 4/5.
Yes, I think so. "-v" just means "be more verbose"; the fact that
there is currently nothing to be more verbose about with "-m" and "-d"
is irrelevant.
It does make me a little nervous about the "'git branch -v'
automatically means 'git branch --list -v'" patch, though. It closes the
door in the future to us being more or less verbose about branch
creation details (and while helpful, it creates a slight inconsistency
in the interface).
If we are adding "-l" anyway, is it really necessary? It's not much
harder to do "git branch -lv" once that is in place.
-Peff
^ permalink raw reply
* Re: [PATCH 0/5] RFC: patterns for branch list
From: Jeff King @ 2011-09-07 19:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, git, Michael Schubert
In-Reply-To: <7vwre0dsdy.fsf@alter.siamese.dyndns.org>
On Fri, Aug 26, 2011 at 09:55:37AM -0700, Junio C Hamano wrote:
> As we use fnmatch() and not match_pathspec() for this pattern matching,
> "git branch peff/" will not list all the topics under the peff/ hierarchy
> (your example "git branch peff/\*" would be the way), but I would imagine
> that we may someday want to update it to allow the leading path match
> here. And at that point, distinction between
>
> git branch peff ;# to create a "peff" branch
> git branch peff/ ;# to list "peff/" branches, as "peff/" itself is
> ;# an invalid branch name and your auto listing
> ;# heuristic kicks in
>
> while it might be very useful for experts, becomes too subtle and would
> confuse new people. We should instead require an explicit -l/--list, and
> not use the auto listing heuristics (it is fine for -v to imply -l).
Sorry, I'm atrociously behind on reviewing this topic. But FWIW, I
completely agree with this. Detecting invalid branch formats is much too
subtle and error prone, and we are better off making a short and
easy-to-type version of "--list".
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] GitTogether 2011 - Oct 24th/25th
From: Jeff King @ 2011-09-07 19:30 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Scott Chacon, git
In-Reply-To: <CAJo=hJvm62xPAg3v5Ay3ec-ira-i_BZ0Ej7wfdg+5r2Ls0UJQg@mail.gmail.com>
On Wed, Sep 07, 2011 at 11:38:18AM -0700, Shawn O. Pearce wrote:
> As we approach 50 people, does it makes sense to be able to break the
> event down into 2 "tracks", and have 2 meeting spaces available? I
> know a number of the folks on the attendee list are Gerrit Code Review
> / Android sorts of shops and will want to discuss topics related to
> that that aren't necessarily relevant to the GitHub users / Linux
> kernel hacking folks that are also on the list. Being able to break
> off some of those discussions might make the event more interesting
> for everyone involved.
I think that's reasonable, especially as we grow. However, one of the
valuable things (for me, anyway) in previous GitTogethers is throwing
all of these people together to some degree. I'm not terribly interested
in day-to-day Gerrit issues, but sometimes the discussions start from
some minor Gerrit annoyance, and we end up realizing that the right
solution involves changes at a more fundamental layer, and all of git is
better as a result. I'd hate to lose that developer/user interaction.
Maybe we can be segmented for part of the conference, and then bring
everybody together for other parts. I dunno. I guess that involves
predicting which parts will be useful for everybody to be together.
I assume we'll keep largely to the un-conference format, though, so
these are issues that can be ironed out in the first hour as we see
which topics people are interested in discussing.
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/1] sha1_file: normalize alt_odb path before comparing and storing
From: Junio C Hamano @ 2011-09-07 18:46 UTC (permalink / raw)
To: Wang Hui; +Cc: git, tali
In-Reply-To: <1315391867-31277-2-git-send-email-Hui.Wang@windriver.com>
Wang Hui <Hui.Wang@windriver.com> writes:
> From: Hui Wang <Hui.Wang@windriver.com>
>
> When it needs to compare and add an alt object path to the
> alt_odb_list, we normalize this path first since comparing normalized
> path is easy to get correct result.
>
> Use strbuf to replace some string operations, since it is cleaner and
> safer.
Thanks, will queue.
> diff --git a/sha1_file.c b/sha1_file.c
> index f7c3408..fa2484b 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -248,27 +248,27 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative
> ...
> + /* Drop the last '/' from path can make memcmp more accurate */
> + if (pathbuf.buf[pfxlen-1] == '/')
> + pfxlen -= 1;
By the way, I do not necessarily agree with the above comment. As long as
you consistently strip the trailing slashes from all directory paths, or
you consistently leave a single trailing slash after all directory paths,
you can get accurate comparison either way.
Side note: I tend to prefer keeping a single trailing slash when I
know what we are talking about is a directory in general, because
you do not have to worry about the corner case near the root.
Compare ('/' and '/bin/') vs ('/' and '/bin').
In this particular case, the real reason you want to remove the trailing
slash is that the invariants of ent->base[] demands it (after all, it
places another slash immediately after it), and making pathbuf.buf[] an
empty string (i.e. pfxlen == 0) would still be OK to represent an
alternate object store at the root level (this function assigns '/' at
ent->base[pfxlen] immediately before returning, and that '/' names the
root directory).
> + entlen = pfxlen + 43; /* '/' + 2 hex + '/' + 38 hex + NUL */
> + ent = xmalloc(sizeof(*ent) + entlen);
> + memcpy(ent->base, pathbuf.buf, pfxlen);
> + strbuf_release(&pathbuf);
>
> ent->name = ent->base + pfxlen + 1;
> ent->base[pfxlen + 3] = '/';
^ permalink raw reply
* Re: [ANNOUNCE] GitTogether 2011 - Oct 24th/25th
From: Shawn Pearce @ 2011-09-07 18:38 UTC (permalink / raw)
To: Scott Chacon; +Cc: git
In-Reply-To: <CAP2yMaKi7rEZU2Sh_W_413QOMWANTGEswJDoGO_YDKVMsoEwWQ@mail.gmail.com>
On Wed, Sep 7, 2011 at 10:11, Scott Chacon <schacon@gmail.com> wrote:
> On Mon, Sep 5, 2011 at 12:56 PM, Shawn Pearce <spearce@spearce.org> wrote:
>> Google is once again hosting a 2 day user/developer conference for Git
>> users and developers to get together, share experiences, and hack on
>> interesting features. This event will be held October 24th and 25th at
>> Google's headquarters in Mountain View, CA.
>>
>> More details along with sign-up (as space is limited) can be found on the wiki:
>>
>> https://git.wiki.kernel.org/index.php/GitTogether11
>
> It's been like 2 days and we're already overflowing. I've also
> already heard people say they didn't sign up because it was full.
> This is unacceptable. I want to drink with all of you guys.
Indeed! Clearly I didn't really expect this big of a response this year.
> Shawn, if you can't get a bigger venue at Google, we'll rent a meeting
> space either at the hotel that most of the mentors are staying at or a
> nearby one.
As we approach 50 people, does it makes sense to be able to break the
event down into 2 "tracks", and have 2 meeting spaces available? I
know a number of the folks on the attendee list are Gerrit Code Review
/ Android sorts of shops and will want to discuss topics related to
that that aren't necessarily relevant to the GitHub users / Linux
kernel hacking folks that are also on the list. Being able to break
off some of those discussions might make the event more interesting
for everyone involved.
--
Shawn.
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.7.6.2
From: Sverre Rabbelier @ 2011-09-07 18:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vpqjc2ors.fsf@alter.siamese.dyndns.org>
Heya,
On Wed, Sep 7, 2011 at 20:25, Junio C Hamano <gitster@pobox.com> wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>> How would you later re-merge the series when the
>> problems it has are fixed though?
>
> Simple. I won't re-merge nor allow a fix-up series to be queued on top of
> the failed topic.
Ah, as long as the reroll has a different sha1 there is no problem,
makes sense. Simple yet effective :)
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: Re* Git without morning coffee
From: Michael J Gruber @ 2011-09-07 18:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vty8o2p7g.fsf_-_@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 07.09.2011 20:16:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>>
>>>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>>>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>>>> to a commit
>>>> # Huh?
>>>
>>> Interesting.
>>
>> This is because 1c7b76b (Build in merge, 2008-07-07) grabs the name of the
>> commit to be merged using peel_to_type(), which was defined in 8177631
>> (expose a helper function peel_to_type()., 2007-12-24) in terms of
>> get_sha1_1(). It understands $commit~$n, $commit^$n and $ref@{$nth}, but
>> does not understand :/$str, $treeish:$path, and :$stage:$path.
>
> -- >8 --
> Subject: Allow git merge ":/<pattern>"
>
> It probably is not such a good idea to use ":/<pattern>" to specify which
> commit to merge, as ":/<pattern>" can often hit unexpected commits, but
> somebody tried it and got a nonsense error message:
>
> fatal: ':/Foo bar' does not point to a commit
>
> So here is a for-the-sake-of-consistency update that is fairly useless
> that allows users to carefully try not shooting in the foot.
Shooting in the foot can be a good thing, depending on the feet...
My concern is: If a command expects a commit(tish), a user expects to be
able to specify it in any way which git rev-parse can parse. I had no
idea we distinguish even further then what the command itself requires
("branch" needs a branch refname, diff-tree a treeish etc.).
So, for systematic reasons I think the below is an improvement.
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/merge.c | 19 ++++++++++++++-----
> sha1_name.c | 6 ------
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ab4077f..ee56974 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -403,6 +403,16 @@ static void finish(const unsigned char *new_head, const char *msg)
> strbuf_release(&reflog_message);
> }
>
> +static struct object *want_commit(const char *name)
> +{
> + struct object *obj;
> + unsigned char sha1[20];
> + if (get_sha1(name, sha1))
> + return NULL;
> + obj = parse_object(sha1);
> + return peel_to_type(name, 0, obj, OBJ_COMMIT);
> +}
> +
> /* Get the name for the merge commit's message. */
> static void merge_name(const char *remote, struct strbuf *msg)
> {
> @@ -418,7 +428,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
> remote = bname.buf;
>
> memset(branch_head, 0, sizeof(branch_head));
> - remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
> + remote_head = want_commit(remote);
> if (!remote_head)
> die(_("'%s' does not point to a commit"), remote);
>
> @@ -1124,7 +1134,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> if (!allow_fast_forward)
> die(_("Non-fast-forward commit does not make sense into "
> "an empty head"));
> - remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
> + remote_head = want_commit(argv[0]);
> if (!remote_head)
> die(_("%s - not something we can merge"), argv[0]);
> read_empty(remote_head->sha1, 0);
> @@ -1170,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> struct object *o;
> struct commit *commit;
>
> - o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
> + o = want_commit(argv[i]);
> if (!o)
> die(_("%s - not something we can merge"), argv[i]);
> commit = lookup_commit(o->sha1);
> @@ -1238,8 +1248,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> if (have_message)
> strbuf_addstr(&msg,
> " (no commit created; -m option ignored)");
> - o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
> - 0, NULL, OBJ_COMMIT);
> + o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
> if (!o)
> return 1;
>
> diff --git a/sha1_name.c b/sha1_name.c
> index ff5992a..653b065 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -501,12 +501,6 @@ struct object *peel_to_type(const char *name, int namelen,
> {
> if (name && !namelen)
> namelen = strlen(name);
> - if (!o) {
> - unsigned char sha1[20];
> - if (get_sha1_1(name, namelen, sha1))
> - return NULL;
> - o = parse_object(sha1);
> - }
> while (1) {
> if (!o || (!o->parsed && !parse_object(o->sha1)))
> return NULL;
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.7.6.2
From: Junio C Hamano @ 2011-09-07 18:25 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List
In-Reply-To: <CAGdFq_hAm4Avoi1VoFMHcnSE4oDmhEPvqJiodrLUJ5042pKzGA@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> How would you later re-merge the series when the
> problems it has are fixed though?
Simple. I won't re-merge nor allow a fix-up series to be queued on top of
the failed topic.
^ permalink raw reply
* Re* Git without morning coffee
From: Junio C Hamano @ 2011-09-07 18:16 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <7v62l445nw.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>>> to a commit
>>> # Huh?
>>
>> Interesting.
>
> This is because 1c7b76b (Build in merge, 2008-07-07) grabs the name of the
> commit to be merged using peel_to_type(), which was defined in 8177631
> (expose a helper function peel_to_type()., 2007-12-24) in terms of
> get_sha1_1(). It understands $commit~$n, $commit^$n and $ref@{$nth}, but
> does not understand :/$str, $treeish:$path, and :$stage:$path.
-- >8 --
Subject: Allow git merge ":/<pattern>"
It probably is not such a good idea to use ":/<pattern>" to specify which
commit to merge, as ":/<pattern>" can often hit unexpected commits, but
somebody tried it and got a nonsense error message:
fatal: ':/Foo bar' does not point to a commit
So here is a for-the-sake-of-consistency update that is fairly useless
that allows users to carefully try not shooting in the foot.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge.c | 19 ++++++++++++++-----
sha1_name.c | 6 ------
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index ab4077f..ee56974 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -403,6 +403,16 @@ static void finish(const unsigned char *new_head, const char *msg)
strbuf_release(&reflog_message);
}
+static struct object *want_commit(const char *name)
+{
+ struct object *obj;
+ unsigned char sha1[20];
+ if (get_sha1(name, sha1))
+ return NULL;
+ obj = parse_object(sha1);
+ return peel_to_type(name, 0, obj, OBJ_COMMIT);
+}
+
/* Get the name for the merge commit's message. */
static void merge_name(const char *remote, struct strbuf *msg)
{
@@ -418,7 +428,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
remote = bname.buf;
memset(branch_head, 0, sizeof(branch_head));
- remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
+ remote_head = want_commit(remote);
if (!remote_head)
die(_("'%s' does not point to a commit"), remote);
@@ -1124,7 +1134,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (!allow_fast_forward)
die(_("Non-fast-forward commit does not make sense into "
"an empty head"));
- remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
+ remote_head = want_commit(argv[0]);
if (!remote_head)
die(_("%s - not something we can merge"), argv[0]);
read_empty(remote_head->sha1, 0);
@@ -1170,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct object *o;
struct commit *commit;
- o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
+ o = want_commit(argv[i]);
if (!o)
die(_("%s - not something we can merge"), argv[i]);
commit = lookup_commit(o->sha1);
@@ -1238,8 +1248,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (have_message)
strbuf_addstr(&msg,
" (no commit created; -m option ignored)");
- o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
- 0, NULL, OBJ_COMMIT);
+ o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
if (!o)
return 1;
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..653b065 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -501,12 +501,6 @@ struct object *peel_to_type(const char *name, int namelen,
{
if (name && !namelen)
namelen = strlen(name);
- if (!o) {
- unsigned char sha1[20];
- if (get_sha1_1(name, namelen, sha1))
- return NULL;
- o = parse_object(sha1);
- }
while (1) {
if (!o || (!o->parsed && !parse_object(o->sha1)))
return NULL;
^ permalink raw reply related
* Re: [PATCH v17 1/7] bisect: move argument parsing before state modification.
From: Johannes Sixt @ 2011-09-07 18:07 UTC (permalink / raw)
To: Christian Couder; +Cc: Jon Seymour, git, gitster, jnareb, jrnieder
In-Reply-To: <201109070816.16655.chriscool@tuxfamily.org>
Am 07.09.2011 08:16, schrieb Christian Couder:
> If we start bisecting like this:
>
> $ git bisect start HEAD HEAD~20
>
> and then we decide that it was not optimum and we want to start again like
> this:
>
> $ git bisect start HEAD HEAD~6
>
> then issuing the latter command might not work as it did before this patch.
>
> Before this patch the latter command would do a "git checkout $start_head"
> before the repeated rev=$(git rev-parse -q --verify "$arg^{commit}") to
> convert arguments into sha1. And after this patch the order is reversed.
>
> This means that before this patch "HEAD" in the arguments to "git bisect
> start" would refer to $start_head because the "git checkout $start_head"
> changes HEAD. After this patch "HEAD" in the arguments to "git bisect start"
> would refer to the current HEAD.
But isn't this an improvement? HEAD denotes the current head. After the
first 'bisect start HEAD HEAD~20', HEAD is somewhere in the middle, not
the original HEAD anymore; I would *expect* that a different commit is
checked out if I just repeat the command.
IOW, I think the new behavior is *much* better than the old behavior.
-- Hannes
^ permalink raw reply
* Re: Git without morning coffee
From: Junio C Hamano @ 2011-09-07 17:49 UTC (permalink / raw)
To: Michael Witten; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <CAMOZ1BstyMteutmK7tst=3t9djavY9_4vBKJgdj7rhUnE1Wr7w@mail.gmail.com>
Michael Witten <mfwitten@gmail.com> writes:
> I think it would be great if at some point you could write a detailed
> tutorial of how you maintain git...
Is MaintNotes[*1*] taken together with Documentation/howto/maintain-git.txt
insufficient?
[Reference]
*1* http://git-blame.blogspot.com/p/note-from-maintainer.html
^ permalink raw reply
* [PATCH 5/5] for-each-ref: add split message parts to %(contents:*).
From: Jeff King @ 2011-09-07 17:46 UTC (permalink / raw)
To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902175323.GA29761@sigill.intra.peff.net>
From: Michał Górny <mgorny@gentoo.org>
The %(body) placeholder returns the whole body of a tag or
commit, including the signature. However, callers may want
to get just the body without signature, or just the
signature.
Rather than change the meaning of %(body), which might break
some scripts, this patch introduces a new set of
placeholders which break down the %(contents) placeholder
into its constituent parts.
[jk: initial patch by mg, rebased on top of my refactoring
and with tests by me]
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-for-each-ref.txt | 7 ++--
builtin/for-each-ref.c | 32 +++++++++++++++---
t/lib-gpg.sh | 8 +++++
t/t6300-for-each-ref.sh | 60 ++++++++++++++++++++++++++++++++++-
4 files changed, 96 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..c872b88 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,9 +101,10 @@ Fields that have name-email-date tuple as its value (`author`,
`committer`, and `tagger`) can be suffixed with `name`, `email`,
and `date` to extract the named component.
-The first line of the message in a commit and tag object is
-`subject`, the remaining lines are `body`. The whole message
-is `contents`.
+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.
For sorting purposes, fields with numeric values sort in numeric
order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ea2112b..50fba65 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -69,6 +69,9 @@ static struct {
{ "subject" },
{ "body" },
{ "contents" },
+ { "contents:subject" },
+ { "contents:body" },
+ { "contents:signature" },
{ "upstream" },
{ "symref" },
{ "flag" },
@@ -472,7 +475,9 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
static void find_subpos(const char *buf, unsigned long sz,
const char **sub, unsigned long *sublen,
- const char **body, unsigned long *bodylen)
+ const char **body, unsigned long *bodylen,
+ unsigned long *nonsiglen,
+ const char **sig, unsigned long *siglen)
{
const char *eol;
/* skip past header until we hit empty line */
@@ -486,10 +491,14 @@ static void find_subpos(const char *buf, unsigned long sz,
while (*buf == '\n')
buf++;
+ /* parse signature first; we might not even have a subject line */
+ *sig = buf + parse_signature(buf, strlen(buf));
+ *siglen = strlen(*sig);
+
/* subject is first non-empty line */
*sub = buf;
/* subject goes to first empty line */
- while (*buf && *buf != '\n') {
+ while (buf < *sig && *buf && *buf != '\n') {
eol = strchrnul(buf, '\n');
if (*eol)
eol++;
@@ -505,14 +514,15 @@ static void find_subpos(const char *buf, unsigned long sz,
buf++;
*body = buf;
*bodylen = strlen(buf);
+ *nonsiglen = *sig - buf;
}
/* See grab_values */
static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
{
int i;
- const char *subpos = NULL, *bodypos;
- unsigned long sublen, bodylen;
+ const char *subpos = NULL, *bodypos, *sigpos;
+ unsigned long sublen, bodylen, nonsiglen, siglen;
for (i = 0; i < used_atom_cnt; i++) {
const char *name = used_atom[i];
@@ -523,17 +533,27 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
name++;
if (strcmp(name, "subject") &&
strcmp(name, "body") &&
- strcmp(name, "contents"))
+ strcmp(name, "contents") &&
+ strcmp(name, "contents:subject") &&
+ strcmp(name, "contents:body") &&
+ strcmp(name, "contents:signature"))
continue;
if (!subpos)
find_subpos(buf, sz,
&subpos, &sublen,
- &bodypos, &bodylen);
+ &bodypos, &bodylen, &nonsiglen,
+ &sigpos, &siglen);
if (!strcmp(name, "subject"))
v->s = copy_subject(subpos, sublen);
+ else if (!strcmp(name, "contents:subject"))
+ v->s = copy_subject(subpos, sublen);
else if (!strcmp(name, "body"))
v->s = xmemdupz(bodypos, bodylen);
+ else if (!strcmp(name, "contents:body"))
+ v->s = xmemdupz(bodypos, nonsiglen);
+ else if (!strcmp(name, "contents:signature"))
+ v->s = xmemdupz(sigpos, siglen);
else if (!strcmp(name, "contents"))
v->s = xstrdup(subpos);
}
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 28463fb..05824fa 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -24,3 +24,11 @@ else
;;
esac
fi
+
+sanitize_pgp() {
+ perl -ne '
+ /^-----END PGP/ and $in_pgp = 0;
+ print unless $in_pgp;
+ /^-----BEGIN PGP/ and $in_pgp = 1;
+ '
+}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 0c9ff96..1721784 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@
test_description='for-each-ref test'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
# Mon Jul 3 15:18:43 2006 +0000
datestamp=1151939923
@@ -40,9 +41,10 @@ test_atom() {
*) ref=$1 ;;
esac
printf '%s\n' "$3" >expected
- test_expect_${4:-success} "basic atom: $1 $2" "
+ test_expect_${4:-success} $PREREQ "basic atom: $1 $2" "
git for-each-ref --format='%($2)' $ref >actual &&
- test_cmp expected actual
+ sanitize_pgp <actual >actual.clean &&
+ test_cmp expected actual.clean
"
}
@@ -72,7 +74,10 @@ test_atom head taggerdate ''
test_atom head creator 'C O Mitter <committer@example.com> 1151939923 +0200'
test_atom head creatordate 'Mon Jul 3 17:18:43 2006 +0200'
test_atom head subject 'Initial'
+test_atom head contents:subject 'Initial'
test_atom head body ''
+test_atom head contents:body ''
+test_atom head contents:signature ''
test_atom head contents 'Initial
'
@@ -102,7 +107,10 @@ test_atom tag taggerdate 'Mon Jul 3 17:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151939925 +0200'
test_atom tag creatordate 'Mon Jul 3 17:18:45 2006 +0200'
test_atom tag subject 'Tagging at 1151939927'
+test_atom tag contents:subject 'Tagging at 1151939927'
test_atom tag body ''
+test_atom tag contents:body ''
+test_atom tag contents:signature ''
test_atom tag contents 'Tagging at 1151939927
'
@@ -390,9 +398,14 @@ test_expect_success 'create tag with multiline subject' '
git tag -F msg multiline
'
test_atom refs/tags/multiline subject 'first subject line second subject line'
+test_atom refs/tags/multiline contents:subject 'first subject line second subject line'
test_atom refs/tags/multiline body 'first body line
second body line
'
+test_atom refs/tags/multiline contents:body 'first body line
+second body line
+'
+test_atom refs/tags/multiline contents:signature ''
test_atom refs/tags/multiline contents 'first subject line
second subject line
@@ -400,4 +413,47 @@ first body line
second body line
'
+test_expect_success GPG 'create signed tags' '
+ git tag -s -m "" signed-empty &&
+ git tag -s -m "subject line" signed-short &&
+ cat >msg <<-\EOF &&
+ subject line
+
+ body contents
+ EOF
+ git tag -s -F msg signed-long
+'
+
+sig='-----BEGIN PGP SIGNATURE-----
+-----END PGP SIGNATURE-----
+'
+
+PREREQ=GPG
+test_atom refs/tags/signed-empty subject ''
+test_atom refs/tags/signed-empty contents:subject ''
+test_atom refs/tags/signed-empty body "$sig"
+test_atom refs/tags/signed-empty contents:body ''
+test_atom refs/tags/signed-empty contents:signature "$sig"
+test_atom refs/tags/signed-empty contents "$sig"
+
+test_atom refs/tags/signed-short subject 'subject line'
+test_atom refs/tags/signed-short contents:subject 'subject line'
+test_atom refs/tags/signed-short body "$sig"
+test_atom refs/tags/signed-short contents:body ''
+test_atom refs/tags/signed-short contents:signature "$sig"
+test_atom refs/tags/signed-short contents "subject line
+$sig"
+
+test_atom refs/tags/signed-long subject 'subject line'
+test_atom refs/tags/signed-long contents:subject 'subject line'
+test_atom refs/tags/signed-long body "body contents
+$sig"
+test_atom refs/tags/signed-long contents:body 'body contents
+'
+test_atom refs/tags/signed-long contents:signature "$sig"
+test_atom refs/tags/signed-long contents "subject line
+
+body contents
+$sig"
+
test_done
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 4/5] for-each-ref: handle multiline subjects like --pretty
From: Jeff King @ 2011-09-07 17:44 UTC (permalink / raw)
To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902175323.GA29761@sigill.intra.peff.net>
Generally the format of a git tag or commit message is:
subject
body body body
body body body
However, we occasionally see multiline subjects like:
subject
with multiple
lines
body body body
body body body
The rest of git treats these multiline subjects as something
to be concatenated and shown as a single line (e.g., "git
log --pretty=format:%s" will do so since f53bd74). For
consistency, for-each-ref should do the same with its
"%(subject)".
Signed-off-by: Jeff King <peff@peff.net>
---
I split this out from the signature patch to make it more obvious what's
going on.
builtin/for-each-ref.c | 29 ++++++++++++++++++++++++-----
t/t6300-for-each-ref.sh | 21 +++++++++++++++++++++
2 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index bcea027..ea2112b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -361,6 +361,18 @@ static const char *copy_email(const char *buf)
return xmemdupz(email, eoemail + 1 - email);
}
+static char *copy_subject(const char *buf, unsigned long len)
+{
+ char *r = xmemdupz(buf, len);
+ int i;
+
+ for (i = 0; i < len; i++)
+ if (r[i] == '\n')
+ r[i] = ' ';
+
+ return r;
+}
+
static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
{
const char *eoemail = strstr(buf, "> ");
@@ -476,10 +488,17 @@ static void find_subpos(const char *buf, unsigned long sz,
/* subject is first non-empty line */
*sub = buf;
- /* subject goes to end of line */
- eol = strchrnul(buf, '\n');
- *sublen = eol - buf;
- buf = eol;
+ /* subject goes to first empty line */
+ while (*buf && *buf != '\n') {
+ eol = strchrnul(buf, '\n');
+ if (*eol)
+ eol++;
+ buf = eol;
+ }
+ *sublen = buf - *sub;
+ /* drop trailing newline, if present */
+ if (*sublen && (*sub)[*sublen - 1] == '\n')
+ *sublen -= 1;
/* skip any empty lines */
while (*buf == '\n')
@@ -512,7 +531,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
&bodypos, &bodylen);
if (!strcmp(name, "subject"))
- v->s = xmemdupz(subpos, sublen);
+ v->s = copy_subject(subpos, sublen);
else if (!strcmp(name, "body"))
v->s = xmemdupz(bodypos, bodylen);
else if (!strcmp(name, "contents"))
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 6fa4d52..0c9ff96 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -379,4 +379,25 @@ first body line
second body line
'
+test_expect_success 'create tag with multiline subject' '
+ cat >msg <<-\EOF &&
+ first subject line
+ second subject line
+
+ first body line
+ second body line
+ EOF
+ git tag -F msg multiline
+'
+test_atom refs/tags/multiline subject 'first subject line second subject line'
+test_atom refs/tags/multiline body 'first body line
+second body line
+'
+test_atom refs/tags/multiline contents 'first subject line
+second subject line
+
+first body line
+second body line
+'
+
test_done
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 3/5] for-each-ref: refactor subject and body placeholder parsing
From: Jeff King @ 2011-09-07 17:44 UTC (permalink / raw)
To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902175323.GA29761@sigill.intra.peff.net>
The find_subpos function was a little hard to use, as well
as to read. It would sometimes write into the subject and
body pointers, and sometimes not. The body pointer sometimes
could be compared to subject, and sometimes not. When
actually duplicating the subject, the caller was forced to
figure out again how long the subject is (which is not too
big a deal when the subject is a single line, but hard to
extend).
The refactoring makes the function more straightforward, both
to read and to use. We will always put something into the
subject and body pointers, and we return explicit lengths
for them, too.
This lays the groundwork both for more complex subject
parsing (e.g., multiline), as well as splitting the body
into subparts (like the text versus the signature).
Signed-off-by: Jeff King <peff@peff.net>
---
Sorry, the patch is a little bit hard to read. It's probably simpler to
just apply and read the resulting function.
builtin/for-each-ref.c | 54 +++++++++++++++++++++++++----------------------
1 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..bcea027 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -458,38 +458,42 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
}
}
-static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
+static void find_subpos(const char *buf, unsigned long sz,
+ const char **sub, unsigned long *sublen,
+ const char **body, unsigned long *bodylen)
{
- while (*buf) {
- const char *eol = strchr(buf, '\n');
- if (!eol)
- return;
- if (eol[1] == '\n') {
- buf = eol + 1;
- break; /* found end of header */
- }
- buf = eol + 1;
+ const char *eol;
+ /* skip past header until we hit empty line */
+ while (*buf && *buf != '\n') {
+ eol = strchrnul(buf, '\n');
+ if (*eol)
+ eol++;
+ buf = eol;
}
+ /* skip any empty lines */
while (*buf == '\n')
buf++;
- if (!*buf)
- return;
- *sub = buf; /* first non-empty line */
- buf = strchr(buf, '\n');
- if (!buf) {
- *body = "";
- return; /* no body */
- }
+
+ /* subject is first non-empty line */
+ *sub = buf;
+ /* subject goes to end of line */
+ eol = strchrnul(buf, '\n');
+ *sublen = eol - buf;
+ buf = eol;
+
+ /* skip any empty lines */
while (*buf == '\n')
- buf++; /* skip blank between subject and body */
+ buf++;
*body = buf;
+ *bodylen = strlen(buf);
}
/* See grab_values */
static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
{
int i;
- const char *subpos = NULL, *bodypos = NULL;
+ const char *subpos = NULL, *bodypos;
+ unsigned long sublen, bodylen;
for (i = 0; i < used_atom_cnt; i++) {
const char *name = used_atom[i];
@@ -503,14 +507,14 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
strcmp(name, "contents"))
continue;
if (!subpos)
- find_subpos(buf, sz, &subpos, &bodypos);
- if (!subpos)
- return;
+ find_subpos(buf, sz,
+ &subpos, &sublen,
+ &bodypos, &bodylen);
if (!strcmp(name, "subject"))
- v->s = copy_line(subpos);
+ v->s = xmemdupz(subpos, sublen);
else if (!strcmp(name, "body"))
- v->s = xstrdup(bodypos);
+ v->s = xmemdupz(bodypos, bodylen);
else if (!strcmp(name, "contents"))
v->s = xstrdup(subpos);
}
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 2/5] t6300: add more body-parsing tests
From: Jeff King @ 2011-09-07 17:43 UTC (permalink / raw)
To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902175323.GA29761@sigill.intra.peff.net>
The current tests don't actually check parsing commit and
tag messages that have both a subject and a body (they just
have single-line messages).
Signed-off-by: Jeff King <peff@peff.net>
---
This is mostly to help make sure the next patch doesn't regress this
case.
t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7dc8a51..6fa4d52 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -37,6 +37,7 @@ test_atom() {
case "$1" in
head) ref=refs/heads/master ;;
tag) ref=refs/tags/testtag ;;
+ *) ref=$1 ;;
esac
printf '%s\n' "$3" >expected
test_expect_${4:-success} "basic atom: $1 $2" "
@@ -359,4 +360,23 @@ test_expect_success 'an unusual tag with an incomplete line' '
'
+test_expect_success 'create tag with subject and body content' '
+ cat >>msg <<-\EOF &&
+ the subject line
+
+ first body line
+ second body line
+ EOF
+ git tag -F msg subject-body
+'
+test_atom refs/tags/subject-body subject 'the subject line'
+test_atom refs/tags/subject-body body 'first body line
+second body line
+'
+test_atom refs/tags/subject-body contents 'the subject line
+
+first body line
+second body line
+'
+
test_done
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 1/5] t7004: factor out gpg setup
From: Jeff King @ 2011-09-07 17:42 UTC (permalink / raw)
To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902175323.GA29761@sigill.intra.peff.net>
Other test scripts may want to look at or verify signed
tags, and the setup is non-trivial. Let's factor this out
into lib-gpg.sh for other tests to use.
Signed-off-by: Jeff King <peff@peff.net>
---
Similar to the one I sent out earlier, but in that one I accidentally
did:
. ../lib-gpg.sh
which is not right. If --root is used for the test script, then ".." is
not necessarily the right place to find the lib-gpg script. This fixes
it to use $TEST_DIRECTORY.
t/lib-gpg.sh | 26 ++++++++++++++++++++++++++
t/{t7004 => lib-gpg}/pubring.gpg | Bin 1164 -> 1164 bytes
t/{t7004 => lib-gpg}/random_seed | Bin 600 -> 600 bytes
t/{t7004 => lib-gpg}/secring.gpg | Bin 1237 -> 1237 bytes
t/{t7004 => lib-gpg}/trustdb.gpg | Bin 1280 -> 1280 bytes
t/t7004-tag.sh | 29 +----------------------------
6 files changed, 27 insertions(+), 28 deletions(-)
create mode 100755 t/lib-gpg.sh
rename t/{t7004 => lib-gpg}/pubring.gpg (100%)
rename t/{t7004 => lib-gpg}/random_seed (100%)
rename t/{t7004 => lib-gpg}/secring.gpg (100%)
rename t/{t7004 => lib-gpg}/trustdb.gpg (100%)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
new file mode 100755
index 0000000..28463fb
--- /dev/null
+++ b/t/lib-gpg.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+gpg_version=`gpg --version 2>&1`
+if test $? = 127; then
+ say "You do not seem to have gpg installed"
+else
+ # As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
+ # the gpg version 1.0.6 didn't parse trust packets correctly, so for
+ # that version, creation of signed tags using the generated key fails.
+ case "$gpg_version" in
+ 'gpg (GnuPG) 1.0.6'*)
+ say "Your version of gpg (1.0.6) is too buggy for testing"
+ ;;
+ *)
+ # key generation info: gpg --homedir t/lib-gpg --gen-key
+ # Type DSA and Elgamal, size 2048 bits, no expiration date.
+ # Name and email: C O Mitter <committer@example.com>
+ # No password given, to enable non-interactive operation.
+ cp -R "$TEST_DIRECTORY"/lib-gpg ./gpghome
+ chmod 0700 gpghome
+ GNUPGHOME="$(pwd)/gpghome"
+ export GNUPGHOME
+ test_set_prereq GPG
+ ;;
+ esac
+fi
diff --git a/t/t7004/pubring.gpg b/t/lib-gpg/pubring.gpg
similarity index 100%
rename from t/t7004/pubring.gpg
rename to t/lib-gpg/pubring.gpg
diff --git a/t/t7004/random_seed b/t/lib-gpg/random_seed
similarity index 100%
rename from t/t7004/random_seed
rename to t/lib-gpg/random_seed
diff --git a/t/t7004/secring.gpg b/t/lib-gpg/secring.gpg
similarity index 100%
rename from t/t7004/secring.gpg
rename to t/lib-gpg/secring.gpg
diff --git a/t/t7004/trustdb.gpg b/t/lib-gpg/trustdb.gpg
similarity index 100%
rename from t/t7004/trustdb.gpg
rename to t/lib-gpg/trustdb.gpg
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 097ce2b..e93ac73 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -8,6 +8,7 @@ test_description='git tag
Tests for operations with tags.'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
# creating and listing lightweight tags:
@@ -585,24 +586,6 @@ test_expect_success \
test_cmp expect actual
'
-# subsequent tests require gpg; check if it is available
-gpg --version >/dev/null 2>/dev/null
-if [ $? -eq 127 ]; then
- say "# gpg not found - skipping tag signing and verification tests"
-else
- # As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
- # the gpg version 1.0.6 didn't parse trust packets correctly, so for
- # that version, creation of signed tags using the generated key fails.
- case "$(gpg --version)" in
- 'gpg (GnuPG) 1.0.6'*)
- say "Skipping signed tag tests, because a bug in 1.0.6 version"
- ;;
- *)
- test_set_prereq GPG
- ;;
- esac
-fi
-
# trying to verify annotated non-signed tags:
test_expect_success GPG \
@@ -625,16 +608,6 @@ test_expect_success GPG \
# creating and verifying signed tags:
-# key generation info: gpg --homedir t/t7004 --gen-key
-# Type DSA and Elgamal, size 2048 bits, no expiration date.
-# Name and email: C O Mitter <committer@example.com>
-# No password given, to enable non-interactive operation.
-
-cp -R "$TEST_DIRECTORY"/t7004 ./gpghome
-chmod 0700 gpghome
-GNUPGHOME="$(pwd)/gpghome"
-export GNUPGHOME
-
get_tag_header signed-tag $commit commit $time >expect
echo 'A signed tag message' >>expect
echo '-----BEGIN PGP SIGNATURE-----' >>expect
--
1.7.6.10.g62f04
^ permalink raw reply related
* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Jeff King @ 2011-09-07 17:40 UTC (permalink / raw)
To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902175323.GA29761@sigill.intra.peff.net>
On Fri, Sep 02, 2011 at 01:53:23PM -0400, Jeff King wrote:
> But there may be other corner cases. I need to read through the code
> more carefully, which I should have time to do later today.
This ended up a little trickier than I expected, but I think the series
below is what we should do. I tried to add extensive tests, but let me
know if you can think of any other corner cases.
[1/5]: t7004: factor out gpg setup
[2/5]: t6300: add more body-parsing tests
[3/5]: for-each-ref: refactor subject and body placeholder parsing
[4/5]: for-each-ref: handle multiline subjects like --pretty
[5/5]: for-each-ref: add split message parts to %(contents:*).
-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