This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3620

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

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

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG0b39edeff033: tests: fix test-patch.t on pickier /bin/sh implementations
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]
Closed by commit rHG0b39edeff033: tests: fix test-patch.t on pickier /bin/sh implementations (authored by durin42). · Explain WhyMay 19 2018, 3:49 PM
This revision was automatically updated to reflect the committed changes.
martinvonz added a subscriber: martinvonz.May 24 2018, 12:27 PM

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.

durin42 added a comment.May 24 2018, 12:32 PM
In D3620#57367, @martinvonz wrote:

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.

martinvonz added a comment.May 24 2018, 12:41 PM
In D3620#57369, @durin42 wrote:
In D3620#57367, @martinvonz wrote:

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)?

durin42 added a comment.May 24 2018, 1:14 PM
In D3620#57370, @martinvonz wrote:
In D3620#57369, @durin42 wrote:
In D3620#57367, @martinvonz wrote:

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

Revision Contents
Changeset List

PathPackages
Mtests/test-patch.t (7 lines)

Diff 8822

tests/test-patch.t

A patch marker like this was more problematic even after d7452292f9d3: A patch marker like this was more problematic even after d7452292f9d3:
# HG changeset patch # HG changeset patch
# User lines looks like this - but it _is_ just a comment # User lines looks like this - but it _is_ just a comment
Error exit (issue4746) Error exit (issue4746)
$ hg import ../c/p --config ui.patch='sh -c "exit 1"' $ cat >> exit1.py <<EOF
> import sys
> sys.exit(1)
> EOF
$ hg import ../c/p --config ui.patch="$PYTHON '`pwd`/exit1.py'"
pulkitUnsubmitted
Not Done

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]
pulkit: test-check-code.t says hi: ``` + tests/test-patch.t:100: + > $ hg import ../c/p --config…
applying ../c/p applying ../c/p
abort: patch command failed: exited with status 1 abort: patch command failed: exited with status 1
[255] [255]
$ cd .. $ cd ..