New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testshell_markdown should be disabled if `bash` is not available #2077

Closed
ingwinlu opened this Issue Jun 15, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@ingwinlu
Contributor

ingwinlu commented Jun 15, 2018

Steps to Reproduce the Problem

build and run ctest on a system without bash.

Expected Result

bash dependent tests are not executed

Actual Result

138/172 Test #134: testshell_markdown_issue_template ........***Failed    0.00 sec
env: can't execute 'bash': No such file or directory

System Information

  • Elektra Version: master
  • Operating System or Docker Container?
  • Versions of other relevant software?

Further Log Files and Output

https://build.libelektra.org/jenkins/blue/organizations/jenkins/libelektra/detail/PR-2063/52/pipeline/444

@markus2330

This comment has been minimized.

Contributor

markus2330 commented Jun 15, 2018

Thank you for reporting the problem!

As long term goal we want to get rid of bash, see #1938.

But excluding tests that need bash on a system without bash seems to be useful and not too much work. Greping for "env bash" it looks like the shell recorder is the only part which needs bash. So a check and if in add_msr_test should be enough.

@ingwinlu

This comment has been minimized.

Contributor

ingwinlu commented Jun 16, 2018

I tried implementing it via find_program but that produced more exclusion messages as I liked (I wanted to print exactly 1 if bash could not be found)

@ingwinlu

This comment has been minimized.

Contributor

ingwinlu commented Jun 16, 2018

I will probably just rewrite the shell recorder wo work with sh...

@ingwinlu ingwinlu removed the help wanted label Jun 16, 2018

@ingwinlu ingwinlu self-assigned this Jun 16, 2018

@ingwinlu

This comment has been minimized.

Contributor

ingwinlu commented Jun 16, 2018

@sanssecours I am getting close now. But can you explain to me what happens here?

That part isn't even parsing for me on bash :X

EDIT: NVM got it, sorry for the ping

@sanssecours

This comment has been minimized.

Contributor

sanssecours commented Jun 16, 2018

But can you explain to me what happens here?

This line uses a feature called process substitution to compare two “files” with diff. Anyway, you do not need to worry about that stuff, I already removed this code in pull request #2066. If you want you can just delete lines 135 till 145.

That part isn't even parsing for me on bash :X

Wikipedia states that this feature is quite old:

The Bash shell provided process substitution no later than version 1.14, released in 1994.

. Maybe you are using bash in sh-compatibility mode by specifying the hash bang #!/bin/sh:

cat <(echo bla)
#> bla
set -o posix
cat <(echo bla)
# STDERR: syntax error near unexpected token `('

?

@ingwinlu ingwinlu removed their assignment Jun 16, 2018

sanssecours added a commit to sanssecours/elektra that referenced this issue Aug 17, 2018

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