Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bash dependency #617

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Conversation

d-s-e
Copy link
Contributor

@d-s-e d-s-e commented Jul 21, 2023

fixes #616

@erikbosch
Copy link
Contributor

What tests have you performed?

I get an error when trying to run ./genCerts.sh after your fix. With bash it works. Are possibly more changes needed in the file to avoid the bash dependency?

erik@debian3:~/kuksa.val/kuksa_certificates$ ./genCerts.sh 
./genCerts.sh: 27: Syntax error: "(" unexpected

@erikbosch
Copy link
Contributor

The newly added pre-commit check also complains, there seems to be a trailing space in one of the files. See https://github.com/eclipse/kuksa.val/blob/master/README.md#pre-commit-set-up on info how to set up pre-commit checks locally

@SebastianSchildt
Copy link
Contributor

Haven't checked, but maybe try a space before the " (", not sure POSIX has something to say about it.... When going the vanilla "sh" (which in your system might actually be anything), at least try with busybox, as that is at least a "likely" target ot be encountered in embedded environments

@d-s-e d-s-e marked this pull request as draft July 21, 2023 12:22
@d-s-e
Copy link
Contributor Author

d-s-e commented Jul 21, 2023

Sorry, I didn't see that.
Will fix this.

@d-s-e d-s-e marked this pull request as ready for review July 21, 2023 13:41
@d-s-e
Copy link
Contributor Author

d-s-e commented Jul 21, 2023

should work now.

Signed-off-by: Guenther Meyer <[email protected]>
@SebastianSchildt
Copy link
Contributor

I have been using echo all my life, and was wondering if it is really clever going for printf in shell here. Spoiler alert: It is. I've been deep in the rabbit hole just now, and wanted to share: https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo/65819#65819

Copy link
Contributor

@erikbosch erikbosch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran both scripts locally on my Debian VM, they worked and no regressions.

@SebastianSchildt - I have no problems if this PR gets merged before v0.4.0, it does not affect any release artifacts apart from the source zip

@SebastianSchildt SebastianSchildt merged commit 58dd5d1 into eclipse:master Jul 24, 2023
4 checks passed
@d-s-e d-s-e deleted the fix-bash-dependency branch July 25, 2023 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove bash dependency from shell scripts
3 participants