eclipse / kuksa.val.services

Repository for Vehicle Service Related implementations for Eclipse SDV
Apache License 2.0
15 stars 18 forks source link

seat_service: remove bash dependencies from scripts #71

Closed d-s-e closed 1 year ago

d-s-e commented 1 year ago

Removed all bash dependencies and cleaned up the seat service shell scripts.

erikbosch commented 1 year ago

Looks good to me, but I have not tested that they work as expected. @d-s-e - have you verified that the scripts work as expected after conversion?

d-s-e commented 1 year ago

I could not test the full scripts, but I tried to not alter the functionality and manually tested most of the replacements I did. I see no reason, why they shouldn't work.

int0x27 commented 1 year ago

I'll push a PR that supersedes this one, mainly added can-utills to the docker image and keep it bash compatible. Current suggestion is not pure sh compatible, so better stay on bash, and if needed you can just replace the shebang line or: sudo ln -s /bin/sh /bin/bash

d-s-e commented 1 year ago

@int0x27 How could this help!? The scripts contain lots of bashisms, simply replacing the shebang can't fix that! The mentioned symlinks usually exists anyway on systems using bash.

The primary reason for this PR is to remove any dependencies to GPLv3 licensed software (which bash is) on the target.

int0x27 commented 1 year ago

If this works on sh, probably everything else works:

local data=${frame#*#}

our docker image uses ubuntu runtime image, so bash is included there

d-s-e commented 1 year ago

If this works on sh, probably everything else works:

That doesn't matter, there are some other lines, that won't work on a system without bash.

our docker image uses ubuntu runtime image, so bash is included there

I don't use your docker image; Ubuntu is way too big for the use case anyway. I build the container image with a bitbake recipe. The goal is to create a distribution (Eclipse Leda) with preinstalled containers, that can be used without any GPLv3 dependencies.

int0x27 commented 1 year ago

Can you check /bin/sh changes here: https://github.com/eclipse/kuksa.val.services/pull/79 If they are OK we can close this PR

d-s-e commented 1 year ago

@int0x27 Looks good to me.