feat(cmd)!: run abra.sh commands with /bin/bash if available. #229

2022-11-15T13:19:24Z

moritz commented

2022-11-15 13:19:24 +00:00

Solving this issue: coop-cloud/organising#357

BREAKING CHANGE: abra.sh commands that depend on /bin/sh will break.

Solving this issue: https://git.coopcloud.tech/coop-cloud/organising/issues/357 BREAKING CHANGE: abra.sh commands that depend on /bin/sh will break.

moritz added 1 commit 2022-11-15 13:19:25 +00:00

feat(cmd)!: run abra.sh commands with /bin/bash if available.

continuous-integration/drone/pr Build is passing

Details

a48e2e2607

BREAKING CHANGE: abra.sh commands that depend on /bin/sh will break

decentral1se requested changes 2022-11-15 15:36:42 +00:00

decentral1se left a comment

Nice one @moritz! I think the breaking change is OK since a lot of stuff was specifically relying on /bin/sh due to a missing /bin/bash. So, with /bin/bash still missing, those scripts will go on working. Typically, you have some guarantees over the shell environment of the target container and projects rarely switch this up in my experience. Let's see what the rest think.

Nice one @moritz! I think the breaking change is OK since a lot of stuff was specifically relying on `/bin/sh` due to a missing `/bin/bash`. So, with `/bin/bash` still missing, those scripts will go on working. Typically, you have some guarantees over the shell environment of the target container and projects rarely switch this up in my experience. Let's see what the rest think.

cli/app/cmd.go Outdated

						
				@@ -88,3 +88,3 @@

							}

							cmd := exec.Command("/bin/sh", "-c", sourceAndExec)

							var shell string = "/bin/bash"

decentral1se commented

2022-11-15 15:32:59 +00:00

shell := "/bin/bash" is the style we've been using elsewhere in the codebase.

`shell := "/bin/bash"` is the style we've been using elsewhere in the codebase.

moritz marked this conversation as resolved

cli/app/cmd.go Outdated

						
				@@ -201,3 +205,2 @@

					logrus.Debugf("running command: %s", strings.Join(cmd, " "))

					var shell string = "/bin/bash"

decentral1se commented

2022-11-15 15:33:37 +00:00

shell := "/bin/bash"

`shell := "/bin/bash"`

moritz marked this conversation as resolved

cli/app/cmd.go Outdated

						
				@@ -202,2 +206,2 @@

					logrus.Debugf("running command: %s", strings.Join(cmd, " "))

					var shell string = "/bin/bash"

					var findShell []string = []string{"test", "-e", shell}

decentral1se commented

2022-11-15 15:34:02 +00:00

findShell := []string{"test", "-e", shell}

`findShell := []string{"test", "-e", shell}`

moritz marked this conversation as resolved

moritz added 1 commit 2022-11-15 20:29:53 +00:00

Resolve reviews

continuous-integration/drone/pr Build is passing

Details

d7d8689d53

moritz requested review from decentral1se 2022-11-15 21:36:08 +00:00

decentral1se approved these changes 2022-11-15 21:58:35 +00:00

decentral1se left a comment

💯

(folks on the chat seem to think this is great also, so let's merge it!)

💯 (folks on the chat seem to think this is great also, so let's merge it!)

decentral1se referenced this issue from a commit

2022-11-15 22:02:19 +00:00

feat(cmd)!: run abra.sh commands with /bin/bash if available.

decentral1se commented

2022-11-15 22:02:25 +00:00

🤔

This pull request is broken due to missing fork information.

I merged it by hand in 7f745ff19f

🤔 > This pull request is broken due to missing fork information. I merged it by hand in 7f745ff19fa7f33953d0abec18e80fd1630119fe

decentral1se closed this pull request

2022-11-15 22:02:26 +00:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: toolshed/abra#229

Allow edits from maintainers