Skip to content
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

Enable null globs when scanning for config.d/*.sh #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nogweii
Copy link

@nogweii nogweii commented Sep 30, 2022

Without this, an empty directory leads to an false error complaining that the file /etc/dehydrated/config.d/*.sh is not readable or doesn't exist.

With nullglob set, the glob, if it doesn't match anything, will expand into null rather than returning the glob string, causing an empty config.d folder to not throw an error.

Without this, an empty directory leads to an false error complaining that the file `/etc/dehydrated/config.d/*.sh` is not readable or doesn't exist.

With `nullglob` set, the glob, if it doesn't match anything, will expand into null rather than returning the glob string, causing an empty config.d folder to not throw an error.
@silkeh
Copy link
Owner

silkeh commented Oct 9, 2022

Thanks for this PR, seems sane to me generally. However, the unit tests fail because the script now exits with a zero exit code.
You can see this locally:

$ env VERBOSE=yes ./test.sh tests/hooks.sh
Testing using Bash 5.1.16(1)-release
Hooks
 + exit 	[PASS]
 + deploy 	[PASS]
Hook errors
 + exit 	[PASS]
 + deploy 	[PASS]
Hook invalid scripts
 + exit 	[FAIL]
EXPECTED
 ./pdns_api.sh: line 369: /Foo3Xeiz: No such file or directory exit=127
 GOT
 ./pdns_api.sh: line 373: /Foo3Xeiz: No such file or directory
exit=127
 ---
 + deploy 	[FAIL]
EXPECTED
 ./pdns_api.sh: line 375: /dah5Aiph: No such file or directory exit=127
 GOT
 ./pdns_api.sh: line 379: /dah5Aiph: No such file or directory
exit=127
 ---
Tests complete: 2 failures.

Copy link
Owner

@silkeh silkeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above.

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

Successfully merging this pull request may close these issues.

2 participants