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

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

Closed
Laikulo opened this issue Dec 17, 2023 · 7 comments
Closed

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

Laikulo opened this issue Dec 17, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@Laikulo
Copy link

Laikulo commented Dec 17, 2023

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

@Laikulo Laikulo added the bug Something isn't working label Dec 17, 2023
@mryel00
Copy link
Member

mryel00 commented Dec 17, 2023

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:

printf "%s %s\n" "${prefix}" "${msg}" >> "${CROWSNEST_LOG_PATH}"

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

crowsnest/libs/core.sh

Lines 72 to 81 in f7ac6aa

function check_dep {
local dep
dep="$(whereis "${1}" | awk '{print $2}')"
if [[ -z "${dep}" ]]; then
log_msg "Dependency: '${1}' not found. Exiting!"
exit 1
else
log_msg "Dependency: '${1}' found in ${dep}."
fi
}

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

@mryel00
Copy link
Member

mryel00 commented Dec 17, 2023

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
Copy link
Author

Laikulo commented Dec 17, 2023

  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
Copy link
Member

mryel00 commented Dec 17, 2023

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:

install_dependencies() {
local dep
local -a pkg
pkg=()
for dep in ${PKGLIST}; do
pkg+=("${dep}")
done
apt-get --yes --no-install-recommends install "${pkg[@]}" || return 1
}

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
Copy link
Member

mryel00 commented Dec 17, 2023

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.

@mryel00 mryel00 closed this as completed Dec 17, 2023
@Laikulo
Copy link
Author

Laikulo commented Dec 17, 2023

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
Copy link
Member

mryel00 commented Dec 17, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants