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

Fix wait.sh/copy.sh quoting for exec'd/wrapped cmd. #22

Merged
merged 1 commit into from
May 13, 2024
Merged

Conversation

kanaka
Copy link
Member

@kanaka kanaka commented May 9, 2024

Add "Running" comment to copy.sh to mirror wait.sh. Fix minor formating issue in copy.sh.

@jonsmock
Copy link
Contributor

jonsmock commented May 10, 2024

Quoting melts my brain for some reason (also in mal...). I'm working out this failing scenario:

$ ./scripts/wait.sh -c 'sleep 1' -- ./scripts/wait.sh -- echo 'hi'
Command successful: sleep 1
Running: ./scripts/wait.sh -- echo hi
Running: echo hi
hi

$ ./scripts/wait.sh -- ./scripts/wait.sh -c 'sleep 1' -- echo 'hi'
Running: ./scripts/wait.sh -c sleep 1 -- echo hi
sleep: missing operand
Try 'sleep --help' for more information.
Failed: '-c sleep'. Sleep 1 seconds before retry
sleep: missing operand
Try 'sleep --help' for more information.
Failed: '-c sleep'. Sleep 1 seconds before retry
sleep: missing operand
Try 'sleep --help' for more information.
Failed: '-c sleep'. Sleep 1 seconds before retry
^C

Changing exec ${@} to exec "${@}" seems to work, though:

$ ./scripts/wait.sh -c "sleep 1" -- ./scripts/wait.sh -c "sleep 1" -- echo "hi\""
Command successful: sleep 1
Running: ./scripts/wait.sh -c sleep 1 -- echo hi"
Command successful: sleep 1
Running: echo hi"
hi"

$ ./scripts/wait.sh -c 'sleep 1' -- ./scripts/wait.sh -c 'sleep 1' -- echo "hi\""
Command successful: sleep 1
Running: ./scripts/wait.sh -c sleep 1 -- echo hi"
Command successful: sleep 1
Running: echo hi"
hi"

@jonsmock
Copy link
Contributor

Using the original exec "${@}" from the copy.sh also in wait.sh fixes a tshark filter quoting issue (that I forgot I had and didn't understand previously). Let me know if that works for your issue also.

@tpot
Copy link

tpot commented May 11, 2024

Yeah, “${@}” should always be used instead of ${@} see top answer at https://stackoverflow.com/questions/12314451/accessing-bash-command-line-args-vs

Add "Running" comment to copy.sh to match wait.sh. Fix minor formating
issue in copy.sh.
@kanaka
Copy link
Member Author

kanaka commented May 13, 2024

Yeah, that's what I was afraid of. I did a bit more debug work and figured out why "${@}" was failing in my environment. It's due to a behavior of yaml I wasn't previously aware of.

This breaks:

command: /wait -i eth0 -- \
     my-real-command.py

This also breaks:

command: /wait -i eth0 -- 
     my-real-command.py

This however works:

command: |
  /wait -i eth0 -- 
     my-real-command.py

Apparently the previous two versions are preserving the newlines as part of the arguments themselves although curiously, it looks like it's preserving them as whitespace (or some character that's not a newline). So using unquoted ${@} removed the newlines and worked for me (although obviously breaks other critical quoting behavior).

@kanaka kanaka merged commit 0f52463 into master May 13, 2024
2 checks passed
@tpot
Copy link

tpot commented May 14, 2024

@kanaka I spent an hour reading the YAML spec (on an exercise bike so would have been watching YouTube anyway) so you don't have to, and to be honest am not that more enlightened, except to know that the backslash is never a line continuation in YAML. Backslashes are only used as a special character in double-quoted style flow scalars to escape a fixed number of single characters, e.g \n.

I suspect all of the odd behaviour is ultimately bash's fault - prove me wrong! (:

Here's my YAML scalar cheat sheet:

$ cat > foo.yaml
flow-scalar-styles:
  plain: foo \
  double-quoted: "foo \\"
  single-quoted: 'foo \'
block-scalar-styles:
  plain:
    foo
    bar
  literal: |
    foo
    bar
  folded: >
    foo
    bar

$ yq -o json < foo.yaml
{
  "flow-scalar-styles": {
    "plain": "foo \\",
    "double-quoted": "foo \\",
    "single-quoted": "foo \\"
  },
  "block-scalar-styles": {
    "plain": "foo bar",
    "literal": "foo\nbar\n",
    "folded": "foo bar\n"
  }
}

Surprisingly, plain-style block scalars turn out to contain the least amount of bonus whitespace. You are almost compelled to use a backslash line continuation due to muscle memory. if you start the value on the same line as the key. It turns out that these two YAML documents are equivalent, but the second doesn't instantly look like it would be a scalar, at least to my eyes.

command: foo 
  bar
command:
  foo
  bar

and both produce the JSON

{
  "command": "foo bar"
}

It appears impossible to be rid of the trailing newline for literal and folded block scalars, unless it appears at the end of the file and the file has no trailing newline (which is actually a violation of the POSIX definition of a line).

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.

3 participants