Skip to content

scripts: Call bash through env#5704

Closed
fabiankeil wants to merge 1 commit intowolfSSL:masterfrom
fabiankeil:more-portable-test-scripts
Closed

scripts: Call bash through env#5704
fabiankeil wants to merge 1 commit intowolfSSL:masterfrom
fabiankeil:more-portable-test-scripts

Conversation

@fabiankeil
Copy link
Copy Markdown
Contributor

.. so the test scripts work on platforms like ElectroBSD and FreeBSD where bash usually isn't installed in /bin but in /usr/local/bin.

Fixes:

[fk@test-vm ~/git/wolfssl]$ gmake check
gmake -j3 check-recursive
[...]
FAIL: scripts/resume.test
FAIL: scripts/tls13.test
PASS: testsuite/testsuite.test

Testsuite summary for wolfssl 5.5.1

TOTAL: 3

PASS: 1

SKIP: 0

XFAIL: 0

FAIL: 2

XPASS: 0

ERROR: 0

Description

Please describe the scope of the fix or feature addition.

Fixes zd#

Testing

How did you test?

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

.. so the test scripts work on platforms like ElectroBSD and FreeBSD
where bash usually isn't installed in /bin but in /usr/local/bin.

Fixes:

   [fk@test-vm ~/git/wolfssl]$ gmake check
   gmake -j3  check-recursive
   [...]
   FAIL: scripts/resume.test
   FAIL: scripts/tls13.test
   PASS: testsuite/testsuite.test
   ============================================================================
   Testsuite summary for wolfssl 5.5.1
   ============================================================================
   # TOTAL: 3
   # PASS:  1
   # SKIP:  0
   # XFAIL: 0
   # FAIL:  2
   # XPASS: 0
   # ERROR: 0
@wolfSSL-Bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@dgarske dgarske requested a review from douzzer October 17, 2022 14:29
@dgarske
Copy link
Copy Markdown
Member

dgarske commented Oct 17, 2022

OK to test. Contributor agreement on file

@douzzer
Copy link
Copy Markdown
Contributor

douzzer commented Oct 17, 2022

Hi @fabiankeil

There's no right answer here, but we would rather the scripts depend on /bin/bash being usable, than on /usr/bin/env being usable, and moreover, on $PATH having a value that doesn't result in surprises.

If we were to go down the /usr/bin/env route, there are at least 31 bash scripts in wolfssl that would need to be changed, and they would all become reliant on env, and all be subject to hard-to-troubleshoot misbehavior around surprising $PATH results.

The pedantically-correct solution here is for all scripts that use bash to have .in versions headed with #!@bashpath@, with configure detecting the right path for bash (per $PATH, subject to --with-bash override) and replacing @bashpath@ to create the executable script. As I noted above, by my cursory count there are 31 scripts that would need that treatment.
But there is a likely-fatal flaw with this solution: all of those scripts would become unusable outside autotools buildenvs/runtimes (of which we support a great number), which seems far too high a price to pay.

TL;DR: When and where needed, add a symlink at /bin/bash to your preferred bash. I've always arranged for my own FreeBSD installations to have the same bash at /bin/bash as at /usr/local/bin/bash, and it's only ever been a problem solver for me.

@fabiankeil
Copy link
Copy Markdown
Contributor Author

Understood. Thanks for the detailed response.

@fabiankeil fabiankeil closed this Oct 17, 2022
@douzzer
Copy link
Copy Markdown
Contributor

douzzer commented Oct 17, 2022

@fabiankeil you're welcome. And btw, if it's useful to run the scripts à la carte without changing them or adding the /bin/bash symlink, you can do that with e.g.

$ bash scripts/resume.test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants