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

Use the pipeline's shell property in the pod #34

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

Conversation

xrl
Copy link

@xrl xrl commented Jul 16, 2020

I want to override /bin/sh with a shell of my choice. Does this code look like a good start? How would I write a good test for this? One wrinkle, I can't get it working on my side, the pod says it can't find the /bin/bash executable, which is odd. Any tips?

@CLAassistant
Copy link

CLAassistant commented Jul 16, 2020

CLA assistant check
All committers have signed the CLA.

@bradrydzewski
Copy link
Collaborator

bradrydzewski commented Oct 4, 2020

/bin/sh is available on all linux and posix-compliant operating systems. There is no guarantee that /bin/bash will be available. For example, the alpine image (on which many popular images are based) does not have bash:

$ docker run -t -i alpine /bin/sh
/ $ which bash
/ $ echo $?
1
/ $ which sh
/bin/sh
/ $ 

dst.Entrypoint = []string{"/bin/sh", "-c"}
dst.Command = []string{`echo "$DRONE_SCRIPT" | /bin/sh`}
} else {
dst.Entrypoint = []string{dst.Shell, "-c"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

-dst.Entrypoint = []string{dst.Shell, "-c"}
+dst.Entrypoint = []string{"/bin/sh", "-c"}

I think this line can be reverted. There is no guarantee the user-defined shell supports -c or that the flag will have the same meaning. But more importantly, I don't think it is necessary since we pipe the script to the target shell in the line below, which is where it really counts.

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