devspace-sh / devspace

DevSpace - The Fastest Developer Tool for Kubernetes ⚡ Automate your deployment workflow with DevSpace and develop software directly inside Kubernetes.
https://devspace.sh
Apache License 2.0
4.36k stars 361 forks source link

Command line argument reference issue when custom command is a bash script #1598

Open nachin opened 3 years ago

nachin commented 3 years ago

What happened? If you have a custom command that is attempting to execute a bash script that references a command line argument $0 will reference the first command line argument not $1 as expected.

What did you expect to happen instead? If I have a custom command called echo-test that executes the following in a container sh -c 'echo $1' and I run devspace run echo-test Hello No output will be echoed, I have to set the index to $0 in the command to get the expected output of "Hello".

How can we reproduce the bug? (as minimally and precisely as possible)

  1. Create a custom command like the following: "devspace enter --container test-container --sh -c 'echo $1'"
  2. run the custom command with one command line argument eg.devspace run echo-test Hello

Notes: If the command is edited to echo $0 the command line argument is printed as expected but if you do not provide a command line argument the printed value is correct, it prints sh.

Local Environment:

Kubernetes Cluster:

Anything else we need to know?

/kind bug

LukasGentele commented 3 years ago

Correct me if I misunderstood you but here are some clarifications:

Custom commands are executed by appending the arguments, so the result in your example would be: sh -c "echo $1" [arg1] [arg2] ...

To be able to do what you want to achieve, you need a bash function:

command: run() { sh -c "echo $1" }; run 

=> This would be executed as: run() { sh -c "echo $1" }; run [arg1] [arg2] ...

I know that it seems uncomfortable and annoying that you cannot just use $1, $2, etc. @FabianKramm Do we just generally want to wrap commands in run() { COMMAND }; run ? This would allow $1, $2, etc. usage out of the box as users seem to expect it. I had to recommend this workaround at least 5 times in the past 3 months, so it's clearly something most folks would be benefiting from and it's clearly not ideal/intuitive as it is right now because nobody understands that commands are just appending the args instead of passing them to a command. A simple wrap would fix that. An alternative could be to write a temp file and then execute it and pass the args. Not sure if other alternatives would be possible in this case.

FabianKramm commented 3 years ago

@nachin yes the correct answer for the current version is:

commands:
  - name: echo-test
    command: run() { devspace enter --container test-container -- sh -c "echo $1"; }; run

But I agree with @LukasGentele that is currently a very cumbersome solution. Wrapping the command unfortunately only works if the "inner" command uses the "$1" variables, but the following wouldn't work:

# Prints nothing
run() { echo }; run test

However, to improve the current situation we will change for the next version the following 2 things:

  1. We will add an option called commands[*].appendArgs that by default is enabled
  2. We will tell the internal command line interpreter to parse the args as parameters so that constructs such as $#, $@, $0 etc. work correctly.

In one of the next config versions we then might disable commands[*].appendArgs by default. So this would simplify your use case to:

commands:
- name: echo-test
  appendArgs: false
  command: devspace enter --container test-container -- sh -c 'echo $1'
nachin commented 3 years ago

@LukasGentele Thank you so much for workaround