{start,stop}-mycroft.sh: make POSIX sh compatible and fix shellcheck issues

[PureTryOut]

Description

Makes the start and stop scripts POSIX sh compatible, and fixes any shellcheck issues found.

This doesn't really depend on #2802, but without it bin/mycroft-start and bin/mycroft-stop will break. Thus, #2802 should be merged first.

How to test

Run all the usual variants of this test like normal (e.g. ./start-mycroft.sh) and see if anything behaves differently than before

Contributor license agreement signed?

CLA [x] (Whether you have signed a CLA - Contributor Licensing Agreement

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[PureTryOut]

Hmm, not sure why Travis fails.

$ ./start-mycroft.sh unittest
./start-mycroft.sh: 85: .: Can't open /home/travis/build/MycroftAI/mycroft-core/.venv/bin/activate

This works fine for me locally. Does .venv/bin/activate not exist on Travis for some reason?

[forslund]

The python env in Travis exists already so the .venv/bin/activate is not actually used here. Earlier the command failed and carried on with the already active venv. (See here for example https://travis-ci.org/github/forslund/mycroft-core/jobs/635713266#L1410)...so either add || true or perhaps the better way is to do a check if a venv already is active...

The following oneliner should be best according to stack overflow
python -c 'import sys;sys.exit(not (hasattr(sys, "real_prefix") or (hasattr(sys, "base_prefix") and sys.base_prefix != sys.prefix)))'

Rather long but still a single line...

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[PureTryOut]

I do not care if it's one line or more to be honest, but then again I'm not sure how to split that up into multiple 😂

Thanks for the explanation, I'll add that check.

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[krisgesling]

Hey, this looks great, thanks!

Only one small issue with shift that I think we need to resolve.

Thanks @fermulator and @forslund for reviewing and testing this already too!

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[krisgesling]

Sorry, looks like we aren't quite done...

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[PureTryOut]

Just rebased this. @krisgesling anything holding this back other than #2802 to be merged?

[devops-mycroft]

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

[pep8speaks]

Hello @PureTryOut! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-11-04 10:07:59 UTC

[codecov-commenter]

Codecov Report

Merging #2686 (50ecaaa) into dev (fd12c88) will not change coverage.
The diff coverage is n/a.

❗ Current head 50ecaaa differs from pull request most recent head 97e52d3. Consider uploading reports for the commit 97e52d3 to get more accurate results

@@           Coverage Diff           @@
##              dev    #2686   +/-   ##
=======================================
  Coverage   53.04%   53.04%           
=======================================
  Files         123      123           
  Lines       11170    11170           
=======================================
  Hits         5925     5925           
  Misses       5245     5245           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd12c88...97e52d3. Read the comment docs.

[forslund]

Looks really good and seems to work well. I did find a place where the check-dependenciescall hadn't been updated (L202)

[krisgesling]

With Ake's final catches addressed, this seems good to go!

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

Requested changes have been addressed.