diff mbox series

[bug#43920,SHEPHERD] shepherd: Move log file to XDG_CACHE_DIR.

Message ID 20201011092017.31901-1-efraim@flashner.co.il
State Accepted
Headers show
Series [bug#43920,SHEPHERD] shepherd: Move log file to XDG_CACHE_DIR. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job

Commit Message

Efraim Flashner Oct. 11, 2020, 9:20 a.m. UTC
* modules/shepherd/support.scm (%user-log-dir): New variable.
(user-default-log-file): Use it.
* doc/shepherd.texi: (Invoking shepherd): Adjust listed default log
file.
* tests/basic.sh: Adjust for separate log directory.
---
 doc/shepherd.texi            |  2 +-
 modules/shepherd/support.scm | 10 ++++++++--
 tests/basic.sh               |  4 ++++
 3 files changed, 13 insertions(+), 3 deletions(-)

Comments

Ludovic Courtès Oct. 22, 2020, 2:23 p.m. UTC | #1
Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> * modules/shepherd/support.scm (%user-log-dir): New variable.
> (user-default-log-file): Use it.
> * doc/shepherd.texi: (Invoking shepherd): Adjust listed default log
> file.
> * tests/basic.sh: Adjust for separate log directory.

[...]

>  For unprivileged users, the default log file is
> -@file{$XDG_CONFIG_HOME/shepherd/shepherd.log}.
> +@file{$XDG_CACHE_DIR/shepherd/shepherd.log}.

The previous location wasn’t perfect, but the proposed one isn’t great
either: one can generally assume “rm -rf ~/.cache” won’t lose anything
that cannot be reconstructed, and the log is something that cannot be
reconstructed.

Do the XDG spec have something else that might be more appropriate?

Thanks,
Ludo’.
Leo Prikler Oct. 23, 2020, 10:17 p.m. UTC | #2
> The previous location wasn’t perfect, but the proposed one isn’t
> great
> either: one can generally assume “rm -rf ~/.cache” won’t lose
> anything
> that cannot be reconstructed, and the log is something that cannot be
> reconstructed.
That is actually not part of the XDG specification – it merely states,
that non-essential files be put there.  Whether or not one considers
the shepherd log essential is debatable.

You could alternatively put it under $XDG_DATA_HOME.  Some projects
like Xorg seem to do that.  Of course, there always remains the option
of having yet another environment variable or command line parameter
for the placement of the log file.

Regards, Leo
Ludovic Courtès Oct. 24, 2020, 4:15 p.m. UTC | #3
Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

>> The previous location wasn’t perfect, but the proposed one isn’t
>> great
>> either: one can generally assume “rm -rf ~/.cache” won’t lose
>> anything
>> that cannot be reconstructed, and the log is something that cannot be
>> reconstructed.
> That is actually not part of the XDG specification – it merely states,
> that non-essential files be put there.  Whether or not one considers
> the shepherd log essential is debatable.

Right, that’s my interpretation of what “cache” means.

> You could alternatively put it under $XDG_DATA_HOME.  Some projects
> like Xorg seem to do that.  Of course, there always remains the option
> of having yet another environment variable or command line parameter
> for the placement of the log file.

Yup, XDG_DATA_HOME would work for me; it seems to be rather in line with
what the XDG states and what Xorg does, as you note.

  https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

WDYT, Efraim & Janneke?

Ludo’.
Efraim Flashner Oct. 24, 2020, 4:51 p.m. UTC | #4
On October 24, 2020 4:15:33 PM UTC, "Ludovic Courtès" <ludo@gnu.org> wrote:
>Hi,
>
>Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>
>>> The previous location wasn’t perfect, but the proposed one isn’t
>>> great
>>> either: one can generally assume “rm -rf ~/.cache” won’t lose
>>> anything
>>> that cannot be reconstructed, and the log is something that cannot be
>>> reconstructed.
>> That is actually not part of the XDG specification – it merely states,
>> that non-essential files be put there.  Whether or not one considers
>> the shepherd log essential is debatable.
>
>Right, that’s my interpretation of what “cache” means.
>

I felt like it belongs in cache much more than in a config dir.

>> You could alternatively put it under $XDG_DATA_HOME.  Some projects
>> like Xorg seem to do that.  Of course, there always remains the option
>> of having yet another environment variable or command line parameter
>> for the placement of the log file.
>
>Yup, XDG_DATA_HOME would work for me; it seems to be rather in line with
>what the XDG states and what Xorg does, as you note.
>
>  https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
>
>WDYT, Efraim & Janneke?

I don't think it's really data, it's not like it belongs in something/share. As another data point keybase writes its (verbose) logs in .cache/keybase.

>
>Ludo’.
Janneke Nieuwenhuizen Oct. 24, 2020, 10:58 p.m. UTC | #5
Ludovic Courtès writes:

Hello,

> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>
>>> The previous location wasn’t perfect, but the proposed one isn’t
>>> great
>>> either: one can generally assume “rm -rf ~/.cache” won’t lose
>>> anything
>>> that cannot be reconstructed, and the log is something that cannot be
>>> reconstructed.
>> That is actually not part of the XDG specification – it merely states,
>> that non-essential files be put there.  Whether or not one considers
>> the shepherd log essential is debatable.
>
> Right, that’s my interpretation of what “cache” means.

Yes, essential is pretty vague; a cache is something that can be
re-generated.

>> You could alternatively put it under $XDG_DATA_HOME.  Some projects
>> like Xorg seem to do that.  Of course, there always remains the option
>> of having yet another environment variable or command line parameter
>> for the placement of the log file.
>
> Yup, XDG_DATA_HOME would work for me; it seems to be rather in line with
> what the XDG states and what Xorg does, as you note.
>
>   https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
>
> WDYT, Efraim & Janneke?

Data sounds a bit strong (I would consider a database to be data), and
cache is too loose.  Otoh, losing a log can be crucial; so I'd go for
XDG_DATA_HOME.

Janneke
Ludovic Courtès Oct. 26, 2020, 10:27 a.m. UTC | #6
Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On October 24, 2020 4:15:33 PM UTC, "Ludovic Courtès" <ludo@gnu.org> wrote:

[...]

>>>> The previous location wasn’t perfect, but the proposed one isn’t
>>>> great
>>>> either: one can generally assume “rm -rf ~/.cache” won’t lose
>>>> anything
>>>> that cannot be reconstructed, and the log is something that cannot be
>>>> reconstructed.

[...]

> I don't think it's really data, it's not like it belongs in something/share. As another data point keybase writes its (verbose) logs in .cache/keybase.

To me the question whether it’s OK to lose the log.  In a way, storing
the log in ~/.cache akin to storing it in /tmp.

If we agree on that, then we have the choice between ~/.local/share
aka. XDG_DATA_HOME (like Xorg does), or maybe using ~/.local/var/log or
something similar that goes beyond XDG?

Thoughts?

Naming directories is hard.  :-)

Ludo’.
Ludovic Courtès Nov. 18, 2020, 9:35 p.m. UTC | #7
Hi,

Ludovic Courtès <ludo@gnu.org> skribis:

> Efraim Flashner <efraim@flashner.co.il> skribis:
>
>> On October 24, 2020 4:15:33 PM UTC, "Ludovic Courtès" <ludo@gnu.org> wrote:
>
> [...]
>
>>>>> The previous location wasn’t perfect, but the proposed one isn’t
>>>>> great
>>>>> either: one can generally assume “rm -rf ~/.cache” won’t lose
>>>>> anything
>>>>> that cannot be reconstructed, and the log is something that cannot be
>>>>> reconstructed.
>
> [...]
>
>> I don't think it's really data, it's not like it belongs in something/share. As another data point keybase writes its (verbose) logs in .cache/keybase.
>
> To me the question whether it’s OK to lose the log.  In a way, storing
> the log in ~/.cache akin to storing it in /tmp.
>
> If we agree on that, then we have the choice between ~/.local/share
> aka. XDG_DATA_HOME (like Xorg does), or maybe using ~/.local/var/log or
> something similar that goes beyond XDG?

I pushed a variant of your patch that uses XDG_DATA_HOME instead:

  https://git.savannah.gnu.org/cgit/shepherd.git/commit/?id=c6382a6898f2851d78b675f209787c5ff9ad4320

There’s a now ‘%user-log-dir’ so it’ll be easier to adjust going
forward if we change our mind.

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/doc/shepherd.texi b/doc/shepherd.texi
index 696477e..0c3901e 100644
--- a/doc/shepherd.texi
+++ b/doc/shepherd.texi
@@ -417,7 +417,7 @@  permissions are not as expected.
 Log output into @var{file}.
 
 For unprivileged users, the default log file is
-@file{$XDG_CONFIG_HOME/shepherd/shepherd.log}.
+@file{$XDG_CACHE_DIR/shepherd/shepherd.log}.
 
 @cindex syslog
 When running as root, the default behavior is to connect to
diff --git a/modules/shepherd/support.scm b/modules/shepherd/support.scm
index cdb7b35..008b280 100644
--- a/modules/shepherd/support.scm
+++ b/modules/shepherd/support.scm
@@ -271,6 +271,12 @@  There is NO WARRANTY, to the extent permitted by law.")))
                      (string-append user-homedir "/.config"))
                  "/shepherd"))
 
+(define %user-log-dir
+  ;; Default log directory if shepherd is run as a normal user.
+  (string-append (or (getenv "XDG_CACHE_DIR")
+                     (string-append user-homedir "/.cache"))
+                 "/shepherd"))
+
 (define %user-runtime-dir
   ;; Default runtime directory if shepherd is run as a normal user.
   (string-append (or (getenv "XDG_RUNTIME_DIR")
@@ -302,8 +308,8 @@  TARGET should be a string representing a filepath + name."
 ;; Logging.
 (define (user-default-log-file)
   "Return the file name of the user's default log file."
-  (mkdir-p %user-config-dir #o700)
-  (string-append %user-config-dir "/shepherd.log"))
+  (mkdir-p %user-log-dir #o700)
+  (string-append %user-log-dir "/shepherd.log"))
 
 (define default-logfile-date-format
   ;; 'strftime' format string to prefix each entry in the log.
diff --git a/tests/basic.sh b/tests/basic.sh
index a6148fc..145c0d3 100644
--- a/tests/basic.sh
+++ b/tests/basic.sh
@@ -24,6 +24,7 @@  herd --version
 socket="t-socket-$$"
 conf="t-conf-$$"
 confdir="t-confdir-$$"
+cachedir="t-cachedir-$$"
 log="t-log-$$"
 stamp="t-stamp-$$"
 pid="t-pid-$$"
@@ -259,7 +260,9 @@  test -f "$log"
 
 # Set XDG_CONFIG_HOME for configuration files.
 export XDG_CONFIG_HOME=$confdir
+export XDG_CACHE_DIR=$cachedir
 mkdir -p $confdir/shepherd
+mkdir -p $cachedir/shepherd
 mv $conf $confdir/shepherd/init.scm
 rm -f "$pid" "$socket"
 shepherd -I -s "$socket" --pid="$pid" &
@@ -281,3 +284,4 @@  $herd stop root
 ! kill -0 $shepherd_pid
 
 rm -rf $confdir
+rm -rf $cachedir