kubernetes / git-sync

A sidecar app which clones a git repo and keeps it in sync with the upstream.
Apache License 2.0
2.13k stars 406 forks source link

consistently use bash shebang for shell scripts #894

Closed sdowell closed 2 weeks ago

sdowell commented 2 weeks ago

This updates the build scripts to use a bash shebang for consistency with other scripts as well as better portability. If using a non-alpine BUILDIMAGE (e.g. debian) the build scripts are not valid sh syntax.

sdowell commented 2 weeks ago

/assign @thockin

sdowell commented 2 weeks ago

/assign @nan-yu

k8s-ci-robot commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nan-yu, sdowell

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: - ~~[OWNERS](https://github.com/kubernetes/git-sync/blob/master/OWNERS)~~ [nan-yu] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
thockin commented 1 week ago

As #895 shows, "bash" is not available in the BUILD_IMAGE (golang:1.22-alpine). We should make the scripts be stricter sh

sdowell commented 1 week ago

As #895 shows, "bash" is not available in the BUILD_IMAGE (golang:1.22-alpine). We should make the scripts be stricter sh

Could we switch the default BUILD_IMAGE to be the debian-based golang:1.22? That image ships with bash and would be a more consistent OS with BASEIMAGE.

I'm good with also making the script more strict/portable, but I don't see a benefit in using the alpine image in this instance.

sdowell commented 1 week ago

Are there not presubmits that verify the build?

thockin commented 1 week ago

There should be. I'm not sure why this would have passed

On Fri, Jun 21, 2024 at 10:21 AM Sam Dowell @.***> wrote:

Are there not presubmits that verify the build?

— Reply to this email directly, view it on GitHub https://github.com/kubernetes/git-sync/pull/894#issuecomment-2183144346, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKWAVEJFZ3YVDYESBXVLM3ZIROIBAVCNFSM6AAAAABJUUY3LOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBTGE2DIMZUGY . You are receiving this because you were assigned.Message ID: @.***>