Allow script mode to accept scripts that do not start with a shebang. #1691
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
the
size/M
|
The following is the coverage report on pkg/.
|
|
The following is the coverage report on pkg/.
|
|
The following is the coverage report on pkg/.
|
pkg/pod/script.go
Outdated
| lines := strings.Split(s.Script, "\n") | ||
| hasShebang := false | ||
| for _, l := range lines { | ||
| if strings.TrimSpace(l) == "" { |
There was a problem hiding this comment.
Does TrimSpace ignore blank lines? I'd love to be able to avoid splitting all the lines just to find the first non-empty one.
| available within the step's container. | ||
| Scripts that do not start with a shebang | ||
| [shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)) line will use a default | ||
| value of `#!/bin/sh`, although users can override this by starting their script |
There was a problem hiding this comment.
small suggestion, feel free to ignore since this is a dockerism, but #!/usr/bin/env sh will account for situations where sh is in /usr/bin or /usr/sbin of the image instead of /bin
There was a problem hiding this comment.
Yeah, I considered that but decided to stick with the docker version, I suppose there might be images with sh but not env?
This prefixes scripts that do not start with a shebang with a default value of "/bin/sh". This matches the default entrypoint/shell for docker containers, and should help prevent an easy-to-make mistake.
|
The following is the coverage report on pkg/.
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester 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 |
tekton-robot
added
the
approved
Changes
This prefixes scripts that do not start with a shebang with a default value of "/bin/sh".
This matches the default entrypoint/shell for docker containers, and should help
prevent an easy-to-make mistake.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes