pull: make --rebase --verify-signatures illegal
PATCH 1/1 pull: make --rebase --verify-signatures illegal
Previously git-pull would silently ignore the --verify-signatures option.
---
We had some discussion back in December about a patch that added --verify-signatures to git-pull, that was declined.
I proposed making git-pull --rebase --verify-signatures illegal then, although it still remained open whether to just warn or make it an error.
In this patch I opted into making it an error, which breaks the previously accepted git pull --rebase --verify-signatures of course, albeit I'd argue that it probably didn't do what the user expected anyways.
I don't know if there are compatibility concerns with this though.
builtin/pull.c | 2 ++ t/t5520-pull.sh | 14 ++++++++++++++ 2 files changed, 16 insertions(+)
builtin/pull.c +2 -0
@@ -809,6 +809,8 @@ static int run_rebase(const unsigned char *curr_head,argv_array_push (&args,"--no-autostash" ); else if (opt_autostash == 1) argv_array_push(&args, "--autostash"); +if (opt_verify_signatures &&strcmp (opt_verify_signatures,"--verify-signatures" ) ==0 ) +die (_ ("The --verify-signatures option does not work for --rebase." ));argv_array_push (&args,"--onto" );argv_array_push (&args,sha1_to_hex (merge_head));
t/t5520-pull.sh +14 -0
@@ -341,6 +341,20 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' +test_expect_success "pull --rebase --verify-signatures is illegal" ' + git reset --hard before-rebase && + test_must_fail git pull --rebase --verify-signatures . copy 2>err && + test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_i18ngrep "The --verify-signatures option does not work for --rebase." err +' + +test_expect_success "pull --rebase --no-verify-signatures" ' + git reset --hard before-rebase && + git pull --rebase --no-verify-signatures . copy && + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" +' + # 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.
Signature · 1 line
2.8.0
PATCH v2 1/1 pull: make --rebase --verify-signatures illegal
Previously git-pull would silently ignore the --verify-signatures option.
---
I made the error-message conform to the CodingGuidelines (removed capitalization and full stop).
Also, in the previous mail I said that I proposed a patch for git-pull last December, when I actually meant git-rebase.
builtin/pull.c | 2 ++ t/t5520-pull.sh | 14 ++++++++++++++ 2 files changed, 16 insertions(+)
builtin/pull.c +2 -0
@@ -809,6 +809,8 @@ static int run_rebase(const unsigned char *curr_head,argv_array_push (&args,"--no-autostash" ); else if (opt_autostash == 1) argv_array_push(&args, "--autostash"); +if (opt_verify_signatures &&strcmp (opt_verify_signatures,"--verify-signatures" ) ==0 ) +die (_ ("the --verify-signatures option does not work for --rebase" ));argv_array_push (&args,"--onto" );argv_array_push (&args,sha1_to_hex (merge_head));
t/t5520-pull.sh +14 -0
@@ -341,6 +341,20 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' +test_expect_success "pull --rebase --verify-signatures is illegal" ' + git reset --hard before-rebase && + test_must_fail git pull --rebase --verify-signatures . copy 2>err && + test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_i18ngrep "The --verify-signatures option does not work for --rebase." err +' + +test_expect_success "pull --rebase --no-verify-signatures" ' + git reset --hard before-rebase && + git pull --rebase --no-verify-signatures . copy && + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" +' + # 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.
Signature · 1 line
2.8.0
Quoted 2 lines · Alexander 'z33ky' Hirsch <[email protected]> writes:
Previously git-pull would silently ignore the --verify-signatures option.
I do not see a point of making it error out. Adding a warning about the option being ignored might be a worthwhile thing to do (e.g. it may solicit responses from those who have been depending on it not erroring out, saying "these warnings are pointless and noisy--here is my valid use case that ends up passing --rebase and --v-s at the same time"), though.
Quoted 51 lines
Signed-off-by Alexander 'z33ky' Hirsch <[email protected]>---
I made the error-message conform to the CodingGuidelines (removed capitalization and full stop).
Also, in the previous mail I said that I proposed a patch for git-pull last December, when I actually meant git-rebase.
builtin/pull.c | 2 ++ t/t5520-pull.sh | 14 ++++++++++++++ 2 files changed, 16 insertions(+)builtin/pull.c +2
-0@@ -809,6 +809,8 @@ static int run_rebase(const unsigned char *curr_head,argv_array_push (&args,"--no-autostash" ); else if (opt_autostash == 1) argv_array_push(&args, "--autostash"); +if (opt_verify_signatures &&strcmp (opt_verify_signatures,"--verify-signatures" ) ==0 ) +die (_ ("the --verify-signatures option does not work for --rebase" ));argv_array_push (&args,"--onto" );argv_array_push (&args,sha1_to_hex (merge_head));t/t5520-pull.sh +14
-0@@ -341,6 +341,20 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' +test_expect_success "pull --rebase --verify-signatures is illegal" ' + git reset --hard before-rebase && + test_must_fail git pull --rebase --verify-signatures . copy 2>err && + test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_i18ngrep "The --verify-signatures option does not work for --rebase." err +' + +test_expect_success "pull --rebase --no-verify-signatures" ' + git reset --hard before-rebase && + git pull --rebase --no-verify-signatures . copy && + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" +' + # 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.
Quoted 27 lines · On Thu, Apr 28, 2016 at 2:52 AM, Alexander 'z33ky' Hirsch <[email protected]>…
Previously git-pull would silently ignore the --verify-signatures option.
Signed-off-by Alexander 'z33ky' Hirsch <[email protected]>---
We had some discussion back in December about a patch that added --verify-signatures to git-pull, that was declined.
I proposed making git-pull --rebase --verify-signatures illegal then, although it still remained open whether to just warn or make it an error.In this patch I opted into making it an error, which breaks the previously accepted git pull --rebase --verify-signatures of course, albeit I'd argue that it probably didn't do what the user expected anyways.
I don't know if there are compatibility concerns with this though.builtin/pull.c | 2 ++ t/t5520-pull.sh | 14 ++++++++++++++ 2 files changed, 16 insertions(+)builtin/pull.c +0
-0
builtin/pull.c static int run_rebase(const unsigned char *curr_head, · line 813
Quoted 5 lines
builtin/pull.c +2
-0@@ -809,6 +809,8 @@ static int run_rebase(const unsigned char *curr_head,argv_array_push (&args,"--no-autostash" ); else if (opt_autostash == 1) argv_array_push(&args, "--autostash"); +if (opt_verify_signatures &&strcmp (opt_verify_signatures,"--verify-signatures" ) ==0 ) +die (_ ("The --verify-signatures option does not work for --rebase." ));
Can you explain why it doesn't work (In the commit message)?
Quoted 35 lines
builtin/pull.c +0
-0argv_array_push (&args,"--onto" );argv_array_push (&args,sha1_to_hex (merge_head));t/t5520-pull.sh +14
-1@@ -341,6 +341,20 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' +test_expect_success "pull --rebase --verify-signatures is illegal" ' + git reset --hard before-rebase && + test_must_fail git pull --rebase --verify-signatures . copy 2>err && + test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_i18ngrep "The --verify-signatures option does not work for --rebase." err +' + +test_expect_success "pull --rebase --no-verify-signatures" ' + git reset --hard before-rebase && + git pull --rebase --no-verify-signatures . copy && + test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test new = "$(git show HEAD:file2)" +' + # 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. --2.8.0
--
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html