-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
allow no matches when grepping for unsupported_cgroups #2743
allow no matches when grepping for unsupported_cgroups #2743
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pushing images |
/retest |
@@ -27,6 +27,11 @@ if grep -Eqv "0[[:space:]]+0[[:space:]]+4294967295" /proc/self/uid_map; then | |||
echo 'INFO: running in a user namespace (experimental)' | |||
fi | |||
|
|||
grep_allow_nomatch() { | |||
# grep exits 0 on match, 1 on no match, 2 on error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EXIT STATUS
Normally the exit status is 0 if a line is selected, 1 if no lines were
selected, and 2 if an error occurred. However, if the -q or --quiet or
--silent is used and a line is selected, the exit status is 0 even if
an error occurred.
is it better to use --silent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-q
, --quiet
, and --silent
are the same flag, and no that's not a suitable option:
-q, --quiet, --silent
Quiet; do not write anything to standard output. Exit
immediately with zero status if any match is found, even if
an error was detected. Also see the -s or --no-messages
option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also we're not trying to swallow errors, we're remapping from:
- match: 0
- no-match: 1
- error: 2
to:
- match: 0
- no-match: 0
- error: non-zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh
/lgtm |
@@ -27,6 +27,11 @@ if grep -Eqv "0[[:space:]]+0[[:space:]]+4294967295" /proc/self/uid_map; then | |||
echo 'INFO: running in a user namespace (experimental)' | |||
fi | |||
|
|||
grep_allow_nomatch() { | |||
# grep exits 0 on match, 1 on no match, 2 on error | |||
grep "$@" || [[ $? == 1 ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: We could consider using a less terse version and retain the exact exit code.
right now that's not important and I'd rather not have to rebase and re-build #2737 again unless there's a seriours error here.
We probably should follow-up with preserving the exact status for future usage.
Or just switch to pure awk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it seems that awk would look nicer. Something like
unsupported_cgroups=$(findmnt -lun -o source,target -t cgroup | awk '!/'"${current_cgroup}"'/ {print $2}')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels slightly wrong that we're regex matching in either case instead of substring literal but :this-is-fine:
I suppose we could regex escape it with some more bash 🙈
It looks like if we stuck with grep we could use -F
/ --fix-strings
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For awk
, one can use something index(string, substring) == -1
. Some implementations treat substr as a regexp, but GNU awk does a fixed substring match.
see: #2737 (comment)
https://linuxcommand.org/lc3_man_pages/grep1.html#:~:text=is%20not%20set.-,EXIT%20STATUS,-Normally%20the%20exit