YunoHost / issues

General issue tracker for the YunoHost project
72 stars 8 forks source link

ynh_use_logrotate assume file ends with .log, which breaks shaarli_ynh #1390

Closed alexAubin closed 5 years ago

alexAubin commented 5 years ago

Shaarli uses logrotate here :

https://github.com/YunoHost-Apps/shaarli_ynh/blob/980120eef0ad8b647242f1c2345f247ee8936ca2/scripts/install#L155

with ynh_use_logrotate "$final_path/data/log.txt"

Later in the helper here, the helper expect the file to end with log https://github.com/YunoHost/yunohost/blob/stretch-unstable/data/helpers.d/logrotate#L57

which is not the case and ends up redefining logfile=$logfile/*.log (assuming that logfile is a folder)

logfile is now $final_path/data/log.txt/*.log

Annnd then runs later :

sudo mkdir -p $(dirname "$logfile")

and now $final_path/data/log.txt is a folder ...

which then triggers https://github.com/YunoHost-Apps/shaarli_ynh/issues/45 (fail2ban is unhappy with the logfile being a folder)

maniackcrudelis commented 5 years ago

A log file as a txt file... That's not that easy to manage... I can't see other way than adding another argument to specify or force something...

Or, shaarli may use a symlink to use a regular .log file.

alexAubin commented 5 years ago

But what are we really testing here ? Don't we want to just test if it was a directory that was given in argument or a regular file ?

A log file is just a text file, there's no absolute convention about naming it .log ... (e.g. mail.err and mail.info are log files ... same as /var/log/messages and debug ... :/

maniackcrudelis commented 5 years ago

Right. That just that a '.txt' file for a log is bothering me. But you're indeed right.

Problem is we can't check the file as it may not exist yet.

maniackcrudelis commented 5 years ago

So, we could add a argument to force the type of the log file.

alexAubin commented 5 years ago

Eh yeah :/

Or can't we have something "smarter" like checking for a non-empty extension ? Or are there cases like using ynh_use_logrotate /var/log/something.d ? (which would then be a folder) Or maybe checking the extension is at least 3 chars ..

maniackcrudelis commented 5 years ago

As you prove me earlier, a log file could as well not have any extension !

lapineige commented 5 years ago

I can't see other way than adding another argument to specify or force something...

Could this be an argument to specify the file extension ? (something like .log by default to preserve the compatibility, but .txt or no extension as an option)

maniackcrudelis commented 5 years ago

Could be that yes, just to specify the extension. However, could be difficult to manage if there's no extension at all. May be better to specify if the first argument is a file or a directory.

JOduMonT commented 5 years ago

Hi..;

@maniackcrudelis as you probably know, Linux doesn't really care about the extension of a file, it's more of a convention for humans conditioned by MS Windows.

I agree with @lapineige, yunohost must be flexible and be able to parade whimsy developers.

The only problem now is that they think differently from the yunohost team and agree with a.txt file as a log file.

If the yunohost team bends and adds a new variable to its system, this case will be solved and may not reappear in a future where another application with another point of view will store a file with another state of mind.

and remember

”Bamboo is flexible, bending with the wind but never breaking, capable of adapting to any circumstance. It suggests resilience, meaning that we have the ability to bounce back even from the most difficult times. . . . Your ability to thrive depends, in the end, on your attitude to your life circumstances. Take everything in stride with grace, putting forth energy when it is needed, yet always staying calm inwardly.” ― Ping Fu

alexAubin commented 5 years ago

Fixed in https://github.com/YunoHost/yunohost/pull/810