-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
generate systemd: quote arguments with whitespace #7350
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg 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 |
@mheon a late candidate for 2.0. @edsantiago, I need a head nod from you before we can merge. Can you think of a case where the changes could lead to a regression? |
LGTM |
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.
I'm embarrassed not to have caught this in my testing. LGTM in principle but I'd like an added test, and possibly a rethinking of using regex.
7d0ad7c
to
d70727d
Compare
/lgtm |
Nice catch, LGTM |
LGTM |
No, it's the CI hang. @vrothberg you need to rebase on master, I'm sorry. |
Make sure that arguments with whitespace are properly quoted so they are interpreted as one (and not multiple ones) by systemd. Now `-e tz="america/new york"` will be generated as `-e "tz=america/new york"`. The quotes are moving but the argument is still correct. Fixes: containers#7285 Signed-off-by: Valentin Rothberg <[email protected]>
merge me :) |
/lgtm |
/hold cancel |
Make sure that arguments with whitespace are properly quoted so they are
interpreted as one (and not multiple ones) by systemd.
Now
-e tz="america/new york"
will be generated as-e "tz=america/new york"
.The quotes are moving but the argument is still correct.
Fixes: #7285
Signed-off-by: Valentin Rothberg [email protected]