* [PATCH] debuginfod-client: default to XDG cache.
@ 2020-02-19 17:41 Aaron Merey
2020-02-19 21:04 ` Aaron Merey
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Merey @ 2020-02-19 17:41 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 2 bytes --]
[-- Attachment #2: 0001-debuginfod-client-default-to-XDG-cache.patch --]
[-- Type: text/x-patch, Size: 11833 bytes --]
From 41c69d75ddcad755a51bfcb7554a124375c11846 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Wed, 19 Feb 2020 12:30:14 -0500
Subject: [PATCH] debuginfod-client: 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 | 6 ++
debuginfod/debuginfod-client.c | 98 ++++++++++++++++++++++++++-------
doc/debuginfod.8 | 10 +++-
doc/debuginfod_find_debuginfo.3 | 8 ++-
tests/ChangeLog | 4 ++
tests/run-debuginfod-find.sh | 39 ++++++++++++-
6 files changed, 139 insertions(+), 26 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index d812e6d7..fc076737 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2020-02-19 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-05 Frank Ch. Eigler <fche@redhat.com>
* debuginfod.cxx (argp options): Add -Z option.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index e5a2e824..723790f6 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -98,6 +98,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. */
@@ -359,6 +360,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)
@@ -373,15 +384,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;
@@ -447,24 +458,65 @@ debuginfod_query_server (debuginfod_client *c,
target_cache_path: $HOME/.debuginfod_cache/0123abcd/source#PATH#TO#SOURCE ?
*/
- 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
+ {
+ char *cmd;
+
+ xalloc_str (cmd, "mkdir -p \'%s\'", cachedir);
+ rc = system (cmd);
+ free (cmd);
+ if (rc == 0)
+ {
+ free (cache_path);
+ xalloc_str (cache_path, "%s/%s", cachedir, cache_xdg_name);
+ }
+ else
+ {
+ rc = -EINVAL;
+ 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;
@@ -776,6 +828,12 @@ debuginfod_query_server (debuginfod_client *c,
free (server_urls);
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 ca844aed..ac231551 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -406,8 +406,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
@@ -418,8 +420,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 1f55a291..ec081b3c 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2020-02-19 Aaron Merey <amerey@redhat.com>
+ * run-debuginfod-find.sh: Run tests for verifying default
+ client cache locations.
+
2020-02-08 Mark Wielaard <mark@klomp.org>
* run-pt_gnu_prop-tests.sh: New test.
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 1cc8f406..9740f4aa 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}/.client_cache*
+ rm -rf F R D L Z ${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.
+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.
+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=`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.
+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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] debuginfod-client: default to XDG cache.
2020-02-19 17:41 [PATCH] debuginfod-client: default to XDG cache Aaron Merey
@ 2020-02-19 21:04 ` Aaron Merey
2020-02-21 16:13 ` Mark Wielaard
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Merey @ 2020-02-19 21:04 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 71 bytes --]
Reposting the patch since the original contained a silly memory leak.
[-- Attachment #2: 0001-debuginfod-client-default-to-XDG-cache.patch --]
[-- Type: text/x-patch, Size: 12514 bytes --]
From 8eb82eb58747078a3e914576a7c6ff3f7b2c7cb4 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Wed, 19 Feb 2020 12:30:14 -0500
Subject: [PATCH] PR25502: debuginfod-client 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 | 6 ++
debuginfod/debuginfod-client.c | 106 +++++++++++++++++++++++++-------
doc/debuginfod.8 | 10 ++-
doc/debuginfod_find_debuginfo.3 | 8 ++-
tests/ChangeLog | 4 ++
tests/run-debuginfod-find.sh | 39 +++++++++++-
6 files changed, 145 insertions(+), 28 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index d812e6d7..fc076737 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2020-02-19 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-05 Frank Ch. Eigler <fche@redhat.com>
* debuginfod.cxx (argp options): Add -Z option.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index e5a2e824..5772367c 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -98,6 +98,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. */
@@ -359,6 +360,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)
@@ -373,15 +384,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;
@@ -447,24 +458,65 @@ debuginfod_query_server (debuginfod_client *c,
target_cache_path: $HOME/.debuginfod_cache/0123abcd/source#PATH#TO#SOURCE ?
*/
- 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
+ {
+ char *cmd;
+
+ xalloc_str (cmd, "mkdir -p \'%s\'", cachedir);
+ rc = system (cmd);
+ free (cmd);
+ if (rc == 0)
+ {
+ free (cache_path);
+ xalloc_str (cache_path, "%s/%s", cachedir, cache_xdg_name);
+ }
+ else
+ {
+ rc = -EINVAL;
+ 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;
@@ -479,7 +531,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;
@@ -754,12 +807,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:
@@ -775,7 +830,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 ca844aed..ac231551 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -406,8 +406,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
@@ -418,8 +420,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 1f55a291..ec081b3c 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2020-02-19 Aaron Merey <amerey@redhat.com>
+ * run-debuginfod-find.sh: Run tests for verifying default
+ client cache locations.
+
2020-02-08 Mark Wielaard <mark@klomp.org>
* run-pt_gnu_prop-tests.sh: New test.
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 1cc8f406..9740f4aa 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}/.client_cache*
+ rm -rf F R D L Z ${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.
+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.
+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=`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.
+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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] debuginfod-client: default to XDG cache.
2020-02-19 21:04 ` Aaron Merey
@ 2020-02-21 16:13 ` Mark Wielaard
2020-02-21 17:22 ` Aaron Merey
0 siblings, 1 reply; 7+ messages in thread
From: Mark Wielaard @ 2020-02-21 16:13 UTC (permalink / raw)
To: Aaron Merey, elfutils-devel
Hi Aaron,
On Wed, 2020-02-19 at 16:04 -0500, Aaron Merey wrote:
> From 8eb82eb58747078a3e914576a7c6ff3f7b2c7cb4 Mon Sep 17 00:00:00 2001
> From: Aaron Merey <amerey@redhat.com>
> Date: Wed, 19 Feb 2020 12:30:14 -0500
> Subject: [PATCH] PR25502: debuginfod-client 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.
Another option might be to rename $HOME/.debuginfod_client_cache to the
new XDG cache debuginfod_client location. But that might be tricky if
it crosses file systems/mount points. So then you would probably need
to give up and fallback to what you do now if you get EXDEV from rename
(or actually any error). Might be too much work and a little bit too
tricky/clever...?
> diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
> index d812e6d7..fc076737 100644
> --- a/debuginfod/ChangeLog
> +++ b/debuginfod/ChangeLog
> @@ -1,3 +1,9 @@
> +2020-02-19 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-05 Frank Ch. Eigler <fche@redhat.com>
>
> * debuginfod.cxx (argp options): Add -Z option.
> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
> index e5a2e824..5772367c 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -98,6 +98,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. */
> @@ -359,6 +360,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)
> @@ -373,15 +384,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;
>
> @@ -447,24 +458,65 @@ debuginfod_query_server (debuginfod_client *c,
> target_cache_path: $HOME/.debuginfod_cache/0123abcd/source#PATH#TO#SOURCE ?
> */
>
> - 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" ?: "/"));
Are these brackets correct? Maybe ..., 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
> + {
> + char *cmd;
> +
> + xalloc_str (cmd, "mkdir -p \'%s\'", cachedir);
> + rc = system (cmd);
Please don't use system. Use rc = mkdir (cachedir, 0700);
> + free (cmd);
> + if (rc == 0)
> + {
> + free (cache_path);
> + xalloc_str (cache_path, "%s/%s", cachedir, cache_xdg_name);
> + }
> + else
> + {
> + rc = -EINVAL;
> + goto out;
> + }
The creation of the directory might race with another client, so
specifically test for EEXIST and then test S_ISDIR again before
returning failure.
> + }
> + }
> }
>
> - /* 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;
> @@ -479,7 +531,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;
> @@ -754,12 +807,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:
> @@ -775,7 +830,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;
> }
I am happy with the more dynamic creating of the strings and the
cleanup. Thanks!
> diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
> index ca844aed..ac231551 100644
> --- a/doc/debuginfod.8
> +++ b/doc/debuginfod.8
> @@ -406,8 +406,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
> @@ -418,8 +420,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
Should we mention the old location might still be used?
Maybe simply ignoring it is fine.
> .SH "SEE ALSO"
> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index 1f55a291..ec081b3c 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-02-19 Aaron Merey <amerey@redhat.com>
> + * run-debuginfod-find.sh: Run tests for verifying default
> + client cache locations.
Probably too much work for this patch. But it feels run-debuginfod-
find.sh is doing a lot of different testing. Might be good to put some
tests in their own test file.
> 2020-02-08 Mark Wielaard <mark@klomp.org>
>
> * run-pt_gnu_prop-tests.sh: New test.
> diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
> index 1cc8f406..9740f4aa 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}/.client_cache*
> + rm -rf F R D L Z ${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.
> +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.
> +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=`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.
> +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
You need to use testrun to run "test" binaries (or at least make sure
LD_LIBRARY_PATH is setup correctly) to make sure the test environment
is setup correctly. The above doesn't work when using srcdir!=builddir
and testrun will make sure that valgrind is ran (as is done with make
distcheck) when you configure with --enable-valgrind (which would have
caught the previous small memleak).
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 317c687c..083ff59b 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -152,14 +152,14 @@ cmp $filename ${PWD}/prog.c
mkdir tmphome
# $HOME/.cache should be created.
-env HOME=$PWD/tmphome XDG_CACHE_HOME= DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
+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.
-env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
+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
@@ -172,11 +172,11 @@ 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=`env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo deadbeef`
+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.
-env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH=$PWD/tmpcache ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
+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
Cheers,
Mark
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] debuginfod-client: default to XDG cache.
2020-02-21 16:13 ` Mark Wielaard
@ 2020-02-21 17:22 ` Aaron Merey
2020-02-21 20:09 ` Mark Wielaard
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Merey @ 2020-02-21 17:22 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel
On Fri, Feb 21, 2020 at 11:13 AM Mark Wielaard <mark@klomp.org> wrote:
> On Wed, 2020-02-19 at 16:04 -0500, Aaron Merey wrote:
> > 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.
>
> Another option might be to rename $HOME/.debuginfod_client_cache to the
> new XDG cache debuginfod_client location. But that might be tricky if
> it crosses file systems/mount points. So then you would probably need
> to give up and fallback to what you do now if you get EXDEV from rename
> (or actually any error). Might be too much work and a little bit too
> tricky/clever...?
We can try a simple rename and fallback if there's any error. We could
mention this briefly in the man page.
> > + if (xdg != NULL && strlen (xdg) > 0)
> > + snprintf (cachedir, PATH_MAX, "%s", xdg);
> > + else
> > + snprintf (cachedir, PATH_MAX, "%s/.cache", getenv ("HOME" ?: "/"));
>
> Are these brackets correct? Maybe ..., getenv ("HOME") ?: "/");
You're right it should be 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
> > + {
> > + char *cmd;
> > +
> > + xalloc_str (cmd, "mkdir -p \'%s\'", cachedir);
> > + rc = system (cmd);
>
> Please don't use system. Use rc = mkdir (cachedir, 0700);
Sure. My goal was to have any nonexisting parent directories created as
easily as possible and I wasn't aware of anything besides `mkdir -p`.
Should we not bother creating nonexisting parent directories?
> > + free (cmd);
> > + if (rc == 0)
> > + {
> > + free (cache_path);
> > + xalloc_str (cache_path, "%s/%s", cachedir, cache_xdg_name);
> > + }
> > + else
> > + {
> > + rc = -EINVAL;
> > + goto out;
> > + }
>
> The creation of the directory might race with another client, so
> specifically test for EEXIST and then test S_ISDIR again before
> returning failure.
Ok.
> > .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
>
> Should we mention the old location might still be used?
> Maybe simply ignoring it is fine.
Here I could mention the possibility of a rename when using a client
cache at the old location.
> > .SH "SEE ALSO"
> > diff --git a/tests/ChangeLog b/tests/ChangeLog
> > index 1f55a291..ec081b3c 100644
> > --- a/tests/ChangeLog
> > +++ b/tests/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2020-02-19 Aaron Merey <amerey@redhat.com>
> > + * run-debuginfod-find.sh: Run tests for verifying default
> > + client cache locations.
>
> Probably too much work for this patch. But it feels run-debuginfod-
> find.sh is doing a lot of different testing. Might be good to put some
> tests in their own test file.
That's a good idea for a future patch, maybe separate client and server
tests into two files (to the extent that they can be separated).
> You need to use testrun to run "test" binaries (or at least make sure
> LD_LIBRARY_PATH is setup correctly) to make sure the test environment
> is setup correctly. The above doesn't work when using srcdir!=builddir
> and testrun will make sure that valgrind is ran (as is done with make
> distcheck) when you configure with --enable-valgrind (which would have
> caught the previous small memleak).
>
> diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
> index 317c687c..083ff59b 100755
> --- a/tests/run-debuginfod-find.sh
> +++ b/tests/run-debuginfod-find.sh
> @@ -152,14 +152,14 @@ cmp $filename ${PWD}/prog.c
> mkdir tmphome
>
> # $HOME/.cache should be created.
> -env HOME=$PWD/tmphome XDG_CACHE_HOME= DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
> +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.
> -env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
> +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
> @@ -172,11 +172,11 @@ 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=`env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH= ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo deadbeef`
> +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.
> -env HOME=$PWD/tmphome XDG_CACHE_HOME=$PWD/tmpxdg DEBUGINFOD_CACHE_PATH=$PWD/tmpcache ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID
> +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
Ok I'll include that.
Thanks,
Aaron
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] debuginfod-client: default to XDG cache.
2020-02-21 17:22 ` Aaron Merey
@ 2020-02-21 20:09 ` Mark Wielaard
2020-02-28 0:01 ` Aaron Merey
0 siblings, 1 reply; 7+ messages in thread
From: Mark Wielaard @ 2020-02-21 20:09 UTC (permalink / raw)
To: Aaron Merey; +Cc: elfutils-devel
Hi Aaron,
On Fri, 2020-02-21 at 12:21 -0500, Aaron Merey wrote:
> On Fri, Feb 21, 2020 at 11:13 AM Mark Wielaard <mark@klomp.org> wrote:
> > On Wed, 2020-02-19 at 16:04 -0500, Aaron Merey wrote:
> > > + /* 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
> > > + {
> > > + char *cmd;
> > > +
> > > + xalloc_str (cmd, "mkdir -p \'%s\'", cachedir);
> > > + rc = system (cmd);
> >
> > Please don't use system. Use rc = mkdir (cachedir, 0700);
>
> Sure. My goal was to have any nonexisting parent directories created as
> easily as possible and I wasn't aware of anything besides `mkdir -p`.
> Should we not bother creating nonexisting parent directories?
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).
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 ?
*/
Cheers,
Mark
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] debuginfod-client: default to XDG cache.
2020-02-21 20:09 ` Mark Wielaard
@ 2020-02-28 0:01 ` Aaron Merey
2020-02-28 13:33 ` Mark Wielaard
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Merey @ 2020-02-28 0:01 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]
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
[-- Attachment #2: 0001-debuginfod-client-default-to-XDG-cache.patch --]
[-- Type: text/x-patch, Size: 13409 bytes --]
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] debuginfod-client: default to XDG cache.
2020-02-28 0:01 ` Aaron Merey
@ 2020-02-28 13:33 ` Mark Wielaard
0 siblings, 0 replies; 7+ messages in thread
From: Mark Wielaard @ 2020-02-28 13:33 UTC (permalink / raw)
To: Aaron Merey; +Cc: elfutils-devel
Hi Aaron,
On Thu, 2020-02-27 at 19:00 -0500, Aaron Merey wrote:
> On Fri, Feb 21, 2020 at 3:09 PM Mark Wielaard <mark@klomp.org> wrote:
> > 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.
And it gets rid of the system call. Nice.
> 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.
Yes, make sense.
Looks really good. Pushed to master.
Thanks,
Mark
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-28 13:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 17:41 [PATCH] debuginfod-client: default to XDG cache Aaron Merey
2020-02-19 21:04 ` Aaron Merey
2020-02-21 16:13 ` Mark Wielaard
2020-02-21 17:22 ` Aaron Merey
2020-02-21 20:09 ` Mark Wielaard
2020-02-28 0:01 ` Aaron Merey
2020-02-28 13:33 ` Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).