This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH] debuginfod-client: default to XDG cache.
On Fri, Feb 21, 2020 at 3:09 PM Mark Wielaard <mark@klomp.org> wrote:
> I think the spec is slightly ambiguous whether or not you need to
> create all parents.
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
>
> If, when attempting to write a file, the destination directory is
> non-existant an attempt should be made to create it with permission
> 0700.
>
> But I think that means we should only create the cachedir (either
> $XDG_CACHE_HOME or $HOME/.cache). Which is what we are doing here. But
> I don't think we are expected to create all parents. I think it is
> reasonable to assume they already exist (it would almost always be
> $HOME anyway).
>
> Later in the code we create target_cache_path (the debuginfod_client
> subdir), again just as one directory (we made sure its parent exists).
Makes sense, I stuck with this approach in the revised patch I've attached.
Also I ended up not attempting to rename a cache at the old default location.
IMO it brings negligible benefits while adding extra complexity plus the risk
of confusing users is greater than if we just leave the old cache where it is.
> BTW. Looking at the code again I see this comment should be adjusted
> too:
>
> /* set paths needed to perform the query
>
> example format
> cache_path: $HOME/.debuginfod_cache
> target_cache_dir: $HOME/.debuginfod_cache/0123abcd
> target_cache_path: $HOME/.debuginfod_cache/0123abcd/debuginfo
> target_cache_path:
> $HOME/.debuginfod_cache/0123abcd/source#PATH#TO#SOURCE ?
> */
Fixed.
Aaron
From 8d29714f6bca61d08fe9aac279c49d30db35345f Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Thu, 27 Feb 2020 18:10:01 -0500
Subject: [PATCH] debuginfod-client: default to XDG cache.
PR25502: debuginfod client should default to XDG cache
Location of client cache now defaults to $XDG_CACHE_HOME/debuginfod_client.
If XDG_CACHE_HOME is not set then fallback to $HOME/.cache/debuginfod_client.
Also maintain backwards compatibility with the previous default path-
if $HOME/.debuginfod_client_cache exists, use that instead of the new
defaults.
Signed-off-by: Aaron Merey <amerey@redhat.com>
---
debuginfod/ChangeLog | 7 ++
debuginfod/debuginfod-client.c | 119 +++++++++++++++++++++++++-------
doc/debuginfod.8 | 10 ++-
doc/debuginfod_find_debuginfo.3 | 8 ++-
tests/ChangeLog | 5 ++
tests/run-debuginfod-find.sh | 39 ++++++++++-
6 files changed, 156 insertions(+), 32 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 70970504..21d9ed87 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,10 @@
+2020-02-27 Aaron Merey <amerey@redhat.com>
+
+ * debuginfod-client.c (xalloc_str): New macro. Call
+ asprintf with error checking.
+ (debuginfod_query_server): Use XDG_CACHE_HOME as a default
+ cache location if it is set. Replace snprintf with xalloc_str.
+
2020-02-26 Konrad Kleine <kkleine@redhat.com>
* debuginfod-client.c (debuginfod_query_server): Handle curl's
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index c1174486..2825e625 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -99,6 +99,7 @@ static const time_t cache_default_max_unused_age_s = 604800; /* 1 week */
/* Location of the cache of files downloaded from debuginfods.
The default parent directory is $HOME, or '/' if $HOME doesn't exist. */
static const char *cache_default_name = ".debuginfod_client_cache";
+static const char *cache_xdg_name = "debuginfod_client";
static const char *cache_path_envvar = DEBUGINFOD_CACHE_PATH_ENV_VAR;
/* URLs of debuginfods, separated by url_delim. */
@@ -370,6 +371,16 @@ add_extra_headers(CURL *handle)
}
+#define xalloc_str(p, fmt, args...) \
+ do \
+ { \
+ if (asprintf (&p, fmt, args) < 0) \
+ { \
+ p = NULL; \
+ rc = -ENOMEM; \
+ goto out; \
+ } \
+ } while (0)
/* Query each of the server URLs found in $DEBUGINFOD_URLS for the file
with the specified build-id, type (debuginfo, executable or source)
@@ -384,15 +395,15 @@ debuginfod_query_server (debuginfod_client *c,
const char *filename,
char **path)
{
- char *urls_envvar;
char *server_urls;
- char cache_path[PATH_MAX];
- char maxage_path[PATH_MAX*3]; /* These *3 multipliers are to shut up gcc -Wformat-truncation */
- char interval_path[PATH_MAX*4];
- char target_cache_dir[PATH_MAX*2];
- char target_cache_path[PATH_MAX*4];
- char target_cache_tmppath[PATH_MAX*5];
- char suffix[PATH_MAX*2];
+ char *urls_envvar;
+ char *cache_path = NULL;
+ char *maxage_path = NULL;
+ char *interval_path = NULL;
+ char *target_cache_dir = NULL;
+ char *target_cache_path = NULL;
+ char *target_cache_tmppath = NULL;
+ char suffix[PATH_MAX];
char build_id_bytes[MAX_BUILD_ID_BYTES * 2 + 1];
int rc;
@@ -452,30 +463,76 @@ debuginfod_query_server (debuginfod_client *c,
/* set paths needed to perform the query
example format
- cache_path: $HOME/.debuginfod_cache
- target_cache_dir: $HOME/.debuginfod_cache/0123abcd
- target_cache_path: $HOME/.debuginfod_cache/0123abcd/debuginfo
- target_cache_path: $HOME/.debuginfod_cache/0123abcd/source#PATH#TO#SOURCE ?
+ cache_path: $HOME/.cache
+ target_cache_dir: $HOME/.cache/0123abcd
+ target_cache_path: $HOME/.cache/0123abcd/debuginfo
+ target_cache_path: $HOME/.cache/0123abcd/source#PATH#TO#SOURCE ?
+
+ $XDG_CACHE_HOME takes priority over $HOME/.cache.
+ $DEBUGINFOD_CACHE_PATH takes priority over $HOME/.cache and $XDG_CACHE_HOME.
*/
- if (getenv(cache_path_envvar))
- strcpy(cache_path, getenv(cache_path_envvar));
+ /* Determine location of the cache. The path specified by the debuginfod
+ cache environment variable takes priority. */
+ char *cache_var = getenv(cache_path_envvar);
+ if (cache_var != NULL && strlen (cache_var) > 0)
+ xalloc_str (cache_path, "%s", cache_var);
else
{
- if (getenv("HOME"))
- sprintf(cache_path, "%s/%s", getenv("HOME"), cache_default_name);
- else
- sprintf(cache_path, "/%s", cache_default_name);
+ /* If a cache already exists in $HOME ('/' if $HOME isn't set), then use
+ that. Otherwise use the XDG cache directory naming format. */
+ xalloc_str (cache_path, "%s/%s", getenv ("HOME") ?: "/", cache_default_name);
+
+ struct stat st;
+ if (stat (cache_path, &st) < 0)
+ {
+ char cachedir[PATH_MAX];
+ char *xdg = getenv ("XDG_CACHE_HOME");
+
+ if (xdg != NULL && strlen (xdg) > 0)
+ snprintf (cachedir, PATH_MAX, "%s", xdg);
+ else
+ snprintf (cachedir, PATH_MAX, "%s/.cache", getenv ("HOME") ?: "/");
+
+ /* Create XDG cache directory if it doesn't exist. */
+ if (stat (cachedir, &st) == 0)
+ {
+ if (! S_ISDIR (st.st_mode))
+ {
+ rc = -EEXIST;
+ goto out;
+ }
+ }
+ else
+ {
+ rc = mkdir (cachedir, 0700);
+
+ /* Also check for EEXIST and S_ISDIR in case another client just
+ happened to create the cache. */
+ if (rc == 0
+ || (errno == EEXIST
+ && stat (cachedir, &st) != 0
+ && S_ISDIR (st.st_mode)))
+ {
+ free (cache_path);
+ xalloc_str (cache_path, "%s/%s", cachedir, cache_xdg_name);
+ }
+ else
+ {
+ rc = -errno;
+ goto out;
+ }
+ }
+ }
}
- /* avoid using snprintf here due to compiler warning. */
- snprintf(target_cache_dir, sizeof(target_cache_dir), "%s/%s", cache_path, build_id_bytes);
- snprintf(target_cache_path, sizeof(target_cache_path), "%s/%s%s", target_cache_dir, type, suffix);
- snprintf(target_cache_tmppath, sizeof(target_cache_tmppath), "%s.XXXXXX", target_cache_path);
+ xalloc_str (target_cache_dir, "%s/%s", cache_path, build_id_bytes);
+ xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
+ xalloc_str (target_cache_tmppath, "%s.XXXXXX", target_cache_path);
/* XXX combine these */
- snprintf(interval_path, sizeof(interval_path), "%s/%s", cache_path, cache_clean_interval_filename);
- snprintf(maxage_path, sizeof(maxage_path), "%s/%s", cache_path, cache_max_unused_age_filename);
+ xalloc_str (interval_path, "%s/%s", cache_path, cache_clean_interval_filename);
+ xalloc_str (maxage_path, "%s/%s", cache_path, cache_max_unused_age_filename);
rc = debuginfod_init_cache(cache_path, interval_path, maxage_path);
if (rc != 0)
goto out;
@@ -490,7 +547,8 @@ debuginfod_query_server (debuginfod_client *c,
/* Success!!!! */
if (path != NULL)
*path = strdup(target_cache_path);
- return fd;
+ rc = fd;
+ goto out;
}
long timeout = default_timeout;
@@ -779,12 +837,14 @@ debuginfod_query_server (debuginfod_client *c,
curl_multi_cleanup (curlm);
free (data);
free (server_urls);
+
/* don't close fd - we're returning it */
/* don't unlink the tmppath; it's already been renamed. */
if (path != NULL)
*path = strdup(target_cache_path);
- return fd;
+ rc = fd;
+ goto out;
/* error exits */
out1:
@@ -800,7 +860,14 @@ debuginfod_query_server (debuginfod_client *c,
out0:
free (server_urls);
+/* general purpose exit */
out:
+ free (cache_path);
+ free (maxage_path);
+ free (interval_path);
+ free (target_cache_dir);
+ free (target_cache_path);
+ free (target_cache_tmppath);
return rc;
}
diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
index 32fca6c4..39d1904e 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -410,8 +410,10 @@ or negative means "no timeout".)
.B DEBUGINFOD_CACHE_PATH
This environment variable governs the location of the cache where
downloaded files are kept. It is cleaned periodically as this
-program is reexecuted. The default is \%$HOME/.debuginfod_client_cache.
-.\" XXX describe cache eviction policy
+program is reexecuted. If XDG_CACHE_HOME is set then
+$XDG_CACHE_HOME/debuginfod_client is the default location, otherwise
+$HOME/.cache/debuginfod_client is used. For more information regarding
+the client cache see \fIdebuginfod_find_debuginfo(3)\fP.
.SH FILES
.LP
@@ -422,8 +424,10 @@ Default database file.
.PD
.TP 20
-.B $HOME/.debuginfod_client_cache
+.B $XDG_CACHE_HOME/debuginfod_client
Default cache directory for content from upstream debuginfods.
+If XDG_CACHE_HOME is not set then \fB$HOME/.cache/debuginfod_client\fP
+is used.
.PD
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index c232ac71..f9e770b0 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -180,7 +180,10 @@ function output.
.B DEBUGINFOD_CACHE_PATH
This environment variable governs the location of the cache where
downloaded files are kept. It is cleaned periodically as this
-program is reexecuted. The default is $HOME/.debuginfod_client_cache.
+program is reexecuted. If XDG_CACHE_HOME is set then
+$XDG_CACHE_HOME/debuginfod_client is the default location, otherwise
+$HOME/.cache/debuginfod_client is used.
+
.SH "ERRORS"
The following list is not comprehensive. Error codes may also
@@ -244,7 +247,8 @@ the timeout duration. See debuginfod(8) for more information.
.PD .1v
.TP 20
.B $HOME/.debuginfod_client_cache
-Default cache directory.
+Default cache directory. If XDG_CACHE_HOME is not set then
+\fB$HOME/.cache/debuginfod_client\fP is used.
.PD
.SH "SEE ALSO"
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 98e5c954..33a5cf62 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-19 Aaron Merey <amerey@redhat.com>
+
+ * run-debuginfod-find.sh: Run tests for verifying default
+ client cache locations.
+
2020-02-26 Konrad Kleine <kkleine@redhat.com>
* run-debuginfod-find.sh: added tests for DEBUGINFOD_URLS beginning
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 0facc555..56e61216 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -40,7 +40,7 @@ cleanup()
if [ $PID2 -ne 0 ]; then kill $PID2; wait $PID2; fi
if [ $PID3 -ne 0 ]; then kill $PID3; wait $PID3; fi
- rm -rf F R D L Z ${PWD}/mocktree ${PWD}/.client_cache*
+ rm -rf F R D L Z ${PWD}/mocktree ${PWD}/.client_cache* ${PWD}/tmp*
exit_cleanup
}
@@ -147,6 +147,43 @@ cmp $filename ${PWD}/prog.c
########################################################################
+# Test whether the cache default locations are correct
+
+mkdir tmphome
+
+# $HOME/.cache should be created.
+testrun env HOME=$PWD/tmphome XDG_CACHE_HOME= DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
+if [ ! -f $PWD/tmphome/.cache/debuginfod_client/$BUILDID/debuginfo ]; then
+ echo "could not find cache in $PWD/tmphome/.cache"
+ exit 1
+fi
+
+# $XDG_CACHE_HOME should take priority over $HOME.cache.
+testrun env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
+if [ ! -f $PWD/tmpxdg/debuginfod_client/$BUILDID/debuginfo ]; then
+ echo "could not find cache in $PWD/tmpxdg/"
+ exit 1
+fi
+
+# A cache at the old default location ($HOME/.debuginfod_client_cache) should take
+# priority over $HOME/.cache, $XDG_CACHE_HOME.
+cp -r $DEBUGINFOD_CACHE_PATH tmphome/.debuginfod_client_cache
+
+# Add a file that doesn't exist in $HOME/.cache, $XDG_CACHE_HOME.
+mkdir tmphome/.debuginfod_client_cache/deadbeef
+echo ELF... > tmphome/.debuginfod_client_cache/deadbeef/debuginfo
+filename=`testrun env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo deadbeef`
+cmp $filename tmphome/.debuginfod_client_cache/deadbeef/debuginfo
+
+# $DEBUGINFO_CACHE_PATH should take priority over all else.
+testrun env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH=$PWD/tmpcache ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
+if [ ! -f $PWD/tmpcache/$BUILDID/debuginfo ]; then
+ echo "could not find cache in $PWD/tmpcache/"
+ exit 1
+fi
+
+########################################################################
+
# Add artifacts to the search paths and test whether debuginfod finds them while already running.
# Build another, non-stripped binary
--
2.24.1