mainsail-crew / crowsnest

Webcam Service for multiple Cams
GNU General Public License v3.0
302 stars 71 forks source link

Fails with no useful log output if crudini is not installed. #220

Closed Laikulo closed 6 months ago

Laikulo commented 6 months ago

What happened

On a system with bash 4, and crudini not installed, mainsail will exit with errors from logging.sh

Dec 17 01:35:09 voron crowsnest[25120]: /opt/crowsnest/libs/logging.sh: line 64: : No such file or directory
Dec 17 01:35:09 voron crowsnest[25120]: /opt/crowsnest/libs/logging.sh: line 64: : No such file or directory

What did you expect to happen

An error indicating that crudini is not installed, instructing the user to install it.

How to reproduce

Run mainsail w/o crudini installed.

Additional information

No response

mryel00 commented 6 months ago

Here are multiple things that makes no sense.

On a system with bash 4, and crudini installed Run mainsail w/o crudini installed.

Is crudini now installed or not? Also we are talking about crowsnest and not mainsail here ig.

How did you install crowsnest without crudini? It's a dependency that will be installed during installation process or otherwise crowsnest will fail to install.

/opt/crowsnest/libs/logging.sh: line 64: : No such file or directory

A little bit more output would be helpful as this error isn't connected to crudini at all: https://github.com/mainsail-crew/crowsnest/blob/f7ac6aa298143f0019eae9b2b2be8039196ebeda/libs/logging.sh#L64

An error indicating that crudini is not installed, instructing the user to install it.

This code checks for dependencies and shows an error if crudini is missing https://github.com/mainsail-crew/crowsnest/blob/f7ac6aa298143f0019eae9b2b2be8039196ebeda/libs/core.sh#L72-L81

For the reasons above I think you get why I'm confused about your issue.

mryel00 commented 6 months ago

I now read it again and you might have installed with MainsailOS? Then your log_path is wrong => "${CROWSNEST_LOG_PATH}": No such file or directory Another indication would be a warning shown in mainsail at the bell icon as you can see in #186 and #156

Laikulo commented 6 months ago
  1. Crudini is not installed, the reference to where it is is a typo. I'll edit it once I have a real computer.
  2. The log output listed above is the only output, after some inspection, it appears that if CF crudini is not present, The log path expands to empty string.
  3. The dependency check does not appear to function in this case, it may be that it actually does attempt to log a failure, but the logging system is also broken.

Side: I'm running on opensuse leap, my repo is at /opt/crowsnest log at /var/log/crowsnest/crowsnest.log config at /etc/crowsnest.conf.

After installing crudini it worked as expected.

I determined the behavior of the above expansion with strace and bash -x

mryel00 commented 6 months ago

The dependency check is after getting the "${CROWSNEST_LOG_PATH}". I thought for some reason that the log_path will be get with sed but that's wrong. So it ofc makes sense that he never get's to that part.

As I already said, during installation we install all dependencies: https://github.com/mainsail-crew/crowsnest/blob/f7ac6aa298143f0019eae9b2b2be8039196ebeda/tools/libs/core.sh#L155-L163 So atm this doesn't seem like an overall issue and more like a problem with your specific setup maybe.

Did you use the installer to install, or did you manually setup everything? If you used the installer, can you debug the installer maybe and find out, why it didn't install crudini as one of the dependencies?

If there is a bug with the installer, it would be nice, if we can find a solution for it, but I won't fix the error during runtime, as this doesn't seem like an important issue to me. I'm currently rewriting the runtime code in python anyway, so it's not worth the time to spend this runtime issue.

edit: have a look at the next message

mryel00 commented 6 months ago

As I'm not a big linux guy, I didn't know that openSUSE doesn't have apt support 😅 So if you want to use it, you are on your own with it. We are recommending Debian based systems and will only give support for those.

Laikulo commented 6 months ago

Okay. On a related note, would you be open to a PR that sends errors to stderr if the logfile is improperly set, or cannot be opened, or is the current iteration to short in future life for that to be worth?

mryel00 commented 6 months ago

Overall I cannot say, how long I will still need to rewrite everything as I also got some other things that need to be finished until a specific deadline. I'm open to PRs and I need to see it before I can judge it. I would say, don't invest too much time into it.