I'm just trying to prove how important your other patch series is. ;-)
Actually, this shows why the other patch series, especially its last
step, is problematic a bit. The above assignments are followed by a
single command, i.e.
+ (
+ PATH=./test-bin:$PATH
+ test_must_fail git rebase -i -s funny -Xopt -Xfoo master topic
+ ) &&
+ (
+ PATH=./test-bin:$PATH
+ git rebase --continue
+ ) &&
and it would be reasonable to assume that these variable assignments
would not fail. IOW, there is no BUG in breaking &&-chain at these
two places. It is merely that the automated mechanism introduced by
step 29/29 of that other series does not understand that (which is
expected). It is not wrong to have && at the end of the assignment,
though.
Having said that, it would be simpler for at least the latter to
write it using a single-shot environment assignment, perhaps? I.e.
PATH=./test-bin:$PATH git rebase --continue &&
without running in a subshell?
On Wed, Jun 27, 2018 at 9:54 AM, Junio C Hamano [off-list ref] wrote:
+ (
+ PATH=./test-bin:$PATH
+ test_must_fail git rebase -i -s funny -Xopt -Xfoo master topic
+ ) &&
+ (
+ PATH=./test-bin:$PATH
+ git rebase --continue
+ ) &&
and it would be reasonable to assume that these variable assignments
would not fail. IOW, there is no BUG in breaking &&-chain at these
two places. It is merely that the automated mechanism introduced by
step 29/29 of that other series does not understand that (which is
expected). It is not wrong to have && at the end of the assignment,
though.
Having said that, it would be simpler for at least the latter to
write it using a single-shot environment assignment, perhaps? I.e.
PATH=./test-bin:$PATH git rebase --continue &&
without running in a subshell?
Seems reasonable. Since these tests were essentially copies of other
tests within the same file, just for rebase -i instead of -m, should I
also add another patch to the series fixing up the rebase -m testcase
to also replace the subshell with a single-shot environment
assignment?
From: Johannes Sixt <hidden> Date: 2018-06-27 18:17:47
Am 27.06.2018 um 19:27 schrieb Elijah Newren:
On Wed, Jun 27, 2018 at 9:54 AM, Junio C Hamano [off-list ref] wrote:
quoted
Having said that, it would be simpler for at least the latter to
write it using a single-shot environment assignment, perhaps? I.e.
PATH=./test-bin:$PATH git rebase --continue &&
without running in a subshell?
Seems reasonable. Since these tests were essentially copies of other
tests within the same file, just for rebase -i instead of -m, should I
also add another patch to the series fixing up the rebase -m testcase
to also replace the subshell with a single-shot environment
assignment?
Pitfalls ahead!
PATH=... git rebase ...
is OK, but
PATH=... test_must_fail git rebase ...
is not; the latter requires the subshell, otherwise the modified PATH variable survives the command because test_must_fail is a shell function. Yes, it's silly, but that's how it is.
-- Hannes
From: Eric Sunshine <hidden> Date: 2018-06-27 18:24:44
On Wed, Jun 27, 2018 at 2:17 PM Johannes Sixt [off-list ref] wrote:
Pitfalls ahead!
PATH=... git rebase ...
is OK, but
PATH=... test_must_fail git rebase ...
is not; the latter requires the subshell, otherwise the modified PATH
variable survives the command because test_must_fail is a shell
function. Yes, it's silly, but that's how it is.
As an alternative to the subshell, some test use 'env':
test_must_fail env PATH=... git rebase ...
From: SZEDER Gábor <hidden> Date: 2018-06-27 18:35:07
Am 27.06.2018 um 19:27 schrieb Elijah Newren:
quoted
On Wed, Jun 27, 2018 at 9:54 AM, Junio C Hamano [off-list ref] wrote:
quoted
Having said that, it would be simpler for at least the latter to
write it using a single-shot environment assignment, perhaps? I.e.
PATH=./test-bin:$PATH git rebase --continue &&
without running in a subshell?
Seems reasonable. Since these tests were essentially copies of other
tests within the same file, just for rebase -i instead of -m, should I
also add another patch to the series fixing up the rebase -m testcase
to also replace the subshell with a single-shot environment
assignment?
Pitfalls ahead!
PATH=... git rebase ...
is OK, but
PATH=... test_must_fail git rebase ...
is not; the latter requires the subshell, otherwise the modified PATH
variable survives the command because test_must_fail is a shell
function. Yes, it's silly, but that's how it is.
We have the 'test_env' helper function for this case:
test_env PATH=./test-bin:$PATH test_must_fail git rebase --opts