Re: [BUG (maybe)] git rev-parse --verify --quiet isn't quiet
Junio C Hamano gitster at pobox.com writes:
Junio C Hamano gitster at pobox.com writes:
I would suspect that this may be fine.
rev-parse --verify makes sure the named object exists, but in this
case at {u} does not even name any object, does it?
Hmph, but rev-parse --verify no-such-branch does *not* name any
object, we would want to see it barf, and we probably would want to
be able to squelch the message. So it is unclear if at {u} barfing is
a good idea.
That was my counter-argument :) The vibe I get from rev-parse --verify
--quiet is that it should handle anything.
What is the reason why it is inpractical to pass 'quiet' down the
callchain?
Maybe I'm missing something obvious, but wouldn't that mean changing the
signature of 9 different functions, and consequently all of their calls
throughout Git?
That's perhaps not a good argument. Who cares whether a diff is small or
large if it fixes a bug properly? But most (or all) of those functions
do not concern themselves with printing stuff so maybe an additional
quiet? argument would look foreign in most places and make the code
harder to read.
Øsse
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] use a hashmap to make remotes faster
On 29.07.2014 16:43, Patrick Reynolds wrote:
Remotes are stored as an array, so looking one up or adding one without
duplication is an O(n) operation. Reading an entire config file full of
remotes is O(n^2) in the number of remotes. For a repository with tens of
thousands of remotes, the running time can hit multiple minutes.
Hash tables are way faster. So we add a hashmap from remote name to
struct remote and use it for all lookups. The time to add a new remote to
a repo that already has 50,000 remotes drops from ~2 minutes to 1
second.
We retain the old array of remotes so iterators proceed in config-file
order.
Signed-off-by: Patrick Reynolds patrick.reyno...@github.com
---
mark function with no arguments as taking void
remote.c | 63 ++-
remote.h | 3 +++
2 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/remote.c b/remote.c
index a0701f6..52533e4 100644
--- a/remote.c
+++ b/remote.c
@@ -42,6 +42,7 @@ struct rewrites {
static struct remote **remotes;
static int remotes_alloc;
static int remotes_nr;
+static struct hashmap remotes_hash;
static struct branch **branches;
static int branches_alloc;
@@ -136,26 +137,51 @@ static void add_url_alias(struct remote *remote, const
char *url)
add_pushurl_alias(remote, url);
}
+struct remotes_hash_key {
+ const char *str;
+ int len;
+};
+
+static int remotes_hash_cmp(const struct remote *a, const struct remote *b,
const struct remotes_hash_key *key)
+{
+ if (key)
+ return strncmp(a-name, key-str, key-len) ||
a-name[key-len];
+ else
+ return strcmp(a-name, b-name);
+}
+
+static inline void init_remotes_hash(void)
+{
+ if (!remotes_hash.cmpfn)
+ hashmap_init(remotes_hash, (hashmap_cmp_fn)remotes_hash_cmp,
0);
+}
+
static struct remote *make_remote(const char *name, int len)
{
- struct remote *ret;
- int i;
+ struct remote *ret, *replaced;
+ struct remotes_hash_key lookup;
+ struct hashmap_entry lookup_entry;
- for (i = 0; i remotes_nr; i++) {
- if (len ? (!strncmp(name, remotes[i]-name, len)
-!remotes[i]-name[len]) :
- !strcmp(name, remotes[i]-name))
- return remotes[i];
- }
+ if (!len)
+ len = strlen(name);
+
+ init_remotes_hash();
+ lookup.str = name;
+ lookup.len = len;
+ hashmap_entry_init(lookup_entry, memhash(name, len));
+
+ if ((ret = hashmap_get(remotes_hash, lookup_entry, lookup)) != NULL)
+ return ret;
ret = xcalloc(1, sizeof(struct remote));
ret-prune = -1; /* unspecified */
ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
remotes[remotes_nr++] = ret;
- if (len)
- ret-name = xstrndup(name, len);
- else
- ret-name = xstrdup(name);
+ ret-name = xstrndup(name, len);
+
+ hashmap_entry_init(ret, lookup_entry.hash);
+ replaced = hashmap_put(remotes_hash, ret);
+ assert(replaced == NULL); /* no previous entry overwritten */
return ret;
}
@@ -722,13 +748,16 @@ struct remote *pushremote_get(const char *name)
int remote_is_configured(const char *name)
{
- int i;
+ struct remotes_hash_key lookup;
+ struct hashmap_entry lookup_entry;
read_config();
- for (i = 0; i remotes_nr; i++)
- if (!strcmp(name, remotes[i]-name))
- return 1;
- return 0;
+ init_remotes_hash();
+ lookup.str = name;
+ lookup.len = strlen(name);
+ hashmap_entry_init(lookup_entry, memhash(name, lookup.len));
+
+ return hashmap_get(remotes_hash, lookup_entry, lookup) != NULL;
}
int for_each_remote(each_remote_fn fn, void *priv)
diff --git a/remote.h b/remote.h
index 917d383..81cb5ff 100644
--- a/remote.h
+++ b/remote.h
@@ -1,6 +1,7 @@
#ifndef REMOTE_H
#define REMOTE_H
+#include hashmap.h
#include parse-options.h
enum {
@@ -10,6 +11,8 @@ enum {
};
struct remote {
+ struct hashmap_entry ent; /* must be first */
+
I stumbled about this comment /* must be first */
when reading the changelog.
Why does it need to be first?
Is it a common reason I'm just not aware of,
or would it make sense to put the reason into the comment as well?
Thanks,
Stefan
const char *name;
int origin;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCHv3 0/4] am: patch-format
I've re-ordered things in this round. Hopefully the first 3 patches are uncontroversial, Junio has expressed reservations about the 4th. I haven't looked at moving the changes into mailsplit yet or I could try and update gitk to use something other than diff-tree or I could write some external filter programs. Chris Packham (4): am: avoid re-directing stdin twice t/am: add test for stgit patch format t/am: add tests for hg patch format am: add gitk patch format Documentation/git-am.txt | 3 +- git-am.sh| 38 +++- t/t4150-am.sh| 76 3 files changed, 115 insertions(+), 2 deletions(-) -- 2.1.0.64.gc343089 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCHv3 1/4] am: avoid re-directing stdin twice
In check_patch_format we feed $1 to a block that attempts to determine
the patch format. Since we've already redirected $1 to stdin there is no
need to redirect it again when we invoke tr. This prevents the following
errors when invoking git am
$ git am patch.patch
tr: write error: Broken pipe
tr: write error
Patch format detection failed.
Cc: Stephen Boyd bebar...@gmail.com
Signed-off-by: Chris Packham judge.pack...@gmail.com
---
Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425
git-am.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..fade7f8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,7 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with the
# header field names...
- tr -d '\015' $1 |
+ tr -d '\015' |
sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
--
2.1.0.64.gc343089
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCHv3 4/4] am: add gitk patch format
Patches created using gitk's write commit to file functionality (which
uses 'git diff-tree -p --pretty' under the hood) need some massaging in
order to apply cleanly. This consists of dropping the 'commit' line
automatically determining the subject and removing leading whitespace.
Signed-off-by: Chris Packham judge.pack...@gmail.com
---
This hasn't materially changed from the version Junio expressed
reservations about[1]. It solves my immediate problem but perhaps this
(as well as stgit and hg) belong as external filters in a pipeline
before git am. Or maybe mailsplit should absorb the functionality?
[1] - http://article.gmane.org/gmane.comp.version-control.git/256426
Documentation/git-am.txt | 3 ++-
git-am.sh| 36
t/t4150-am.sh| 23 +++
3 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..b59d2b3 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -101,7 +101,8 @@ default. You can use `--no-utf8` to override this.
By default the command will try to detect the patch format
automatically. This option allows the user to bypass the automatic
detection and specify the patch format that the patch(es) should be
- interpreted as. Valid formats are mbox, stgit, stgit-series and hg.
+ interpreted as. Valid formats are mbox, stgit, stgit-series, hg and
+ gitk.
-i::
--interactive::
diff --git a/git-am.sh b/git-am.sh
index fade7f8..5d69c89 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -227,6 +227,9 @@ check_patch_format () {
# HG changeset patch)
patch_format=hg
;;
+ 'commit '*)
+ patch_format=gitk
+ ;;
*)
# if the second line is empty and the third is
# a From, Author or Date entry, this is very
@@ -357,6 +360,39 @@ split_patches () {
this=
msgnum=
;;
+ gitk)
+ # These patches are generates with 'git diff-tree -p --pretty'
+ # we discard the 'commit' line, after that the first line not
+ # starting with 'Author:' or 'Date:' is the subject. We also
+ # need to strip leading whitespace from the message body.
+ this=0
+ for gitk in $@
+ do
+ this=$(expr $this + 1)
+ msgnum=$(printf %0${prec}d $this)
+ @@PERL@@ -ne 'BEGIN { $subject = 0; $diff = 0; }
+ if (!$diff) { s/^// ; }
+ if ($subject 1) { print ; }
+ elsif (/^commit\s.*$/) { next ; }
+ elsif (/^\s+$/) { next ; }
+ elsif (/^Author:/) { s/Author/From/ ; print ; }
+ elsif (/^Date:/) { print ;}
+ elsif (/^diff --git/) { $diff = 1 ; print ;}
+ elsif ($subject) {
+ $subject = 2 ;
+ print \n ;
+ print ;
+ } else {
+ print Subject: , $_ ;
+ $subject = 1;
+ }
+ ' $gitk $dotest/$msgnum || clean_abort
+
+ done
+ echo $this $dotest/last
+ this=
+ msgnum=
+ ;;
*)
if test -n $patch_format
then
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 8ee81cf..5d4f7be 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -120,6 +120,7 @@ test_expect_success setup '
echo ---
git diff-tree --stat -p second | sed -e 1d
} patch1-hg.eml
+ git diff-tree -p --pretty second patch1-gitk.eml
sed -n -e 3,\$p msg file
git add file
@@ -239,6 +240,28 @@ test_expect_failure 'am applies patch using
--patch-format=hg' '
git diff --exit-code second
'
+test_expect_success 'am applies patch generated by gitk' '
+ rm -fr .git/rebase-apply
+ git reset --hard
+ git checkout first
+ git am patch1-gitk.eml
+ test_path_is_missing .git/rebase-apply
+ git diff --exit-code second
+ test $(git rev-parse second) = $(git rev-parse HEAD)
+ test $(git rev-parse second^) = $(git rev-parse HEAD^)
+'
+
+test_expect_failure 'am applies patch using --patch-format=gitk' '
+ rm -fr .git/rebase-apply
+ git reset --hard
+ git checkout first
+ git am --patch-format=gitk patch1-gitk.eml
+ test_path_is_missing .git/rebase-apply
[RFC PATCHv3 2/4] t/am: add test for stgit patch format
This adds a tests which exercise the detection of the stgit format.
There is a current know breakage in that the code that deals with stgit
in split_patches can't handle reading from stdin.
Signed-off-by: Chris Packham judge.pack...@gmail.com
---
t/t4150-am.sh | 27 +++
1 file changed, 27 insertions(+)
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 5edb79a..dbe3475 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -103,6 +103,15 @@ test_expect_success setup '
echo X-Fake-Field: Line Three
git format-patch --stdout first | sed -e 1d
} patch1-ws.eml
+ {
+ echo From: $GIT_AUTHOR_NAME $GIT_AUTHOR_EMAIL
+ echo
+ cat msg
+ echo
+ echo Signed-off-by: $GIT_COMMITTER_NAME
$GIT_COMMITTER_EMAIL
+ echo ---
+ git diff-tree --stat -p second | sed -e 1d
+ } patch1-stgit.eml
sed -n -e 3,\$p msg file
git add file
@@ -186,6 +195,24 @@ test_expect_success 'am applies patch e-mail with
preceding whitespace' '
test $(git rev-parse second^) = $(git rev-parse HEAD^)
'
+test_expect_success 'am applies patch generated by stgit' '
+ rm -fr .git/rebase-apply
+ git reset --hard
+ git checkout first
+ git am patch1-stgit.eml
+ test_path_is_missing .git/rebase-apply
+ git diff --exit-code second
+'
+
+test_expect_failure 'am applies patch using --patch-format=stgit' '
+ rm -fr .git/rebase-apply
+ git reset --hard
+ git checkout first
+ git am --patch-format=stgit patch1-stgit.eml
+ test_path_is_missing .git/rebase-apply
+ git diff --exit-code second
+'
+
test_expect_success 'setup: new author and committer' '
GIT_AUTHOR_NAME=Another Thor
GIT_AUTHOR_EMAIL=a.t...@example.com
--
2.1.0.64.gc343089
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCHv3 3/4] t/am: add tests for hg patch format
This adds a tests which exercise the detection of the hg format. As
with stgit there is a current know breakage in where split_patches can't
handle reading from stdin with these patch formats.
Cc: Giuseppe Bilotta giuseppe.bilo...@gmail.com
Signed-off-by: Chris Packham judge.pack...@gmail.com
---
Note. I don't have access to a mercurial repository (plus I know next to
nothing about it) so the patch I've generated for the test was created
by looking at the format detection code. If someone can show me an
actual example of what mercurial produces that'd be helpful.
t/t4150-am.sh | 26 ++
1 file changed, 26 insertions(+)
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index dbe3475..8ee81cf 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -112,6 +112,14 @@ test_expect_success setup '
echo ---
git diff-tree --stat -p second | sed -e 1d
} patch1-stgit.eml
+ {
+ echo # HG changeset patch
+ echo # User $GIT_AUTHOR_NAME $GIT_AUTHOR_EMAIL
+ echo
+ cat msg
+ echo ---
+ git diff-tree --stat -p second | sed -e 1d
+ } patch1-hg.eml
sed -n -e 3,\$p msg file
git add file
@@ -213,6 +221,24 @@ test_expect_failure 'am applies patch using
--patch-format=stgit' '
git diff --exit-code second
'
+test_expect_success 'am applies patch generated by hg' '
+ rm -fr .git/rebase-apply
+ git reset --hard
+ git checkout first
+ git am patch1-hg.eml
+ test_path_is_missing .git/rebase-apply
+ git diff --exit-code second
+'
+
+test_expect_failure 'am applies patch using --patch-format=hg' '
+ rm -fr .git/rebase-apply
+ git reset --hard
+ git checkout first
+ git am --patch-format=hg patch1-hg.eml
+ test_path_is_missing .git/rebase-apply
+ git diff --exit-code second
+'
+
test_expect_success 'setup: new author and committer' '
GIT_AUTHOR_NAME=Another Thor
GIT_AUTHOR_EMAIL=a.t...@example.com
--
2.1.0.64.gc343089
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git rebase: yet another newbie quest.
Hello, Caveat: this is somewhat long, sorry. Recently I've run into yet another rather tricky trouble with pull/rebase. No, neither of my troubles is in the usual rewriting published history group. (The first trouble I ran earlier was caused by the fact that git pull breaks local merges when pull.rebase configuration variable is set to true.) Below is shell script to create test repo that shows the steps I did that lead me to the trouble, along with explanations why I did them. Then follows description of the actual quest I ran through to figure a work-around. I must admit I still don't know what I should have done differently in the script below to avoid this problem in the first place, nor do I entirely understood what git is doing. $ git --version git version 1.9.3 - 8 - Test repo creation - 8 - #!/bin/sh # Git output is prefixed with '# below # Prepare some initial state for the show. Create 2 files to be changed # in 2 different branches to avoid content conflicts later. git init t cd t echo A a echo B b git add a b git commit -m A -a # Pretend we have some origin/master. It was really the case in the # original scenario. Here I just set a tag instead, as it doesn't # change the outcome. git tag origin_master # OK. Here we start. On the branch 'master' (that tracked origin/master in # original scenario) I do some small change: echo B b git commit -m B -a # Then I realize I need more changes and it gets complex enough to # warrant a topic branch. I create the 'topic' branch that will track # 'master' branch and reset 'master' back to its origin (remote # origin/master in original scenario). git checkout -b topic git branch --force master origin_master git branch -u master # Now I do more work on the topic branch... echo C b git commit -m C -a # Meanwhile something non-conflicting (in another file) changed upstream... git checkout master echo A a git commit -m D -a git checkout topic - 8 - Test repo creation end - 8 - The creation of the test setup complete, we end-up with the following simple history: $ git log --all --oneline --graph --decorate * 335fc12 (master) D | * 2cefe9f (HEAD, topic) C | * aed3006 B |/ * 951b15b (tag: origin_master) A $ And my 'topic' branch is set to track 'master': $ git branch -vv --list topic * topic 2cefe9f [master: ahead 2, behind 1] C Exactly what I wanted to achieve. Now we are ready to go to the essence of the quest. Being on the 'topic' branch I pull from upstream, rebasing. Can you guess what I expectated? Yeah, something as simple as B and C being rebased on top of D, like this: $ git pull --rebase ... $ git log --all --oneline --graph --decorate * xxx (HEAD, topic) C * xxx B * 335fc12 (master) D * 951b15b (tag: origin_master) A Here is what I got instead: $ git pull --rebase From . * branchmaster - FETCH_HEAD First, rewinding head to replay your work on top of it... Applying: C Using index info to reconstruct a base tree... M b Falling back to patching base and 3-way merge... Auto-merging b CONFLICT (content): Merge conflict in b Recorded preimage for 'b' Failed to merge in the changes. Patch failed at 0001 C The copy of the patch that failed is found in: /home/osv/f/t/t/.git/rebase-apply/patch When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort. $ Wait... Conflict?! That's unexpected. Let's first get back to a sane state: $ git rebase --abort $ I suspect it's not easy to figure what happens here, even having this simple imitation. It was even harder in real life as there were a lot of unrelated things happening simultaneously. What I did: Looking closer at the git rebase output, it applies C, but not (yet?) B. Why doesn't it apply B? Let's first exclude git pull from the equation: $ git rebase ... $ Still the same outcome as with 'git pull --rebase above -- at least git is consistent at kicking me into the stomach ;-) Good... Let's figure how to check which commits are to be rebased: $ git help rebase ... This is the same set of commits that would be shown by git log upstream..HEAD $ git rebase --abort $ git log --oneline master..HEAD 2cefe9f C aed3006 B $ So it should apply B and C, but only applies C. The manual lies? Unlikely. Let's dig more into the manual... Ah, maybe here it is: Note that any commits in HEAD which introduce the same textual changes as a commit in HEAD..upstream are omitted Check... $ git log --oneline HEAD..master 335fc12 D $ No, there is no way missed commit 'B' textually matches commit 'D'. Wondering further... let's try to specify upstream explicitly: $ git --rebase master First, rewinding head to replay your work on top of it... Applying: B Applying: C $ Wow! This way it works! But the manual says: If upstream is not specified, the upstream
Re: [PATCH 22/32] checkout: support checking out into a new working directory
On Fri, Sep 5, 2014 at 10:26 AM, Scott Schmit i.g...@comcast.net wrote: Each linked working tree has a private sub-directory in the repository's $GIT_DIR/worktrees directory. The private sub-directory's name is usually the base name of the linked working tree's path, possibly appended with a number to make it unique. For example, when `$GIT_DIR=/path/main/.git` the command `git checkout --to /path/other/test-next next` creates the linked working tree in `/path/other/test-next` and also creates a `$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1` if `test-next` is already taken). As a user, this leaves me with one other question -- what happens when I'm done with the test-next working tree and want to delete/rename it? If you rename it, the link remains and you can still continue to use test-next with its new name. If you move it, or rename on a filesystem without hardlink support, then you may need to run something from the linked worktree for it to fix up the link to main checkout. If you move it on the same filesystem that supports hardlink, it's all good. If you delete it, the part in $GIT_DIR/worktrees remains and can be cleaned up using git prune --repos (which I should rename to git prune --worktrees as well). At some point in future, this pruning should be part of git-gc. See patch 25/32 [1] for detail. I know I haven't added some user documentation about pruning. You're welcome to write up something about this ;) Note to self, you someone moves the _main_ checkout away, then they're screwed. Should probably make a note about that somewhere in documents. Is that cleaned up automatically, or do I need to register that I'm getting rid of/renaming it? (Another use case is if I put the working tree on removable media for some reason.) Removable media is covered in [1] as well. Though you'll need to lock the worktree in order to stop it from being pruned. In earlier iterations, this locking could be done automatically at git checkout --to time, if it detects that the new worktree is on a different filesystem than the main one. But that got dropped. We can add it back when this feature matures a bit. [1] http://article.gmane.org/gmane.comp.version-control.git/256236 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] use a hashmap to make remotes faster
On Fri, Sep 05, 2014 at 11:55:06AM +0200, Stefan Beller wrote:
struct remote {
+ struct hashmap_entry ent; /* must be first */
+
I stumbled about this comment /* must be first */
when reading the changelog.
Why does it need to be first?
Is it a common reason I'm just not aware of,
or would it make sense to put the reason into the comment as well?
Yes, it's a requirement of the hashmap code. It stores arbitrary
structs, but uses the front of the type for its bookkeeping data (the
hash and a linked list pointer to the next item in the bucket). This is
documented in Documentation/technical/api-hashmap.txt.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git rebase: yet another newbie quest.
I'm not going to say what you *should* have done, since it's not clear whether anything close to what you were doing is a supported workflow. But I can tell you what I *do* myself. Personally, I vastly distrust git pull --rebase. So in general, my pulls are all the equivalent of git pull --ff-only, and if I want to rebase the topic branch (which in general, is a bad idea to do regularly; I will generally not do it at all until I'm almost done). So I'll branch the topic branch off of origin (which tracks origin/master, typically): git checkout -b topic1 origin hack hack hack git commit . . . Then I might do something like this to do a build: git fetch origin ; git branch -f origin origin/master# this is optional git checkout -B build origin git merge topic1 git merge topic2 ... make In general, I will only rebase a topic branch when it's needed to fix a serious conflcit caused by significant changes upstream. And in that case, I might do something like this: git checkout topic1 git rebase origin/master make make check This basically goes to a philosophical question of whether it's simpler to tell users to use a single command, such as git pull --rebase, or whether to tell users to use a 2 or 3 commands that conceptually much more simple. Personally, I type fast enough that I tend to use simple commands, and not try to use things like automated branch tracking. That way I don't have to strain my brain thinking about things like fork points. :-) OTOH, some people feel that it's better to make things like git pull --rebase work and do the right thing automatically, because competing DSCM allows you to do it in a single command. And indeed, if you use git pull --rebase without any topic branches, it works fine. But then when you start wanting to do things that are more complicated, the automated command starts getting actually harder and more confusing (at least in my opinion). I don't know if a workflow involving topic branches was even expected to work with git pull --rebase, and if so, how to set things up so that they do work smoothly. All I know is that the issue never arises with me, because it's rare that I use git pull, let alone git pull --rebase. That's because I usually like to take a quick look at what I've pulled (using gitk, or git log) before do the merge operation. If I'm doing a pull from a repo that I control, and so I think I'm sure I know what's there, I might skip the git fetch, and do a git pull --ff-only instead. But in general I prefer to do the merging separate from the pull operation. Cheers, - Ted P.S. There is a separate, and completely valid discussion which is how to prevent a newbie from falling into a same trap you did. I'll defer that discussion to others... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] po/TEAMS: add new member to German translation team
Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/TEAMS | 1 + 1 file changed, 1 insertion(+) diff --git a/po/TEAMS b/po/TEAMS index 461cc14..a33a38e 100644 --- a/po/TEAMS +++ b/po/TEAMS @@ -17,6 +17,7 @@ Members: Thomas Rast t...@thomasrast.ch Christian Stimming stimm...@tuhh.de Phillip Szelat phillip.sze...@gmail.com Matthias Rüster matthias.rues...@gmail.com + Magnus Görlitz magnus.goerl...@googlemail.com Language: fr (French) Repository:https://github.com/jnavila/git -- 2.1.0.223.gc60e7ff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv2 1/2] am: add gitk patch format
Chris Packham judge.pack...@gmail.com writes: So teaching git mailinfo to do s/^// (either when asked to or using some heuristic) would be a better approach? I also think we should accept Author: as an acceptable fallback if From: is not present. Not as a fallback in the sense that Author: should not be treated any specially when am (which stands for apply mail) is operating on the patches in e-mails. Whatever wants to convert the output from log --pretty as if it came from log --pretty=email would certainly need to flip Author: to From: (what should happen when it sees From: in the input, though???), and whatgver wannts to pick metainfo from log --pretty output like mailinfo does for log --pretty=email output would certainly need to pick the authorship from Author: (without paying any attention to From: if one exists). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Translations for ambiguous refname warning
Here is a pair of commit which allow messages from git to be translated when running an ambiguous checkout such as when a branch name and a tag name are the same. Sandy Carter (2): i18n: ambiguous refname message is not translated i18n translate builtin warning, error, usage, fatal messages sha1_name.c | 6 +++--- usage.c | 8 2 files changed, 7 insertions(+), 7 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] i18n translate builtin warning, error, usage, fatal messages
Allow warnings, errors, usage and fatal messages to be translated to user
when checking out a ambiguous refname for example
Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
usage.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/usage.c b/usage.c
index ed14645..24a450e 100644
--- a/usage.c
+++ b/usage.c
@@ -27,24 +27,24 @@ void vwritef(int fd, const char *prefix, const char *err,
va_list params)
static NORETURN void usage_builtin(const char *err, va_list params)
{
- vreportf(usage: , err, params);
+ vreportf(_(usage: ), err, params);
exit(129);
}
static NORETURN void die_builtin(const char *err, va_list params)
{
- vreportf(fatal: , err, params);
+ vreportf(_(fatal: ), err, params);
exit(128);
}
static void error_builtin(const char *err, va_list params)
{
- vreportf(error: , err, params);
+ vreportf(_(error: ), err, params);
}
static void warn_builtin(const char *warn, va_list params)
{
- vreportf(warning: , warn, params);
+ vreportf(_(warning: ), warn, params);
}
static int die_is_recursing_builtin(void)
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i18n: ambiguous refname message is not translated
Allow warning message about ambuiguous refname to be exported to .pot files
when checking out a refname which is the name of a tag and of a branch for
example.
Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
sha1_name.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 63ee66f..6571287 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -434,7 +434,7 @@ static int interpret_nth_prior_checkout(const char *name,
int namelen, struct st
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
{
- static const char *warn_msg = refname '%.*s' is ambiguous.;
+ static const char *warn_msg = N_(refname '%.*s' is ambiguous.);
static const char *object_name_msg = N_(
Git normally never creates a ref that ends with 40 hex characters\n
because it will be ignored when you just specify 40-hex. These refs\n
@@ -454,7 +454,7 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
if (warn_ambiguous_refs warn_on_object_refname_ambiguity) {
refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
if (refs_found 0) {
- warning(warn_msg, len, str);
+ warning(_(warn_msg), len, str);
if (advice_object_name_warning)
fprintf(stderr, %s\n,
_(object_name_msg));
}
@@ -514,7 +514,7 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
if (warn_ambiguous_refs
(refs_found 1 ||
!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
- warning(warn_msg, len, str);
+ warning(_(warn_msg), len, str);
if (reflog_len) {
int nth, i;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i18n translate builtin warning, error, usage, fatal messages
Sandy Carter sandy.car...@savoirfairelinux.com writes:
Allow warnings, errors, usage and fatal messages to be translated to user
when checking out a ambiguous refname for example
Signed-off-by: Sandy Carter sandy.car...@savoirfairelinux.com
---
H. Doesn't this break the plumbing commands whose messages are
meant to be matched and interpreted by scripts?
usage.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/usage.c b/usage.c
index ed14645..24a450e 100644
--- a/usage.c
+++ b/usage.c
@@ -27,24 +27,24 @@ void vwritef(int fd, const char *prefix, const char *err,
va_list params)
static NORETURN void usage_builtin(const char *err, va_list params)
{
- vreportf(usage: , err, params);
+ vreportf(_(usage: ), err, params);
exit(129);
}
static NORETURN void die_builtin(const char *err, va_list params)
{
- vreportf(fatal: , err, params);
+ vreportf(_(fatal: ), err, params);
exit(128);
}
static void error_builtin(const char *err, va_list params)
{
- vreportf(error: , err, params);
+ vreportf(_(error: ), err, params);
}
static void warn_builtin(const char *warn, va_list params)
{
- vreportf(warning: , warn, params);
+ vreportf(_(warning: ), warn, params);
}
static int die_is_recursing_builtin(void)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
Am 05.09.2014 12:06, schrieb Chris Packham:
In check_patch_format we feed $1 to a block that attempts to determine
the patch format. Since we've already redirected $1 to stdin there is no
need to redirect it again when we invoke tr. This prevents the following
errors when invoking git am
$ git am patch.patch
tr: write error: Broken pipe
tr: write error
Patch format detection failed.
Cc: Stephen Boyd bebar...@gmail.com
Signed-off-by: Chris Packham judge.pack...@gmail.com
---
Nothing new since
http://article.gmane.org/gmane.comp.version-control.git/256425
git-am.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..fade7f8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,7 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with the
# header field names...
- tr -d '\015' $1 |
+ tr -d '\015' |
sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
I think this change is wrong. This pipeline checks whether one of the
lines at the top of the file contains something that looks like an email
header. With your change, the first three lines would not be looked at
because they were already consumed earlier.
I wonder why tr (assuming it is *this* instance of tr) dies with a write
error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
the tr invocation inherits this ignore SIGPIPE setting?
The only thing your version changes is that tr writes a bit less text
into the pipe. Perhaps its just sufficient that the output fits into the
pipe buffer, and no error occurs anymore? Then the new version is not a
real fix: make the patch text a bit longer, and the error is back.
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 00/22] Signed push
The first round is found at $gmane/255520. The second round is found at $gmane/255701. The third round is found at $gmane/256464. While signed tags and commits assert that the objects thusly signed came from you, who signed these objects, there is not a good way to assert that you wanted to have a particular object at the tip of a particular branch. My signing v2.0.1 tag only means I want to call the version v2.0.1, and it does not mean I want to push it out to my 'master' branch---it is likely that I only want it in 'maint', so the signature on the object alone is insufficient. The only assurance to you that 'maint' points at what I wanted to place there comes from your trust on the hosting site and my authentication with it, which cannot easily audited later. This series introduces a cryptographic assurance for ref updates done by git push by introducing a mechanism that allows you to sign a push certificate (for the lack of better name) every time you push. Think of it as working on an axis orthogonal to the traditional signed tags. In addition to typofixes in the log messages and comments in the earlier parts of the series since the last round, notable changes and additions in this round include the following: - The logic to generate nonce was revamped, based on HMAC-SHA1-80 modeled after RFC 2104, suggested in $gmane/256491 [*1*]. - Tentative --push-cert/--no-push-cert command line options to control if receive-pack expects/accepts a signed push is gone, as it is hard to arrange what goes on its command line. It is now controled by receive.certnonceseed configuration variable. If you supply an HMAC secret to be used for generating nonce, you accept git push --signed. If you don't, you don't. - The last patch is new to help stateless RPC codepath to participate in the nonce dance. I didn't feel bold enough to add smart-http tests to the last patch. Contributions are very much welcomed ;-) [Reference] *1* http://thread.gmane.org/gmane.comp.version-control.git/255520/focus=256491 Junio C Hamano (22): receive-pack: do not overallocate command structure receive-pack: parse feature request a bit earlier receive-pack: do not reuse old_sha1[] for other things receive-pack: factor out queueing of command send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher send-pack: refactor decision to send update per ref send-pack: always send capabilities send-pack: factor out capability string generation receive-pack: factor out capability string generation send-pack: rename new_refs to need_pack_data send-pack: refactor inspecting and resetting status and sending commands send-pack: clarify that cmds_sent is a boolean gpg-interface: move parse_gpg_output() to where it should be gpg-interface: move parse_signature() to where it should be pack-protocol doc: typofix for PKT-LINE push: the beginning of git push --signed receive-pack: GPG-validate push certificates send-pack: send feature request on push-cert packet signed push: remove duplicated protocol info signed push: add pushee header to push certificate signed push: fortify against replay attacks signed push: allow stale nonce in stateless mode Documentation/config.txt | 6 + Documentation/git-push.txt| 9 +- Documentation/git-receive-pack.txt| 63 +++- Documentation/technical/pack-protocol.txt | 49 ++- Documentation/technical/protocol-capabilities.txt | 13 +- builtin/push.c| 1 + builtin/receive-pack.c| 354 +++--- commit.c | 36 --- gpg-interface.c | 57 gpg-interface.h | 17 +- send-pack.c | 201 +--- send-pack.h | 2 + t/t5534-push-signed.sh| 116 +++ tag.c | 20 -- tag.h | 1 - transport.c | 5 + transport.h | 5 + 17 files changed, 799 insertions(+), 156 deletions(-) create mode 100755 t/t5534-push-signed.sh -- 2.1.0-399-g2df620b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 01/22] receive-pack: do not overallocate command structure
An update command in the protocol exchange consists of 40-hex old object name, SP, 40-hex new object name, SP, and a refname, but the first instance is further followed by a NUL with feature requests. The command structure, which has a flex-array member that stores the refname at the end, was allocated based on the whole length of the update command, without excluding the trailing feature requests. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/receive-pack.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f93ac45..1663beb 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -872,10 +872,11 @@ static struct command *read_head_info(struct sha1_array *shallow) if (parse_feature_request(feature_list, quiet)) quiet = 1; } - cmd = xcalloc(1, sizeof(struct command) + len - 80); + cmd = xcalloc(1, sizeof(struct command) + reflen + 1); hashcpy(cmd-old_sha1, old_sha1); hashcpy(cmd-new_sha1, new_sha1); - memcpy(cmd-ref_name, line + 82, len - 81); + memcpy(cmd-ref_name, refname, reflen); + cmd-ref_name[reflen] = '\0'; *p = cmd; p = cmd-next; } -- 2.1.0-399-g2df620b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/22] receive-pack: do not reuse old_sha1[] for other things
This piece of code reads object names of shallow boundaries, not
old_sha1[], i.e. the current value the ref points at, which is to be
replaced by what is in new_sha1[].
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/receive-pack.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a91eec8..c9b92bf 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -847,9 +847,11 @@ static struct command *read_head_info(struct sha1_array
*shallow)
break;
if (len == 48 starts_with(line, shallow )) {
- if (get_sha1_hex(line + 8, old_sha1))
- die(protocol error: expected shallow sha, got
'%s', line + 8);
- sha1_array_append(shallow, old_sha1);
+ unsigned char sha1[20];
+ if (get_sha1_hex(line + 8, sha1))
+ die(protocol error: expected shallow sha, got
'%s',
+ line + 8);
+ sha1_array_append(shallow, sha1);
continue;
}
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 04/22] receive-pack: factor out queueing of command
Make a helper function to accept a line of a protocol message and
queue an update command out of the code from read_head_info().
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/receive-pack.c | 50 +-
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c9b92bf..587f7da 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -831,16 +831,40 @@ static void execute_commands(struct command *commands,
the reported refs above);
}
+static struct command **queue_command(struct command **tail,
+ const char *line,
+ int linelen)
+{
+ unsigned char old_sha1[20], new_sha1[20];
+ struct command *cmd;
+ const char *refname;
+ int reflen;
+
+ if (linelen 83 ||
+ line[40] != ' ' ||
+ line[81] != ' ' ||
+ get_sha1_hex(line, old_sha1) ||
+ get_sha1_hex(line + 41, new_sha1))
+ die(protocol error: expected old/new/ref, got '%s', line);
+
+ refname = line + 82;
+ reflen = linelen - 82;
+ cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
+ hashcpy(cmd-old_sha1, old_sha1);
+ hashcpy(cmd-new_sha1, new_sha1);
+ memcpy(cmd-ref_name, refname, reflen);
+ cmd-ref_name[reflen] = '\0';
+ *tail = cmd;
+ return cmd-next;
+}
+
static struct command *read_head_info(struct sha1_array *shallow)
{
struct command *commands = NULL;
struct command **p = commands;
for (;;) {
char *line;
- unsigned char old_sha1[20], new_sha1[20];
- struct command *cmd;
- char *refname;
- int len, reflen, linelen;
+ int len, linelen;
line = packet_read_line(0, len);
if (!line)
@@ -866,23 +890,7 @@ static struct command *read_head_info(struct sha1_array
*shallow)
quiet = 1;
}
- if (linelen 83 ||
- line[40] != ' ' ||
- line[81] != ' ' ||
- get_sha1_hex(line, old_sha1) ||
- get_sha1_hex(line + 41, new_sha1))
- die(protocol error: expected old/new/ref, got '%s',
- line);
-
- refname = line + 82;
- reflen = linelen - 82;
- cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
- hashcpy(cmd-old_sha1, old_sha1);
- hashcpy(cmd-new_sha1, new_sha1);
- memcpy(cmd-ref_name, refname, reflen);
- cmd-ref_name[reflen] = '\0';
- *p = cmd;
- p = cmd-next;
+ p = queue_command(p, line, linelen);
}
return commands;
}
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/22] send-pack: always send capabilities
We tried to avoid sending one extra byte, NUL and nothing behind it
to signal there is no protocol capabilities being sent, on the first
command packet on the wire, but it just made the code look ugly.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 43e98fa..e81f741 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -281,8 +281,7 @@ int send_pack(struct send_pack_args *args,
char *new_hex = sha1_to_hex(ref-new_sha1);
int quiet = quiet_supported (args-quiet ||
!args-progress);
- if (!cmds_sent (status_report || use_sideband ||
- quiet || agent_supported)) {
+ if (!cmds_sent)
packet_buf_write(req_buf,
%s %s %s%c%s%s%s%s%s,
old_hex, new_hex, ref-name, 0,
@@ -292,7 +291,6 @@ int send_pack(struct send_pack_args *args,
agent_supported ? agent= :
,
agent_supported ?
git_user_agent_sanitized() :
);
- }
else
packet_buf_write(req_buf, %s %s %s,
old_hex, new_hex, ref-name);
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/22] receive-pack: factor out capability string generation
Similar to the previous one for send-pack, make it easier and
cleaner to add to capability advertisement.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/receive-pack.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 587f7da..cbbad54 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -137,15 +137,21 @@ static void show_ref(const char *path, const unsigned
char *sha1)
if (ref_is_hidden(path))
return;
- if (sent_capabilities)
+ if (sent_capabilities) {
packet_write(1, %s %s\n, sha1_to_hex(sha1), path);
- else
- packet_write(1, %s %s%c%s%s agent=%s\n,
-sha1_to_hex(sha1), path, 0,
- report-status delete-refs side-band-64k quiet,
-prefer_ofs_delta ? ofs-delta : ,
-git_user_agent_sanitized());
- sent_capabilities = 1;
+ } else {
+ struct strbuf cap = STRBUF_INIT;
+
+ strbuf_addstr(cap,
+ report-status delete-refs side-band-64k quiet);
+ if (prefer_ofs_delta)
+ strbuf_addstr(cap, ofs-delta);
+ strbuf_addf(cap, agent=%s, git_user_agent_sanitized());
+ packet_write(1, %s %s%c%s\n,
+sha1_to_hex(sha1), path, 0, cap.buf);
+ strbuf_release(cap);
+ sent_capabilities = 1;
+ }
}
static int show_ref_cb(const char *path, const unsigned char *sha1, int flag,
void *unused)
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 11/22] send-pack: refactor inspecting and resetting status and sending commands
The main loop over remote_refs list inspects the ref status
to see if we need to generate pack data (i.e. a delete-only push
does not need to send any additional data), resets it to expecting
the status report state, and formats the actual update commands
to be sent.
Split the former two out of the main loop, as it will become
conditional in later steps.
Besides, we should have code that does real thing here, before the
Finally, tell the other end! part ;-)
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 49 ++---
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 716c11b..6dc8a46 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -274,7 +274,8 @@ int send_pack(struct send_pack_args *args,
advertise_shallow_grafts_buf(req_buf);
/*
-* Finally, tell the other end!
+* Clear the status for each ref and see if we need to send
+* the pack data.
*/
for (ref = remote_refs; ref; ref = ref-next) {
if (!ref_update_to_be_sent(ref, args))
@@ -283,25 +284,35 @@ int send_pack(struct send_pack_args *args,
if (!ref-deletion)
need_pack_data = 1;
- if (args-dry_run) {
+ if (args-dry_run || !status_report)
ref-status = REF_STATUS_OK;
- } else {
- char *old_hex = sha1_to_hex(ref-old_sha1);
- char *new_hex = sha1_to_hex(ref-new_sha1);
-
- if (!cmds_sent)
- packet_buf_write(req_buf,
-%s %s %s%c%s,
-old_hex, new_hex, ref-name, 0,
-cap_buf.buf);
- else
- packet_buf_write(req_buf, %s %s %s,
-old_hex, new_hex, ref-name);
- ref-status = status_report ?
- REF_STATUS_EXPECTING_REPORT :
- REF_STATUS_OK;
- cmds_sent++;
- }
+ else
+ ref-status = REF_STATUS_EXPECTING_REPORT;
+ }
+
+ /*
+* Finally, tell the other end!
+*/
+ for (ref = remote_refs; ref; ref = ref-next) {
+ char *old_hex, *new_hex;
+
+ if (args-dry_run)
+ continue;
+
+ if (!ref_update_to_be_sent(ref, args))
+ continue;
+
+ old_hex = sha1_to_hex(ref-old_sha1);
+ new_hex = sha1_to_hex(ref-new_sha1);
+ if (!cmds_sent)
+ packet_buf_write(req_buf,
+%s %s %s%c%s,
+old_hex, new_hex, ref-name, 0,
+cap_buf.buf);
+ else
+ packet_buf_write(req_buf, %s %s %s,
+old_hex, new_hex, ref-name);
+ cmds_sent++;
}
if (args-stateless_rpc) {
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 10/22] send-pack: rename new_refs to need_pack_data
The variable counts how many non-deleting command is being sent, but
is only checked with 0-ness to decide if we need to send the pack
data.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 0cb44ab..716c11b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -220,7 +220,7 @@ int send_pack(struct send_pack_args *args,
struct strbuf req_buf = STRBUF_INIT;
struct strbuf cap_buf = STRBUF_INIT;
struct ref *ref;
- int new_refs;
+ int need_pack_data = 0;
int allow_deleting_refs = 0;
int status_report = 0;
int use_sideband = 0;
@@ -276,13 +276,12 @@ int send_pack(struct send_pack_args *args,
/*
* Finally, tell the other end!
*/
- new_refs = 0;
for (ref = remote_refs; ref; ref = ref-next) {
if (!ref_update_to_be_sent(ref, args))
continue;
if (!ref-deletion)
- new_refs++;
+ need_pack_data = 1;
if (args-dry_run) {
ref-status = REF_STATUS_OK;
@@ -327,7 +326,7 @@ int send_pack(struct send_pack_args *args,
in = demux.out;
}
- if (new_refs cmds_sent) {
+ if (need_pack_data cmds_sent) {
if (pack_objects(out, remote_refs, extra_have, args) 0) {
for (ref = remote_refs; ref; ref = ref-next)
ref-status = REF_STATUS_NONE;
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 06/22] send-pack: refactor decision to send update per ref
A new helper function ref_update_to_be_sent() decides for each ref
if the update is to be sent based on the status previously set by
set_ref_status_for_push() and also if this is a mirrored push.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 36 +---
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 22a1709..43e98fa 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,6 +190,26 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
for_each_commit_graft(advertise_shallow_grafts_cb, sb);
}
+static int ref_update_to_be_sent(const struct ref *ref, const struct
send_pack_args *args)
+{
+ if (!ref-peer_ref !args-send_mirror)
+ return 0;
+
+ /* Check for statuses set by set_ref_status_for_push() */
+ switch (ref-status) {
+ case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ case REF_STATUS_REJECT_FETCH_FIRST:
+ case REF_STATUS_REJECT_NEEDS_FORCE:
+ case REF_STATUS_REJECT_STALE:
+ case REF_STATUS_REJECT_NODELETE:
+ case REF_STATUS_UPTODATE:
+ return 0;
+ default:
+ return 1;
+ }
+}
+
int send_pack(struct send_pack_args *args,
int fd[], struct child_process *conn,
struct ref *remote_refs,
@@ -248,23 +268,9 @@ int send_pack(struct send_pack_args *args,
*/
new_refs = 0;
for (ref = remote_refs; ref; ref = ref-next) {
- if (!ref-peer_ref !args-send_mirror)
+ if (!ref_update_to_be_sent(ref, args))
continue;
- /* Check for statuses set by set_ref_status_for_push() */
- switch (ref-status) {
- case REF_STATUS_REJECT_NONFASTFORWARD:
- case REF_STATUS_REJECT_ALREADY_EXISTS:
- case REF_STATUS_REJECT_FETCH_FIRST:
- case REF_STATUS_REJECT_NEEDS_FORCE:
- case REF_STATUS_REJECT_STALE:
- case REF_STATUS_REJECT_NODELETE:
- case REF_STATUS_UPTODATE:
- continue;
- default:
- ; /* do nothing */
- }
-
if (!ref-deletion)
new_refs++;
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 17/22] receive-pack: GPG-validate push certificates
Reusing the GPG signature check helpers we already have, verify
the signature in receive-pack and give the results to the hooks
via GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.
Policy decisions, such as accepting or rejecting a good signature by
a key that is not fully trusted, is left to the hook and kept
outside of the core.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-receive-pack.txt | 25 -
builtin/receive-pack.c | 31 +++
t/t5534-push-signed.sh | 18 --
3 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-receive-pack.txt
b/Documentation/git-receive-pack.txt
index 9c4d17c..e6df234 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -56,7 +56,21 @@ sha1-old and sha1-new should be valid objects in the
repository.
When accepting a signed push (see linkgit:git-push[1]), the signed
push certificate is stored in a blob and an environment variable
`GIT_PUSH_CERT` can be consulted for its object name. See the
-description of `post-receive` hook for an example.
+description of `post-receive` hook for an example. In addition, the
+certificate is verified using GPG and the result is exported with
+the following environment variables:
+
+`GIT_PUSH_CERT_SIGNER`::
+ The name and the e-mail address of the owner of the key that
+ signed the push certificate.
+
+`GIT_PUSH_CERT_KEY`::
+ The GPG key ID of the key that signed the push certificate.
+
+`GIT_PUSH_CERT_STATUS`::
+ The status of GPG verification of the push certificate,
+ using the same mnemonic as used in `%G?` format of `git log`
+ family of commands (see linkgit:git-log[1]).
This hook is called before any refname is updated and before any
fast-forward checks are performed.
@@ -106,13 +120,14 @@ the update. Refs that were created will have sha1-old
equal to
0\{40}, otherwise sha1-old and sha1-new should be valid objects in
the repository.
-The `GIT_PUSH_CERT` environment variable can be inspected, just as
+The `GIT_PUSH_CERT*` environment variables can be inspected, just as
in `pre-receive` hook, after accepting a signed push.
Using this hook, it is easy to generate mails describing the updates
to the repository. This example script sends one mail message per
ref listing the commits pushed to the repository, and logs the push
-certificates of signed pushes to a file:
+certificates of signed pushes with good signatures to a logger
+service:
#!/bin/sh
# mail out commit update information.
@@ -129,11 +144,11 @@ certificates of signed pushes to a file:
mail -s Changes to ref $ref commit-list@mydomain
done
# log signed push certificate, if any
- if test -n ${GIT_PUSH_CERT-}
+ if test -n ${GIT_PUSH_CERT-} test ${GIT_PUSH_CERT_STATUS} = G
then
(
git cat-file blob ${GIT_PUSH_CERT}
- ) | mail -s push certificate push-log@mydomain
+ ) | mail -s push certificate from $GIT_PUSH_CERT_SIGNER
push-log@mydomain
fi
exit 0
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 610b085..c0a3189 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -15,6 +15,8 @@
#include connected.h
#include argv-array.h
#include version.h
+#include tag.h
+#include gpg-interface.h
static const char receive_pack_usage[] = git receive-pack git-dir;
@@ -49,6 +51,7 @@ static const char *alt_shallow_file;
static int accept_push_cert = 1;
static struct strbuf push_cert = STRBUF_INIT;
static unsigned char push_cert_sha1[20];
+static struct signature_check sigcheck;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -277,12 +280,40 @@ static void prepare_push_cert_sha1(struct child_process
*proc)
return;
if (!already_done) {
+ struct strbuf gpg_output = STRBUF_INIT;
+ struct strbuf gpg_status = STRBUF_INIT;
+ int bogs /* beginning_of_gpg_sig */;
+
already_done = 1;
if (write_sha1_file(push_cert.buf, push_cert.len, blob,
push_cert_sha1))
hashclr(push_cert_sha1);
+
+ memset(sigcheck, '\0', sizeof(sigcheck));
+ sigcheck.result = 'N';
+
+ bogs = parse_signature(push_cert.buf, push_cert.len);
+ if (verify_signed_buffer(push_cert.buf, bogs,
+push_cert.buf + bogs, push_cert.len -
bogs,
+gpg_output, gpg_status) 0) {
+ ; /* error running gpg */
+ } else {
+ sigcheck.payload = push_cert.buf;
+ sigcheck.gpg_output = gpg_output.buf;
+ sigcheck.gpg_status =
[PATCH v4 16/22] push: the beginning of git push --signed
While signed tags and commits assert that the objects thusly signed
came from you, who signed these objects, there is not a good way to
assert that you wanted to have a particular object at the tip of a
particular branch. My signing v2.0.1 tag only means I want to call
the version v2.0.1, and it does not mean I want to push it out to my
'master' branch---it is likely that I only want it in 'maint', so
the signature on the object alone is insufficient.
The only assurance to you that 'maint' points at what I wanted to
place there comes from your trust on the hosting site and my
authentication with it, which cannot easily audited later.
Introduce a mechanism that allows you to sign a push certificate
(for the lack of better name) every time you push, asserting that
what object you are pushing to update which ref that used to point
at what other object. Think of it as a cryptographic protection for
ref updates, similar to signed tags/commits but working on an
orthogonal axis.
The basic flow based on this mechanism goes like this:
1. You push out your work with git push --signed.
2. The sending side learns where the remote refs are as usual,
together with what protocol extension the receiving end
supports. If the receiving end does not advertise the protocol
extension push-cert, an attempt to git push --signed fails.
Otherwise, a text file, that looks like the following, is
prepared in core:
certificate version 0.1
pusher Junio C Hamano gits...@pobox.com 1315427886 -0700
7339ca65... 21580ecb... refs/heads/master
3793ac56... 12850bec... refs/heads/next
The file begins with a few header lines, which may grow as we
gain more experience. The 'pusher' header records the name of
the signer (the value of user.signingkey configuration variable,
falling back to GIT_COMMITTER_{NAME|EMAIL}) and the time of the
certificate generation. After the header, a blank line follows,
followed by a copy of the protocol message lines.
Each line shows the old and the new object name at the tip of
the ref this push tries to update, in the way identical to how
the underlying git push protocol exchange tells the ref
updates to the receiving end (by recording the old object
name, the push certificate also protects against replaying). It
is expected that new command packet types other than the
old-new-refname kind will be included in push certificate in the
same way as would appear in the plain vanilla command packets in
unsigned pushes.
The user then is asked to sign this push certificate using GPG,
formatted in a way similar to how signed tag objects are signed,
and the result is sent to the other side (i.e. receive-pack).
In the protocol exchange, this step comes immediately before the
sender tells what the result of the push should be, which in
turn comes before it sends the pack data.
3. When the receiving end sees a push certificate, the certificate
is written out as a blob. The pre-receive hook can learn about
the certificate by checking GIT_PUSH_CERT environment variable,
which, if present, tells the object name of this blob, and make
the decision to allow or reject this push. Additionally, the
post-receive hook can also look at the certificate, which may be
a good place to log all the received certificates for later
audits.
Because a push certificate carry the same information as the usual
command packets in the protocol exchange, we can omit the latter
when a push certificate is in use and reduce the protocol overhead.
This however is not included in this patch to make it easier to
review (in other words, the series at this step should never be
released without the remainder of the series, as it implements an
interim protocol that will be incompatible with the final one,
merely for reviewing purposes). As such, the documentation update
for the protocol is left out of this step.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 6 +++
Documentation/git-push.txt | 9 +++-
Documentation/git-receive-pack.txt | 18 +++-
builtin/push.c | 1 +
builtin/receive-pack.c | 52 +++
send-pack.c| 64
send-pack.h| 1 +
t/t5534-push-signed.sh | 85 ++
transport.c| 4 ++
transport.h| 5 +++
10 files changed, 243 insertions(+), 2 deletions(-)
create mode 100755 t/t5534-push-signed.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c55c22a..0d01e32 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2038,6 +2038,12 @@ rebase.autostash::
successful rebase might result in non-trivial conflicts.
Defaults to false.
[PATCH v4 08/22] send-pack: factor out capability string generation
A run of 'var ? var : ' fed to a long printf string in a deeply
nested block was hard to read. Move it outside the loop and format
it into a strbuf.
As an added bonus, the trick to add agent=agent-name by using
two conditionals is replaced by a more readable version.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index e81f741..0cb44ab 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -218,6 +218,7 @@ int send_pack(struct send_pack_args *args,
int in = fd[0];
int out = fd[1];
struct strbuf req_buf = STRBUF_INIT;
+ struct strbuf cap_buf = STRBUF_INIT;
struct ref *ref;
int new_refs;
int allow_deleting_refs = 0;
@@ -251,6 +252,15 @@ int send_pack(struct send_pack_args *args,
return 0;
}
+ if (status_report)
+ strbuf_addstr(cap_buf, report-status);
+ if (use_sideband)
+ strbuf_addstr(cap_buf, side-band-64k);
+ if (quiet_supported (args-quiet || !args-progress))
+ strbuf_addstr(cap_buf, quiet);
+ if (agent_supported)
+ strbuf_addf(cap_buf, agent=%s, git_user_agent_sanitized());
+
/*
* NEEDSWORK: why does delete-refs have to be so specific to
* send-pack machinery that set_ref_status_for_push() cannot
@@ -279,18 +289,12 @@ int send_pack(struct send_pack_args *args,
} else {
char *old_hex = sha1_to_hex(ref-old_sha1);
char *new_hex = sha1_to_hex(ref-new_sha1);
- int quiet = quiet_supported (args-quiet ||
!args-progress);
if (!cmds_sent)
packet_buf_write(req_buf,
-%s %s %s%c%s%s%s%s%s,
+%s %s %s%c%s,
old_hex, new_hex, ref-name, 0,
-status_report ?
report-status : ,
-use_sideband ?
side-band-64k : ,
-quiet ? quiet : ,
-agent_supported ? agent= :
,
-agent_supported ?
git_user_agent_sanitized() :
- );
+cap_buf.buf);
else
packet_buf_write(req_buf, %s %s %s,
old_hex, new_hex, ref-name);
@@ -311,6 +315,7 @@ int send_pack(struct send_pack_args *args,
packet_flush(out);
}
strbuf_release(req_buf);
+ strbuf_release(cap_buf);
if (use_sideband cmds_sent) {
memset(demux, 0, sizeof(demux));
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 20/22] signed push: add pushee header to push certificate
Record the URL of the intended recipient for a push (after
anonymizing it if it has authentication material) on a new pushee
URL header. Because the networking configuration (SSH-tunnels,
proxies, etc.) on the pushing user's side varies, the receiving
repository may not know the single canonical URL all the pushing
users would refer it as (besides, many sites allow pushing over
ssh://host/path and https://host/path protocols to the same
repository but with different local part of the path). So this
value may not be reliably used for replay-attack prevention
purposes, but this will still serve as a human readable hint to
identify the repository the certificate refers to.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/technical/pack-protocol.txt | 6 ++
send-pack.c | 5 +
send-pack.h | 1 +
transport.c | 1 +
4 files changed, 13 insertions(+)
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index 4a5c2e8..7b543dc 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -484,6 +484,7 @@ references.
push-cert = PKT-LINE(push-cert NUL capability-list LF)
PKT-LINE(certificate version 0.1 LF)
PKT-LINE(pusher SP ident LF)
+ PKT-LINE(pushee SP url LF)
PKT-LINE(LF)
*PKT-LINE(command LF)
*PKT-LINE(gpg-signature-lines LF)
@@ -527,6 +528,11 @@ Currently, the following header fields are defined:
Identify the GPG key in Human Readable Name email@address
format.
+`pushee` url::
+ The repository URL (anonymized, if the URL contains
+ authentication material) the user who ran `git push`
+ intended to push into.
+
The GPG signature lines are a detached signature for the contents
recorded in the push certificate before the signature block begins.
The detached signature is used to certify that the commands were
diff --git a/send-pack.c b/send-pack.c
index 857beb3..9c2c649 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -240,6 +240,11 @@ static int generate_push_cert(struct strbuf *req_buf,
datestamp(stamp, sizeof(stamp));
strbuf_addf(cert, certificate version 0.1\n);
strbuf_addf(cert, pusher %s %s\n, signing_key, stamp);
+ if (args-url *args-url) {
+ char *anon_url = transport_anonymize_url(args-url);
+ strbuf_addf(cert, pushee %s\n, anon_url);
+ free(anon_url);
+ }
strbuf_addstr(cert, \n);
for (ref = remote_refs; ref; ref = ref-next) {
diff --git a/send-pack.h b/send-pack.h
index 3555d8e..5635457 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -2,6 +2,7 @@
#define SEND_PACK_H
struct send_pack_args {
+ const char *url;
unsigned verbose:1,
quiet:1,
porcelain:1,
diff --git a/transport.c b/transport.c
index 07fdf86..1df1375 100644
--- a/transport.c
+++ b/transport.c
@@ -827,6 +827,7 @@ static int git_transport_push(struct transport *transport,
struct ref *remote_re
args.dry_run = !!(flags TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags TRANSPORT_PUSH_PORCELAIN);
args.push_cert = !!(flags TRANSPORT_PUSH_CERT);
+ args.url = transport-url;
ret = send_pack(args, data-fd, data-conn, remote_refs,
data-extra_have);
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 22/22] signed push: allow stale nonce in stateless mode
When operating with the stateless RPC mode, we will receive a nonce
issued by another instance of us that advertised our capability and
refs some time ago. Update the logic to check received nonce to
detect this case, compute how much time has passed since the nonce
was issued and report the status with a new environment variable
GIT_PUSH_CERT_NONCE_SLOP to the hooks.
GIT_PUSH_CERT_NONCE_STATUS will report SLOP in such a case. The
hooks are free to decide how large a slop it is willing to accept.
Strictly speaking, the nonce is not really a nonce anymore in
the stateless RPC mode, as it will happily take any nonce issued
by it (which is protected by HMAC and its secret key) as long as it
is fresh enough. The degree of this security degradation, relative
to the native protocol, is about the same as the we make sure that
the 'git push' decided to update our refs with new objects based on
the freshest observation of our refs by making sure the values they
claim the original value of the refs they ask us to update exactly
match the current state security is loosened to accomodate the
stateless RPC mode in the existing code without this series, so
there is no need for those who are already using smart HTTP to push
to their repositories to be alarmed any more than they already are.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-receive-pack.txt | 11
builtin/receive-pack.c | 57 --
2 files changed, 59 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-receive-pack.txt
b/Documentation/git-receive-pack.txt
index 2d4b452..2e5131d 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -89,6 +89,17 @@ the following environment variables:
git push --signed sent a bogus nonce.
`OK`;;
git push --signed sent the nonce we asked it to send.
+`SLOP`;;
+ git push --signed sent a nonce different from what we
+ asked it to send now, but in a previous session. See
+ `GIT_PUSH_CERT_NONCE_SLOP` environment variable.
+
+`GIT_PUSH_CERT_NONCE_SLOP`::
+ git push --signed sent a nonce different from what we
+ asked it to send now, but in a different session whose
+ starting time is different by this many seconds from the
+ current session. Only meaningful when
+ `GIT_PUSH_CERT_NONCE_STATUS` says `SLOP`.
This hook is called before any refname is updated and before any
fast-forward checks are performed.
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a1823e5..86fb5a4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -43,6 +43,8 @@ static int prefer_ofs_delta = 1;
static int auto_update_server_info;
static int auto_gc = 1;
static int fix_thin = 1;
+static int stateless_rpc;
+static const char *service_dir;
static const char *head_name;
static void *head_name_to_free;
static int sent_capabilities;
@@ -58,7 +60,9 @@ static const char *NONCE_UNSOLICITED = UNSOLICITED;
static const char *NONCE_BAD = BAD;
static const char *NONCE_MISSING = MISSING;
static const char *NONCE_OK = OK;
+static const char *NONCE_SLOP = SLOP;
static const char *nonce_status;
+static long nonce_stamp_slop;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -359,6 +363,8 @@ static const char *find_header(const char *msg, size_t len,
const char *key)
static const char *check_nonce(const char *buf, size_t len)
{
const char *nonce = find_header(buf, len, nonce);
+ unsigned long stamp, ostamp;
+ char *bohmac, *expect;
if (!nonce)
return NONCE_MISSING;
@@ -368,7 +374,39 @@ static const char *check_nonce(const char *buf, size_t len)
return NONCE_OK;
/* returned nonce MUST match what we gave out earlier */
- return NONCE_BAD;
+ if (!stateless_rpc)
+ return NONCE_BAD;
+
+ /*
+* In stateless mode, we may be receiving a nonce issued
+* by another instance of the server that serving the same
+* repository, and the timestamps may not match, but the
+* nonce-seed and dir should match, so we can recompute
+* and report the time slop.
+*/
+
+ /* nonce is concat(seconds-since-epoch, -, hmac) */
+ if (*nonce = '0' || '9' *nonce)
+ return NONCE_BAD;
+ stamp = strtoul(nonce, bohmac, 10);
+ if (bohmac == nonce || bohmac[1] != '-')
+ return NONCE_BAD;
+
+ expect = prepare_push_cert_nonce(service_dir, stamp);
+ if (strcmp(expect, nonce)) {
+ free(expect);
+ return NONCE_BAD;
+ }
+ free(expect);
+
+ /*
+* By how many seconds is this nonce stale? Negative
+* value would mean it was issued by another server
+* with its clock skewed in the future.
+*/
+ ostamp = strtoul(push_cert_nonce, NULL, 10);
+
[PATCH v4 12/22] send-pack: clarify that cmds_sent is a boolean
We use it to make sure that the feature request is sent only once on
the very first request packet (ignoring the shallow line, which
was an unfortunate mistake we cannot retroactively fix with existing
receive-pack already deployed in the field) and we set it to true
with cmds_sent++, not because we care about the actual number of
updates sent but because it is merely an idiomatic way.
Set it explicitly to one to clarify that the code that uses this
variable only cares about its zero-ness.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 6dc8a46..bb13599 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -304,15 +304,16 @@ int send_pack(struct send_pack_args *args,
old_hex = sha1_to_hex(ref-old_sha1);
new_hex = sha1_to_hex(ref-new_sha1);
- if (!cmds_sent)
+ if (!cmds_sent) {
packet_buf_write(req_buf,
%s %s %s%c%s,
old_hex, new_hex, ref-name, 0,
cap_buf.buf);
- else
+ cmds_sent = 1;
+ } else {
packet_buf_write(req_buf, %s %s %s,
old_hex, new_hex, ref-name);
- cmds_sent++;
+ }
}
if (args-stateless_rpc) {
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 02/22] receive-pack: parse feature request a bit earlier
Ideally, we should have also allowed the first shallow to carry
the feature request trailer, but that is water under the bridge
now. This makes the next step to factor out the queuing of commands
easier to review.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/receive-pack.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1663beb..a91eec8 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -840,7 +840,7 @@ static struct command *read_head_info(struct sha1_array
*shallow)
unsigned char old_sha1[20], new_sha1[20];
struct command *cmd;
char *refname;
- int len, reflen;
+ int len, reflen, linelen;
line = packet_read_line(0, len);
if (!line)
@@ -853,7 +853,18 @@ static struct command *read_head_info(struct sha1_array
*shallow)
continue;
}
- if (len 83 ||
+ linelen = strlen(line);
+ if (linelen len) {
+ const char *feature_list = line + linelen + 1;
+ if (parse_feature_request(feature_list,
report-status))
+ report_status = 1;
+ if (parse_feature_request(feature_list,
side-band-64k))
+ use_sideband = LARGE_PACKET_MAX;
+ if (parse_feature_request(feature_list, quiet))
+ quiet = 1;
+ }
+
+ if (linelen 83 ||
line[40] != ' ' ||
line[81] != ' ' ||
get_sha1_hex(line, old_sha1) ||
@@ -862,16 +873,7 @@ static struct command *read_head_info(struct sha1_array
*shallow)
line);
refname = line + 82;
- reflen = strlen(refname);
- if (reflen + 82 len) {
- const char *feature_list = refname + reflen + 1;
- if (parse_feature_request(feature_list,
report-status))
- report_status = 1;
- if (parse_feature_request(feature_list,
side-band-64k))
- use_sideband = LARGE_PACKET_MAX;
- if (parse_feature_request(feature_list, quiet))
- quiet = 1;
- }
+ reflen = linelen - 82;
cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
hashcpy(cmd-old_sha1, old_sha1);
hashcpy(cmd-new_sha1, new_sha1);
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 13/22] gpg-interface: move parse_gpg_output() to where it should be
Earlier, ffb6d7d5 (Move commit GPG signature verification to
commit.c, 2013-03-31) moved this helper that used to be in pretty.c
(i.e. the output code path) to commit.c for better reusability.
It was a good first step in the right direction, but still suffers
from a myopic view that commits will be the only thing we would ever
want to sign---we would actually want to be able to reuse it even
wider.
The function interprets what GPG said; gpg-interface is obviously a
better place. Move it there.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
commit.c| 36
gpg-interface.c | 36
gpg-interface.h | 16 +++-
3 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/commit.c b/commit.c
index ae7f2b1..01cdad2 100644
--- a/commit.c
+++ b/commit.c
@@ -1220,42 +1220,6 @@ free_return:
free(buf);
}
-static struct {
- char result;
- const char *check;
-} sigcheck_gpg_status[] = {
- { 'G', \n[GNUPG:] GOODSIG },
- { 'B', \n[GNUPG:] BADSIG },
- { 'U', \n[GNUPG:] TRUST_NEVER },
- { 'U', \n[GNUPG:] TRUST_UNDEFINED },
-};
-
-static void parse_gpg_output(struct signature_check *sigc)
-{
- const char *buf = sigc-gpg_status;
- int i;
-
- /* Iterate over all search strings */
- for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
- const char *found, *next;
-
- if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1,
found)) {
- found = strstr(buf, sigcheck_gpg_status[i].check);
- if (!found)
- continue;
- found += strlen(sigcheck_gpg_status[i].check);
- }
- sigc-result = sigcheck_gpg_status[i].result;
- /* The trust messages are not followed by key/signer
information */
- if (sigc-result != 'U') {
- sigc-key = xmemdupz(found, 16);
- found += 17;
- next = strchrnul(found, '\n');
- sigc-signer = xmemdupz(found, next - found);
- }
- }
-}
-
void check_commit_signature(const struct commit* commit, struct
signature_check *sigc)
{
struct strbuf payload = STRBUF_INIT;
diff --git a/gpg-interface.c b/gpg-interface.c
index ff07012..3c9624c 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -21,6 +21,42 @@ void signature_check_clear(struct signature_check *sigc)
sigc-key = NULL;
}
+static struct {
+ char result;
+ const char *check;
+} sigcheck_gpg_status[] = {
+ { 'G', \n[GNUPG:] GOODSIG },
+ { 'B', \n[GNUPG:] BADSIG },
+ { 'U', \n[GNUPG:] TRUST_NEVER },
+ { 'U', \n[GNUPG:] TRUST_UNDEFINED },
+};
+
+void parse_gpg_output(struct signature_check *sigc)
+{
+ const char *buf = sigc-gpg_status;
+ int i;
+
+ /* Iterate over all search strings */
+ for (i = 0; i ARRAY_SIZE(sigcheck_gpg_status); i++) {
+ const char *found, *next;
+
+ if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1,
found)) {
+ found = strstr(buf, sigcheck_gpg_status[i].check);
+ if (!found)
+ continue;
+ found += strlen(sigcheck_gpg_status[i].check);
+ }
+ sigc-result = sigcheck_gpg_status[i].result;
+ /* The trust messages are not followed by key/signer
information */
+ if (sigc-result != 'U') {
+ sigc-key = xmemdupz(found, 16);
+ found += 17;
+ next = strchrnul(found, '\n');
+ sigc-signer = xmemdupz(found, next - found);
+ }
+ }
+}
+
void set_signing_key(const char *key)
{
free(configured_signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index 37c23da..82493b7 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -5,16 +5,22 @@ struct signature_check {
char *payload;
char *gpg_output;
char *gpg_status;
- char result; /* 0 (not checked),
- * N (checked but no further result),
- * U (untrusted good),
- * G (good)
- * B (bad) */
+
+ /*
+* possible result:
+* 0 (not checked)
+* N (checked but no further result)
+* U (untrusted good)
+* G (good)
+* B (bad)
+*/
+ char result;
char *signer;
char *key;
};
extern void signature_check_clear(struct signature_check *sigc);
+extern void parse_gpg_output(struct signature_check *);
extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const
char *signing_key);
extern int verify_signed_buffer(const char *payload, size_t payload_size,
const char *signature, size_t
[PATCH v4 05/22] send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
20e8b465 (refactor ref status logic for pushing, 2010-01-08)
restructured the code to set status for each ref to be pushed, but
did not quite go far enough. We inspect the status set earlier by
set_refs_status_for_push() and then perform yet another update to
the status of a ref with an otherwise OK status to be deleted to
mark it with REF_STATUS_REJECT_NODELETE when the protocol tells us
never to delete.
Split the latter into a separate loop that comes before we enter the
per-ref loop. This way we would have one less condition to check in
the main loop.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 6129b0f..22a1709 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -231,6 +231,15 @@ int send_pack(struct send_pack_args *args,
return 0;
}
+ /*
+* NEEDSWORK: why does delete-refs have to be so specific to
+* send-pack machinery that set_ref_status_for_push() cannot
+* set this bit for us???
+*/
+ for (ref = remote_refs; ref; ref = ref-next)
+ if (ref-deletion !allow_deleting_refs)
+ ref-status = REF_STATUS_REJECT_NODELETE;
+
if (!args-dry_run)
advertise_shallow_grafts_buf(req_buf);
@@ -249,17 +258,13 @@ int send_pack(struct send_pack_args *args,
case REF_STATUS_REJECT_FETCH_FIRST:
case REF_STATUS_REJECT_NEEDS_FORCE:
case REF_STATUS_REJECT_STALE:
+ case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_UPTODATE:
continue;
default:
; /* do nothing */
}
- if (ref-deletion !allow_deleting_refs) {
- ref-status = REF_STATUS_REJECT_NODELETE;
- continue;
- }
-
if (!ref-deletion)
new_refs++;
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 19/22] signed push: remove duplicated protocol info
With the interim protocol, we used to send the update commands even though we already send a signed copy of the same information when push certificate is in use. Update the send-pack/receive-pack pair not to do so. The notable thing on the receive-pack side is that it makes sure that there is no command sent over the traditional protocol packet outside the push certificate. Otherwise a pusher can claim to be pushing one set of ref updates in the signed certificate while issuing commands to update unrelated refs, and such an update will evade later audits. Finally, start documenting the protocol. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/technical/pack-protocol.txt | 33 ++- Documentation/technical/protocol-capabilities.txt | 12 +++-- builtin/receive-pack.c| 26 ++ send-pack.c | 2 +- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index a845d51..4a5c2e8 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -465,7 +465,7 @@ contain all the objects that the server will need to complete the new references. - update-request= *shallow command-list [pack-file] + update-request= *shallow ( command-list | push-cert ) [pack-file] shallow = PKT-LINE(shallow SP obj-id) @@ -481,12 +481,25 @@ references. old-id= obj-id new-id= obj-id + push-cert = PKT-LINE(push-cert NUL capability-list LF) + PKT-LINE(certificate version 0.1 LF) + PKT-LINE(pusher SP ident LF) + PKT-LINE(LF) + *PKT-LINE(command LF) + *PKT-LINE(gpg-signature-lines LF) + PKT-LINE(push-cert-end LF) + pack-file = PACK 28*(OCTET) If the receiving end does not support delete-refs, the sending end MUST NOT ask for delete command. +If the receiving end does not support push-cert, the sending end +MUST NOT send a push-cert command. When a push-cert command is +sent, command-list MUST NOT be sent; the commands recorded in the +push certificate is used instead. + The pack-file MUST NOT be sent if the only command used is 'delete'. A pack-file MUST be sent if either create or update command is used, @@ -501,6 +514,24 @@ was being processed (the obj-id is still the same as the old-id), and it will run any update hooks to make sure that the update is acceptable. If all of that is fine, the server will then update the references. +Push Certificate + + +A push certificate begins with a set of header lines. After the +header and an empty line, the protocol commands follow, one per +line. + +Currently, the following header fields are defined: + +`pusher` ident:: + Identify the GPG key in Human Readable Name email@address + format. + +The GPG signature lines are a detached signature for the contents +recorded in the push certificate before the signature block begins. +The detached signature is used to certify that the commands were +given by the pusher, who must be the signer. + Report Status - diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index e174343..a478cc4 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -18,8 +18,8 @@ was sent. Server MUST NOT ignore capabilities that client requested and server advertised. As a consequence of these rules, server MUST NOT advertise capabilities it does not understand. -The 'report-status', 'delete-refs', and 'quiet' capabilities are sent and -recognized by the receive-pack (push to server) process. +The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities +are sent and recognized by the receive-pack (push to server) process. The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized by both upload-pack and receive-pack protocols. The 'agent' capability @@ -250,3 +250,11 @@ allow-tip-sha1-in-want If the upload-pack server advertises this capability, fetch-pack may send want lines with SHA-1s that exist at the server but are not advertised by upload-pack. + +push-cert +- + +The receive-pack server that advertises this capability is willing +to accept a signed push certificate. A send-pack client MUST NOT +send a push-cert packet unless the receive-pack server advertises +this capability. diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c0a3189..431af39 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -926,6 +926,28 @@ static struct command **queue_command(struct command **tail, return cmd-next; } +static void
[PATCH v4 21/22] signed push: fortify against replay attacks
In order to prevent a valid push certificate for pushing into an
repository from getting replayed to push to an unrelated one, send a
nonce string from the receive-pack process and have the signer
include it in the push certificate. The receiving end uses an HMAC
hash of the path to the repository it serves and the current time,
hashed with a secret key (this does not have to be per-repository
but can be defined in /etc/gitconfig) to ensure that a random third
party cannot forge a nonce that looks like it originated from it.
The original nonce is exported as GIT_PUSH_CERT_NONCE for the hooks
to examine and match against the value on the nonce header in the
certificate to notice a replay. Returned nonce header is examined
and the validation state is exported as GIT_PUSH_CERT_NONCE_STATUS
to the hooks.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 12 +--
Documentation/git-receive-pack.txt| 19
Documentation/technical/pack-protocol.txt | 6 ++
Documentation/technical/protocol-capabilities.txt | 7 +-
builtin/receive-pack.c| 123 --
send-pack.c | 18 +++-
t/t5534-push-signed.sh| 20 ++--
7 files changed, 176 insertions(+), 29 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0d01e32..dd6fd65 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2038,17 +2038,17 @@ rebase.autostash::
successful rebase might result in non-trivial conflicts.
Defaults to false.
-receive.acceptpushcert::
- By default, `git receive-pack` will advertise that it
- accepts `git push --signed`. Setting this variable to
- false disables it (this is a tentative variable that
- will go away at the end of this series).
-
receive.autogc::
By default, git-receive-pack will run git-gc --auto after
receiving data from git-push and updating refs. You can stop
it by setting this variable to false.
+receive.certnonceseed::
+ By setting this variable to a string, `git receive-pack`
+ will accept a `git push --signed` and verifies it by using
+ a nonce protected by HMAC using this string as a secret
+ key.
+
receive.fsckObjects::
If it is set to true, git-receive-pack will check all received
objects. It will abort in the case of a malformed object or a
diff --git a/Documentation/git-receive-pack.txt
b/Documentation/git-receive-pack.txt
index e6df234..2d4b452 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -72,6 +72,24 @@ the following environment variables:
using the same mnemonic as used in `%G?` format of `git log`
family of commands (see linkgit:git-log[1]).
+`GIT_PUSH_CERT_NONCE`::
+ The nonce string the process asked the signer to include
+ in the push certificate. If this does not match the value
+ recorded on the nonce header in the push certificate, it
+ may indicate that the certificate is a valid one that is
+ being replayed from a separate git push session.
+
+`GIT_PUSH_CERT_NONCE_STATUS`::
+`UNSOLICITED`;;
+ git push --signed sent a nonce when we did not ask it to
+ send one.
+`MISSING`;;
+ git push --signed did not send any nonce header.
+`BAD`;;
+ git push --signed sent a bogus nonce.
+`OK`;;
+ git push --signed sent the nonce we asked it to send.
+
This hook is called before any refname is updated and before any
fast-forward checks are performed.
@@ -147,6 +165,7 @@ service:
if test -n ${GIT_PUSH_CERT-} test ${GIT_PUSH_CERT_STATUS} = G
then
(
+ echo expected nonce is ${GIT_PUSH_NONCE}
git cat-file blob ${GIT_PUSH_CERT}
) | mail -s push certificate from $GIT_PUSH_CERT_SIGNER
push-log@mydomain
fi
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index 7b543dc..dda1206 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -485,6 +485,7 @@ references.
PKT-LINE(certificate version 0.1 LF)
PKT-LINE(pusher SP ident LF)
PKT-LINE(pushee SP url LF)
+ PKT-LINE(nonce SP nonce LF)
PKT-LINE(LF)
*PKT-LINE(command LF)
*PKT-LINE(gpg-signature-lines LF)
@@ -533,6 +534,11 @@ Currently, the following header fields are defined:
authentication material) the user who ran `git push`
intended to push into.
+`nonce` nonce::
+ The 'nonce' string the receiving repository asked the
+ pushing user to include in the certificate, to prevent
+ replay attacks.
+
The GPG signature
[PATCH v4 18/22] send-pack: send feature request on push-cert packet
We would want to update the interim protocol so that we do not send
the usual update commands when the push certificate feature is in
use, as the same information is in the certificate. Once that
happens, the push-cert packet may become the only protocol command,
but then there is no packet to put the feature request behind, like
we always did.
As we have prepared the receiving end that understands the push-cert
feature to accept the feature request on the first protocol packet
(other than shallow , which was an unfortunate historical mistake
that has to come before everything else), we can give the feature
request on the push-cert packet instead of the first update protocol
packet, in preparation for the next step to actually update to the
final protocol.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
send-pack.c| 13 -
t/t5534-push-signed.sh | 13 +
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index ef93f33..d392f5b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -225,9 +225,10 @@ static const char *next_line(const char *line, size_t len)
return nl + 1;
}
-static void generate_push_cert(struct strbuf *req_buf,
- const struct ref *remote_refs,
- struct send_pack_args *args)
+static int generate_push_cert(struct strbuf *req_buf,
+ const struct ref *remote_refs,
+ struct send_pack_args *args,
+ const char *cap_string)
{
const struct ref *ref;
char stamp[60];
@@ -256,7 +257,7 @@ static void generate_push_cert(struct strbuf *req_buf,
if (sign_buffer(cert, cert, signing_key))
die(_(failed to sign the push certificate));
- packet_buf_write(req_buf, push-cert\n);
+ packet_buf_write(req_buf, push-cert%c%s, 0, cap_string);
for (cp = cert.buf; cp cert.buf + cert.len; cp = np) {
np = next_line(cp, cert.buf + cert.len - cp);
packet_buf_write(req_buf,
@@ -267,6 +268,7 @@ static void generate_push_cert(struct strbuf *req_buf,
free_return:
free(signing_key);
strbuf_release(cert);
+ return update_seen;
}
int send_pack(struct send_pack_args *args,
@@ -335,7 +337,8 @@ int send_pack(struct send_pack_args *args,
advertise_shallow_grafts_buf(req_buf);
if (!args-dry_run args-push_cert)
- generate_push_cert(req_buf, remote_refs, args);
+ cmds_sent = generate_push_cert(req_buf, remote_refs, args,
+ cap_buf.buf);
/*
* Clear the status for each ref and see if we need to send
diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
index ad004d4..e664110 100755
--- a/t/t5534-push-signed.sh
+++ b/t/t5534-push-signed.sh
@@ -67,6 +67,19 @@ test_expect_success 'push --sign fails with a receiver
without push certificate
test_i18ngrep the receiving end does not support err
'
+test_expect_success GPG 'no certificate for a signed push with no update' '
+ prepare_dst
+ mkdir -p dst/.git/hooks
+ write_script dst/.git/hooks/post-receive -\EOF
+ if test -n ${GIT_PUSH_CERT-}
+ then
+ git cat-file blob $GIT_PUSH_CERT ../push-cert
+ fi
+ EOF
+ git push dst noop
+ ! test -f dst/push-cert
+'
+
test_expect_success GPG 'signed push sends push certificate' '
prepare_dst
mkdir -p dst/.git/hooks
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 15/22] pack-protocol doc: typofix for PKT-LINE
Everywhere else we use PKT-LINE to denote the pkt-line formatted data, but shallow/deepen messages are described with PKT_LINE(). Fix them. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/technical/pack-protocol.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 18dea8d..a845d51 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -212,9 +212,9 @@ out of what the server said it could do with the first 'want' line. want-list = first-want *additional-want - shallow-line = PKT_LINE(shallow SP obj-id) + shallow-line = PKT-LINE(shallow SP obj-id) - depth-request = PKT_LINE(deepen SP depth) + depth-request = PKT-LINE(deepen SP depth) first-want= PKT-LINE(want SP obj-id SP capability-list LF) additional-want = PKT-LINE(want SP obj-id LF) -- 2.1.0-399-g2df620b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 14/22] gpg-interface: move parse_signature() to where it should be
Our signed-tag objects set the standard format used by Git to store
GPG-signed payload (i.e. the payload followed by its detached
signature) [*1*], and it made sense to have a helper to find the
boundary between the payload and its signature in tag.c back then.
Newer code added later to parse other kinds of objects that learned
to use the same format to store GPG-signed payload (e.g. signed
commits), however, kept using the helper from the same location.
Move it to gpg-interface; the helper is no longer about signed tag,
but it is how our code and data interact with GPG.
[Reference]
*1* http://thread.gmane.org/gmane.linux.kernel/297998/focus=1383
Signed-off-by: Junio C Hamano gits...@pobox.com
---
gpg-interface.c | 21 +
gpg-interface.h | 1 +
tag.c | 20
tag.h | 1 -
4 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 3c9624c..0dd11ea 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -7,6 +7,9 @@
static char *configured_signing_key;
static const char *gpg_program = gpg;
+#define PGP_SIGNATURE -BEGIN PGP SIGNATURE-
+#define PGP_MESSAGE -BEGIN PGP MESSAGE-
+
void signature_check_clear(struct signature_check *sigc)
{
free(sigc-payload);
@@ -57,6 +60,24 @@ void parse_gpg_output(struct signature_check *sigc)
}
}
+/*
+ * Look at GPG signed content (e.g. a signed tag object), whose
+ * payload is followed by a detached signature on it. Return the
+ * offset where the embedded detached signature begins, or the end of
+ * the data when there is no such signature.
+ */
+size_t parse_signature(const char *buf, unsigned long size)
+{
+ char *eol;
+ size_t len = 0;
+ while (len size !starts_with(buf + len, PGP_SIGNATURE)
+ !starts_with(buf + len, PGP_MESSAGE)) {
+ eol = memchr(buf + len, '\n', size - len);
+ len += eol ? eol - (buf + len) + 1 : size - len;
+ }
+ return len;
+}
+
void set_signing_key(const char *key)
{
free(configured_signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index 82493b7..87a4f2e 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -20,6 +20,7 @@ struct signature_check {
};
extern void signature_check_clear(struct signature_check *sigc);
+extern size_t parse_signature(const char *buf, unsigned long size);
extern void parse_gpg_output(struct signature_check *);
extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const
char *signing_key);
extern int verify_signed_buffer(const char *payload, size_t payload_size,
const char *signature, size_t signature_size, struct strbuf *gpg_output, struct
strbuf *gpg_status);
diff --git a/tag.c b/tag.c
index 82d841b..5b0ac62 100644
--- a/tag.c
+++ b/tag.c
@@ -4,9 +4,6 @@
#include tree.h
#include blob.h
-#define PGP_SIGNATURE -BEGIN PGP SIGNATURE-
-#define PGP_MESSAGE -BEGIN PGP MESSAGE-
-
const char *tag_type = tag;
struct object *deref_tag(struct object *o, const char *warn, int warnlen)
@@ -143,20 +140,3 @@ int parse_tag(struct tag *item)
free(data);
return ret;
}
-
-/*
- * Look at a signed tag object, and return the offset where
- * the embedded detached signature begins, or the end of the
- * data when there is no such signature.
- */
-size_t parse_signature(const char *buf, unsigned long size)
-{
- char *eol;
- size_t len = 0;
- while (len size !starts_with(buf + len, PGP_SIGNATURE)
- !starts_with(buf + len, PGP_MESSAGE)) {
- eol = memchr(buf + len, '\n', size - len);
- len += eol ? eol - (buf + len) + 1 : size - len;
- }
- return len;
-}
diff --git a/tag.h b/tag.h
index bc8a1e4..f4580ae 100644
--- a/tag.h
+++ b/tag.h
@@ -17,6 +17,5 @@ extern int parse_tag_buffer(struct tag *item, const void
*data, unsigned long si
extern int parse_tag(struct tag *item);
extern struct object *deref_tag(struct object *, const char *, int);
extern struct object *deref_tag_noverify(struct object *);
-extern size_t parse_signature(const char *buf, unsigned long size);
#endif /* TAG_H */
--
2.1.0-399-g2df620b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option
Hi Johan, Johan Herland writes: A colleague of mine noticed that cherry-pick does not accept the --no-verify option to skip running the pre-commit/commit-msg hooks. neither git-cherry-pick nor git-revert execute the pre-commit or commit-msg hooks at the moment. The underlying rationale can be found in the log message of commit 9fa4db5 (Do not verify reverted/cherry-picked/rebased patches.). Indeed, the sequencer uses git-commit internally which executes the two verify hooks by default. However, the particular command line being used implicitly specifies the --no-verify option. This behaviour is implemented in sequencer.c#run_git_commit as well, right before the configurable git-commit options are handled. I guess that's easily overlooked since the documentation doesn't mention it and the implementation uses the short version -n of --no-verify. The reasons why the new test cases succeed nonetheless are manifold. I hope they're still understandable even though I don't put the comments next to the code. The revert with failing hook test case fails if run in isolation, which can be achieved by using the very cool --run option of test-lib. More specifically, git-revert does not fail because it executes the failing hook but because the preceding test case leaves behind an uncommitted index. In the cherry-pick with failing hook test case, git-cherry-pick really fails because it doesn't know the --no-verify option yet, which presumably ended up there only by accident. This test case is meaningless if run in isolation because it assumes that revert with failing hook creates a commit (else HEAD^ points nowhere). I like your patchset for that it makes it explicit in both the documentation and the tests whether the commits resulting from cherry-picks are being verified or not. Kind regards, Fabian Here's a first attempt at adding --no-verify to the revert/cherry-pick. Have fun! :) ...Johan Johan Herland (3): t7503/4: Add failing testcases for revert/cherry-pick --no-verify revert/cherry-pick: Add --no-verify option, and pass it on to commit revert/cherry-pick --no-verify: Update documentation Documentation/git-cherry-pick.txt | 4 Documentation/git-revert.txt | 4 Documentation/githooks.txt| 20 ++-- builtin/revert.c | 1 + sequencer.c | 7 +++ sequencer.h | 1 + t/t7503-pre-commit-hook.sh| 24 t/t7504-commit-msg-hook.sh| 24 8 files changed, 75 insertions(+), 10 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt j...@kdbg.org wrote:
Am 05.09.2014 12:06, schrieb Chris Packham:
In check_patch_format we feed $1 to a block that attempts to determine
the patch format. Since we've already redirected $1 to stdin there is no
need to redirect it again when we invoke tr. This prevents the following
errors when invoking git am
$ git am patch.patch
tr: write error: Broken pipe
tr: write error
Patch format detection failed.
Cc: Stephen Boyd bebar...@gmail.com
Signed-off-by: Chris Packham judge.pack...@gmail.com
---
Nothing new since
http://article.gmane.org/gmane.comp.version-control.git/256425
git-am.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..fade7f8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,7 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with the
# header field names...
- tr -d '\015' $1 |
+ tr -d '\015' |
sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
I think this change is wrong. This pipeline checks whether one of the
lines at the top of the file contains something that looks like an email
header. With your change, the first three lines would not be looked at
because they were already consumed earlier.
I wonder why tr (assuming it is *this* instance of tr) dies with a write
error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
the tr invocation inherits this ignore SIGPIPE setting?
The only thing your version changes is that tr writes a bit less text
into the pipe. Perhaps its just sufficient that the output fits into the
pipe buffer, and no error occurs anymore? Then the new version is not a
real fix: make the patch text a bit longer, and the error is back.
-- Hannes
I did notice some oddities when attempting to reproduce this issue.
They would be explained by the output fitting into the buffer. So yes
perhaps this solution has just changed enough so that it no longer
triggers on the particular patch I was testing with. It still seems a
bit funny that we start re-reading the input part way through
processing it.
Perhaps putting the tr outside the whole block would be a better solution?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv2 1/2] am: add gitk patch format
On Sat, Sep 6, 2014 at 6:29 AM, Junio C Hamano gits...@pobox.com wrote: Chris Packham judge.pack...@gmail.com writes: So teaching git mailinfo to do s/^// (either when asked to or using some heuristic) would be a better approach? I also think we should accept Author: as an acceptable fallback if From: is not present. Not as a fallback in the sense that Author: should not be treated any specially when am (which stands for apply mail) is\ operating on the patches in e-mails. I was proposing we avoid the Patch does not have a valid e-mail address. error when we can dwim and determine the email address from Author:. I originally was going to say From: should take precedence but it would be another way to indicate that the true author is not necessarily the person who sent the email. Whatever wants to convert the output from log --pretty as if it came from log --pretty=email would certainly need to flip Author: to From: (what should happen when it sees From: in the input, though???), and whatgver wannts to pick metainfo from log --pretty output like mailinfo does for log --pretty=email output would certainly need to pick the authorship from Author: (without paying any attention to From: if one exists). Wow. I didn't know --pretty=email existed. Better yet it works for diff-tree so gitk should probably be using that to produce something that can be exported/imported easily. I do wonder what the original use-case for write commit to file was. Once it's been written to a file what is one supposed to do with it? It's not something that 'git am' can consume (currently). Using 'git apply' or 'patch' would lose the commit message plus you have to manually stage/commit the changes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
Chris Packham judge.pack...@gmail.com writes: On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt j...@kdbg.org wrote: Am 05.09.2014 12:06, schrieb Chris Packham: In check_patch_format we feed $1 to a block that attempts to determine the patch format. Since we've already redirected $1 to stdin there is no need to redirect it again when we invoke tr. This prevents the following errors when invoking git am $ git am patch.patch tr: write error: Broken pipe tr: write error Patch format detection failed. ... I wonder why tr (assuming it is *this* instance of tr) dies with a write error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then the tr invocation inherits this ignore SIGPIPE setting? ... Perhaps putting the tr outside the whole block would be a better solution? Perhaps fixing the root cause of your (but not other people's) tr failing is the right solution, no? Also, - tr -d '\015' $1 | sed -n -e '/^$/q' -e '/^[ ]/d' -e p | sane_egrep -v '^[!-9;-~]+:' /dev/null || patch_format=mbox as the tr is at an upsteam of this pipeline, it does not really matter to the outcome if it gives a write-error error message or not (the downstream sane_egrep would have decided, based on the data it was given, if the payload was mbox format), so... An easier workaround may be to update the sed script downstream of tr. It stops reading as soon as it finished to save cycles, and tr should know that it does not have to produce any more output. For a broken tr installation, the sed script could be taught to slurp everything in the message body (without passing it to downstream sane_egrep, of course), and your tr would not see a broken pipe. But that is still a workaround, not a fix, and an expensive one at that. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
Junio C Hamano gits...@pobox.com writes:
Also,
- tr -d '\015' $1 |
sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
as the tr is at an upsteam of this pipeline, it does not really
matter to the outcome if it gives a write-error error message or not
(the downstream sane_egrep would have decided, based on the data it
was given, if the payload was mbox format), so...
An easier workaround may be to update the sed script downstream of
tr. It stops reading as soon as it finished to save cycles, and tr
should know that it does not have to produce any more output. For a
broken tr installation, the sed script could be taught to slurp
everything in the message body (without passing it to downstream
sane_egrep, of course), and your tr would not see a broken pipe.
But that is still a workaround, not a fix, and an expensive one at
that.
Redoing what e3f67d30 (am: fix patch format detection for
Thunderbird Save As emails, 2010-01-25) tried to do without
wasting a fork and a pipe may be a workable improvement.
I see Stephen who wrote the original Thunderbird save-as is
already on the Cc list. How about doing it this way instead?
git-am.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..9db3846 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,8 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with the
# header field names...
- tr -d '\015' $1 |
- sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
+ sed -n -e '/^$/q' -e '/^\r$/q' -e '/^[ ]/d' -e p $1
|
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
fi
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git rebase: yet another newbie quest.
On Fri, Sep 05, 2014 at 02:28:46PM +0400, Sergey Organov wrote: ... # Then I realize I need more changes and it gets complex enough to # warrant a topic branch. I create the 'topic' branch that will track # 'master' branch and reset 'master' back to its origin (remote # origin/master in original scenario). git checkout -b topic git branch --force master origin_master This line is the problem, because the purpose of the `--fork-point` argument to `git rebase` is designed to help people recover from upstream rebases, which is essentially what you create here. So when rebase calculates the local changes it realises (from the reflog) that the state of master before this command was before you created the branch, so only commits after it should be picked. For the case when the upstream of a branch is remote, this is normally what you want. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
Junio C Hamano gits...@pobox.com writes:
Redoing what e3f67d30 (am: fix patch format detection for
Thunderbird Save As emails, 2010-01-25) tried to do without
wasting a fork and a pipe may be a workable improvement.
I see Stephen who wrote the original Thunderbird save-as is
already on the Cc list. How about doing it this way instead?
Not that way, but more like this.
git-am.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..32e3039 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,8 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with the
# header field names...
- tr -d '\015' $1 |
- sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
+ sed -n -e 's/\r$//' -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
fi
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
(replying from webmail interface)
On Fri, Sep 5, 2014 at 3:25 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Redoing what e3f67d30 (am: fix patch format detection for
Thunderbird Save As emails, 2010-01-25) tried to do without
wasting a fork and a pipe may be a workable improvement.
I see Stephen who wrote the original Thunderbird save-as is
already on the Cc list. How about doing it this way instead?
It was so long ago I can't even remember writing that patch. But I
googled the thread from 4.5 years ago and I see that you suggested we
use tr because \r is not portable[1].
Not that way, but more like this.
git-am.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..32e3039 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -250,8 +250,7 @@ check_patch_format () {
# discarding the indented remainder of folded lines,
# and see if it looks like that they all begin with
the
# header field names...
- tr -d '\015' $1 |
- sed -n -e '/^$/q' -e '/^[ ]/d' -e p |
+ sed -n -e 's/\r$//' -e '/^$/q' -e '/^[ ]/d' -e p |
sane_egrep -v '^[!-9;-~]+:' /dev/null ||
patch_format=mbox
fi
[1]
http://git.661346.n2.nabble.com/PATCH-am-fix-patch-format-detection-for-Thunderbird-quot-Save-As-quot-emails-td4184273.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
