* Re: [PATCH 2/2] push -s: skeleton
From: Jeff King @ 2011-09-08 19:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vbouw2hqg.fsf@alter.siamese.dyndns.org>
On Wed, Sep 07, 2011 at 01:57:27PM -0700, Junio C Hamano wrote:
> 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:
I think this is the right direction, but I was a little turned off by
the idea that it needs a protocol extension. As I see it, there are two
ways to care about the contents of a push certificate:
1. The server might care, because it only wants to accept pushes that
are accompanied by a certificate matching a certain key.
2. A client fetching from the server might care, because they want the
integrity and authenticity of the data to be ensured by the gpg key
of the pusher, not by trusting the server.
I think (1) is actually not all that interesting. The server already has
credentials for each user via ssh or http. So it knows who each pusher
is already. It can't relay that information cryptographically to a
client who fetches later, of course, but we are just talking about
whether or not to accept the push at this moment.
But if you really did want to do that, it seems like a pre-receive hook
would be sufficient.
For (2), you don't want to trust the server, so the user's
authentication to the server isn't enough. You want a cryptographic
chain leading back to the original pusher. But the server doesn't
actually need to see or understand that cryptographic chain for this
purpose. If it were stored in a notes-tree or other format pointed to by
a ref, then a client could pull down those notes and do the verification
themselves.
Which means you can start using this immediately, without having to care
about whether your hosting provider supports it or not (or even whether
your provider supports the git protocol. Such a system would Just Work
across dumb http, local clones, sneakernet bundles, etc).
The only issue I foresee is one of atomicity. IIRC, we never have a
whole-repo lock during push, so it's possible that a client might
succeed in pushing the ref with the certificate, but fail at one or more
refs that the certificate mentions. And maybe a protocol extension is
required for that. You've looked much more closely at this than I have,
so maybe you already considered something simpler.
-Peff
^ permalink raw reply
* Re: t5800-*.sh: Intermittent test failures
From: Ramsay Jones @ 2011-09-08 17:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sverre Rabbelier, GIT Mailing-list, Jeff King, Jonathan Nieder
In-Reply-To: <7vpqjgyvn1.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>> On Tue, Aug 9, 2011 at 20:30, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
>>> The git-fast-import is hung in the read() syscall waiting for data which will
>>> never arrive. This is because the git(fast-export) process, started by the above
>>> git(push), executes (producing it's data on stdout) and completes successfully
>>> and exits *before* the above git-fast-import process starts.
>>>
>>> I haven't looked to see how the git(fast-export)/git-fast-import processes are
>>> plumbed together, but there seems to be a synchronization problem somewhere ...
>> This seems odd, before the fast-export process is even started it's
>> stdout are wired to the stdin of the helper (and thus the fast-import
>> process). What indication do you have that fast-import hasn't started
>> and that fast-export has finished?
>>
>> Also, you say git remote-test everywhere, but it should be git
>> remote-testgit, typo?
>
> FWIW, I have been seeing this every once in a while.
Good to know I'm not alone ;-P
Unfortunately, I haven't had the time to debug this further than I've
already reported ...
As I said, it's obviously a process plumbing/synchronization problem; the reading
end of the fast-export output pipe must be open for read by someone (probably by
it's parent), otherwise it would receive SIGPIPE (also, the output is small enough
not to fill the pipe) rather than exiting with success.
When I run the tests with "make test >test-out", I see a failure rate of about
1 in 10. If I then set the debug environment variables (GIT_TRANSPORT_HELPER_DEBUG,
GIT_TRANSLOOP_DEBUG and GIT_DEBUG_TESTGIT) and run the test script directly (-v),
then the failure rate goes up to about 1 in 3.
Well, ... I added debug code to git-fast-{im,ex}port which writes the debug info
to a file (can't write to stdout/stderr obviously), so that may well be affecting
the timing enough to increase the chance of a failure. Having said that, If I'm
listening to music (rhythmbox) at the same time, then the failure rate seems to
increase ...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] send-mail: Add option to sleep between sending each email.
From: Georgi Chorbadzhiyski @ 2011-09-08 17:14 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matthieu Moy, Ramkumar Ramachandra, git
In-Reply-To: <m37h5jt19v.fsf@localhost.localdomain>
On 9/8/11 8:10 PM, Jakub Narebski wrote:
> Georgi Chorbadzhiyski<gf@unixsol.org> writes:
>> Around 09/08/2011 04:58 PM, Georgi Chorbadzhiyski scribbled:
> [...]
>>> In order for this to work, confirmation should be split from send_message()
>>> and from a quick look this not seem very easy. Might be easier to just
>>> disable the sleep if user was asked for confirmation. It'll be good to
>>> not sleep after last email, but main "foreach my $t (@files) {" loop should
>>> pass some hint to send_message().
>>
>> The attached patch (apply on on top of the original) should implement the
>> idea.
>>
>> --
>> Georgi Chorbadzhiyski
>> http://georgi.unixsol.org/
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 7239fd4..d4559c9 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1149,7 +1149,7 @@ X-Mailer: git-send-email $gitversion
>> }
>> }
>>
>> - if (!$dry_run&& $sleep) {
>> + if (!$dry_run&& $sleep&& $message_num< scalar $#files&& $confirm eq 'never') {
> ^^^^^^^^^^^^^^
>
>> print "Sleeping: $sleep second(s).\n" if (!$quiet);
>> sleep($sleep);
>> };
>
> Errr... what? If we have @files array, then '$#files' is index of
> last element in array, which is scalar anyway, and 'scalar $#files' is
> a no-op.
>
> You can get number of elements in array with 'scalar @files', though
> _implicit_ scalar context would also work, like e.g. right hand side
> of '<' operator.
Correct, my perl is rusty and I wasn't sure $#xx was what I needed so
so I copied it from "$time = time - scalar $#files;" somewhere in
the same file.
--
Georgi Chorbadzhiyski
http://georgi.unixsol.org/
^ permalink raw reply
* Re: The imporantance of including http credential caching in 1.7.7
From: Jeff King @ 2011-09-08 19:18 UTC (permalink / raw)
To: John Szakmeister; +Cc: Michael J Gruber, Kyle Neath, git
In-Reply-To: <CAEBDL5VAFaWYctJotxTA8ajy_0KtR8H_4SoDHK29Ofd65mYdKw@mail.gmail.com>
On Thu, Sep 08, 2011 at 11:02:11AM -0400, John Szakmeister wrote:
> On Thu, Sep 8, 2011 at 9:17 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
> [snip]
> > It would be interesting to know what we can rely on in the user group
> > you're thinking about (which I called ssh-challenged). Setting up ssh
> > keys is too complicated. Can we require a working gpg setup? They do
> > want to check sigs, don't they?
>
> I don't think you can require a working gpg setup (at least for not
> addressing the ssh-challenged group).
Agreed. Anything harder than ssh keys is right out the window, because
they're always the alternative these people could be using (but can't or
don't want to).
We could make our own gpg-based password wallet system, but I think it's
a really bad idea, for two reasons:
1. It's reinventing the wheel. Which is bad enough as it is, but is
doubly bad with security-related code, because it's very easy to
screw something up when you're writing a lot of new code.
2. It's inconvenient for users. Nobody wants a separate wallet system
with its own master password. They want to integrate with the
wallet system they're already using. Which is generally going to be
way nicer _anyway_, because it's going to be part of the OS and do
helpful things like unlock the secret store using their login
credentials.
> > So: What credential store/password wallet/etc. can we rely on for this
> > group? Is gpg fair game?
>
> I think there probably need to be providers for using Keychain under
> the Mac, gnome-keyring and kwallet under Linux, and probably something
> using the wincrypt API under Windows. I don't think there's a
> one-store-fits-all solution here, unfortunately. :-(
Exactly. That's why the helpers communicate via pipes. They don't have
to be included with core git at all; you should be able to just drop a
third-party git-credential-foo into your PATH.
> I'm actually tempted try and work on a couple of those myself.
Please do! I mentioned a few people working on helpers elsewhere in this
thread, so you may want to see what they've done and/or coordinate to
avoid duplicate effort. Let me know if you have trouble finding the
appropriate threads in the list archive.
-Peff
^ permalink raw reply
* Re: Git Bug - diff in commit message.
From: Michael Witten @ 2011-09-08 16:26 UTC (permalink / raw)
To: anikey; +Cc: git
In-Reply-To: <1315493353942-6772145.post@n2.nabble.com>
On Thu, Sep 8, 2011 at 14:49, anikey <arty.anikey@gmail.com> wrote:
> Hi, peops. I'm pretty much sure that's a bug.
>
> What I did was putting git diff (i needed to tell people that for my changes
> to start working they needed to aplly message-inline patch to some code
> which was not under git) in commit message. Like adding:
>
> diff --git a/app/controllers/settings_controller.rb
> b/app/controllers/settings_controller.rb
> index 937da74..0e8440d 100644
> --- a/app/controllers/settings_controller.rb
> +++ b/app/controllers/settings_controller.rb
> @@ -42,7 +42,7 @@ class SettingsController < ApplicationController
> end
>
> def snmp_mibs
> - render layout: 'ext3'
> + render layout: 'ext3_2'
> end
>
> def cfg_auth_keys(auth_type=:all)
>
> though the commit itself didn't contain that change. So while `git rebase
> some_branch_name` I started getting:
>
> First, rewinding head to replay your work on top of it...
> Applying: My cool patch.
> fatal: sha1 information is lacking or useless
> (app/controllers/settings_controller.rb).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 My cool patch.
>
> When you have resolved this problem run "git rebase --continue".
> If you would prefer to skip this patch, instead run "git rebase --skip".
> To restore the original branch and stop rebasing run "git rebase --abort".
Ha!
It would appear that `git rebase' is in fact producing patches with
`git format-patch' and then applying the resulting patches with `git
am', which gets confused by your inline diff; this can be clearly seen
in the `git-rebase--am[.sh]' file.
Perhaps `git rebase' should be reimplemented to use `git cherry-pick',
or does that suffer from the same problem?
^ permalink raw reply
* Re: [PATCH] send-mail: Add option to sleep between sending each email.
From: Matthieu Moy @ 2011-09-08 20:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramkumar Ramachandra, Georgi Chorbadzhiyski, git
In-Reply-To: <7vbouvx8j5.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> There have been discussion (and IIRC a patch) proposing this already in
>> the past. One advantage of sleeping a bit between each email is that it
>> increase the chances for the receiver to receive the emails in the right
>> order.
>
> Huh? Even in the presense of MTAs in the middle that are free to reorder
> messages?
I didn't say it _ensures_ reception in the right order. I said it
increases the chances.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCHv2 4/5] branch: introduce --list option
From: Junio C Hamano @ 2011-09-08 21:17 UTC (permalink / raw)
To: Michael J Gruber, Jeff King; +Cc: git
In-Reply-To: <7vr53qwxtz.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> Jeff King venit, vidit, dixit 07.09.2011 21:56:
>> ...
>>> 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).
>
> Hasn't 'git branch -v' meant listing in verbose mode for a long enough
> time that changing it now would mean a moderately major regression?
>
> At least my copy of v1.7.0 seems to list with "git branch -v".
Ah, nevermind.
As the series is already in 'next', here is what I came up with.
-- >8 --
From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Thu, 8 Sep 2011 14:09:50 -0700
Subject: [PATCH] branch: -v does not automatically imply --list
"branch -v" without other options or parameters still works in the list
mode, but that is not because there is "-v" but because there is no
parameter nor option.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-branch.txt | 6 +++---
builtin/branch.c | 3 +--
t/t3203-branch-output.sh | 8 ++++++--
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 2b8bc84..f46013c 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -21,7 +21,7 @@ DESCRIPTION
With no arguments, existing branches are listed and the current branch will
be highlighted with an asterisk. Option `-r` causes the remote-tracking
branches to be listed, and option `-a` shows both. This list mode is also
-activated by the `--list` and `-v` options (see below).
+activated by the `--list` option (see below).
<pattern> restricts the output to matching branches, the pattern is a shell
wildcard (i.e., matched using fnmatch(3))
Multiple patterns may be given; if any of them matches, the tag is shown.
@@ -120,10 +120,10 @@ OPTIONS
-v::
--verbose::
- Show sha1 and commit subject line for each head, along with
+ When in list mode,
+ show sha1 and commit subject line for each head, along with
relationship to upstream branch (if any). If given twice, print
the name of the upstream branch, as well.
- `--list` is implied by all verbosity options.
--abbrev=<length>::
Alter the sha1's minimum display length in the output listing.
diff --git a/builtin/branch.c b/builtin/branch.c
index 98a420f..099c75c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -712,8 +712,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
0);
- if (!delete && !rename && !force_create &&
- (argc == 0 || (verbose && argc)))
+ if (!delete && !rename && !force_create && argc == 0)
list = 1;
if (!!delete + !!rename + !!force_create + !!list > 1)
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index f2b294b..76fe7e0 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -84,12 +84,16 @@ cat >expect <<'EOF'
two
one
EOF
-test_expect_success 'git branch -v pattern shows branch summaries' '
- git branch -v branch* >tmp &&
+test_expect_success 'git branch --list -v pattern shows branch summaries' '
+ git branch --list -v branch* >tmp &&
awk "{print \$NF}" <tmp >actual &&
test_cmp expect actual
'
+test_expect_success 'git branch -v pattern does not show branch summaries' '
+ test_must_fail git branch -v branch*
+'
+
cat >expect <<'EOF'
* (no branch)
branch-one
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
* Re: git rebase fails with: Patch does not have a valid e-mail address.
From: Junio C Hamano @ 2011-09-08 17:15 UTC (permalink / raw)
To: James Blackburn; +Cc: git
In-Reply-To: <CACyv8dckmRxgb9_FDTW+=1Y9bS27b3XZQCHnhjpfHiJig5p6wg@mail.gmail.com>
James Blackburn <jamesblackburn@gmail.com> writes:
> I'm trying to rewrite some history and git's telling me:
>
> -bash:jamesb:lc-cam-025:33079> git rebase
> 7f58969b933745d4cb9bb128bbd3fa8d441cdb92
> First, rewinding head to replay your work on top of it...
> Patch does not have a valid e-mail address.
>
> Now it's true there isn't an email address for the author - the author
> no longer works for the company, and the email address was removed
> during the conversion. Therefore the repo contains "Author <>".
>
> Given git doesn't ordinarily complain about this, should this prevent
> rebase from working?
Perhaps you used "filter-branch" for conversion and your "doesn't
ordinarily complain about this" refers to it? If so, I have to say that it
is filter-branch that needs to be fixed to error out.
^ permalink raw reply
* Re: The imporantance of including http credential caching in 1.7.7
From: Jeff King @ 2011-09-08 19:10 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Kyle Neath, git
In-Reply-To: <4E6769E3.4070003@drmicha.warpmail.net>
On Wed, Sep 07, 2011 at 02:56:03PM +0200, Michael J Gruber 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 don't know whether
> jk/http-auth-keyring has been done on github payroll or during spare
> time.
To be absolutely clear here, this feature was 100% paid for by GitHub
(which isn't to say that I don't think it's a good idea. On the
contrary, I think it's awesome; but GitHub money is what provided the
time for me to work on it).
When I started at GitHub in January, I was given a giant list of things
that GitHub felt would make core git better, but that they hadn't the
personnel to improve. And I was told to use my own judgement in adding
or removing items from the list based on what I thought git needed, and
to prioritize as I saw fit. The fact that it took six months for me to
come up with credential patches is because that's how long it took me to
figure out what I wanted to write, and to clear my backlog of other git
tasks.
So I think the wheels have been turning on this for quite a while from
GitHub's perspective.
At the same time, I agree very much with Junio; releasing something with
a bad API and then having to fix it later is much worse than delaying
the release of a feature by a little bit. And we have very little data
on whether the API is "right" at this point. Initially I was concerned
that there wasn't going to be enough interest while the patches were in
'next', and that we would have to make a release in order to get people
interested enough in writing helpers. But right after I said that, Lukas
Sandström showed up with a kdewallet helper. And Ted Zlatanov is working
on something for the freedesktop secrets API.
And already there's been some discussion that perhaps the current
interface isn't quite what we want and is going to need tweaking.
So we are moving forward, and I still hope that we can target the next
release of "master" in 8-10 weeks. But this time with more confidence
that what's being released is actually right.
In the meantime, the best thing we can do to push it forward is to write
helpers. I implemented some basic ones that should work anywhere, but
aren't as nice as integration with existing keychains. Some people are
working on Linux ones. The single best thing GitHub can do to push this
forward right now is to provide a well-written OS X Keychain helper, and
to provide feedback on whether git's end of the API is good enough.
-Peff
^ permalink raw reply
* Re: [PATCH] send-mail: Add option to sleep between sending each email.
From: Junio C Hamano @ 2011-09-08 17:12 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Ramkumar Ramachandra, Georgi Chorbadzhiyski, git
In-Reply-To: <vpq7h5jtngj.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> There have been discussion (and IIRC a patch) proposing this already in
> the past. One advantage of sleeping a bit between each email is that it
> increase the chances for the receiver to receive the emails in the right
> order.
Huh? Even in the presense of MTAs in the middle that are free to reorder
messages?
IIRC, "git send-email" does its best to force ordering by assigning
monotonically increasing timestamps on the Date: field, so that the
recipients can sort the messages based on it, in addition to the
In-Reply-To field to help threading. I personally do not think there is
anything more than that that should done in the program.
^ permalink raw reply
* Re: [PATCH 1/3] remove prefix argument from pathspec_prefix
From: Junio C Hamano @ 2011-09-08 16:51 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git
In-Reply-To: <20110908071237.GA4408@ecki>
Clemens Buchacher <drizzd@aon.at> writes:
>> > We also didn't free() in the earlier code (because we do not know if it
>> > can be freed) and leaking xmemdupz() if the function didn't return the
>> > "prefix", but now you plugged the small leak. Isn't it something you
>> > should advertise?
>>
>> Nah, the leak is not necessarily plugged in all callers anyway, so scratch
>> that part. I've rewritten it like this:
>
> Ok.
>
> The only other caller, though, is cmd_ls_files(). And it would be
> trivial to plug that leak as well.
>
> But is it considered a leak, if the program is going to terminate
> right after the function returns?
it not a big deal to leak immediately before exit, and a patch whose sole
purpose is to plug them is of little value.
But if you are already in the vicinity, updating a function that happens
to have such a leak, the cost to decide not plugging the leak would be
about the same as plugging it, so it would be worth doing in such a case.
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Junio C Hamano @ 2011-09-08 20:48 UTC (permalink / raw)
To: Jeff King; +Cc: git, Shawn O. Pearce
In-Reply-To: <20110908193555.GC16064@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think (1) is actually not all that interesting. The server already has
> credentials for each user via ssh or http. So it knows who each pusher
> is already. It can't relay that information cryptographically to a
> client who fetches later, of course, but we are just talking about
> whether or not to accept the push at this moment.
>
> But if you really did want to do that, it seems like a pre-receive hook
> would be sufficient.
I see two flaws in that reasoning. The server's authentication may be
found not trustworthy for some reason long after commits hit the tree, and
GPG signature made by the _pusher_ would assert the integrity. Also this
will open the door to accept push over an unauthenticated connection and
allowing only signed pushes.
> For (2), you don't want to trust the server, so the user's
> authentication to the server isn't enough. You want a cryptographic
> chain leading back to the original pusher. But the server doesn't
> actually need to see or understand that cryptographic chain for this
> purpose.
Exactly. That is why the signed push certificate is stored without the
server doing anything funky, only to annotate the pushed commits in the
notes tree---the fetchers can peek the notes and verify the GPG signature.
But not _forcing_ that the push certificate be placed in a notes tree on
the client side allows different server hosting sites to additionally do
different things using that data.
> The only issue I foresee is one of atomicity.
The very initial thinking was to create a notes tree commit on the client
side and push that along with what is pushed, but that approach has an
inherent flaw of causing unnecessary collisions between two people who are
pushing to unrelated branches, and that is why I decided to let the server
side handle it.
^ permalink raw reply
* Re: Git Bug - diff in commit message.
From: Junio C Hamano @ 2011-09-08 17:27 UTC (permalink / raw)
To: anikey; +Cc: git
In-Reply-To: <1315493353942-6772145.post@n2.nabble.com>
anikey <arty.anikey@gmail.com> writes:
> Hi, peops. I'm pretty much sure that's a bug.
I am not Peops, but will try to help ;-).
As "rebase" is essentially "format-patch" output piped into "am",
unindented diff in a random place inside the commit message will be
indistingushable from the beginning of the patch. You cannot have an
unadulterated "diff" output in your commit message for this reason, and
this is not likely to change.
Often what people do is to indent them, as the reason they quote patch in
the message is to make it serve as a supporting material. Incidentally,
that makes the resulting commit log message easier to read as well; e.g.
Fix bloop bug
An earlier commit had this patch that is quite bogus.
diff --git a/bloop.php b/bloop.php
index 937da74..0e8440d 100644
--- a/bloop.php
+++ b/bloop.php
@@ -42,7 +42,7 @@
...
The assignment to frotz in the above hunk should happen after
nitfol variable is initialized.
This patch fixes it.
Signed-off-by: Cont Ributer <contributor@example.com>
^ permalink raw reply
* Re: [PATCH 5/2] push -s: receiving end
From: Junio C Hamano @ 2011-09-08 16:43 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Shawn O. Pearce
In-Reply-To: <201109081131.58362.johan@herland.net>
Johan Herland <johan@herland.net> writes:
>> +static void get_note_text(struct strbuf *buf, struct notes_tree *t,
>> + const unsigned char *object)
>> +{
>> + const unsigned char *sha1 = get_note(t, object);
>> + char *text;
>> + unsigned long len;
>> + enum object_type type;
>> +
>> + if (!sha1)
>> + return;
>> + text = read_sha1_file(sha1, &type, &len);
>> + if (text && len && type == OBJ_BLOB)
>> + strbuf_add(buf, text, len);
>> + free(text);
>> +}
>> +
>
> What about adding this function to notes.h as a convenience to other
> users of the notes API?
I actually was hoping to hear that I do not have to do this "check
existing and concatenate", and should let the add_note() function
run its default combine_notes method to do the concatenation.
I found a few things I wasn't quite sure in the notes/notes-merge API, by
the way.
- The combine_notes callback is run when a note is inserted into the
in-core notes tree. I felt that this is way too early if you want to
avoid racing with another process (and the patch tries to wrap
create-notes-commit with lock-ref/write-ref-sha1 pair), but perhaps
this is to deal with a case where the calling program calls add_notes()
on the same object multiple times.
- create_notes_commit() dies under a few conditions, but some callers
that are recording advisory/optional notes might want to get an error
and continue.
I think ideally this patch should handle notes like the following, which
is not quite how I coded it:
- initialize in-core notes tree;
- add bunch of notes, without regard to the existing ones, to in-core
notes tree by calling add_notes();
- lock the notes ref and read the "parent"; we may want to add "wait and
retry for a few times until we get the lock" support at lockfile API
level, but doing it at the application level would be fine.
- call create-notes-commit, which in turn merges the in-core
notes with what collides with those already in "parent" by
calling the combine-notes callback, merges and re-balances
the notes tree, and makes a notes commit object;
- update the notes ref with that notes commit, releasing the lock on
the ref.
^ permalink raw reply
* Re: [PATCHv2 4/5] branch: introduce --list option
From: Junio C Hamano @ 2011-09-08 21:03 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jeff King, git
In-Reply-To: <4E6889DF.7030404@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Jeff King venit, vidit, dixit 07.09.2011 21:56:
> ...
>> 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).
Hasn't 'git branch -v' meant listing in verbose mode for a long enough
time that changing it now would mean a moderately major regression?
At least my copy of v1.7.0 seems to list with "git branch -v".
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Junio C Hamano @ 2011-09-08 16:24 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <CAJo=hJsLx1Q9ZDoxGn=dww5J-rO9GitH47rEme_1L8Lg0RmAqw@mail.gmail.com>
Shawn Pearce <spearce@spearce.org> writes:
> This sounds too late to me. I think you just caused 2 HTTP POSTs, one
> a partial one with the commands and no pack data, and another with the
> push certificate and the pack. Neither is useful.
Well, then I'd stop looking at this area and let others make it useful
while I hack in other areas.
^ permalink raw reply
* Re: [PATCHv2 1/2] remote: write correct fetch spec when renaming remote 'remote'
From: Junio C Hamano @ 2011-09-08 16:46 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Jeff King
In-Reply-To: <alpine.DEB.2.00.1109080458500.12564@debian>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> On Wed, 7 Sep 2011, Junio C Hamano wrote:
>
>> Later in the same mv() function, the other one renames refs/remotes/$OLD/
>> to refs/remotes/$NEW/, even when you did not find any fetch refspec that
>> stores under "refs/remotes/$OLD/<anything>" in the earlier logic.
>
> Ah, of course. Sorry for being slow and thanks for the explanation. So
> if there are two configured refspecs with RHSs "refs/remotes/$OLD/foo"
> and "refs/remotes/$OLD/bar/*", we should remember that we updated
> those and only update refs that match the same patterns. I will see
> what I can do and will hopefully soon get back with a "patch 4/2".
What I had in mind was nothing that elaborate--it would be sufficiently
safe to just remember if you replaced any in the config, and if you
didn't, not to move anything.
^ permalink raw reply
* Re: [PATCHv2 4/5] branch: introduce --list option
From: Jeff King @ 2011-09-08 21:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7vr53qwxtz.fsf@alter.siamese.dyndns.org>
On Thu, Sep 08, 2011 at 02:03:52PM -0700, Junio C Hamano wrote:
> Hasn't 'git branch -v' meant listing in verbose mode for a long enough
> time that changing it now would mean a moderately major regression?
>
> At least my copy of v1.7.0 seems to list with "git branch -v".
No, it will mean the same thing as in v1.7.0; it is not the "-v" which
does it, but the lack of non-option arguments.
Right now, "-v" just means "if we are listing, do it verbosely". And if
you don't specify any non-option arguments, it means "list". So right
now:
git branch ;# list
git branch -v ;# list verbosely
git branch -v foo ;# create branch 'foo', -v does nothing
Michael's proposal was:
git branch -v foo ;# assume verbose list, interpret 'foo' as pattern
which is actually a regression, albeit one that probably doesn't matter
(because "-v" didn't ever do anything with a non-option argument).
Whereas mine is:
git branch -v foo ;# create branch 'foo' verbosely
Which happens to do exactly the same thing as the current behavior,
because there are no verbose messages to add to "git branch". But it
leaves the door open to adding them in the future (and it's consistent
with "git branch -m -v" and "git branch -d -v", both of which could use
"-v" to do their operations more verbosely).
-Peff
^ permalink raw reply
* [PATCH v2 0/7] "push -s"
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
Slightly re-rolled from last night.
- Marked push-certificate format as version 0 while we are
still experimenting;
- The push certificate records new object name and the refname;
- Add support for an external hook "pre-receive-signature".
One issue internally debated was if we want to list the refs that matched
the pushing criteria but were found to be already up to date, and this can
be argued both ways.
- You can say that you are making assertion that you want to have a
certain object at that ref, in which case you would want to include
them.
- On the other hand, for the purpose of leaving audit-trail, if the ref
you tried to push already had the object you wanted to see at the tip
of a ref, you weren't the person who made the ref point at the object,
and it would be sensible not to include them.
Taking the latter stance is slightly easier on the end users, because
"Everything up-to-date" case becomes a no-op as the natural consequence,
and we do not have to ask them to unlock their GPG key in such a case. It
however makes it impossible to say "Earlier I pushed that object to the
tip of my branch but forgot to sign the push, and I want to make a signed
push, even though I didn't add anything to my history."
People who configured to push out more than one branches with "git push"
often work on one branch, run "git push" which ends up pushing that branch
but not other branches, then work on another branch and run "git push" to
push out that other branch, while the branch he earlier pushed out stays
the same since his last push. For such people, the first "push" is not
necessarily even an assertion that he wants to have both branches pointing
at certain commits, and from that point of view, not including the latter
branch he hasn't worked on (and stayed up-to-date) in the push certifiate
is a sensible thing to do.
As there is no single right answer, this round of re-roll keeps the latter
semantics to record only what you pushed out as the original series.
Junio C Hamano (7):
send-pack: typofix error message
Split GPG interface into its own helper library
push -s: skeleton
push -s: send signed push certificate
push -s: receiving end
refactor run_receive_hook()
push -s: support pre-receive-signature hook
Makefile | 2 +
builtin/push.c | 1 +
builtin/receive-pack.c | 206 +++++++++++++++++++++++++++++++++++++++++++-----
builtin/send-pack.c | 61 +++++++++++++-
builtin/tag.c | 60 ++------------
builtin/verify-tag.c | 35 +--------
gpg-interface.c | 94 ++++++++++++++++++++++
gpg-interface.h | 11 +++
send-pack.h | 1 +
transport.c | 4 +
transport.h | 4 +
11 files changed, 369 insertions(+), 110 deletions(-)
create mode 100644 gpg-interface.c
create mode 100644 gpg-interface.h
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply
* [PATCH v2 3/7] push -s: skeleton
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>
If a tag is GPG-signed, and if you trust the cryptographic robustness of
the SHA-1 hash and GPG, you can sleep well knowing that all the history
leading to the signed commit cannot be 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 all.
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 based on this idea would go 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 core,
that looks like the following:
Push-Certificate-Version: 0
Pusher: Junio C Hamano <gitster@pobox.com> 1315427886 -0700
Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next
Each "Update" line shows the new object name at the tip of the ref
this push tries to update.
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 keeps the signed push certificate in core, receives
the pack data and unpacks (or stores and indexes) it 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-receive-signature hook, feeds the push certificate to it
and asks it to veto the ref updates.
And here is a skeleton to implement this. The patch has 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-receive-signature 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 | 64 +++++++++++++++++++++++++++++++++++++++++++++---
send-pack.h | 1 +
transport.c | 4 +++
transport.h | 4 +++
6 files changed, 123 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..20b6799 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-receive-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..7f4778c 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,18 @@ 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) {
+ strbuf_addstr(&push_cert, "Push-Certificate-Version: 0\n");
+ strbuf_addf(&push_cert, "Pusher: %s\n", git_committer_info(0));
+ }
+
/*
* Finally, tell the other end!
*/
@@ -301,15 +336,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)
+ strbuf_addf(&push_cert, "Update: %s %s\n",
+ new_hex, ref->name);
ref->status = status_report ?
REF_STATUS_EXPECTING_REPORT :
REF_STATUS_OK;
@@ -326,6 +365,23 @@ int send_pack(struct send_pack_args *args,
safe_write(out, req_buf.buf, req_buf.len);
packet_flush(out);
}
+
+ if (signed_push && cmds_sent) {
+ 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.188.g3793ac
^ permalink raw reply related
* [PATCH v2 7/7] push -s: support pre-receive-signature hook
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/receive-pack.c | 31 +++++++++++++++++++++++++++----
1 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 939b867..b5a54e7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -303,6 +303,27 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
return status;
}
+static int feed_signature_hook(void *state_, const char **bufp, size_t *sizep)
+{
+ const char **cert_p = state_;
+
+ if (!*cert_p)
+ return -1; /* EOF */
+ *bufp = *cert_p;
+ *cert_p = NULL; /* just return once */
+ *sizep = strlen(*bufp);
+ return 0;
+}
+
+static int run_receive_signature_hook(const char *cert)
+{
+ static const char hook[] = "hooks/pre-receive-signature";
+
+ if (!cert)
+ return 0;
+ return run_and_feed_hook(hook, feed_signature_hook, &cert);
+}
+
static int run_update_hook(struct command *cmd)
{
static const char update_hook[] = "hooks/update";
@@ -642,10 +663,6 @@ static int record_signed_push(char *cert)
* 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-receive-signature" hook.
*/
size_t total, payload;
char *cp, *ep;
@@ -714,6 +731,12 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
return;
}
+ if (run_receive_signature_hook(push_certificate)) {
+ for (cmd = commands; cmd; cmd = cmd->next)
+ cmd->error_string = "n/a (pre-receive-signature hook declined)";
+ return;
+ }
+
if (record_signed_push(push_certificate)) {
for (cmd = commands; cmd; cmd = cmd->next)
cmd->error_string = "n/a (push signature error)";
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
* [PATCH v2 4/7] push -s: send signed push certificate
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>
And this uses the GPG interface to sign the push certificate. The format
of the signed certificate is very similar to a signed tag, in that the
result is a concatenation of the payload, immediately followed by a
detached signature.
This places the same constraint as an annotated tag on the push
certificate payload; it has to be a text file and the final line
must not be an incomplete line.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/send-pack.c | 29 ++++++++++++-----------------
1 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7f4778c..f715324 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -8,6 +8,7 @@
#include "send-pack.h"
#include "quote.h"
#include "transport.h"
+#include "gpg-interface.h"
static const char send_pack_usage[] =
"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
@@ -237,25 +238,18 @@ static int sideband_demux(int in, int out, void *data)
return ret;
}
-static void sign_push_certificate(struct strbuf *cert)
+/*
+ * Take the contents of cert->buf, and have the user GPG sign it, and
+ * read it back in the strbuf.
+ */
+static int 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.
+ * You may want to append some extra info to cert before
+ * giving it to GPG, possibly via a hook, here.
*/
- char *cp;
- for (cp = cert->buf; *cp; cp++) {
- int ch = *cp;
- if ('a' <= ch && ch <= 'z')
- *cp = toupper(ch);
- }
- return;
+
+ return sign_buffer(cert, git_committer_info(IDENT_NO_DATE));
}
int send_pack(struct send_pack_args *args,
@@ -369,7 +363,8 @@ int send_pack(struct send_pack_args *args,
if (signed_push && cmds_sent) {
char *cp, *ep;
- sign_push_certificate(&push_cert);
+ if (sign_push_certificate(&push_cert))
+ return error(_("failed to sign push certificate"));
strbuf_reset(&req_buf);
for (cp = push_cert.buf; *cp; cp = ep) {
ep = strchrnul(cp, '\n');
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
* [PATCH v2 2/7] Split GPG interface into its own helper library
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>
This mostly moves existing code from builtin/tag.c (for signing)
and builtin/verify-tag.c (for verifying) to a new gpg-interface.c
file to provide a more generic library interface.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 2 +
builtin/tag.c | 60 ++++----------------------------
builtin/verify-tag.c | 35 ++-----------------
gpg-interface.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
gpg-interface.h | 11 ++++++
5 files changed, 117 insertions(+), 85 deletions(-)
create mode 100644 gpg-interface.c
create mode 100644 gpg-interface.h
diff --git a/Makefile b/Makefile
index 8d6d451..2183223 100644
--- a/Makefile
+++ b/Makefile
@@ -530,6 +530,7 @@ LIB_H += exec_cmd.h
LIB_H += fsck.h
LIB_H += gettext.h
LIB_H += git-compat-util.h
+LIB_H += gpg-interface.h
LIB_H += graph.h
LIB_H += grep.h
LIB_H += hash.h
@@ -620,6 +621,7 @@ LIB_OBJS += entry.o
LIB_OBJS += environment.o
LIB_OBJS += exec_cmd.o
LIB_OBJS += fsck.o
+LIB_OBJS += gpg-interface.o
LIB_OBJS += graph.o
LIB_OBJS += grep.o
LIB_OBJS += hash.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..e9d36fa 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -14,6 +14,7 @@
#include "parse-options.h"
#include "diff.h"
#include "revision.h"
+#include "gpg-interface.h"
static const char * const git_tag_usage[] = {
"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
@@ -208,60 +209,13 @@ static int verify_tag(const char *name, const char *ref,
static int do_sign(struct strbuf *buffer)
{
- struct child_process gpg;
- const char *args[4];
- char *bracket;
- int len;
- int i, j;
+ const char *key;
- if (!*signingkey) {
- if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
- sizeof(signingkey)) > sizeof(signingkey) - 1)
- return error(_("committer info too long."));
- bracket = strchr(signingkey, '>');
- if (bracket)
- bracket[1] = '\0';
- }
-
- /* When the username signingkey is bad, program could be terminated
- * because gpg exits without reading and then write gets SIGPIPE. */
- signal(SIGPIPE, SIG_IGN);
-
- memset(&gpg, 0, sizeof(gpg));
- gpg.argv = args;
- gpg.in = -1;
- gpg.out = -1;
- args[0] = "gpg";
- args[1] = "-bsau";
- args[2] = signingkey;
- args[3] = NULL;
-
- if (start_command(&gpg))
- return error(_("could not run gpg."));
-
- if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
- close(gpg.in);
- close(gpg.out);
- finish_command(&gpg);
- return error(_("gpg did not accept the tag data"));
- }
- close(gpg.in);
- len = strbuf_read(buffer, gpg.out, 1024);
- close(gpg.out);
-
- if (finish_command(&gpg) || !len || len < 0)
- return error(_("gpg failed to sign the tag"));
-
- /* Strip CR from the line endings, in case we are on Windows. */
- for (i = j = 0; i < buffer->len; i++)
- if (buffer->buf[i] != '\r') {
- if (i != j)
- buffer->buf[j] = buffer->buf[i];
- j++;
- }
- strbuf_setlen(buffer, j);
-
- return 0;
+ if (*signingkey)
+ key = signingkey;
+ else
+ key = git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+ return sign_buffer(buffer, key);
}
static const char tag_template[] =
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 3134766..8b4f742 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -11,6 +11,7 @@
#include "run-command.h"
#include <signal.h>
#include "parse-options.h"
+#include "gpg-interface.h"
static const char * const verify_tag_usage[] = {
"git verify-tag [-v|--verbose] <tag>...",
@@ -19,42 +20,12 @@ static const char * const verify_tag_usage[] = {
static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
{
- struct child_process gpg;
- const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
- char path[PATH_MAX];
- size_t len;
- int fd, ret;
+ int len;
- fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
- if (fd < 0)
- return error("could not create temporary file '%s': %s",
- path, strerror(errno));
- if (write_in_full(fd, buf, size) < 0)
- return error("failed writing temporary file '%s': %s",
- path, strerror(errno));
- close(fd);
-
- /* find the length without signature */
len = parse_signature(buf, size);
if (verbose)
write_in_full(1, buf, len);
-
- memset(&gpg, 0, sizeof(gpg));
- gpg.argv = args_gpg;
- gpg.in = -1;
- args_gpg[2] = path;
- if (start_command(&gpg)) {
- unlink(path);
- return error("could not run gpg.");
- }
-
- write_in_full(gpg.in, buf, len);
- close(gpg.in);
- ret = finish_command(&gpg);
-
- unlink_or_warn(path);
-
- return ret;
+ return verify_signed_buffer(buf, size, len);
}
static int verify_tag(const char *name, int verbose)
diff --git a/gpg-interface.c b/gpg-interface.c
new file mode 100644
index 0000000..b83cca1
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "gpg-interface.h"
+#include "sigchain.h"
+
+int sign_buffer(struct strbuf *buffer, const char *signing_key)
+{
+ struct child_process gpg;
+ const char *args[4];
+ ssize_t len;
+ int i, j;
+
+ memset(&gpg, 0, sizeof(gpg));
+ gpg.argv = args;
+ gpg.in = -1;
+ gpg.out = -1;
+ args[0] = "gpg";
+ args[1] = "-bsau";
+ args[2] = signing_key;
+ args[3] = NULL;
+
+ if (start_command(&gpg))
+ return error(_("could not run gpg."));
+
+ /*
+ * When the username signingkey is bad, program could be terminated
+ * because gpg exits without reading and then write gets SIGPIPE.
+ */
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
+ close(gpg.in);
+ close(gpg.out);
+ finish_command(&gpg);
+ return error(_("gpg did not accept the data"));
+ }
+ close(gpg.in);
+ len = strbuf_read(buffer, gpg.out, 1024);
+ close(gpg.out);
+
+ sigchain_pop(SIGPIPE);
+
+ if (finish_command(&gpg) || !len || len < 0)
+ return error(_("gpg failed to sign the data"));
+
+ /* Strip CR from the line endings, in case we are on Windows. */
+ for (i = j = 0; i < buffer->len; i++)
+ if (buffer->buf[i] != '\r') {
+ if (i != j)
+ buffer->buf[j] = buffer->buf[i];
+ j++;
+ }
+ strbuf_setlen(buffer, j);
+
+ return 0;
+}
+
+int verify_signed_buffer(const char *buf, size_t total, size_t payload)
+{
+ struct child_process gpg;
+ const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+ char path[PATH_MAX];
+ int fd, ret;
+
+ fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+ if (fd < 0)
+ return error("could not create temporary file '%s': %s",
+ path, strerror(errno));
+ if (write_in_full(fd, buf, total) < 0)
+ return error("failed writing temporary file '%s': %s",
+ path, strerror(errno));
+ close(fd);
+
+ memset(&gpg, 0, sizeof(gpg));
+ gpg.argv = args_gpg;
+ gpg.in = -1;
+ args_gpg[2] = path;
+ if (start_command(&gpg)) {
+ unlink(path);
+ return error("could not run gpg.");
+ }
+
+ write_in_full(gpg.in, buf, payload);
+ close(gpg.in);
+ ret = finish_command(&gpg);
+
+ unlink_or_warn(path);
+
+ return ret;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
new file mode 100644
index 0000000..7689357
--- /dev/null
+++ b/gpg-interface.h
@@ -0,0 +1,11 @@
+#ifndef GPG_INTERFACE_H
+#define GPG_INTERFACE_H
+
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+
+extern int sign_buffer(struct strbuf *buffer, const char *signing_key);
+extern int verify_signed_buffer(const char *buffer, size_t total, size_t payload);
+
+#endif
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
* [PATCH v2 6/7] refactor run_receive_hook()
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>
Running a hook has to make complex set-up to establish web of
communication between child process and multiplexer, which is common
regardless of what kind of data is fed to the hook. Refactor the parts
that is specific to the data fed to the particular set of hooks from the
part that runs the hook, so that the code can be reused to drive hooks
that take different kind of data.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/receive-pack.c | 71 +++++++++++++++++++++++++++++++++++-------------
1 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 344660e..939b867 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -212,21 +212,15 @@ static int copy_to_sideband(int in, int out, void *arg)
return 0;
}
-static int run_receive_hook(struct command *commands, const char *hook_name)
+typedef int (*feed_fn)(void *, const char **, size_t *);
+static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_state)
{
- static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
- struct command *cmd;
struct child_process proc;
struct async muxer;
const char *argv[2];
- int have_input = 0, code;
-
- for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
- if (!cmd->error_string)
- have_input = 1;
- }
+ int code;
- if (!have_input || access(hook_name, X_OK) < 0)
+ if (access(hook_name, X_OK) < 0)
return 0;
argv[0] = hook_name;
@@ -254,15 +248,13 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
return code;
}
- for (cmd = commands; cmd; cmd = cmd->next) {
- if (!cmd->error_string) {
- size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
- sha1_to_hex(cmd->old_sha1),
- sha1_to_hex(cmd->new_sha1),
- cmd->ref_name);
- if (write_in_full(proc.in, buf, n) != n)
- break;
- }
+ while (1) {
+ const char *buf;
+ size_t n;
+ if (feed(feed_state, &buf, &n))
+ break;
+ if (write_in_full(proc.in, buf, n) != n)
+ break;
}
close(proc.in);
if (use_sideband)
@@ -270,6 +262,47 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
return finish_command(&proc);
}
+struct receive_hook_feed_state {
+ struct command *cmd;
+ struct strbuf buf;
+};
+
+static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
+{
+ struct receive_hook_feed_state *state = state_;
+ struct command *cmd = state->cmd;
+
+ while (cmd && cmd->error_string)
+ cmd = cmd->next;
+ if (!cmd)
+ return -1; /* EOF */
+ strbuf_reset(&state->buf);
+ strbuf_addf(&state->buf, "%s %s %s\n",
+ sha1_to_hex(cmd->old_sha1), sha1_to_hex(cmd->new_sha1),
+ cmd->ref_name);
+ state->cmd = cmd->next;
+ if (bufp) {
+ *bufp = state->buf.buf;
+ *sizep = state->buf.len;
+ }
+ return 0;
+}
+
+static int run_receive_hook(struct command *commands, const char *hook_name)
+{
+ struct receive_hook_feed_state state;
+ int status;
+
+ strbuf_init(&state.buf, 0);
+ state.cmd = commands;
+ if (feed_receive_hook(&state, NULL, NULL))
+ return 0;
+ state.cmd = commands;
+ status = run_and_feed_hook(hook_name, feed_receive_hook, &state);
+ strbuf_release(&state.buf);
+ return status;
+}
+
static int run_update_hook(struct command *cmd)
{
static const char update_hook[] = "hooks/update";
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
* [PATCH v2 5/7] push -s: receiving end
From: Junio C Hamano @ 2011-09-08 20:01 UTC (permalink / raw)
To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>
This stores the GPG signed push certificate in the receiving repository
using the notes mechanism. The certificate is appended to a note in the
refs/notes/signed-push tree for each object that appears on the right
hand side of the push certificate, i.e. the object that was pushed to
update the tip of a ref.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/receive-pack.c | 72 +++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 65 insertions(+), 7 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 20b6799..344660e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -11,6 +11,11 @@
#include "transport.h"
#include "string-list.h"
#include "sha1-array.h"
+#include "gpg-interface.h"
+#include "notes.h"
+#include "notes-merge.h"
+#include "blob.h"
+#include "tag.h"
static const char receive_pack_usage[] = "git receive-pack <git-dir>";
@@ -581,6 +586,22 @@ static void check_aliased_updates(struct command *commands)
string_list_clear(&ref_list, 0);
}
+static void get_note_text(struct strbuf *buf, struct notes_tree *t,
+ const unsigned char *object)
+{
+ const unsigned char *sha1 = get_note(t, object);
+ char *text;
+ unsigned long len;
+ enum object_type type;
+
+ if (!sha1)
+ return;
+ text = read_sha1_file(sha1, &type, &len);
+ if (text && len && type == OBJ_BLOB)
+ strbuf_add(buf, text, len);
+ free(text);
+}
+
static int record_signed_push(char *cert)
{
/*
@@ -592,18 +613,55 @@ static int record_signed_push(char *cert)
* 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-receive-signature" hook.
- *
- * Here we just throw it to stderr to demonstrate that the
- * codepath is being exercised.
*/
+ size_t total, payload;
char *cp, *ep;
- for (cp = cert; *cp; cp = ep) {
+ int ret = 0;
+ struct notes_tree *t;
+ struct strbuf nbuf = STRBUF_INIT;
+
+ if (!cert)
+ return 0;
+
+ init_notes(NULL, "refs/notes/signed-push", NULL, 0);
+ t = &default_notes_tree;
+
+ total = strlen(cert);
+ payload = parse_signature(cert, total);
+ for (cp = cert; cp < cert + payload; cp = ep) {
+ unsigned char sha1[20], nsha1[20];
+
ep = strchrnul(cp, '\n');
if (*ep == '\n')
ep++;
- fprintf(stderr, "RSP: %.*s", (int)(ep - cp), cp);
+ if (prefixcmp(cp, "Update: "))
+ continue;
+ cp += strlen("Update: ");
+ if (get_sha1_hex(cp, sha1) || cp[40] != ' ')
+ continue;
+
+ get_note_text(&nbuf, t, sha1);
+ if (nbuf.len)
+ strbuf_addch(&nbuf, '\n');
+ strbuf_add(&nbuf, cert, total);
+ if (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||
+ add_note(t, sha1, nsha1, NULL))
+ ret = error(_("unable to write note object"));
+ strbuf_reset(&nbuf);
}
- return 0;
+
+ if (!ret) {
+ unsigned char commit[20];
+ unsigned char parent[20];
+ struct ref_lock *lock;
+
+ resolve_ref(t->ref, parent, 0, NULL);
+ lock = lock_any_ref_for_update(t->ref, parent, 0);
+ create_notes_commit(t, NULL, "push", commit);
+ ret = write_ref_sha1(lock, commit, "push");
+ }
+ free_notes(t);
+ return ret;
}
static void execute_commands(struct command *commands, const char *unpacker_error)
@@ -623,7 +681,7 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
return;
}
- if (push_certificate && record_signed_push(push_certificate)) {
+ if (record_signed_push(push_certificate)) {
for (cmd = commands; cmd; cmd = cmd->next)
cmd->error_string = "n/a (push signature error)";
return;
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
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