toolshed/abra
toolshed/abra
4
2
Fork 18
Code Issues 29 Pull Requests 3 Packages Releases 40 Activity

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

Closed
moritz wants to merge 2 commits from moritz/abra:main into main
pull from: moritz/abra:main
merge into: toolshed:main
Conversation 3 Commits 2 Files Changed 1 +30 -15
moritz commented 2022-11-15 13:19:24 +00:00
Member
Copy Link

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.
All checks were successful
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
Owner
Copy Link

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
Owner
Copy Link

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
Owner
Copy Link

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
Owner
Copy Link

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
All checks were successful
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
Owner
Copy Link

💯

(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 commented 2022-11-15 22:02:25 +00:00
Owner
Copy Link

🤔

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
All checks were successful
continuous-integration/drone/pr Build is passing
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
decentral1se
Labels
Clear labels
bug
Something is not working
build
go build related issues
ci/cd
Building things with CI/CD
contributing
Dealing with contributing
critical fix
https://docs.coopcloud.tech/federation/resolutions/passed/010/
design
UI/UX
documentation
Documenting all the things
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
meta
relating to the project or repo itself
question
More information is needed
release
Release management
release-candidate
Related to the new release candidate
security
Security related
test
Unit/integration testing
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
3wordchant abra-bot ammaratef45 Apfelwurm carla cas decentral1se fauno kawaiipunk knoflook lambdabundesverband moritz p4u1 simon trav val yksflip
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#229
No description provided.
Delete Branch "moritz/abra:main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?