[PATCH v2 1/2] merge: add config option for verifySignatures
git merge --verify-signatures can be used to verify that the tip commit of the branch being merged in is properly signed, but it's cumbersome to have to specify that every time. Add a configuration option that enables this behaviour by default, which can be overridden by --no-verify-signatures. Signed-off-by: Hans Jerry Illikainen --- Documentation/merge-config.txt | 4 builtin/merge.c| 2 ++ t/t7612-merge-verify-signatures.sh | 39 ++ 3 files changed, 45 insertions(+) diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index df3ea3779..12b6bbf59 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -26,6 +26,10 @@ merge.ff:: allowed (equivalent to giving the `--ff-only` option from the command line). +merge.verifySignatures:: + If true, this is equivalent to the --verify-signatures command + line option. See linkgit:git-merge[1] for details. + include::fmt-merge-msg-config.txt[] merge.renameLimit:: diff --git a/builtin/merge.c b/builtin/merge.c index 612dd7bfb..30264cfd7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -567,6 +567,8 @@ static int git_merge_config(const char *k, const char *v, void *cb) if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) show_diffstat = git_config_bool(k, v); + else if (!strcmp(k, "merge.verifysignatures")) + verify_signatures = git_config_bool(k, v); else if (!strcmp(k, "pull.twohead")) return git_config_string(&pull_twohead, k, v); else if (!strcmp(k, "pull.octopus")) diff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh index 8ae69a61c..2344995a1 100755 --- a/t/t7612-merge-verify-signatures.sh +++ b/t/t7612-merge-verify-signatures.sh @@ -39,23 +39,62 @@ test_expect_success GPG 'merge unsigned commit with verification' ' test_i18ngrep "does not have a GPG signature" mergeerror ' +test_expect_success GPG 'merge unsigned commit with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + test_must_fail git merge --ff-only side-unsigned 2>mergeerror && + test_i18ngrep "does not have a GPG signature" mergeerror +' + test_expect_success GPG 'merge commit with bad signature with verification' ' test_must_fail git merge --ff-only --verify-signatures $(cat forged.commit) 2>mergeerror && test_i18ngrep "has a bad GPG signature" mergeerror ' +test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + test_must_fail git merge --ff-only $(cat forged.commit) 2>mergeerror && + test_i18ngrep "has a bad GPG signature" mergeerror +' + test_expect_success GPG 'merge commit with untrusted signature with verification' ' test_must_fail git merge --ff-only --verify-signatures side-untrusted 2>mergeerror && test_i18ngrep "has an untrusted GPG signature" mergeerror ' +test_expect_success GPG 'merge commit with untrusted signature with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + test_must_fail git merge --ff-only side-untrusted 2>mergeerror && + test_i18ngrep "has an untrusted GPG signature" mergeerror +' + test_expect_success GPG 'merge signed commit with verification' ' + test_when_finished "git checkout initial" && git merge --verbose --ff-only --verify-signatures side-signed >mergeoutput && test_i18ngrep "has a good GPG signature" mergeoutput ' +test_expect_success GPG 'merge signed commit with merge.verifySignatures=true' ' + test_when_finished "git checkout initial" && + test_config merge.verifySignatures true && + git merge --verbose --ff-only side-signed >mergeoutput && + test_i18ngrep "has a good GPG signature" mergeoutput +' + test_expect_success GPG 'merge commit with bad signature without verification' ' + test_when_finished "git checkout initial" && + git merge $(cat forged.commit) +' + +test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=false' ' + test_when_finished "git checkout initial" && + test_config merge.verifySignatures false && git merge $(cat forged.commit) ' +test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true and --no-verify-signatures' ' + test_when_finished "git checkout initial" && + test_config merge.verifySignatures true && + git merge --no-verify-signatures $(cat forged.commit) +' + test_done -- 2.15.1.356.g13e4cf275
[PATCH v2 2/2] t: add tests for pull --verify-signatures
Add tests for pull --verify-signatures with untrusted, bad and no signatures. Previously the only test for --verify-signatures was to make sure that pull --rebase --verify-signatures result in a warning (t5520-pull.sh). Signed-off-by: Hans Jerry Illikainen --- t/t5573-pull-verify-signatures.sh | 78 +++ 1 file changed, 78 insertions(+) create mode 100755 t/t5573-pull-verify-signatures.sh diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh new file mode 100755 index 0..8ae331f40 --- /dev/null +++ b/t/t5573-pull-verify-signatures.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='pull signature verification tests' +. ./test-lib.sh +. "$TEST_DIRECTORY/lib-gpg.sh" + +test_expect_success GPG 'create repositories with signed commits' ' + echo 1 >a && git add a && + test_tick && git commit -m initial && + git tag initial && + + git clone . signed && + ( + cd signed && + echo 2 >b && git add b && + test_tick && git commit -S -m "signed" + ) && + + git clone . unsigned && + ( + cd unsigned && + echo 3 >c && git add c && + test_tick && git commit -m "unsigned" + ) && + + git clone . bad && + ( + cd bad && + echo 4 >d && git add d && + test_tick && git commit -S -m "bad" && + git cat-file commit HEAD >raw && + sed -e "s/bad/forged bad/" raw >forged && + git hash-object -w -t commit forged >forged.commit && + git checkout $(cat forged.commit) + ) && + + git clone . untrusted && + ( + cd untrusted && + echo 5 >e && git add e && + test_tick && git commit -SB7227189 -m "untrusted" + ) +' + +test_expect_success GPG 'pull unsigned commit with --verify-signatures' ' + test_must_fail git pull --ff-only --verify-signatures unsigned 2>pullerror && + test_i18ngrep "does not have a GPG signature" pullerror +' + +test_expect_success GPG 'pull commit with bad signature with --verify-signatures' ' + test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror && + test_i18ngrep "has a bad GPG signature" pullerror +' + +test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' ' + test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror && + test_i18ngrep "has an untrusted GPG signature" pullerror +' + +test_expect_success GPG 'pull signed commit with --verify-signatures' ' + test_when_finished "git checkout initial" && + git pull --verify-signatures signed >pulloutput && + test_i18ngrep "has a good GPG signature" pulloutput +' + +test_expect_success GPG 'pull commit with bad signature without verification' ' + test_when_finished "git checkout initial" && + git pull --ff-only bad 2>pullerror +' + +test_expect_success GPG 'pull commit with bad signature with --no-verify-signatures' ' + test_when_finished "git checkout initial" && + test_config merge.verifySignatures true && + test_config pull.verifySignatures true && + git pull --ff-only --no-verify-signatures bad 2>pullerror +' + +test_done -- 2.15.1.356.g13e4cf275
Re: [PATCH 3/3] pull: add config option for verifySignatures
On Sat, Dec 09, 2017 at 01:06:23PM +0100, Kevin Daudt wrote: > On Sat, Dec 09, 2017 at 09:05:30AM +, Hans Jerry Illikainen wrote: > > Verify the signature of the tip commit when `pull.verifySignatures` is > > true. This option overrides `merge.verifySignatures` on pull, and can > > be disabled with the option `--no-verify-signatures`. > > Is there a reason why git pull would need a different behaviour from git > merge? Pull itself is just a convenience command for fetch + > merge/rebase. > > One precedent for having a separate configuration option for pull > however is 'pull.ff', so there might be a usecase for it. > > I guess your commit message could use a motivation on why you want to > set this differently from 'merge.verifySignature'. Thanks for the review! I wasn't sure whether pull.verifySignatures made sense -- I included it to be consistent with pull.ff/merge.ff, but it's scrapped in v2. -- hji
Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, noworking tree file changes)
Hi Philip,
On 09/12/2017 20:01, Phillip Wood wrote:
>
> > But thanks for clarifying, anyway, it does feel like `git rebase
> > -i --autosquash` could be smarter in this regards, if `git rebase
> > --onto` does it better...?
>
> Creating the fixup directly on A rather than on top of B avoids the
> conflicting merge B f!A A. Creating the fixup on top of B and then
> using git commit --onto A would suffer from the same conflicts as
> rebase does.
I`m a bit confused here, as you`re replying to the part where we
strictly discussed `rebase --autosquash` versus `rebase --onto`,
having the latter succeed where the former fails - but you`re
mentioning `git _commit_ --onto` instead, comparing it with `rebase`...
and which one of the two ("--autosquash", I assume)?
Even further, while I do seem to understand (and agree with) what
you`re talking about with `commit --onto` and `rebase --autosquah`
suffering from the same conflicts in attempt to take f!A, originally
created on top of B, and apply it on top of A - the thing is that
Alexei actually pointed to B being the problematic one, failing to
rebase on top of already (successfully) autosquashed A' (where A' = A
+ f!A, fixup applied through --autosquash), while it doesn`t fail
rebasing --onto f!A when f!A is being committed on top of A directly
(and not through --autosquash).
In that (very?) specific case, proposed `git commit --onto-parent`[1]
doesn`t suffer from this, as once f!A is successfully applied onto A
(either squashed in with --amend, or on top of it), we take original
f!A _snapshot_ (not patch!) made on top of B, and just "declare" it
B` (being equal to B + f!A, which we already know, and being
correct), without a need to (try to) apply B patch on top of fixed-up
A to create B', as `rebase` does (and fails).
> I don't think there is any way for 'git rebase --autosquash' to
> avoid the conflicts unless it used a special fixup merge strategy
> that somehow took advantage of the DAG to resolve the conflicts by
> realizing they come from a later commit. However I don't think that
> could be implemented reliably as sometimes one wants those
> conflicting lines from the later commit to be moved to the earlier
> commit with the fixup.
I think I agree on this part being tricky (if possible at all), but I
also think this is not what Alexei was complaining about, nor what we
were discussing (as I tried to explain above) - but please do correct
me if I misunderstood you.
That said, and what I mentioned already, we might really benefit from
simple test case(s), showing "rebase --autosquash" failing where
"rebase --onto" works, as Alexei explained, giving some more (and
firm) context to the discussion.
I *think* I`ve experienced this in the past myself, but now I can`t
seem to wrap my head around a reproducible example just yet... :$
Regards, Buga
[1]
https://public-inbox.org/git/4a92e34c-d713-25d3-e1ac-100525011...@talktalk.net/T/#m72f45ad7a8f1c733266a875bca087ee82cc781e7
[PATCH] sequencer: make sign_off_header a file local symbol
Commit d0aaa46fd3 ("commit: move empty message checks to libgit",
2017-11-10) removes the last use of 'sign_off_header' outside of
the "sequencer.c" source file. Remove the extern declaration from
the header file and mark the definition of the symbol with the
static keyword.
Signed-off-by: Ramsay Jones
---
Hi Junio,
Now that the 'pw/sequencer-in-process-commit' branch has
graduated to 'next', the static-check.pl script is barking on
that branch. This patch applies to the next (and pu) branches
without conflict. You mentioned that the previous version of
this patch (see [1]) conflicted with an in-flight patch series.
('ot/pretty' and 'lb/rebase-i-short-command-names' have nearby
changes, but don't conflict).
Was this an 'not in any integration branch, but I'm still holding
onto it' topic branch? If it is still a problem, please just ignore
this patch.
Thanks!
[1]
https://public-inbox.org/git/%3cd9d05477-5d2f-04fc-adee-10d18bc09...@ramsayjones.plus.com%3E/
ATB,
Ramsay Jones
sequencer.c | 2 +-
sequencer.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0f17b4d32..8c2af94ed 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -26,7 +26,7 @@
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-const char sign_off_header[] = "Signed-off-by: ";
+static const char sign_off_header[] = "Signed-off-by: ";
static const char cherry_picked_prefix[] = "(cherry picked from commit ";
GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
diff --git a/sequencer.h b/sequencer.h
index 77cb174b2..688b0276d 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -53,8 +53,6 @@ int check_todo_list(void);
int skip_unnecessary_picks(void);
int rearrange_squash(void);
-extern const char sign_off_header[];
-
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
void append_conflicts_hint(struct strbuf *msgbuf);
int git_sequencer_config(const char *k, const char *v, void *cb);
--
2.15.0
[Proposed] Externalize man/html ref for quick-install-man and quick-install-html
I am proposing the following trivial change to allow the external
specification of the reference used for quick-install-man. The justification
is that I cannot have my production team modifying scripts when non-master
versions of git are installed in production (that violates so many rules
that I would have trouble enumerating). This does add any requirements for
changes to the automation for building either git-manpages or git-htmldocs.
What it does is allow the top level make to pass GIT_MAN_REF down to the
underlying shell script with a default of master if unspecific. Where I am
uncertain is what else would be required for this change (documentation,
unit tests).
I humbly submit this for consideration.
Sincerely,
Randall
>From 6acc4a4238b3e3e62674bf8a5d0b9084258a0967 Mon Sep 17 00:00:00 2001
From: "Randall S. Becker"
Date: Sat, 9 Dec 2017 15:52:44 -0600
Subject: Externalize man/html ref for quick-install-man and
quick-install-html
---
Documentation/Makefile | 6 --
Documentation/install-doc-quick.sh | 7 ---
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 3e39e28..4f1e6df 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -39,6 +39,8 @@ MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT))
MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT))
+GIT_MAN_REF = master
+
OBSOLETE_HTML += everyday.html
OBSOLETE_HTML += git-remote-helpers.html
DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML)
@@ -415,14 +417,14 @@ require-manrepo::
then echo "git-manpages repository must exist at $(MAN_REPO)"; exit
1; fi
quick-install-man: require-manrepo
- '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO)
$(DESTDIR)$(mandir)
+ '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO)
$(DESTDIR)$(mandir) $(GIT_MAN_REF)
require-htmlrepo::
@if test ! -d $(HTML_REPO); \
then echo "git-htmldocs repository must exist at $(HTML_REPO)"; exit
1; fi
quick-install-html: require-htmlrepo
- '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO)
$(DESTDIR)$(htmldir)
+ '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO)
$(DESTDIR)$(htmldir) $(GIT_MAN_REF)
print-man1:
@for i in $(MAN1_TXT); do echo $$i; done
diff --git a/Documentation/install-doc-quick.sh
b/Documentation/install-doc-quick.sh
index 327f69b..a7715eb 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -4,7 +4,8 @@
repository=${1?repository}
destdir=${2?destination}
-head=master GIT_DIR=
+GIT_MAN_REF=${3?master}
+GIT_DIR=
for d in "$repository/.git" "$repository"
do
if GIT_DIR="$d" git rev-parse refs/heads/master >/dev/null 2>&1
@@ -27,12 +28,12 @@ export GIT_INDEX_FILE GIT_WORK_TREE
rm -f "$GIT_INDEX_FILE"
trap 'rm -f "$GIT_INDEX_FILE"' 0
-git read-tree $head
+git read-tree $GIT_MAN_REF
git checkout-index -a -f --prefix="$destdir"/
if test -n "$GZ"
then
- git ls-tree -r --name-only $head |
+ git ls-tree -r --name-only $GIT_MAN_REF |
xargs printf "$destdir/%s\n" |
xargs gzip -f
fi
--
2.5.6.18.ga013bef
[PATCH v3 6/7] diff: add tests for --relative without optional prefix value
From: Jacob Keller
We already have tests for --relative, but they currently only test when
a prefix has been provided. This fails to test the case where --relative
by itself should use the current directory as the prefix.
Teach the check_$type functions to take a directory argument to indicate
which subdirectory to run the git commands in. Add a new test which uses
this to test --relative without a prefix value.
Signed-off-by: Jacob Keller
---
t/t4045-diff-relative.sh | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 3950f5034d..7d68a6e2a5 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -13,6 +13,7 @@ test_expect_success 'setup' '
'
check_diff() {
+dir=$1; shift
expect=$1; shift
cat >expected actual &&
test_cmp expected actual
"
}
check_numstat() {
+dir=$1; shift
expect=$1; shift
cat >expected actual &&
+ git -C '$dir' diff --numstat $* HEAD^ >actual &&
test_cmp expected actual
"
}
check_stat() {
+dir=$1; shift
expect=$1; shift
cat >expected actual &&
test_i18ncmp expected actual
"
}
check_raw() {
+dir=$1; shift
expect=$1; shift
cat >expected actual &&
test_cmp expected actual
"
}
for type in diff numstat stat raw; do
- check_$type file2 --relative=subdir/
- check_$type file2 --relative=subdir
- check_$type dir/file2 --relative=sub
+ check_$type . file2 --relative=subdir/
+ check_$type . file2 --relative=subdir
+ check_$type . dir/file2 --relative=sub
+ check_$type subdir file2 --relative
done
test_done
--
2.15.1.361.g8b07d831d0
[PATCH v3 1/7] git-compat-util: introduce skip_to_optional_arg()
From: Christian Couder
We often accept both a "--key" option and a "--key=" option.
These options currently are parsed using something like:
if (!strcmp(arg, "--key")) {
/* do something */
} else if (skip_prefix(arg, "--key=", &arg)) {
/* do something with arg */
}
which is a bit cumbersome compared to just:
if (skip_to_optional_arg(arg, "--key", &arg)) {
/* do something with arg */
}
This also introduces skip_to_optional_arg_default() for the few
cases where something different should be done when the first
argument is exactly "--key" than when it is exactly "--key=".
In general it is better for UI consistency and simplicity if
"--key" and "--key=" do the same thing though, so that using
skip_to_optional_arg() should be encouraged compared to
skip_to_optional_arg_default().
Note that these functions can be used to parse any "key=value"
string where "key" is also considered as valid, not just
command line options.
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
git-compat-util.h | 23 +++
strbuf.c | 22 ++
2 files changed, 45 insertions(+)
The changes compared to v2 are:
- s/_val/_arg/ in the name of the functions
- s/val/arg/ in the name of the third argument of the functions
- works with NULL as third argument of the functions
- "--relative" is handled correctly in a separate new patch from Junio
- tests are added for "--relative" in a separate new patch from Jake
- a new test reindenting patch from Junio is added
diff --git a/git-compat-util.h b/git-compat-util.h
index cedad4d581..68b2ad531e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -484,6 +484,29 @@ static inline int skip_prefix(const char *str, const char
*prefix,
return 0;
}
+/*
+ * If the string "str" is the same as the string in "prefix", then the "arg"
+ * parameter is set to the "def" parameter and 1 is returned.
+ * If the string "str" begins with the string found in "prefix" and then a
+ * "=" sign, then the "arg" parameter is set to "str + strlen(prefix) + 1"
+ * (i.e., to the point in the string right after the prefix and the "=" sign),
+ * and 1 is returned.
+ *
+ * Otherwise, return 0 and leave "arg" untouched.
+ *
+ * When we accept both a "--key" and a "--key=" option, this function
+ * can be used instead of !strcmp(arg, "--key") and then
+ * skip_prefix(arg, "--key=", &arg) to parse such an option.
+ */
+int skip_to_optional_arg_default(const char *str, const char *prefix,
+const char **arg, const char *def);
+
+static inline int skip_to_optional_arg(const char *str, const char *prefix,
+ const char **arg)
+{
+ return skip_to_optional_arg_default(str, prefix, arg, "");
+}
+
/*
* Like skip_prefix, but promises never to read past "len" bytes of the input
* buffer, and returns the remaining number of bytes in "out" via "outlen".
diff --git a/strbuf.c b/strbuf.c
index 323c49ceb3..29169b8ef8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -11,6 +11,28 @@ int starts_with(const char *str, const char *prefix)
return 0;
}
+int skip_to_optional_arg_default(const char *str, const char *prefix,
+const char **arg, const char *def)
+{
+ const char *p;
+
+ if (!skip_prefix(str, prefix, &p))
+ return 0;
+
+ if (!*p) {
+ if (arg)
+ *arg = def;
+ return 1;
+ }
+
+ if (*p != '=')
+ return 0;
+
+ if (arg)
+ *arg = p + 1;
+ return 1;
+}
+
/*
* Used as the default ->buf value, so that people can always assume
* buf is non NULL and ->buf is NUL terminated even for a freshly
--
2.15.1.361.g8b07d831d0
[PATCH v3 4/7] diff: use skip_to_optional_arg_default()
From: Christian Couder
Let's simplify diff option parsing using
skip_to_optional_arg_default().
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
diff.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/diff.c b/diff.c
index 464a53adb5..28e1ab168f 100644
--- a/diff.c
+++ b/diff.c
@@ -4623,9 +4623,7 @@ int diff_opt_parse(struct diff_options *options,
else if (!strcmp(arg, "--no-follow")) {
options->flags.follow_renames = 0;
options->flags.default_follow_renames = 0;
- } else if (!strcmp(arg, "--color"))
- options->use_color = 1;
- else if (skip_prefix(arg, "--color=", &arg)) {
+ } else if (skip_to_optional_arg_default(arg, "--color", &arg,
"always")) {
int value = git_config_colorbool(NULL, arg);
if (value < 0)
return error("option `color' expects \"always\",
\"auto\", or \"never\"");
@@ -4645,14 +4643,9 @@ int diff_opt_parse(struct diff_options *options,
if (cm < 0)
die("bad --color-moved argument: %s", arg);
options->color_moved = cm;
- } else if (!strcmp(arg, "--color-words")) {
- options->use_color = 1;
- options->word_diff = DIFF_WORDS_COLOR;
- }
- else if (skip_prefix(arg, "--color-words=", &arg)) {
+ } else if (skip_to_optional_arg_default(arg, "--color-words",
&options->word_regex, NULL)) {
options->use_color = 1;
options->word_diff = DIFF_WORDS_COLOR;
- options->word_regex = arg;
}
else if (!strcmp(arg, "--word-diff")) {
if (options->word_diff == DIFF_WORDS_NONE)
@@ -4691,15 +4684,10 @@ int diff_opt_parse(struct diff_options *options,
options->flags.textconv_set_via_cmdline = 1;
} else if (!strcmp(arg, "--no-textconv"))
options->flags.allow_textconv = 0;
- else if (!strcmp(arg, "--ignore-submodules")) {
- options->flags.override_submodule_config = 1;
- handle_ignore_submodules_arg(options, "all");
- } else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {
+ else if (skip_to_optional_arg_default(arg, "--ignore-submodules", &arg,
"all")) {
options->flags.override_submodule_config = 1;
handle_ignore_submodules_arg(options, arg);
- } else if (!strcmp(arg, "--submodule"))
- options->submodule_format = DIFF_SUBMODULE_LOG;
- else if (skip_prefix(arg, "--submodule=", &arg))
+ } else if (skip_to_optional_arg_default(arg, "--submodule", &arg,
"log"))
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
return parse_ws_error_highlight_opt(options, arg);
--
2.15.1.361.g8b07d831d0
[PATCH v3 2/7] index-pack: use skip_to_optional_arg()
From: Christian Couder
Let's simplify index-pack option parsing using
skip_to_optional_arg().
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
builtin/index-pack.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 8ec459f522..4c51aec81f 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1660,10 +1660,7 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
from_stdin = 1;
} else if (!strcmp(arg, "--fix-thin")) {
fix_thin_pack = 1;
- } else if (!strcmp(arg, "--strict")) {
- strict = 1;
- do_fsck_object = 1;
- } else if (skip_prefix(arg, "--strict=", &arg)) {
+ } else if (skip_to_optional_arg(arg, "--strict", &arg))
{
strict = 1;
do_fsck_object = 1;
fsck_set_msg_types(&fsck_options, arg);
@@ -1679,10 +1676,8 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
verify = 1;
show_stat = 1;
stat_only = 1;
- } else if (!strcmp(arg, "--keep")) {
- keep_msg = "";
- } else if (starts_with(arg, "--keep=")) {
- keep_msg = arg + 7;
+ } else if (skip_to_optional_arg(arg, "--keep",
&keep_msg)) {
+ ; /* nothing to do */
} else if (starts_with(arg, "--threads=")) {
char *end;
nr_threads = strtoul(arg+10, &end, 0);
--
2.15.1.361.g8b07d831d0
[PATCH v3 3/7] diff: use skip_to_optional_arg()
From: Christian Couder
Let's simplify diff option parsing using skip_to_optional_arg().
Signed-off-by: Christian Couder
Signed-off-by: Junio C Hamano
---
diff.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/diff.c b/diff.c
index 2ebe2227b4..464a53adb5 100644
--- a/diff.c
+++ b/diff.c
@@ -4508,17 +4508,12 @@ int diff_opt_parse(struct diff_options *options,
options->output_format |= DIFF_FORMAT_NUMSTAT;
else if (!strcmp(arg, "--shortstat"))
options->output_format |= DIFF_FORMAT_SHORTSTAT;
- else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
- return parse_dirstat_opt(options, "");
- else if (skip_prefix(arg, "-X", &arg))
- return parse_dirstat_opt(options, arg);
- else if (skip_prefix(arg, "--dirstat=", &arg))
+ else if (skip_prefix(arg, "-X", &arg) ||
+skip_to_optional_arg(arg, "--dirstat", &arg))
return parse_dirstat_opt(options, arg);
else if (!strcmp(arg, "--cumulative"))
return parse_dirstat_opt(options, "cumulative");
- else if (!strcmp(arg, "--dirstat-by-file"))
- return parse_dirstat_opt(options, "files");
- else if (skip_prefix(arg, "--dirstat-by-file=", &arg)) {
+ else if (skip_to_optional_arg(arg, "--dirstat-by-file", &arg)) {
parse_dirstat_opt(options, "files");
return parse_dirstat_opt(options, arg);
}
@@ -4540,13 +4535,13 @@ int diff_opt_parse(struct diff_options *options,
return stat_opt(options, av);
/* renames options */
- else if (starts_with(arg, "-B") || starts_with(arg,
"--break-rewrites=") ||
-!strcmp(arg, "--break-rewrites")) {
+ else if (starts_with(arg, "-B") ||
+skip_to_optional_arg(arg, "--break-rewrites", NULL)) {
if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
return error("invalid argument to -B: %s", arg+2);
}
- else if (starts_with(arg, "-M") || starts_with(arg, "--find-renames=")
||
-!strcmp(arg, "--find-renames")) {
+ else if (starts_with(arg, "-M") ||
+skip_to_optional_arg(arg, "--find-renames", NULL)) {
if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
return error("invalid argument to -M: %s", arg+2);
options->detect_rename = DIFF_DETECT_RENAME;
@@ -4554,8 +4549,8 @@ int diff_opt_parse(struct diff_options *options,
else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
options->irreversible_delete = 1;
}
- else if (starts_with(arg, "-C") || starts_with(arg, "--find-copies=") ||
-!strcmp(arg, "--find-copies")) {
+ else if (starts_with(arg, "-C") ||
+skip_to_optional_arg(arg, "--find-copies", NULL)) {
if (options->detect_rename == DIFF_DETECT_COPY)
options->flags.find_copies_harder = 1;
if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
--
2.15.1.361.g8b07d831d0
[PATCH v3 5/7] diff: use skip_to_optional_arg_default() in parsing --relative
From: Junio C Hamano
Helped-by: Jacob Keller
Helped-by: Jeff King
Signed-off-by: Junio C Hamano
---
diff.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index 28e1ab168f..3f14cdace6 100644
--- a/diff.c
+++ b/diff.c
@@ -4563,11 +4563,10 @@ int diff_opt_parse(struct diff_options *options,
options->flags.rename_empty = 1;
else if (!strcmp(arg, "--no-rename-empty"))
options->flags.rename_empty = 0;
- else if (!strcmp(arg, "--relative"))
+ else if (skip_to_optional_arg_default(arg, "--relative", &arg, NULL)) {
options->flags.relative_name = 1;
- else if (skip_prefix(arg, "--relative=", &arg)) {
- options->flags.relative_name = 1;
- options->prefix = arg;
+ if (arg)
+ options->prefix = arg;
}
/* xdiff options */
--
2.15.1.361.g8b07d831d0
[PATCH v3 7/7] t4045: reindent to make helpers readable
From: Junio C Hamano
Signed-off-by: Junio C Hamano
---
t/t4045-diff-relative.sh | 104 +--
1 file changed, 56 insertions(+), 48 deletions(-)
diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 7d68a6e2a5..4df55884c4 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -12,60 +12,68 @@ test_expect_success 'setup' '
git commit -m one
'
-check_diff() {
-dir=$1; shift
-expect=$1; shift
-cat >expected expected <<-EOF
+ diff --git a/$expect b/$expect
+ new file mode 100644
+ index 000..25c05ef
+ --- /dev/null
+ +++ b/$expect
+ @@ -0,0 +1 @@
+ +other content
+ EOF
+ test_expect_success "-p $*" "
+ git -C '$dir' diff -p $* HEAD^ >actual &&
+ test_cmp expected actual
+ "
}
-check_numstat() {
-dir=$1; shift
-expect=$1; shift
-cat >expected actual &&
- test_cmp expected actual
-"
+check_numstat () {
+ dir=$1
+ shift
+ expect=$1
+ shift
+ cat >expected <<-EOF
+ 1 0 $expect
+ EOF
+ test_expect_success "--numstat $*" "
+ echo '1 0 $expect' >expected &&
+ git -C '$dir' diff --numstat $* HEAD^ >actual &&
+ test_cmp expected actual
+ "
}
-check_stat() {
-dir=$1; shift
-expect=$1; shift
-cat >expected expected <<-EOF
+$expect | 1 +
+1 file changed, 1 insertion(+)
+ EOF
+ test_expect_success "--stat $*" "
+ git -C '$dir' diff --stat $* HEAD^ >actual &&
+ test_i18ncmp expected actual
+ "
}
-check_raw() {
-dir=$1; shift
-expect=$1; shift
-cat >expected expected <<-EOF
+ :00 100644
25c05ef3639d2d270e7fe765a67668f098092bc5 A $expect
+ EOF
+ test_expect_success "--raw $*" "
+ git -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&
+ test_cmp expected actual
+ "
}
for type in diff numstat stat raw; do
--
2.15.1.361.g8b07d831d0
Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking tree file changes)
Hi Igor On 09/12/17 03:03, Igor Djordjevic wrote: > > Hi Alexei, > > On 09/12/2017 03:18, Alexei Lozovsky wrote: >> >>> Chris reported in this very topic[1] that sometimes, due to >>> conflicts with later commits, "checkout > commit > [checkout >] >>> rebase --onto" is "much easier to do", where "commit --fixup > >>> rebase -i" "breaks" (does not apply cleanly). >> >> It was more of a rant about conflict resolution by rebase rather than >> a concern about modification time of files. While I'd prefer git to >> not touch the source tree unnecessarily, it's not really a big deal >> for me if it does and some parts of the project need to be rebuilt. > > Nevertheless, I found it valuable in supporting the case where > "commit --fixup > rebase -i" seems to require even more work than > otherwise necessary :) > > But thanks for clarifying, anyway, it does feel like `git rebase -i > --autosquash` could be smarter in this regards, if `git rebase > --onto` does it better...? Creating the fixup directly on A rather than on top of B avoids the conflicting merge B f!A A. Creating the fixup on top of B and then using git commit --onto A would suffer from the same conflicts as rebase does. I don't think there is any way for 'git rebase --autosquash' to avoid the conflicts unless it used a special fixup merge strategy that somehow took advantage of the DAG to resolve the conflicts by realizing they come from a later commit. However I don't think that could be implemented reliably as sometimes one wants those conflicting lines from the later commit to be moved to the earlier commit with the fixup. Best Wishes Phillip > > Even though your explanation seems clear, having a real, easily > reproducible case would help as well, I guess. > >> I kinda hoped that you may know this magic and incorporate it into >> "commit --onto" which will allow to immediately get to the result of >> the rebase: >> >> ---A---f!A---B' >> >> without spelling it all manually. > > If you mind enough to be bothered testing it out, might be even > existing/initial state of originally proposed `git commit > --onto-parent` script would work for you, as it does incorporate some > trivial three-way merge resolution. > > In your starting situation: > > ---A---B > > you would just do something like: > > git commit --onto-parent A > > hopefully ending up in the desired state (hopefully = conflicts > automatically resolved): > > ---A---C---B' > > You could even do this instead: > > git commit --onto-parent A --amend > > ending up with: > > ---A'---B' > > as that is basically what you wanted in the first place ;) > >> (And yeah, I'm actually Alexei, not Chris. That was my MUA being >> dumb and using an old pseudonym than Google insists I'm called by.) > > Ah, sorry for the confusion :) > > Regards, Buga >
Re: [PATCH v4 7/9] sequencer: load commit related config
On 05/12/17 11:21, Phillip Wood wrote:
> On 04/12/17 18:30, Junio C Hamano wrote:
>> Phillip Wood writes:
>>
>>> --- a/builtin/rebase--helper.c
>>> +++ b/builtin/rebase--helper.c
>>> @@ -9,6 +9,17 @@ static const char * const builtin_rebase_helper_usage[] = {
>>> NULL
>>> };
>>>
>>> +static int git_rebase_helper_config(const char *k, const char *v, void *cb)
>>> +{
>>> + int status;
>>> +
>>> + status = git_sequencer_config(k, v, NULL);
>>> + if (status)
>>> + return status;
>>> +
>>> + return git_default_config(k, v, NULL);
>>> +}
>>> +
>>
>> Sorry for spotting the problem this late, but this code is
>> unfortunate and we will need to revisit it later; we may want to do
>> so sooner rather than later.
>
> If it needs fixing then doing it before it hits next makes sense.
>
>> When k,v is a valid configuration that is handled by
>> sequencer_config() successfully, this code still needs to call into
>> default_config() with the same k,v, only to get it ignored.
>
> I'm a bit confused by this sentence. Do you mean that when k,v is a
> valid configuration that is successfully handled by sequencer_config()
> this code unnecessarily calls default_config() as there is no need to
> call default_config() if k,v have already been handled?
>
>> The problem lies in the (mis)design of git_sequencer_config(). The
>> function should either allow the caller to notice that (k,v) has
>> already been handled, or be the last one in the callback by making a
>> call to default_config() itself.
>
> The problem is that git_gpg_config() provides no indication if it has
> handled k,v so there's no way to avoid the call to default_config() in
> that case. builtin/am.c and builtin/commit.c both do something like
>
> static int git_am_config(const char *k, const char *v, void *cb)
> {
> int status;
>
> status = git_gpg_config(k, v, NULL);
> if (status)
> return status;
>
> return git_default_config(k, v, NULL);
> }
>
>
> Looking more generally at sequencer_config() I wonder if we should be
> providing a warning or an error if the config contains an invalid
> cleanup mode. Also should it be calling git_diff_ui_config() to set
> things up for print_commit_summary()? (I'm not sure if anything in that
> function is affected by diff config settings)
I think the answer maybe yes so that it loads the setting for external
diff commands and textconv filters but I'm not sure, perhaps someone
more familiar with the diff code would know? Also I think we should be
loading the basic diff config for creating the patch when we stop with
conflicts?
If anyone has any thought on this, please share them!
Best Wishes
Phillip
>
> Let me know what you think. I should have time to update this patch set
> later in the week.
>
> Best Wishes
>
> Phillip
>
>> For the former, because this helper does not have to be called
>> directly as a git_config() callback, but instead it is always meant
>> to be called indirectly from another git_config() callback (like
>> git_rebase_helper_config() here, and common_config() in
>> builtin/revert.c like we see below), it does *not* have to be
>> constrained by the function signature required for it to be a config
>> callback. It could take a pointer to an int that stores if 'k' was
>> handled inside the function,
>>
>> int git_sequencer_config_helper(char *k, char *v, int *handled);
>>
>> for example.
>>
>
Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, noworking tree file changes)
Hi Igor On 09/12/17 03:03, Igor Djordjevic wrote: > > Hi Alexei, > > On 09/12/2017 03:18, Alexei Lozovsky wrote: >> >>> Chris reported in this very topic[1] that sometimes, due to >>> conflicts with later commits, "checkout > commit > [checkout >] >>> rebase --onto" is "much easier to do", where "commit --fixup > >>> rebase -i" "breaks" (does not apply cleanly). >> >> It was more of a rant about conflict resolution by rebase rather than >> a concern about modification time of files. While I'd prefer git to >> not touch the source tree unnecessarily, it's not really a big deal >> for me if it does and some parts of the project need to be rebuilt. > > Nevertheless, I found it valuable in supporting the case where > "commit --fixup > rebase -i" seems to require even more work than > otherwise necessary :) > > But thanks for clarifying, anyway, it does feel like `git rebase -i > --autosquash` could be smarter in this regards, if `git rebase > --onto` does it better...? Creating the fixup directly on A rather than on top of B avoids the conflicting merge B f!A A. Creating the fixup on top of B and then using git commit --onto A would suffer from the same conflicts as rebase does. I don't think there is any way for 'git rebase --autosquash' to avoid the conflicts unless it used a special fixup merge strategy that somehow took advantage of the DAG to resolve the conflicts by realizing they come from a later commit. However I don't think that could be implemented reliably as sometimes one wants those conflicting lines from the later commit to be moved to the earlier commit with the fixup. Best Wishes Phillip > > Even though your explanation seems clear, having a real, easily > reproducible case would help as well, I guess. > >> I kinda hoped that you may know this magic and incorporate it into >> "commit --onto" which will allow to immediately get to the result of >> the rebase: >> >> ---A---f!A---B' >> >> without spelling it all manually. > > If you mind enough to be bothered testing it out, might be even > existing/initial state of originally proposed `git commit > --onto-parent` script would work for you, as it does incorporate some > trivial three-way merge resolution. > > In your starting situation: > > ---A---B > > you would just do something like: > > git commit --onto-parent A > > hopefully ending up in the desired state (hopefully = conflicts > automatically resolved): > > ---A---C---B' > > You could even do this instead: > > git commit --onto-parent A --amend > > ending up with: > > ---A'---B' > > as that is basically what you wanted in the first place ;) > >> (And yeah, I'm actually Alexei, not Chris. That was my MUA being >> dumb and using an old pseudonym than Google insists I'm called by.) > > Ah, sorry for the confusion :) > > Regards, Buga >
[PATCH] send-email: extract email-parsing code into a subroutine
The existing code mixes parsing of email header with regular
expression and actual code. Extract the parsing code into a new
subroutine "parse_header_line()". This improves the code readability
and make parse_header_line reusable in other place.
"parsed_header_line()" and "filter_body()" could be used for
refactoring the part of code which parses the header to prepare the
email and send it.
Signed-off-by: Nathan Payre
Signed-off-by: Matthieu Moy
Signed-off-by: Timothee Albertin
Signed-off-by: Daniel Bensoussan
Signed-off-by: Junio C Hamano
Thanks-to: Ævar Arnfjörð Bjarmason
---
I fixed the last reported problems and removed some other old
variable as $need_8bit_cte.
git-send-email.perl | 110 ++--
1 file changed, 73 insertions(+), 37 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 2208dcc21..ac36c6aac 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -685,7 +685,7 @@ Lines beginning in "GIT:" will be removed.
Consider including an overall diffstat or table of contents
for the patch you are writing.
-Clear the body content if you don't wish to send a summary.
+Clear the body content if you dont wish to send a summary.
EOT2
From: $tpl_sender
Subject: $tpl_subject
@@ -709,51 +709,61 @@ EOT3
open $c, "<", $compose_filename
or die sprintf(__("Failed to open %s: %s"), $compose_filename,
$!);
- my $need_8bit_cte = file_has_nonascii($compose_filename);
- my $in_body = 0;
- my $summary_empty = 1;
if (!defined $compose_encoding) {
$compose_encoding = "UTF-8";
}
- while(<$c>) {
- next if m/^GIT:/;
- if ($in_body) {
- $summary_empty = 0 unless (/^\n$/);
- } elsif (/^\n$/) {
- $in_body = 1;
- if ($need_8bit_cte) {
- print $c2 "MIME-Version: 1.0\n",
-"Content-Type: text/plain; ",
- "charset=$compose_encoding\n",
-"Content-Transfer-Encoding: 8bit\n";
- }
- } elsif (/^MIME-Version:/i) {
- $need_8bit_cte = 0;
- } elsif (/^Subject:\s*(.+)\s*$/i) {
- $initial_subject = $1;
- my $subject = $initial_subject;
- $_ = "Subject: " .
- quote_subject($subject, $compose_encoding) .
- "\n";
- } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
- $initial_reply_to = $1;
- next;
- } elsif (/^From:\s*(.+)\s*$/i) {
- $sender = $1;
- next;
- } elsif (/^(?:To|Cc|Bcc):/i) {
- print __("To/Cc/Bcc fields are not interpreted yet,
they have been ignored\n");
- next;
+
+
+ my %parsed_email;
+ $parsed_email{'body'} = '';
+ while (my $line = <$c>) {
+ next if $line =~ m/^GIT:/;
+ parse_header_line($line, \%parsed_email);
+ if ($line =~ /^$/) {
+ $parsed_email{'body'} = filter_body($c);
}
- print $c2 $_;
}
- close $c;
- close $c2;
- if ($summary_empty) {
+ if ($parsed_email{'from'}) {
+ $sender = $parsed_email{'from'};
+ }
+ if ($parsed_email{'in-reply-to'}) {
+ $initial_reply_to = $parsed_email{'in-reply-to'};
+ }
+ if ($parsed_email{'subject'}) {
+ $initial_subject = $parsed_email{'subject'};
+ print $c2 "Subject: " .
+ quote_subject($parsed_email{'subject'},
$compose_encoding) .
+ "\n";
+ }
+ if ($parsed_email{'mime-version'}) {
+ print $c2 "MIME-Version: $parsed_email{'mime-version'}\n",
+ "Content-Type:
$parsed_email{'content-type'};\n",
+ "Content-Transfer-Encoding:
$parsed_email{'content-transfer-encoding'}\n";
+ }
+
+ if ($parsed_email{'content-type'}) {
+ print $c2 "MIME-Version: 1.0\n",
+"Content-Type: $parsed_email{'content-type'};\n",
+"Content-Transfer-Encoding: 8bit\n";
+ } elsif (file_has_nonascii($compose_filename)) {
+my $content_type = ($parsed_email{'content-type'} or
+"text/plain; charset=$compose_encoding");
+print $c2 "MIME-Version: 1.0\n",
+ "Content-Type: $content_type\n",
+ "Content-Transfer-Encoding: 8bit\n";
+}
+
+ if ($parsed_email{'body'}) {
+ print $c2 "\n$parsed_em
Assalamu`Alaikum.
Greetings from Dr. mohammad ouattara. Assalamu`Alaikum. My Name is Dr. mohammad ouattara, I am a banker by profession. I'm from Ouagadougou, Burkina Faso, West Africa. My reason for contacting you is to transfer an abandoned $14.6M to your account. The owner of this fund died since 2004 with his Next Of Kin. I want to present you to the bank as the Next of Kin/beneficiary of this fund. Further details of the transaction shall be forwarded to you as soon as I receive your return mail indicating your interest. Have a great day, Dr. mohammad ouattara.
Re: What's cooking in git.git (Dec 2017, #01; Mon, 4)
On Thu, Dec 07, 2017 at 04:33:12PM -0500, Todd Zullinger wrote:
> Jeff Hostetler wrote:
> >I'm looking at t5616 now on my mac.
> >Looks like the MAC doesn't like my line counting in the tests.
> >I'll fix in my next version.
>
[]
> | sort >expect_2.oids &&
> - test "$(wc -l + test_line_count = 8 expect_2.oids &&
> git -C src blame master -- file.1.txt >expect.blame
> '
The problem seems to be the '"' around wc, this would work:
test $(wc -l ) {
/^\s*declare\s+/ and err 'arrays/declare not portable';
/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
/\btest\s+[^=]*==/ and err '"test a == b" is not portable (please use
=)';
+ /\bwc -l.*"\s*=/ and err '`"$(wc -l)"` is not portable, please use
`$(wc -l)`';
/\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable
(please use FOO=bar && expo
Re: [PATCH 1/2] git version --build-options: report the build platform, too
Hi Eric,
On Sat, 9 Dec 2017, Eric Sunshine wrote:
> On Fri, Dec 08, 2017 at 04:19:25PM -0500, Eric Sunshine wrote:
> > On Fri, Dec 8, 2017 at 4:17 PM, Eric Sunshine
> > wrote:
> > > On Fri, Dec 8, 2017 at 12:43 PM, Junio C Hamano wrote:
> > >> Jonathan Nieder writes:
> > >>> Can this use GIT_BUILD_PLATFORM directly instead of going via the
> > >>> indirection
> > >>> of a mutable static string? That is, something like
> > >>>
> > >>> printf("machine: %s\n", GIT_BUILD_PLATFORM);
> > >>
> > >> Good point. And if this is externally identified as "machine",
> > >> probably the macro should also use the same word, not "platform".
> > >> We can go either way, as long as we are consistent, though.
> > >
> > > In Autoconf parlance, this would be called "host architecture"
> > > (GIT_HOST_ARCH).
> >
> > My bad: "host cpu", rather (GIT_HOST_CPU).
>
> Dscho, when you re-roll, perhaps replace the current patch with the
> one below which determines the CPU type automatically rather than
> having to manually maintain and augment a bunch of #ifdefs in help.h.
Will do!
Ciao,
Dscho
Re: [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH
Hi Peff, On Fri, 8 Dec 2017, Jeff King wrote: > On Fri, Dec 08, 2017 at 04:08:19PM +0100, Johannes Schindelin wrote: > > > > Yes, but test-lib.sh sources GIT-BUILD-OPTIONS, which we > > > built during the first "make". And that overrides the > > > environment, giving us the original SHELL_PATH again. > > > > ... and we could simply see whether the environment variable > > TEST_SHELL_PATH (which we would set in t/Makefile from the passed-in > > SHELL_PATH) is set, and override it again. > > > > I still think we can do without recording test-phase details in the > > build-phase (which may, or may not, know what the test-phase wants to do). > > > > In other words, I believe that we can make the invocation you mentioned > > above work, by touching only t/Makefile (to pass SHELL_PATH as > > TEST_SHELL_PATH) and t/test-lib.sh (to override the SHELL_PATH from > > GIT-BUILD-OPTIONS with TEST_SHELL_PATH, if set). > > We could do that, but it makes TEST_SHELL_PATH inconsistent with all of > the other config.mak variables. It is already inconsistent with the other variables because its scope is the "test" phase, not the "build" phase. Ciao, Dscho
Re: [PATCH v2 0/5] SHA1DC fixes & fully moving to a git.git submodule
On Fri, Dec 08, 2017 at 10:29:56PM +, Ævar Arnfjörð Bjarmason wrote: > Here's v2 as promised. Comments per-patch. > > sha1dc: remove in favor of using sha1collisiondetection as a submodule > > Reword & expand commit message. Is this commit missing from the mailing list because the e-mail is too large?
Re: [PATCH 3/4] Makefile: use the sha1collisiondetection submodule by default
On Sat, Dec 09, 2017 at 01:30:14PM +0100, Kevin Daudt wrote:
> On Tue, Dec 05, 2017 at 02:02:50AM -0500, Jeff King wrote:
> > On Tue, Nov 28, 2017 at 09:32:13PM +, Ævar Arnfjörð Bjarmason wrote:
> >
> > > Change the build process so that instead of needing to supply
> > > DC_SHA1_SUBMODULE=YesPlease to use the sha1collisiondetection
> > > submodule instead of the copy of the same code shipped in the sha1dc
> > > directory, it uses the submodule by default unless
> > > NO_DC_SHA1_SUBMODULE=UnfortunatelyYes is supplied.
> > >
> > > This reverses the logic added by me in 86cfd61e6b ("sha1dc: optionally
> > > use sha1collisiondetection as a submodule", 2017-07-01). Git has now
> > > shipped with the submodule in git.git for two major releases, if we're
> > > ever going to migrate to fully using it instead of perpetually
> > > maintaining both sha1collisiondetection and the sha1dc directory this
> > > is a logical first step.
> > >
> > > This change removes the "auto" logic Junio added in
> > > cac87dc01d ("sha1collisiondetection: automatically enable when
> > > submodule is populated", 2017-07-01), I feel that automatically
> > > falling back to using sha1dc would defeat the point, which is to smoke
> > > out any remaining users of git.git who have issues cloning the
> > > submodule for whatever reason.
> >
> > I'm not sure how I feel about this. I see your point that there's no
> > real value in maintaining two systems indefinitely. At the same time, I
> > wonder how much value the submodule strategy is actually bringing us.
> >
> > IOW, are we agreed that the path forward is to get everybody using the
> > submodule?
> >
> > It seems like it's going to cause some minor pain for CI and packaging
> > systems that now need to care about submodules (so at least flipping the
> > switch, but maybe also dealing with having a network dependency for the
> > build that was not already there).
> >
> > I'll admit I'm more sensitive to this than most people, since I happen
> > to maintain a fork of Git that I run through an internal CI system. And
> > that CI otherwise depends only on the locally-held fork, not any
> > external resources. But I'm probably in a fairly unique situation there.
> >
> > -Peff
>
> To add to this point, package systems such as Alpinelinux and Archlinux
> (and probably others) work with released tarballs, not cloned
> repositories. For them, there is no easy way to get the submodule
> contents (the release tarballs would not contain it).
>
> Once we would switch over to submodules only (because we do not want to
> maintain 2 separate systems), it would be a lot of hassle for those
> projects to get the sha1collisiondetection contents.
>
> That's in my opinion a bigger disadvantage of submodules, commands like
> git archive do not support it, making it harder to get self-contained
> tarballs.
>
> Perpahs there is a good solution to that problem, but then I'd like to
> hear it.
>
> Kevin.
I missed the v2 Ævar sent. I see that there `make dist` is adjusted to
include sha1collisiondetection, so that would at least solve this
problem.
Re: [PATCH 3/4] Makefile: use the sha1collisiondetection submodule by default
On Tue, Dec 05, 2017 at 02:02:50AM -0500, Jeff King wrote:
> On Tue, Nov 28, 2017 at 09:32:13PM +, Ævar Arnfjörð Bjarmason wrote:
>
> > Change the build process so that instead of needing to supply
> > DC_SHA1_SUBMODULE=YesPlease to use the sha1collisiondetection
> > submodule instead of the copy of the same code shipped in the sha1dc
> > directory, it uses the submodule by default unless
> > NO_DC_SHA1_SUBMODULE=UnfortunatelyYes is supplied.
> >
> > This reverses the logic added by me in 86cfd61e6b ("sha1dc: optionally
> > use sha1collisiondetection as a submodule", 2017-07-01). Git has now
> > shipped with the submodule in git.git for two major releases, if we're
> > ever going to migrate to fully using it instead of perpetually
> > maintaining both sha1collisiondetection and the sha1dc directory this
> > is a logical first step.
> >
> > This change removes the "auto" logic Junio added in
> > cac87dc01d ("sha1collisiondetection: automatically enable when
> > submodule is populated", 2017-07-01), I feel that automatically
> > falling back to using sha1dc would defeat the point, which is to smoke
> > out any remaining users of git.git who have issues cloning the
> > submodule for whatever reason.
>
> I'm not sure how I feel about this. I see your point that there's no
> real value in maintaining two systems indefinitely. At the same time, I
> wonder how much value the submodule strategy is actually bringing us.
>
> IOW, are we agreed that the path forward is to get everybody using the
> submodule?
>
> It seems like it's going to cause some minor pain for CI and packaging
> systems that now need to care about submodules (so at least flipping the
> switch, but maybe also dealing with having a network dependency for the
> build that was not already there).
>
> I'll admit I'm more sensitive to this than most people, since I happen
> to maintain a fork of Git that I run through an internal CI system. And
> that CI otherwise depends only on the locally-held fork, not any
> external resources. But I'm probably in a fairly unique situation there.
>
> -Peff
To add to this point, package systems such as Alpinelinux and Archlinux
(and probably others) work with released tarballs, not cloned
repositories. For them, there is no easy way to get the submodule
contents (the release tarballs would not contain it).
Once we would switch over to submodules only (because we do not want to
maintain 2 separate systems), it would be a lot of hassle for those
projects to get the sha1collisiondetection contents.
That's in my opinion a bigger disadvantage of submodules, commands like
git archive do not support it, making it harder to get self-contained
tarballs.
Perpahs there is a good solution to that problem, but then I'd like to
hear it.
Kevin.
Re: [PATCH 3/3] pull: add config option for verifySignatures
On Sat, Dec 09, 2017 at 09:05:30AM +, Hans Jerry Illikainen wrote:
> Verify the signature of the tip commit when `pull.verifySignatures` is
> true. This option overrides `merge.verifySignatures` on pull, and can
> be disabled with the option `--no-verify-signatures`.
Is there a reason why git pull would need a different behaviour from git
merge? Pull itself is just a convenience command for fetch +
merge/rebase.
One precedent for having a separate configuration option for pull
however is 'pull.ff', so there might be a usecase for it.
I guess your commit message could use a motivation on why you want to
set this differently from 'merge.verifySignature'.
>
> Signed-off-by: Hans Jerry Illikainen
> ---
> Documentation/config.txt | 8
> builtin/pull.c| 25 +
> t/t5520-pull.sh | 18 ++
> t/t5573-pull-verify-signatures.sh | 32
> 4 files changed, 83 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index c1598ee70..0cd2bc597 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2596,6 +2596,14 @@ pull.ff::
> allowed (equivalent to giving the `--ff-only` option from the
> command line). This setting overrides `merge.ff` when pulling.
>
> +pull.verifySignatures::
> + Verify that the tip commit of the side branch being merged is
> + signed with a valid key, i.e. a key that has a valid uid: in the
> + default trust model, this means the signing key has been signed
> + by a trusted key. If the tip commit of the side branch is not
> + signed with a valid key, the merge is aborted. This setting
> + overrides `merge.verifySignatures` when pulling.
> +
> pull.rebase::
> When true, rebase branches on top of the fetched branch, instead
> of merging the default branch from the default remote when "git
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 166b777ed..791365915 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -300,6 +300,28 @@ static const char *config_get_ff(void)
> }
>
> /**
> + * If pull.verifySignatures is unset, returns NULL. If pull.verifySignatures
> is
> + * "true", returns "--verify-signatures". If pull.verifySignatures is
> "false",
> + * returns "--no-verify-signatures". Otherwise, die with an error.
> + */
> +static const char *config_get_verify_signatures(void)
> +{
> + const char *value;
> +
> + if (git_config_get_value("pull.verifysignatures", &value))
> + return NULL;
> +
> + switch (git_parse_maybe_bool(value)) {
> + case 0:
> + return "--no-verify-signatures";
> + case 1:
> + return "--verify-signatures";
> + default:
> + die(_("Invalid value for pull.verifysignatures: %s"), value);
> + }
> +}
> +
> +/**
> * Returns the default configured value for --rebase. It first looks for the
> * value of "branch.$curr_branch.rebase", where $curr_branch is the current
> * branch, and if HEAD is detached or the configuration key does not exist,
> @@ -849,6 +871,9 @@ int cmd_pull(int argc, const char **argv, const char
> *prefix)
> if (!opt_ff)
> opt_ff = xstrdup_or_null(config_get_ff());
>
> + if (!opt_verify_signatures)
> + opt_verify_signatures =
> xstrdup_or_null(config_get_verify_signatures());
> +
> if (opt_rebase < 0)
> opt_rebase = config_get_rebase();
>
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 59c4b778d..cdf1fd213 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -416,6 +416,15 @@ test_expect_success "pull --rebase warns on
> --verify-signatures" '
> test_i18ngrep "ignoring --verify-signatures for rebase" err
> '
>
> +test_expect_success "pull --rebase warns on pull.verifySignatures=true" '
> + test_config pull.verifySignatures true &&
> + git reset --hard before-rebase &&
> + git pull --rebase . copy 2>err &&
> + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
> + test new = "$(git show HEAD:file2)" &&
> + test_i18ngrep "ignoring --verify-signatures for rebase" err
> +'
> +
> test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
> git reset --hard before-rebase &&
> git pull --rebase --no-verify-signatures . copy 2>err &&
> @@ -424,6 +433,15 @@ test_expect_success "pull --rebase does not warn on
> --no-verify-signatures" '
> test_i18ngrep ! "verify-signatures" err
> '
>
> +test_expect_success "pull --rebase does not warn on
> pull.verifySignatures=false" '
> + test_config pull.verifySignatures false &&
> + git reset --hard before-rebase &&
> + git pull --rebase . copy 2>err &&
> + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
> + test new = "$(git show HEAD:file2)" &&
> + test_i18ngrep ! "verify-signatures" err
> +'
> +
> # add a feature
Re: [PATCH 2/3] t: add tests for pull --verify-signatures
On Sat, Dec 09, 2017 at 09:05:29AM +, Hans Jerry Illikainen wrote: > Add tests for `pull --verify-signatures` with untrusted, bad and no > signatures. Previously the only test for `--verify-signatures` was to > make sure that `pull --rebase --verify-signatures` result in a warning > (t5520-pull.sh). Nice! Same thing regarding the `git checkout initial` commands counts here too. > > Signed-off-by: Hans Jerry Illikainen > --- > t/t5573-pull-verify-signatures.sh | 78 > +++ > 1 file changed, 78 insertions(+) > create mode 100755 t/t5573-pull-verify-signatures.sh > > diff --git a/t/t5573-pull-verify-signatures.sh > b/t/t5573-pull-verify-signatures.sh > new file mode 100755 > index 0..700247910 > --- /dev/null > +++ b/t/t5573-pull-verify-signatures.sh > @@ -0,0 +1,78 @@ > +#!/bin/sh > + > +test_description='pull signature verification tests' > +. ./test-lib.sh > +. "$TEST_DIRECTORY/lib-gpg.sh" > + > +test_expect_success GPG 'create repositories with signed commits' ' > + echo 1 >a && git add a && > + test_tick && git commit -m initial && > + git tag initial && > + > + git clone . signed && > + ( > + cd signed && > + echo 2 >b && git add b && > + test_tick && git commit -S -m "signed" > + ) && > + > + git clone . unsigned && > + ( > + cd unsigned && > + echo 3 >c && git add c && > + test_tick && git commit -m "unsigned" > + ) && > + > + git clone . bad && > + ( > + cd bad && > + echo 4 >d && git add d && > + test_tick && git commit -S -m "bad" && > + git cat-file commit HEAD >raw && > + sed -e "s/bad/forged bad/" raw >forged && > + git hash-object -w -t commit forged >forged.commit && > + git checkout $(cat forged.commit) > + ) && > + > + git clone . untrusted && > + ( > + cd untrusted && > + echo 5 >e && git add e && > + test_tick && git commit -SB7227189 -m "untrusted" > + ) > +' > + > +test_expect_success GPG 'pull unsigned commit with --verify-signatures' ' > + test_must_fail git pull --ff-only --verify-signatures unsigned > 2>pullerror && > + test_i18ngrep "does not have a GPG signature" pullerror > +' > + > +test_expect_success GPG 'pull commit with bad signature with > --verify-signatures' ' > + test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror && > + test_i18ngrep "has a bad GPG signature" pullerror > +' > + > +test_expect_success GPG 'pull commit with untrusted signature with > --verify-signatures' ' > + test_must_fail git pull --ff-only --verify-signatures untrusted > 2>pullerror && > + test_i18ngrep "has an untrusted GPG signature" pullerror > +' > + > +test_expect_success GPG 'pull signed commit with --verify-signatures' ' > + git pull --verify-signatures signed >pulloutput && > + test_i18ngrep "has a good GPG signature" pulloutput && > + git checkout initial > +' > + > +test_expect_success GPG 'pull commit with bad signature without > verification' ' > + git pull --ff-only bad 2>pullerror && > + git checkout initial > +' > + > +test_expect_success GPG 'pull commit with bad signature with > --no-verify-signatures' ' > + test_config merge.verifySignatures true && > + test_config pull.verifySignatures true && > + git pull --ff-only --no-verify-signatures bad 2>pullerror && > + git checkout initial > +' > + > +test_done > -- > 2.11.0 >
Re: [PATCH 1/3] merge: add config option for verifySignatures
Hello Hans Jerry, Thank you for your contribution. I have soem remarks below. On Sat, Dec 09, 2017 at 09:05:28AM +, Hans Jerry Illikainen wrote: > Verify the signature of the tip commit when `merge.verifySignatures` is > true. This can be overridden with `--no-verify-signatures`. > > Signed-off-by: Hans Jerry Illikainen I miss the motivation in the commit message. I imagine something like: git merge --verify-signatures can be used to verify that the tip commit of the branch being merged in is properly signed, but it's cumbersome to have to specify that every time. Add a configuration option that enables this behaviour by default, which can be overridden by --no-verify-signatures. > --- > Documentation/merge-config.txt | 7 +++ > builtin/merge.c| 2 ++ > t/t7612-merge-verify-signatures.sh | 43 > -- > 3 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt > index df3ea3779..76571cd3b 100644 > --- a/Documentation/merge-config.txt > +++ b/Documentation/merge-config.txt > @@ -26,6 +26,13 @@ merge.ff:: > allowed (equivalent to giving the `--ff-only` option from the > command line). > > +merge.verifySignatures:: > + Verify that the tip commit of the side branch being merged is > + signed with a valid key, i.e. a key that has a valid uid: in the > + default trust model, this means the signing key has been signed > + by a trusted key. If the tip commit of the side branch is not > + signed with a valid key, the merge is aborted. > + This is a verbatim copy of the explenation of --verify-signatures. Would it be an idea to refer to the explenation of merge --verify-signatures? > include::fmt-merge-msg-config.txt[] > > merge.renameLimit:: > diff --git a/builtin/merge.c b/builtin/merge.c > index 612dd7bfb..30264cfd7 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -567,6 +567,8 @@ static int git_merge_config(const char *k, const char *v, > void *cb) > > if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) > show_diffstat = git_config_bool(k, v); > + else if (!strcmp(k, "merge.verifysignatures")) > + verify_signatures = git_config_bool(k, v); > else if (!strcmp(k, "pull.twohead")) > return git_config_string(&pull_twohead, k, v); > else if (!strcmp(k, "pull.octopus")) > diff --git a/t/t7612-merge-verify-signatures.sh > b/t/t7612-merge-verify-signatures.sh > index 8ae69a61c..f1a74a683 100755 > --- a/t/t7612-merge-verify-signatures.sh > +++ b/t/t7612-merge-verify-signatures.sh > @@ -39,23 +39,62 @@ test_expect_success GPG 'merge unsigned commit with > verification' ' > test_i18ngrep "does not have a GPG signature" mergeerror > ' > > +test_expect_success GPG 'merge unsigned commit with > merge.verifySignatures=true' ' > + test_config merge.verifySignatures true && > + test_must_fail git merge --ff-only side-unsigned 2>mergeerror && > + test_i18ngrep "does not have a GPG signature" mergeerror > +' > + > test_expect_success GPG 'merge commit with bad signature with verification' ' > test_must_fail git merge --ff-only --verify-signatures $(cat > forged.commit) 2>mergeerror && > test_i18ngrep "has a bad GPG signature" mergeerror > ' > > +test_expect_success GPG 'merge commit with bad signature with > merge.verifySignatures=true' ' > + test_config merge.verifySignatures true && > + test_must_fail git merge --ff-only $(cat forged.commit) 2>mergeerror && > + test_i18ngrep "has a bad GPG signature" mergeerror > +' > + > test_expect_success GPG 'merge commit with untrusted signature with > verification' ' > test_must_fail git merge --ff-only --verify-signatures side-untrusted > 2>mergeerror && > test_i18ngrep "has an untrusted GPG signature" mergeerror > ' > > +test_expect_success GPG 'merge commit with untrusted signature with > merge.verifySignatures=true' ' > + test_config merge.verifySignatures true && > + test_must_fail git merge --ff-only side-untrusted 2>mergeerror && > + test_i18ngrep "has an untrusted GPG signature" mergeerror > +' > + > test_expect_success GPG 'merge signed commit with verification' ' > git merge --verbose --ff-only --verify-signatures side-signed > >mergeoutput && > - test_i18ngrep "has a good GPG signature" mergeoutput > + test_i18ngrep "has a good GPG signature" mergeoutput && > + git checkout initial This looks like a clean up step. If so, it's better to add `test_when_finished 'git checkout initial'` at the beginning to clearly mark it as a clean up step and make sure it's run even when the test fails. Same counts for the other occurances. > +' > + > +test_expect_success GPG 'merge signed commit with > merge.verifySignatures=true' ' > + test_config merge.verifySignatures true && > + gi
Re: [PATCH 1/2] git version --build-options: report the build platform, too
On Fri, Dec 08, 2017 at 04:19:25PM -0500, Eric Sunshine wrote:
> On Fri, Dec 8, 2017 at 4:17 PM, Eric Sunshine wrote:
> > On Fri, Dec 8, 2017 at 12:43 PM, Junio C Hamano wrote:
> >> Jonathan Nieder writes:
> >>> Can this use GIT_BUILD_PLATFORM directly instead of going via the
> >>> indirection
> >>> of a mutable static string? That is, something like
> >>>
> >>> printf("machine: %s\n", GIT_BUILD_PLATFORM);
> >>
> >> Good point. And if this is externally identified as "machine",
> >> probably the macro should also use the same word, not "platform".
> >> We can go either way, as long as we are consistent, though.
> >
> > In Autoconf parlance, this would be called "host architecture"
> > (GIT_HOST_ARCH).
>
> My bad: "host cpu", rather (GIT_HOST_CPU).
Dscho, when you re-roll, perhaps replace the current patch with the
one below which determines the CPU type automatically rather than
having to manually maintain and augment a bunch of #ifdefs in help.h.
I took the liberty of renaming the #define to GIT_HOST_CPU to better
match Autoconf parlance since its conceivable that Git might some day
support cross-compilation officially via the configure script (which
doesn't yet work, though I have some patches which begin addressing
that, but that's a separate topic).
The original plan was to keep Adric Norris as author, but by the time
the patch was done and the commit message rewritten to match, I
realized that none of his work remained, thus ended up dropping his
authorship and both of your sign-offs. Sorry.
--- >8 ---
From: Eric Sunshine
Date: Sat, 9 Dec 2017 04:09:18 -0500
Subject: [PATCH] help: version --build-options: also report host CPU
It can be helpful for bug reports to include information about the
environment in which the bug occurs. "git version --build-options" can
help to supplement this information. In addition to the size of 'long'
already reported by --build-options, also report the host's CPU type.
Example output:
$ git version --build-options
git version 2.9.3.windows.2.826.g06c0f2f
cpu: x86_64
sizeof-long: 4
New Makefile variable HOST_CPU supports cross-compiling.
Suggested-by: Adric Norris
Signed-off-by: Eric Sunshine
---
Makefile | 9 +
help.c | 1 +
2 files changed, 10 insertions(+)
diff --git a/Makefile b/Makefile
index fef9c8d272..5587bccc93 100644
--- a/Makefile
+++ b/Makefile
@@ -425,6 +425,9 @@ all::
#
# to say "export LESS=FRX (and LV=-c) if the environment variable
# LESS (and LV) is not set, respectively".
+#
+# When cross-compiling, define HOST_CPU as the canonical name of the CPU on
+# which the built Git will run (for instance "x86_64").
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1095,6 +1098,12 @@ else
BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
endif
+ifeq (,$(HOST_CPU))
+ BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(firstword $(subst -,
,$(uname_M)))\""
+else
+ BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(HOST_CPU)\""
+endif
+
ifneq (,$(INLINE))
BASIC_CFLAGS += -Dinline=$(INLINE)
endif
diff --git a/help.c b/help.c
index 88a3aeaeb9..cbcb159f36 100644
--- a/help.c
+++ b/help.c
@@ -412,6 +412,7 @@ int cmd_version(int argc, const char **argv, const char
*prefix)
printf("git version %s\n", git_version_string);
if (build_options) {
+ printf("cpu: %s\n", GIT_HOST_CPU);
printf("sizeof-long: %d\n", (int)sizeof(long));
/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
}
--
2.15.1.502.gccaef8de57
[PATCH 1/3] merge: add config option for verifySignatures
Verify the signature of the tip commit when `merge.verifySignatures` is true. This can be overridden with `--no-verify-signatures`. Signed-off-by: Hans Jerry Illikainen --- Documentation/merge-config.txt | 7 +++ builtin/merge.c| 2 ++ t/t7612-merge-verify-signatures.sh | 43 -- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index df3ea3779..76571cd3b 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -26,6 +26,13 @@ merge.ff:: allowed (equivalent to giving the `--ff-only` option from the command line). +merge.verifySignatures:: + Verify that the tip commit of the side branch being merged is + signed with a valid key, i.e. a key that has a valid uid: in the + default trust model, this means the signing key has been signed + by a trusted key. If the tip commit of the side branch is not + signed with a valid key, the merge is aborted. + include::fmt-merge-msg-config.txt[] merge.renameLimit:: diff --git a/builtin/merge.c b/builtin/merge.c index 612dd7bfb..30264cfd7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -567,6 +567,8 @@ static int git_merge_config(const char *k, const char *v, void *cb) if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) show_diffstat = git_config_bool(k, v); + else if (!strcmp(k, "merge.verifysignatures")) + verify_signatures = git_config_bool(k, v); else if (!strcmp(k, "pull.twohead")) return git_config_string(&pull_twohead, k, v); else if (!strcmp(k, "pull.octopus")) diff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh index 8ae69a61c..f1a74a683 100755 --- a/t/t7612-merge-verify-signatures.sh +++ b/t/t7612-merge-verify-signatures.sh @@ -39,23 +39,62 @@ test_expect_success GPG 'merge unsigned commit with verification' ' test_i18ngrep "does not have a GPG signature" mergeerror ' +test_expect_success GPG 'merge unsigned commit with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + test_must_fail git merge --ff-only side-unsigned 2>mergeerror && + test_i18ngrep "does not have a GPG signature" mergeerror +' + test_expect_success GPG 'merge commit with bad signature with verification' ' test_must_fail git merge --ff-only --verify-signatures $(cat forged.commit) 2>mergeerror && test_i18ngrep "has a bad GPG signature" mergeerror ' +test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + test_must_fail git merge --ff-only $(cat forged.commit) 2>mergeerror && + test_i18ngrep "has a bad GPG signature" mergeerror +' + test_expect_success GPG 'merge commit with untrusted signature with verification' ' test_must_fail git merge --ff-only --verify-signatures side-untrusted 2>mergeerror && test_i18ngrep "has an untrusted GPG signature" mergeerror ' +test_expect_success GPG 'merge commit with untrusted signature with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + test_must_fail git merge --ff-only side-untrusted 2>mergeerror && + test_i18ngrep "has an untrusted GPG signature" mergeerror +' + test_expect_success GPG 'merge signed commit with verification' ' git merge --verbose --ff-only --verify-signatures side-signed >mergeoutput && - test_i18ngrep "has a good GPG signature" mergeoutput + test_i18ngrep "has a good GPG signature" mergeoutput && + git checkout initial +' + +test_expect_success GPG 'merge signed commit with merge.verifySignatures=true' ' + test_config merge.verifySignatures true && + git merge --verbose --ff-only side-signed >mergeoutput && + test_i18ngrep "has a good GPG signature" mergeoutput && + git checkout initial ' test_expect_success GPG 'merge commit with bad signature without verification' ' - git merge $(cat forged.commit) + git merge $(cat forged.commit) && + git checkout initial +' + +test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=false' ' + test_config merge.verifySignatures false && + git merge $(cat forged.commit) && + git checkout initial +' + +test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true and --no-verify-signatures' ' + test_config merge.verifySignatures true && + git merge --no-verify-signatures $(cat forged.commit) && + git checkout initial ' test_done -- 2.11.0
[PATCH 3/3] pull: add config option for verifySignatures
Verify the signature of the tip commit when `pull.verifySignatures` is
true. This option overrides `merge.verifySignatures` on pull, and can
be disabled with the option `--no-verify-signatures`.
Signed-off-by: Hans Jerry Illikainen
---
Documentation/config.txt | 8
builtin/pull.c| 25 +
t/t5520-pull.sh | 18 ++
t/t5573-pull-verify-signatures.sh | 32
4 files changed, 83 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1598ee70..0cd2bc597 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2596,6 +2596,14 @@ pull.ff::
allowed (equivalent to giving the `--ff-only` option from the
command line). This setting overrides `merge.ff` when pulling.
+pull.verifySignatures::
+ Verify that the tip commit of the side branch being merged is
+ signed with a valid key, i.e. a key that has a valid uid: in the
+ default trust model, this means the signing key has been signed
+ by a trusted key. If the tip commit of the side branch is not
+ signed with a valid key, the merge is aborted. This setting
+ overrides `merge.verifySignatures` when pulling.
+
pull.rebase::
When true, rebase branches on top of the fetched branch, instead
of merging the default branch from the default remote when "git
diff --git a/builtin/pull.c b/builtin/pull.c
index 166b777ed..791365915 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -300,6 +300,28 @@ static const char *config_get_ff(void)
}
/**
+ * If pull.verifySignatures is unset, returns NULL. If pull.verifySignatures is
+ * "true", returns "--verify-signatures". If pull.verifySignatures is "false",
+ * returns "--no-verify-signatures". Otherwise, die with an error.
+ */
+static const char *config_get_verify_signatures(void)
+{
+ const char *value;
+
+ if (git_config_get_value("pull.verifysignatures", &value))
+ return NULL;
+
+ switch (git_parse_maybe_bool(value)) {
+ case 0:
+ return "--no-verify-signatures";
+ case 1:
+ return "--verify-signatures";
+ default:
+ die(_("Invalid value for pull.verifysignatures: %s"), value);
+ }
+}
+
+/**
* Returns the default configured value for --rebase. It first looks for the
* value of "branch.$curr_branch.rebase", where $curr_branch is the current
* branch, and if HEAD is detached or the configuration key does not exist,
@@ -849,6 +871,9 @@ int cmd_pull(int argc, const char **argv, const char
*prefix)
if (!opt_ff)
opt_ff = xstrdup_or_null(config_get_ff());
+ if (!opt_verify_signatures)
+ opt_verify_signatures =
xstrdup_or_null(config_get_verify_signatures());
+
if (opt_rebase < 0)
opt_rebase = config_get_rebase();
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 59c4b778d..cdf1fd213 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -416,6 +416,15 @@ test_expect_success "pull --rebase warns on
--verify-signatures" '
test_i18ngrep "ignoring --verify-signatures for rebase" err
'
+test_expect_success "pull --rebase warns on pull.verifySignatures=true" '
+ test_config pull.verifySignatures true &&
+ git reset --hard before-rebase &&
+ git pull --rebase . copy 2>err &&
+ test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+ test new = "$(git show HEAD:file2)" &&
+ test_i18ngrep "ignoring --verify-signatures for rebase" err
+'
+
test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
git reset --hard before-rebase &&
git pull --rebase --no-verify-signatures . copy 2>err &&
@@ -424,6 +433,15 @@ test_expect_success "pull --rebase does not warn on
--no-verify-signatures" '
test_i18ngrep ! "verify-signatures" err
'
+test_expect_success "pull --rebase does not warn on
pull.verifySignatures=false" '
+ test_config pull.verifySignatures false &&
+ git reset --hard before-rebase &&
+ git pull --rebase . copy 2>err &&
+ test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+ test new = "$(git show HEAD:file2)" &&
+ test_i18ngrep ! "verify-signatures" err
+'
+
# add a feature branch, keep-merge, that is merged into master, so the
# test can try preserving the merge commit (or not) with various
# --rebase flags/pull.rebase settings.
diff --git a/t/t5573-pull-verify-signatures.sh
b/t/t5573-pull-verify-signatures.sh
index 700247910..d1e8263d9 100755
--- a/t/t5573-pull-verify-signatures.sh
+++ b/t/t5573-pull-verify-signatures.sh
@@ -47,22 +47,54 @@ test_expect_success GPG 'pull unsigned commit with
--verify-signatures' '
test_i18ngrep "does not have a GPG signature" pullerror
'
+test_expect_success GPG 'pull unsigned commit with pull.verifySignatures=true'
'
+
[PATCH 2/3] t: add tests for pull --verify-signatures
Add tests for `pull --verify-signatures` with untrusted, bad and no signatures. Previously the only test for `--verify-signatures` was to make sure that `pull --rebase --verify-signatures` result in a warning (t5520-pull.sh). Signed-off-by: Hans Jerry Illikainen --- t/t5573-pull-verify-signatures.sh | 78 +++ 1 file changed, 78 insertions(+) create mode 100755 t/t5573-pull-verify-signatures.sh diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh new file mode 100755 index 0..700247910 --- /dev/null +++ b/t/t5573-pull-verify-signatures.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='pull signature verification tests' +. ./test-lib.sh +. "$TEST_DIRECTORY/lib-gpg.sh" + +test_expect_success GPG 'create repositories with signed commits' ' + echo 1 >a && git add a && + test_tick && git commit -m initial && + git tag initial && + + git clone . signed && + ( + cd signed && + echo 2 >b && git add b && + test_tick && git commit -S -m "signed" + ) && + + git clone . unsigned && + ( + cd unsigned && + echo 3 >c && git add c && + test_tick && git commit -m "unsigned" + ) && + + git clone . bad && + ( + cd bad && + echo 4 >d && git add d && + test_tick && git commit -S -m "bad" && + git cat-file commit HEAD >raw && + sed -e "s/bad/forged bad/" raw >forged && + git hash-object -w -t commit forged >forged.commit && + git checkout $(cat forged.commit) + ) && + + git clone . untrusted && + ( + cd untrusted && + echo 5 >e && git add e && + test_tick && git commit -SB7227189 -m "untrusted" + ) +' + +test_expect_success GPG 'pull unsigned commit with --verify-signatures' ' + test_must_fail git pull --ff-only --verify-signatures unsigned 2>pullerror && + test_i18ngrep "does not have a GPG signature" pullerror +' + +test_expect_success GPG 'pull commit with bad signature with --verify-signatures' ' + test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror && + test_i18ngrep "has a bad GPG signature" pullerror +' + +test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' ' + test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror && + test_i18ngrep "has an untrusted GPG signature" pullerror +' + +test_expect_success GPG 'pull signed commit with --verify-signatures' ' + git pull --verify-signatures signed >pulloutput && + test_i18ngrep "has a good GPG signature" pulloutput && + git checkout initial +' + +test_expect_success GPG 'pull commit with bad signature without verification' ' + git pull --ff-only bad 2>pullerror && + git checkout initial +' + +test_expect_success GPG 'pull commit with bad signature with --no-verify-signatures' ' + test_config merge.verifySignatures true && + test_config pull.verifySignatures true && + git pull --ff-only --no-verify-signatures bad 2>pullerror && + git checkout initial +' + +test_done -- 2.11.0
Re: [PATCH] docs/pretty-formats: mention commas in %(trailers) syntax
On Thu, Dec 7, 2017 at 9:16 PM, Jeff King wrote: > Commit 84ff053d47 (pretty.c: delimit "%(trailers)" arguments > with ",", 2017-10-01) switched the syntax of the trailers > placeholder, but forgot to update the documentation in > pretty-formats.txt. > > There's need to mention the old syntax; it was never in a > released version of Git. > > Signed-off-by: Jeff King > --- > This should go on top of tb/delimit-pretty-trailers-args-with-comma. > > Documentation/pretty-formats.txt | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/Documentation/pretty-formats.txt > b/Documentation/pretty-formats.txt > index d433d50f81..e664c088a5 100644 > --- a/Documentation/pretty-formats.txt > +++ b/Documentation/pretty-formats.txt > @@ -204,11 +204,13 @@ endif::git-rev-list[] >than given and there are spaces on its left, use those spaces > - '%><()', '%><|()': similar to '% <()', '%<|()' >respectively, but padding both sides (i.e. the text is centered) > -- %(trailers): display the trailers of the body as interpreted by > - linkgit:git-interpret-trailers[1]. If the `:only` option is given, > - omit non-trailer lines from the trailer block. If the `:unfold` > - option is given, behave as if interpret-trailer's `--unfold` option > - was given. E.g., `%(trailers:only:unfold)` to do both. > +- %(trailers[:options]): display the trailers of the body as interpreted > + by linkgit:git-interpret-trailers[1]. The `trailers` string may be > + followed by a colon and zero or more comma-separated options. If the > + `only` option is given, omit non-trailer lines from the trailer block. > + If the `unfold` option is given, behave as if interpret-trailer's > + `--unfold` option was given. E.g., `%(trailers:only,unfold)` to do > + both. > > NOTE: Some placeholders may depend on other options given to the > revision traversal engine. For example, the `%g*` reflog options will > -- > 2.15.1.659.g8bd2eae3ea HAH. I was recently looking at this documentation and going to send a patch. Looks good besides the other reviewers comments. Thanks, Jake
