Page MenuHomePhabricator

tests: fix test-patch.t on picker /bin/sh implementations
ClosedPublic

Authored by durin42 on May 19 2018, 3:16 PM.

Details

Summary

FreeBSD sh(1) doesn't accept -d, so we weren't testing what we
expected there. Let's just use a simple Python script instead.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.May 19 2018, 3:16 PM
pulkit accepted this revision.May 19 2018, 3:19 PM
pulkit added inline comments.May 19 2018, 3:28 PM
tests/test-patch.t
100

test-check-code.t says hi:

+  tests/test-patch.t:100:
+   >   $ hg import ../c/p --config ui.patch="python '$PWD/exit1.py'"
+   don't use 'python', use '$PYTHON'
+   warning: don't use $PWD, use `pwd`
+  [1]
This revision was automatically updated to reflect the committed changes.

FreeBSD sh(1) doesn't accept -d, so we weren't testing what we
expected there. Let's just use a simple Python script instead.

This confused me for longer than it should. I assume you just mean -c instead of -d.

FreeBSD sh(1) doesn't accept -d, so we weren't testing what we
expected there. Let's just use a simple Python script instead.

This confused me for longer than it should. I assume you just mean -c instead of -d.

No, I actually mean -d. We *add* -d $PWD to the invocation of the patch tool internally before invoking it.

FreeBSD sh(1) doesn't accept -d, so we weren't testing what we
expected there. Let's just use a simple Python script instead.

This confused me for longer than it should. I assume you just mean -c instead of -d.

No, I actually mean -d. We *add* -d $PWD to the invocation of the patch tool internally before invoking it.

Hmm, so we were hoping that whatever sh implementation was used would support a -d option and would not fail when it was given $PWD? That's a really odd assumption, so thanks for cleaning that up. Do other shells (than FreeBSD) support a -d option (bash and dash don't seem to)?

FreeBSD sh(1) doesn't accept -d, so we weren't testing what we
expected there. Let's just use a simple Python script instead.

This confused me for longer than it should. I assume you just mean -c instead of -d.

No, I actually mean -d. We *add* -d $PWD to the invocation of the patch tool internally before invoking it.

Hmm, so we were hoping that whatever sh implementation was used would support a -d option and would not fail when it was given $PWD? That's a really odd assumption, so thanks for cleaning that up. Do other shells (than FreeBSD) support a -d option (bash and dash don't seem to)?

It looks like bash accepts unknown flags if -c is in use, but BSD sh doesn't. Maybe dash works the same way? I'm too lazy to ssh to a Debian machine right now to find out.

yuja added a subscriber: yuja.May 25 2018, 9:02 AM
> Hmm, so we were hoping that whatever `sh` implementation was used would support a -d option and would not fail when it was given $PWD? That's a really odd assumption, so thanks for cleaning that up. Do other shells (than FreeBSD) support a -d option (bash and dash don't seem to)?
It looks like bash accepts unknown flags if -c is in use, but BSD sh doesn't. Maybe dash works the same way? I'm too lazy to ssh to a Debian machine right now to find out.

Yes, dash accepts them as the arguments passed to the command.

% dash -c 'echo "$@"' -foo -bar -baz
-bar -baz