plugins/convert_README_to_header.sh doesn't work in bionic (and possibly others after xenial)

[peternewman]

Although your earlier bionic run did run, and fail:
https://travis-ci.org/OpenLightingProject/ola/jobs/609809927#L974-L977

sh ./plugins/convert_README_to_header.sh ./plugins/artnet ./plugins/artnet/ArtNetPluginDescription.h
sh ./plugins/convert_README_to_header.sh ./plugins/dummy ./plugins/dummy/DummyPluginDescription.h
sed: -e expression #6, char 14: unterminated `s' command
sed: -e expression #6, char 14: unterminated `s' command

Is that the same as you get on your personal machine with 19.04?

Originally posted by @peternewman in #1586

See the rest of the discussion in #1586 (review)

We should start by making the script fail so make fails if the sed command fails, and then try and fix the command.

[Keeper-of-the-Keys]

Either it fails elegantly - just doesn't insert newlines - or it fails depending on the version of the regex.

Since the newlines are not important to anything (except the lint check which should not run on autogenerated content) I propose we have enough important stuff to worry about and that we just fix the regex to not insert newlines on any platform.

[peternewman]

This looks like it should be easy enough to fix it:
https://unix.stackexchange.com/a/23028

We need to get it failing reliably, regardless of other stuff for the future.

You're missing my point, the script shouldn't fail silently regardless of the cause, we need to toughen it up, then we can look at fixing it.

Looks like this should do the job with some tidying.

diff --git a/plugins/convert_README_to_header.sh b/plugins/convert_README_to_header.sh
index 86b1b42..159d2a7 100755
--- a/plugins/convert_README_to_header.sh
+++ b/plugins/convert_README_to_header.sh
@@ -33,6 +33,11 @@ outfilename=`basename $outfile`;
 desc=`sed -e ':a' -e 'N' -e '$!ba' -e 's/\"/\\\"/g' -e 's/\n/\\\\n"\\
 "/g' "$path/README.md"`;
 
+if [ $? != 0 ]; then
+  echo "Failed to generate";
+  exit 1;
+fi
+
 identifier=`echo "PLUGINS_${plugin}_${outfilename%.h}_H_" | tr '[:lower:]' '[:upper:]'`
 
 cat <<EOM > $outfile

[peternewman]

@FloEdelmann do you fancy attempting to fix this?

[Keeper-of-the-Keys]

You are right that failure should not be silent - no argument there.
That being said the "not inserting newlines" is not a failure clearly sed returns 0 and is of the opinion it succeeded.

[Keeper-of-the-Keys]

BTW any reason the script is not bash but sh (which is basically subject to whatever sh interpreter your OS chooses)?

[Keeper-of-the-Keys]

Please check out 426654c
If that also works on OS X the everything is resolved.

[peternewman]

That being said the "not inserting newlines" is not a failure clearly sed returns 0 and is of the opinion it succeeded.

Have you tested that? I'd be surprised, and rather disappointed, if sed throws those errors and still exits zero? It doesn't for me (although curiously I get the same error message on Xenial when I run the command locally, which probably in itself needs more digging).

BTW any reason the script is not bash but sh (which is basically subject to whatever sh interpreter your OS chooses)?

Cross-platform, e.g. FreeBSD amongst others:
https://www.freebsd.org/doc/en_US.ISO8859-1/articles/linux-users/shells.html

We have to cater for the lowest common denominator unfortunately (not that I'm saying FreeBSD is, but we need to pick the most basic/common form of something to increase the chance of it working everywhere.

[peternewman]

Please check out 426654c
If that also works on OS X the everything is resolved.

Not quite, if it works on OS X, then we only need to try and test it on some BSDs and Windows. But we still need to make the script fail when it hits an error so we don't hit these issues in future.

[peternewman]

Being done in #1593