Junio C Hamano <gitster@xxxxxxxxx> writes:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>>
>>> Hi Alexandr
>>>
>>> This looks good, thanks for the test
>>>
>>> Best Wishes
>>>
>>> Phillip
>>>
>>> On 16/12/2019 15:47, Alexandr Miloslavskiy via GitGitGadget wrote:
>>>> From: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
>>>>
>>>> I forgot this in my previous patch `--pathspec-from-file` for
>>>> `git commit` [1]. When both `--pathspec-from-file` and `--all` were
>>>> ...
>>>> [1] Commit e440fc58 ("commit: support the --pathspec-from-file option" 2019-11-19)
>>
>> Thanks, both. I will take this separately and queue directly on top
>> of am/pathspec-from-file to fast-track it, rather than leaving it as
>> a part of larger topic that would take more time to mature.
>
> Sigh... the test part of this patch is taken hostage to an earlier
> patches in this series that are iffy, so I cannot quite apply this
> fix alone at this moment.
>
> Yuck.
Here is what I'll queue directly on top of am/pathspec-from-file
e440fc58 ("commit: support the --pathspec-from-file option",
2019-11-19), to be fast-tracked.
-- >8 --
Subject: [PATCH] commit: forbid --pathspec-from-file --all
I forgot this in my previous patch `--pathspec-from-file` for
`git commit` [1]. When both `--pathspec-from-file` and `--all` were
specified, `--all` took precedence and `--pathspec-from-file` was
ignored. Before `--pathspec-from-file` was implemented, this case was
prevented by this check in `parse_and_validate_options()` :
die(_("paths '%s ...' with -a does not make sense"), argv[0]);
It is unfortunate that these two cases are disconnected. This came as
result of how the code was laid out before my patches, where `pathspec`
is parsed outside of `parse_and_validate_options()`. This branch is
already full of refactoring patches and I did not dare to go for another
one.
Fix by mirroring `die()` for `--pathspec-from-file` as well.
[1] Commit e440fc58 ("commit: support the --pathspec-from-file option" 2019-11-19)
Reported-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
[jc: adjusted test not to depend on other patches]
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
builtin/commit.c | 3 +++
t/t7526-commit-pathspec-file.sh | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/builtin/commit.c b/builtin/commit.c
index ed40729355..c040dc92a4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -347,6 +347,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
if (interactive)
die(_("--pathspec-from-file is incompatible with --interactive/--patch"));
+ if (all)
+ die(_("--pathspec-from-file with -a does not make sense"));
+
if (pathspec.nr)
die(_("--pathspec-from-file is incompatible with pathspec arguments"));
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index a06b683534..4b58901ed6 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -127,4 +127,10 @@ test_expect_success 'only touches what was listed' '
verify_expect
'
+test_expect_success '--pathspec-from-file and --all cannot be used together' '
+ restore_checkpoint &&
+ test_must_fail git commit --pathspec-from-file=- --all -m "Commit" 2>err &&
+ test_i18ngrep "[-]-pathspec-from-file with -a does not make sense" err
+'
+
test_done
--
2.24.1-722-g0b9e186032
