Git development
 help / color / mirror / Atom feed
* Re: merge result
From: Lynn Lin @ 2011-09-10  1:07 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: git
In-Reply-To: <CAH5451=BpiN9P8gjL5TYQO+NSH913oXo1-bmscMXyy8QhFg8KQ@mail.gmail.com>

On Fri, Sep 9, 2011 at 8:41 PM, Andrew Ardill <andrew.ardill@gmail.com> wrote:
> Hi Lynn,
>
> If you merge two branches together you are merging the state of the
> head of those trees, not re-applying commits on top of each other. The
> changes introduced in commit 4 will therefore not be applied twice,
> but will more-or-less be ignored.
>
> Perhaps you are not trying a merge operation, but something else? Is
> this issue hypothetical, or is it something you have experienced??


1$ git checkout master
2$ git edit  and commit
3$ git checkout branchA
4$ git edit and commit
5$ git checkout master
6 $ git merge branchA

2 and 4 change is the same

> Regards,
>
> Andrew Ardill
>
>
>
> On 9 September 2011 20:54, Lynn Lin <lynn.xin.lin@gmail.com> wrote:
>> Hi All,
>>   When I merge branch A back to master branch,if there are same
>> commit(developer do double commit) both in master and A branch, there
>> will be two same commit in master branch.For example
>>
>>
>>   1->2->3-4>5        Master
>>       |
>>       4->6->7          A
>>
>> When I merge A branch into master,the two same 4 commit will present
>> in master branch.
>>
>> Is there any wrong with my operation?
>>
>> Thanks for your help
>> Lynn
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* Re: [PATCH 6/6] Add a REFNAME_ALLOW_UNNORMALIZED flag to check_ref_format()
From: Junio C Hamano @ 2011-09-09 23:30 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, cmn
In-Reply-To: <1315568778-3592-7-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> Let the callers of check_ref_format() (and normalize_refname()) decide
> whether to accept unnormalized refnames via a new
> REFNAME_ALLOW_UNNORMALIZED flag.  Change callers to set this flag,
> which preserves their current behavior.  (There are likely places
> where this flag can be removed.)

Hmm, is it just me who finds --no-allow-unnormalized backwards?

More importantly, shouldn't every caller be required to normalize refnames
by default, unless it can justify why it does not have to with a compelling
reason?

In other words, I would be Ok if "--no-require-normalized" was the default
for "git check-ref-format" for scripts' use, and I also would be perfectly
fine if callers feed un-normalized strings that came from the command line
argument and other end user input to normalize_refname(), but once such a
string is normalized, shouldn't the rest of the callchain be passing the
normalized refname all the way?

To put it another way, my knee jerk reaction is that we shouldn't need
such a "flag". Shouldn't it be sufficient for normalize_refname() and
nothing else to allow unnormalized input, and everybody else should barf
when they see an un-normalized input?

^ permalink raw reply

* Re: [ANNOUNCE] GitTogether 2011 - Oct 24th/25th
From: Shawn Pearce @ 2011-09-09 23:10 UTC (permalink / raw)
  To: git
In-Reply-To: <CAJo=hJu48DiVUDexuWJpVgq__zVTfO1Xz=AgfOz6wws00b2EaQ@mail.gmail.com>

On Mon, Sep 5, 2011 at 12:56, 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

Lets try this one more time. Google has found us a much bigger space.
We can now host a lot more people. :-)

Since git.wiki.kernel.org is having issues, I've moved the sign-up list:

  https://docs.google.com/a/google.com/spreadsheet/embeddedform?formkey=dGVTN3M1ZnpTalVFSkVhUUJyRE5mQWc6MQ&ndplr=1

Please re-register, and be sure to join the mailing list.

-- 
Shawn.

^ permalink raw reply

* Branching strategies
From: robert mena @ 2011-09-09 23:01 UTC (permalink / raw)
  To: git

Hi,

I have a project where I have to do a continuous integration, adding
features/making changes on a daily basis.  Some changes are one liners
with no functionality just, for example, textual changes or a new
button.   Some are actual features or bug fixes.

So today my developers do their business and publish the changes in a
beta site where the customer or the qa takes a look.  The problem is
a standard one.  Sometimes features stay already developed (waiting
for review) for a long time and other changes/features get approved
first.

Since some of those can touch the same files how can I make this a
little bit better (manageable)?

I am considering doing feature branches.   The customer requests to
add feature A I open a bug tracking issue and create a branch 1276
corresponding to the bug id.

In my simply view I'd have a master/live branch and every time I need
to create a new branch I do it from here.  When the developer is happy
he merges his branch with a beta branch where the Q&A/customer review
is done.

When this review gets an OK he merges his feature branch with the live
one, redo the tests and publish.

I'd really appreciate feedback for this specially for the weak points
and known problems of my approach with alternatives :)

Regards.

^ permalink raw reply

* Re: [PATCH 1/2] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
From: Junio C Hamano @ 2011-09-09 22:54 UTC (permalink / raw)
  To: Alexey Shumkin; +Cc: git
In-Reply-To: <1315558437-6295-2-git-send-email-zapped@mail.ru>

Alexey Shumkin <zapped@mail.ru> writes:

> The following two ought to give the same output to a terminal:
>
> 	$ git log --oneline --no-color
> 	$ git log --pretty=format:'%h %s'
>
> However, the former pays attention to i18n.logOutputEncoding
> configuration, while the latter does not when it format "%s". A log
> messages written in an encoding i18n.commitEncoding that differs
> from terminal encoding are shown corrupted with the latter even
> when i18n.logOutputEncoding and terminal encoding are the same.

You seem to have inherited bad grammar from some example; I wonder
where... ;-)

> @@ -19,8 +21,12 @@ add_file () {
>  		echo "$name" > "$name" &&
>  		git add "$name" &&
>  		test_tick &&
> -		git commit -m "Add $name"
> +		git config i18n.commitEncoding cp1251 &&
> +		echo "Add $name ($added $name)" \
> +			| iconv -f utf-8 -t cp1251 \
> +			| xargs -I{} git commit -m "{}"

Don't do this. Format it this way instead if you want to use multi-line:

	echo "..." |
        iconv ... |
        xargs ...

I also am not sure if -I{} in xargs is all that portable. Why isn't it
more like this, which would be far easier to read?

	msg_in_cp1251=$(echo "Add ..." | iconv ...) &&
        git commit -m "$msg_in_cp1251" &&
        ...

Also do not break the && chain.

>  	done >/dev/null
> +	git config --unset i18n.commitEncoding
>  	git rev-parse --verify HEAD | cut -c1-7
>  	cd "$owd"
>  }

^ permalink raw reply

* Re: [PATCH v2] git-svn: teach git-svn to populate svn:mergeinfo
From: Eric Wong @ 2011-09-09 22:21 UTC (permalink / raw)
  To: Bryan Jacobs; +Cc: git, Sam Vilain
In-Reply-To: <20110907133605.07ecad1b@robyn.woti.com>

Bryan Jacobs <bjacobs@woti.com> wrote:
> This is the second revision of a patch I posted earlier. I believe
> this patch is now suitable for inclusion.

Some comments inline, but I can clean them up myself and push out in a
bit.

> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -213,6 +213,14 @@ discouraged.
>  	store this information (as a property), and svn clients starting from
>  	version 1.5 can make use of it. 'git svn' currently does not use it
>  	and does not set it automatically.

I noticed this conflicts when applying due to the missing --mergeinfo=
documentation.  Did you intend to remove --mergeinfo entirely and
replace it with this?  I think some folks already depend on it (it's
been around since last year).

> +	if (not defined($push_merge_info)
> +			or $push_merge_info eq "false"
> +			or $push_merge_info eq "no"
> +			or $push_merge_info eq "never") {

I missed this the first time, but "||" and "!" are easier for
C programmers to understand and higher in precedence (ref: perlop manpage)

-- 
Eric Wong

^ permalink raw reply

* [PATCH v3.1 4/4] push -s: signed push
From: Junio C Hamano @ 2011-09-09 21:16 UTC (permalink / raw)
  To: git
In-Reply-To: <1315600904-17032-5-git-send-email-gitster@pobox.com>

The earlier one had some unnecessary bits specific to git transport copied
from the v2 patch, but this alternative design is transport agnostic, so
here is a replacemnt to remove them.

-- >8 --
Subject: [PATCH] push -s: signed push

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.

 3. The signed push certificate is added as notes in the "signed-push"
    notes tree to the objects listed in the certificate. The push refspec
    is altered to push this notes tree to the other side.

Compared to the alternative design posted earlier on the list, this does
not require changes in the receiving end, as the signed push certificate
is added to the notes tree on the sending side. A possible downside is
that it may become more likely that a push is refused due to a conflict
while updating the notes tree if the receiving repository is pushed into
frequently and by multiple people.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |    1 +
 notes.c        |   16 +++++++
 notes.h        |    2 +
 transport.c    |  120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h    |    2 +
 5 files changed, 141 insertions(+), 0 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/notes.c b/notes.c
index 93e9868..d081e7c 100644
--- a/notes.c
+++ b/notes.c
@@ -1296,3 +1296,19 @@ void expand_notes_ref(struct strbuf *sb)
 	else
 		strbuf_insert(sb, 0, "refs/notes/", 11);
 }
+
+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);
+}
diff --git a/notes.h b/notes.h
index c716694..5141e13 100644
--- a/notes.h
+++ b/notes.h
@@ -312,4 +312,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
 /* Expand inplace a note ref like "foo" or "notes/foo" into "refs/notes/foo" */
 void expand_notes_ref(struct strbuf *sb);
 
+void get_note_text(struct strbuf *, struct notes_tree *, const unsigned char *);
+
 #endif
diff --git a/transport.c b/transport.c
index 740a739..702d438 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,12 @@
 #include "branch.h"
 #include "url.h"
 #include "submodule.h"
+#include "gpg-interface.h"
+#include "commit.h"
+#include "notes.h"
+#include "notes-merge.h"
+#include "blob.h"
+#include "tag.h"
 
 /* rsync support */
 
@@ -1004,6 +1010,112 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
 }
 
+static int is_ref_pushed(const struct ref *ref)
+{
+	if (!ref->peer_ref || ref->deletion)
+		return 0;
+
+	/* Filter out unchanged ones */
+	switch (ref->status) {
+	case REF_STATUS_REJECT_NONFASTFORWARD:
+	case REF_STATUS_UPTODATE:
+		return 0;
+	default:
+		; /* ok */
+	}
+
+	return 1;
+}
+
+static const char push_signature_note[] = "refs/notes/signed-push";
+
+static int add_push_signature_note(struct ref *signature_note,
+				   struct ref *ref,
+				   struct strbuf *cert)
+{
+	struct notes_tree *notes_tree;
+	struct strbuf nbuf = STRBUF_INIT;
+	int ret = 0;
+	unsigned char parent[20], commit[20];
+	struct ref_lock *lock;
+
+	init_notes(NULL, push_signature_note, NULL, 0);
+	notes_tree = &default_notes_tree;
+
+	resolve_ref(notes_tree->ref, parent, 0, NULL);
+	lock = lock_any_ref_for_update(notes_tree->ref, parent, 0);
+
+	for ( ; ref; ref = ref->next) {
+		unsigned char nsha1[20];
+
+		if ((ref == signature_note) || !is_ref_pushed(ref))
+			continue;
+		get_note_text(&nbuf, notes_tree, ref->new_sha1);
+		if (nbuf.len)
+			strbuf_addch(&nbuf, '\n');
+		strbuf_add(&nbuf, cert->buf, cert->len);
+		if (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||
+		    add_note(notes_tree, ref->new_sha1, nsha1, NULL))
+			ret = error(_("unable to write note object"));
+		strbuf_reset(&nbuf);
+	}
+
+	if (!ret) {
+		create_notes_commit(notes_tree, NULL, "push", commit);
+		ret = write_ref_sha1(lock, commit, "signed push");
+	}
+	free_notes(notes_tree);
+
+	if (!ret) {
+		hashcpy(signature_note->new_sha1, commit);
+		if (!signature_note->peer_ref)
+			signature_note->peer_ref = alloc_ref(push_signature_note);
+	}
+	return ret;
+}
+
+static int sign_push_certificate(struct strbuf *cert)
+{
+	return sign_buffer(cert, git_committer_info(IDENT_NO_DATE));
+}
+
+static int sign_push(struct transport *transport,
+		     struct ref *remote_refs,
+		     int flags)
+{
+	struct ref *ref, *tail = NULL, *signature_note = NULL;
+	struct strbuf push_cert = STRBUF_INIT;
+	int updates = 0, ret = 0;
+
+	if (flags & TRANSPORT_PUSH_DRY_RUN)
+		return 0;
+
+	strbuf_addstr(&push_cert, "Push-Certificate-Version: 0\n");
+	strbuf_addf(&push_cert, "Pusher: %s\n", git_committer_info(0));
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		tail = ref;
+		if (!strcmp(ref->name, push_signature_note))
+			signature_note = ref;
+		if (!is_ref_pushed(ref))
+			continue;
+		updates++;
+		strbuf_addf(&push_cert, "Update: %s %s\n",
+			    sha1_to_hex(ref->new_sha1), ref->name);
+	}
+
+	if (updates && !sign_push_certificate(&push_cert)) {
+		if (!signature_note) {
+			signature_note = alloc_ref(push_signature_note);
+			tail->next = signature_note;
+		}
+		ret = add_push_signature_note(signature_note,
+					      remote_refs, &push_cert);
+	}
+	strbuf_release(&push_cert);
+	return ret;
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
 		   int *nonfastforward)
@@ -1015,6 +1127,9 @@ int transport_push(struct transport *transport,
 		/* Maybe FIXME. But no important transport uses this case. */
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			die("This transport does not support using --set-upstream");
+		/* Likewise */
+		if (flags & TRANSPORT_PUSH_SIGNED)
+			die("This transport does not support using --signed");
 
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
@@ -1050,6 +1165,11 @@ int transport_push(struct transport *transport,
 					die("There are unpushed submodules, aborting.");
 		}
 
+		if (flags & TRANSPORT_PUSH_SIGNED) {
+			if (sign_push(transport, remote_refs, flags))
+				return -1;
+		}
+
 		push_ret = transport->push_refs(transport, remote_refs, flags);
 		err = push_had_errors(remote_refs);
 		ret = push_ret | err;
diff --git a/transport.h b/transport.h
index 059b330..e525d07 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)
 
-- 
1.7.7.rc0.188.g3793ac

^ permalink raw reply related

* Re: [PATCH] fetch: skip on-demand checking when no submodules are configured
From: Junio C Hamano @ 2011-09-09 21:00 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Git Mailing List, Martin Fick, Heiko Voigt, Michael Haggerty
In-Reply-To: <4E6A594B.9030108@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> It makes no sense to do the - possibly very expensive - call to "rev-list
> <new-ref-sha1> --not --all" in check_for_new_submodule_commits() when
> there aren't any submodules configured.
>
> Leave check_for_new_submodule_commits() early when no name <-> path
> mappings for submodules are found in the configuration. To make that work
> reading the configuration had to be moved further up in cmd_fetch(), as
> doing that after the actual fetch of the superproject was too late.
>
> Reported-by: Martin Fick <mfick@codeaurora.org>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> This achieves the first goal: Don't let people pay a performance penalty
> when they don't even use submodules. On Michael's test repo from [1] the
> time for a full fetch went down from 142 seconds (current master) to one
> second which is - not surprisingly - the same as using current master
> with the --no-recurse-submodules option.
>
> Now back to the drawing board to fix the performance regression for those
> people who are using submodules ...
>
> [1] http://comments.gmane.org/gmane.comp.version-control.git/177103

Thanks.

^ permalink raw reply

* Re: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
From: Junio C Hamano @ 2011-09-09 20:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <20110909190547.GF28480@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> How frequently do you build the html branch? I always assumed it was
> once in a while (every push?), not for every commit.

I don't ;-)

Every time "master" is updated to the official distribution point, i.e.,
kernel.org (which currently is down). A post-update hook runs there,
fetches the "master" branch commit into a separate repository, rebuilds
the documentation, updates "html" and "man" branches and pushes the
results back to that distribution point repository.

The mirrors will pick the result up after all that happens (or they may
lag but they will eventually catch up).

^ permalink raw reply

* Re: [PATCH v3] date.c: Support iso8601 timezone formats
From: Junio C Hamano @ 2011-09-09 20:46 UTC (permalink / raw)
  To: Haitao Li; +Cc: git, Jeff King
In-Reply-To: <7vd3f9ve9m.fsf@alter.siamese.dyndns.org>

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

> Also, I do not quite understand why the match_tz() logic needs to be that
> long.
>
> Wouldn't something like this patch (on top of yours) easier to follow?
>
>  date.c |   50 +++++++++++++++++++++-----------------------------
>  1 files changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/date.c b/date.c
> index f8722c1..6079b1a 100644
> --- a/date.c
> +++ b/date.c
> @@ -551,44 +551,36 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
>  
>  static int match_tz(const char *date, int *offp)
>  {
> +	int min;

Micronit; this should be "int min = 0", as parsing of "hh" form relies on
the if/elseif/else cascade to parse only the hour part and leaving min to
the original value.

>  	char *end;
> +	int hour = strtoul(date + 1, &end, 10);
> +	int n = end - (date + 1);
>  
> +	if (n == 4) {
> +		/* hhmm */
> +		min = hour % 100;
> +		hour = hour / 100;
> +	} else if (n != 2) {
> +		min = 99; /* random crap */
> +	} else if (*end == ':') {
> +		/* hh:mm? */
> +		min = strtoul(end + 1, &end, 10);
> +		if (end - (date + 1) != 5)
> +			min = 99; /* random crap */
>  	}

^ permalink raw reply

* [PATCH v3 4/4] push -s: signed push
From: Junio C Hamano @ 2011-09-09 20:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1315600904-17032-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.

 3. The signed push certificate is added as notes in the "signed-push"
    notes tree to the objects listed in the certificate. The push refspec
    is altered to push this notes tree to the other side.

Compared to the alternative design posted earlier on the list, this does
not require changes in the receiving end, as the signed push certificate
is added to the notes tree on the sending side. A possible downside is
that it may become more likely that a push is refused due to a conflict
while updating the notes tree if the receiving repository is pushed into
frequently and by multiple people.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |    1 +
 notes.c        |   16 +++++++
 notes.h        |    2 +
 transport.c    |  124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h    |    5 ++
 5 files changed, 148 insertions(+), 0 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/notes.c b/notes.c
index 93e9868..d081e7c 100644
--- a/notes.c
+++ b/notes.c
@@ -1296,3 +1296,19 @@ void expand_notes_ref(struct strbuf *sb)
 	else
 		strbuf_insert(sb, 0, "refs/notes/", 11);
 }
+
+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);
+}
diff --git a/notes.h b/notes.h
index c716694..5141e13 100644
--- a/notes.h
+++ b/notes.h
@@ -312,4 +312,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
 /* Expand inplace a note ref like "foo" or "notes/foo" into "refs/notes/foo" */
 void expand_notes_ref(struct strbuf *sb);
 
+void get_note_text(struct strbuf *, struct notes_tree *, const unsigned char *);
+
 #endif
diff --git a/transport.c b/transport.c
index 740a739..de61669 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,12 @@
 #include "branch.h"
 #include "url.h"
 #include "submodule.h"
+#include "gpg-interface.h"
+#include "commit.h"
+#include "notes.h"
+#include "notes-merge.h"
+#include "blob.h"
+#include "tag.h"
 
 /* rsync support */
 
@@ -476,6 +482,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;
 }
@@ -1004,6 +1013,112 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
 }
 
+static int is_ref_pushed(const struct ref *ref)
+{
+	if (!ref->peer_ref || ref->deletion)
+		return 0;
+
+	/* Filter out unchanged ones */
+	switch (ref->status) {
+	case REF_STATUS_REJECT_NONFASTFORWARD:
+	case REF_STATUS_UPTODATE:
+		return 0;
+	default:
+		; /* ok */
+	}
+
+	return 1;
+}
+
+static const char push_signature_note[] = "refs/notes/signed-push";
+
+static int add_push_signature_note(struct ref *signature_note,
+				   struct ref *ref,
+				   struct strbuf *cert)
+{
+	struct notes_tree *notes_tree;
+	struct strbuf nbuf = STRBUF_INIT;
+	int ret = 0;
+	unsigned char parent[20], commit[20];
+	struct ref_lock *lock;
+
+	init_notes(NULL, push_signature_note, NULL, 0);
+	notes_tree = &default_notes_tree;
+
+	resolve_ref(notes_tree->ref, parent, 0, NULL);
+	lock = lock_any_ref_for_update(notes_tree->ref, parent, 0);
+
+	for ( ; ref; ref = ref->next) {
+		unsigned char nsha1[20];
+
+		if ((ref == signature_note) || !is_ref_pushed(ref))
+			continue;
+		get_note_text(&nbuf, notes_tree, ref->new_sha1);
+		if (nbuf.len)
+			strbuf_addch(&nbuf, '\n');
+		strbuf_add(&nbuf, cert->buf, cert->len);
+		if (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||
+		    add_note(notes_tree, ref->new_sha1, nsha1, NULL))
+			ret = error(_("unable to write note object"));
+		strbuf_reset(&nbuf);
+	}
+
+	if (!ret) {
+		create_notes_commit(notes_tree, NULL, "push", commit);
+		ret = write_ref_sha1(lock, commit, "signed push");
+	}
+	free_notes(notes_tree);
+
+	if (!ret) {
+		hashcpy(signature_note->new_sha1, commit);
+		if (!signature_note->peer_ref)
+			signature_note->peer_ref = alloc_ref(push_signature_note);
+	}
+	return ret;
+}
+
+static int sign_push_certificate(struct strbuf *cert)
+{
+	return sign_buffer(cert, git_committer_info(IDENT_NO_DATE));
+}
+
+static int sign_push(struct transport *transport,
+		     struct ref *remote_refs,
+		     int flags)
+{
+	struct ref *ref, *tail = NULL, *signature_note = NULL;
+	struct strbuf push_cert = STRBUF_INIT;
+	int updates = 0, ret = 0;
+
+	if (flags & TRANSPORT_PUSH_DRY_RUN)
+		return 0;
+
+	strbuf_addstr(&push_cert, "Push-Certificate-Version: 0\n");
+	strbuf_addf(&push_cert, "Pusher: %s\n", git_committer_info(0));
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		tail = ref;
+		if (!strcmp(ref->name, push_signature_note))
+			signature_note = ref;
+		if (!is_ref_pushed(ref))
+			continue;
+		updates++;
+		strbuf_addf(&push_cert, "Update: %s %s\n",
+			    sha1_to_hex(ref->new_sha1), ref->name);
+	}
+
+	if (updates && !sign_push_certificate(&push_cert)) {
+		if (!signature_note) {
+			signature_note = alloc_ref(push_signature_note);
+			tail->next = signature_note;
+		}
+		ret = add_push_signature_note(signature_note,
+					      remote_refs, &push_cert);
+	}
+	strbuf_release(&push_cert);
+	return ret;
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
 		   int *nonfastforward)
@@ -1015,6 +1130,9 @@ int transport_push(struct transport *transport,
 		/* Maybe FIXME. But no important transport uses this case. */
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			die("This transport does not support using --set-upstream");
+		/* Likewise */
+		if (flags & TRANSPORT_PUSH_SIGNED)
+			die("This transport does not support using --signed");
 
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
@@ -1050,7 +1168,13 @@ int transport_push(struct transport *transport,
 					die("There are unpushed submodules, aborting.");
 		}
 
+		if (flags & TRANSPORT_PUSH_SIGNED) {
+			if (sign_push(transport, remote_refs, flags))
+				return -1;
+		}
+
 		push_ret = transport->push_refs(transport, remote_refs, flags);
+
 		err = push_had_errors(remote_refs);
 		ret = push_ret | err;
 
diff --git a/transport.h b/transport.h
index 059b330..034ff5a 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,9 @@ struct transport *transport_get(struct remote *, const char *);
 /* Aggressively fetch annotated tags if possible */
 #define TRANS_OPT_FOLLOWTAGS "followtags"
 
+/* Signed push */
+#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 v3 2/4] Split GPG interface into its own helper library
From: Junio C Hamano @ 2011-09-09 20:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1315600904-17032-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 v3 3/4] rename "match_refs()" to "match_push_refs()"
From: Junio C Hamano @ 2011-09-09 20:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1315600904-17032-1-git-send-email-gitster@pobox.com>

Yes, there is a warning that says the function is only used by push in big
red letters in front of this function, but it didn't say a more important
thing it should have said: what the function is for and what it does.

Rename it and document it to avoid future confusion.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/remote.c    |    4 ++--
 builtin/send-pack.c |    2 +-
 http-push.c         |    4 ++--
 remote.c            |   13 ++++++++-----
 remote.h            |    4 ++--
 transport.c         |    4 ++--
 6 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index f2a9c26..f16b544 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -389,8 +389,8 @@ static int get_push_ref_states(const struct ref *remote_refs,
 	local_refs = get_local_heads();
 	push_map = copy_ref_list(remote_refs);
 
-	match_refs(local_refs, &push_map, remote->push_refspec_nr,
-		   remote->push_refspec, MATCH_REFS_NONE);
+	match_push_refs(local_refs, &push_map, remote->push_refspec_nr,
+			remote->push_refspec, MATCH_REFS_NONE);
 
 	states->push.strdup_strings = 1;
 	for (ref = push_map; ref; ref = ref->next) {
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 87833f4..e0b8030 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -509,7 +509,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 		flags |= MATCH_REFS_MIRROR;
 
 	/* match them up */
-	if (match_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
+	if (match_push_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
 		return -1;
 
 	set_ref_status_for_push(remote_refs, args.send_mirror,
diff --git a/http-push.c b/http-push.c
index 376331a..02f46a5 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1877,8 +1877,8 @@ int main(int argc, char **argv)
 	}
 
 	/* match them up */
-	if (match_refs(local_refs, &remote_refs,
-		       nr_refspec, (const char **) refspec, push_all)) {
+	if (match_push_refs(local_refs, &remote_refs,
+			    nr_refspec, (const char **) refspec, push_all)) {
 		rc = -1;
 		goto cleanup;
 	}
diff --git a/remote.c b/remote.c
index b8ecfa5..536ffa3 100644
--- a/remote.c
+++ b/remote.c
@@ -1170,12 +1170,15 @@ static struct ref **tail_ref(struct ref **head)
 }
 
 /*
- * Note. This is used only by "push"; refspec matching rules for
- * push and fetch are subtly different, so do not try to reuse it
- * without thinking.
+ * Given the set of refs the local repository has, the set of refs the
+ * remote repository has, and the refspec used for push, determine
+ * what remote refs we will update and with what value by setting
+ * peer_ref (which object is being pushed) and force (if the push is
+ * forced) in elements of "dst". The function may add new elements to
+ * dst (e.g. pushing to a new branch, done in match_explicit_refs).
  */
-int match_refs(struct ref *src, struct ref **dst,
-	       int nr_refspec, const char **refspec, int flags)
+int match_push_refs(struct ref *src, struct ref **dst,
+		    int nr_refspec, const char **refspec, int flags)
 {
 	struct refspec *rs;
 	int send_all = flags & MATCH_REFS_ALL;
diff --git a/remote.h b/remote.h
index 9a30a9d..6729477 100644
--- a/remote.h
+++ b/remote.h
@@ -96,8 +96,8 @@ void free_refspec(int nr_refspec, struct refspec *refspec);
 char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
 		     const char *name);
 
-int match_refs(struct ref *src, struct ref **dst,
-	       int nr_refspec, const char **refspec, int all);
+int match_push_refs(struct ref *src, struct ref **dst,
+		    int nr_refspec, const char **refspec, int all);
 void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 	int force_update);
 
diff --git a/transport.c b/transport.c
index fa279d5..740a739 100644
--- a/transport.c
+++ b/transport.c
@@ -1033,8 +1033,8 @@ int transport_push(struct transport *transport,
 		if (flags & TRANSPORT_PUSH_MIRROR)
 			match_flags |= MATCH_REFS_MIRROR;
 
-		if (match_refs(local_refs, &remote_refs,
-			       refspec_nr, refspec, match_flags)) {
+		if (match_push_refs(local_refs, &remote_refs,
+				    refspec_nr, refspec, match_flags)) {
 			return -1;
 		}
 
-- 
1.7.7.rc0.188.g3793ac

^ permalink raw reply related

* [PATCH v3 0/4] Signed push
From: Junio C Hamano @ 2011-09-09 20:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1315512102-19022-1-git-send-email-gitster@pobox.com>

This is based an alternative design we discussed on the list to prepare
and push the notes tree on the sending side.

Junio C Hamano (4):
  send-pack: typofix error message
  Split GPG interface into its own helper library
  rename "match_refs()" to "match_push_refs()"
  push -s: signed push

 Makefile             |    2 +
 builtin/push.c       |    1 +
 builtin/remote.c     |    4 +-
 builtin/send-pack.c  |    4 +-
 builtin/tag.c        |   60 +++---------------------
 builtin/verify-tag.c |   35 +------------
 gpg-interface.c      |   94 ++++++++++++++++++++++++++++++++++++
 gpg-interface.h      |   11 ++++
 http-push.c          |    4 +-
 notes.c              |   16 ++++++
 notes.h              |    2 +
 remote.c             |   13 +++--
 remote.h             |    4 +-
 transport.c          |  128 +++++++++++++++++++++++++++++++++++++++++++++++++-
 transport.h          |    5 ++
 15 files changed, 283 insertions(+), 100 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h

-- 
1.7.7.rc0.188.g3793ac

^ permalink raw reply

* [PATCH v3 1/4] send-pack: typofix error message
From: Junio C Hamano @ 2011-09-09 20:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1315600904-17032-1-git-send-email-gitster@pobox.com>

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.188.g3793ac

^ permalink raw reply related

* git checkout under 1.7.6 does not properly list untracked files and aborts
From: Joshua Jensen @ 2011-09-09 20:04 UTC (permalink / raw)
  To: git@vger.kernel.org

This may be an msysGit 1.7.6 issue, as that is what I am using.  It also 
occurs in msysGit 1.7.5, but I am almost certain it did not happen in 
msysGit 1.7.2.

Given an untracked file in the working directory that has been added to 
an alternate branch, when switching to that alternate branch, 'git 
checkout' exits with an error code but does not print anything to the 
console.  It should print an untracked file error.

I have been trying to track this down in code.  The point where the 
error messages are printed, display_error_msgs, is never hit.

I don't have a debugger to trace this on Windows, so I've just been 
adding lots of debug output.  So far, I have not found the catalyst.

I am out on vacation now for the next week and a half.  However, I 
thought I would send this along to see if someone else can confirm the 
the problem.  I'll continue investigation when I'm back.

Thanks.

Josh

^ permalink raw reply

* Re: [PATCH] t3200: test branch creation with -v
From: Jeff King @ 2011-09-09 19:45 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <20110909194357.GA31446@sigill.intra.peff.net>

On Fri, Sep 09, 2011 at 03:43:57PM -0400, Jeff King wrote:

> On Fri, Sep 09, 2011 at 09:40:59PM +0200, Michael J Gruber wrote:
> 
> > +test_expect_success 'git branch -v t should work' '
> > +	git branch -v t &&
> > +	test .git/refs/heads/t &&
> 
> test -f ?

Hmm, this also seems to be a problem in the other tests, too.

-Peff

^ permalink raw reply

* Re: [PATCH] t3200: test branch creation with -v
From: Jeff King @ 2011-09-09 19:43 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <49578782dd114220aa2562b5bd29755fc2bdd0fa.1315597137.git.git@drmicha.warpmail.net>

On Fri, Sep 09, 2011 at 09:40:59PM +0200, Michael J Gruber wrote:

> +test_expect_success 'git branch -v t should work' '
> +	git branch -v t &&
> +	test .git/refs/heads/t &&

test -f ?

Also, don't we have test_path_is_file which yields slightly prettier
output (and maybe some portability benefits; I don't remember)?

> +	git branch -d t &&
> +	test ! -f .git/refs/heads/t

Ditto for 'test_path_is_missing' here.

-Peff

^ permalink raw reply

* [PATCH] t3200: test branch creation with -v
From: Michael J Gruber @ 2011-09-09 19:40 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <20110909193033.GA31184@sigill.intra.peff.net>

Make sure that "git branch -v t" creates branch "t".

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t3200-branch.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index c466b20..8381f0c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -105,6 +105,13 @@ test_expect_success 'git branch -v -d t should work' '
 	test ! -f .git/refs/heads/t
 '
 
+test_expect_success 'git branch -v t should work' '
+	git branch -v t &&
+	test .git/refs/heads/t &&
+	git branch -d t &&
+	test ! -f .git/refs/heads/t
+'
+
 test_expect_success 'git branch -v -m t s should work' '
 	git branch t &&
 	test .git/refs/heads/t &&
-- 
1.7.7.rc0.469.g9eb94

^ permalink raw reply related

* Re: Rebase & Trailing Whitespace
From: Jeff King @ 2011-09-09 19:38 UTC (permalink / raw)
  To: Clemens Buchacher
  Cc: Michael J Gruber, Philip Oakley, Hilco Wijbenga, Git Users
In-Reply-To: <20110903201832.GB8362@ecki>

On Sat, Sep 03, 2011 at 10:18:32PM +0200, Clemens Buchacher wrote:

> I am looking at the output of:
> 
>  git diff --check 4b825 -- $(git ls-files '*.[ch]'|grep -v ^compat)
> 
> Most of those are false positives from multiline function argument
> lists, which are aligned with the first line. So the above command
> would break more than it would fix.
> 
> I don't care either way, but for future reference: Do we or do we
> not want indentation with spaces in this case?

I thought our policy was that a tab is 8 spaces, and that indentations
of size N should be floor(N/8) tabs and (N%8) spaces. I know other
projects disagree (e.g., they want tabs for structural indentation and
spaces for all other alignment), but that seems to be the general use in
git.

-Peff

^ permalink raw reply

* Re: [PATCHv2 4/5] branch: introduce --list option
From: Jeff King @ 2011-09-09 19:30 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git
In-Reply-To: <4E6A691C.2060803@drmicha.warpmail.net>

On Fri, Sep 09, 2011 at 09:29:32PM +0200, Michael J Gruber wrote:

> > I recall Peff had some comments on your new tests last night, by the way.
> 
> Yes, we have tests (now) for "-v -d", "-v -m", "-v branch*" and
> combinations with --list. Testing "-v foo" for creation would be good
> (and belong into t3200). Is there anything to amend?

I think my comment was just to add the "-v foo" test.

-Peff

^ permalink raw reply

* Re: [PATCHv2 4/5] branch: introduce --list option
From: Michael J Gruber @ 2011-09-09 19:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vlitxvh45.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 09.09.2011 18:02:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Junio C Hamano venit, vidit, dixit 08.09.2011 23:17:
>>> Ah, nevermind.
>>>
>>> As the series is already in 'next', here is what I came up with.
>>
>> I thought you'll rebuild next anyways after 1.7.7, but either way it's
>> fine. Thanks for holding this series in next long enough to really cook
>> it (and Jeff for revisiting it), it's much better now, keeping the
>> (undocumented, but expected) behavior of "git branch -v foo".
> 
> Thank *you* for all the work.
> 
> I recall Peff had some comments on your new tests last night, by the way.
> 

Yes, we have tests (now) for "-v -d", "-v -m", "-v branch*" and
combinations with --list. Testing "-v foo" for creation would be good
(and belong into t3200). Is there anything to amend?

Michael

^ permalink raw reply

* Re: can Git encrypt/decrypt .gpg on push/fetch?
From: Jeff King @ 2011-09-09 19:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, Aneesh Bhasin, tzz, git
In-Reply-To: <7vvct1tu3n.fsf@alter.siamese.dyndns.org>

On Fri, Sep 09, 2011 at 12:05:00PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >> B) Keep blobs encrypted, checkout decrypted
> >> - Use Use "*.gpg filter=gpg" in your attributes and
> >> [filter "gpg"]
> >> 	smudge = gpg -d
> >> 	clean = gpg -e -r yourgpgkey
> >>   in your config.
> >> 
> >> I use A on a regular basis. B is untested (but patterned after a similar
> >> gzip filter I use). You may or may not have better results with "gpg -ea".
> >
> > Yeah, I think that would work but have never tried it either.
> 
> Unless "gpg -e" encrypts the same cleartext into the same cyphertext every
> time, the above "clean" filter probably wouldn't be very useful.

Ah, right, I remember now running into that at some point. You could get
around that by using a symmetric cipher in block mode, or with a
non-random IV, but then you're opening yourself up to some cryptanalytic
attacks.

-Peff

^ permalink raw reply

* Re: can Git encrypt/decrypt .gpg on push/fetch?
From: Michael J Gruber @ 2011-09-09 19:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Aneesh Bhasin, tzz, git
In-Reply-To: <7vvct1tu3n.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 09.09.2011 21:05:
> Jeff King <peff@peff.net> writes:
> 
>>> B) Keep blobs encrypted, checkout decrypted
>>> - Use Use "*.gpg filter=gpg" in your attributes and
>>> [filter "gpg"]
>>> 	smudge = gpg -d
>>> 	clean = gpg -e -r yourgpgkey
>>>   in your config.
>>>
>>> I use A on a regular basis. B is untested (but patterned after a similar
>>> gzip filter I use). You may or may not have better results with "gpg -ea".
>>
>> Yeah, I think that would work but have never tried it either.
> 
> Unless "gpg -e" encrypts the same cleartext into the same cyphertext every
> time, the above "clean" filter probably wouldn't be very useful.
> 

Uh, right, this would only make sense with specific versions of debian's
openssl then. Only that gpg does not use that ;)

I'm not sure whether "gpg --symmetric" has the same issue, but version
A) seemed better before that already.

Michael

^ permalink raw reply

* Re: [PATCH 2/2] push -s: skeleton
From: Jeff King @ 2011-09-09 19:12 UTC (permalink / raw)
  To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <20110909160301.GA9707@gnu.kitenet.net>

On Fri, Sep 09, 2011 at 12:03:01PM -0400, Joey Hess wrote:

> The most credible attack I have so far does not involve binary files in
> tree. Someone pointed out that git log, git show, etc stop printing
> commit messages at NULL.

It was me.

> It might be worth ameloriating that attack by making git log always
> show the full buffer. Or it would be easy to write a tool that finds
> any commits that have a NULL in their message.

Unfortunately, that is going to involve a pretty huge code audit of git,
as the "tack a \0 to the end of an object just in case" code dates back
quite a while (e871b64, unpack_sha1_file: zero-pad the unpacked
object, 2005-05-25). So I suspect there is a lot of code built on
top of the assumption that commit messages are NUL-terminated strings.

Besides which, that is only one form of hiding. If collision attacks
against sha1 become a possibility, I think we are better to talk about
moving to a new hash. Even sha-256 truncated to 160 bits would be better
than sha-1 (AFAIK, that family of SHA does not suffer from the same
attacks, so we would still be in the 2^80 range for collision attacks).

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox