Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

Fix bash dependency #617

Merged
merged 4 commits into from
Jul 24, 2023

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 <g.meyer@signum-media.de>
@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-archived:master Jul 24, 2023
@d-s-e d-s-e deleted the fix-bash-dependency branch July 25, 2023 08:30
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove bash dependency from shell scripts
3 participants