Hi Housam, This is a very cool feature. Thanks again for working on this!
On Fri, Jan 19, 2024 at 11:47 AM Housam Alamour <halam...@redhat.com> wrote: > > * srcfiles.cxx: Introduce new --zip option that places all the > source files associated with a specified dwarf/elf file > into a zip file and sends it to stdout. Files may be > fetched from debuginfod (if applicable) or locally as > a backup. > Added -b option to disable the backup of checking > for files locally in -z mode. > > * run-srcfiles-self.sh: Added test-case for the new zip > feature that archives the source files of the srcfiles > tool and checks archive integrity. An additional test > ensures that if debuginfod is enabled, the files are > fetched and archived properly while maintaing integrity. Should be 'maintaining'. > > * debuginfod-subr.sh: On very slow/remote storage, it can > take O(minute) to finish indexing the entire elfutils > build tree, so a wait_ready4 shell function is one > way to let a longer debuginfod wait operation work. > > * srcfiles.1, NEWS: Added documentation for the new zip feature. > > * configure.ac: Simplify check for libarchive for srcfiles.cxx > by integrating it into the same check for debuginfod. > > * Makefile.am: build with local copy of debuginfod-client. > > Example: > % ./src/srcfiles -z -e /bin/ls > output.zip > > https://sourceware.org/bugzilla/show_bug.cgi?id=30991 > > Signed-off-by: Housam Alamour <halam...@redhat.com> > --- > NEWS | 8 + > configure.ac | 5 +- > debuginfod/debuginfod.cxx | 2 - > doc/srcfiles.1 | 26 +++- > src/Makefile.am | 9 +- > src/srcfiles.cxx | 307 ++++++++++++++++++++++++++++++++----- > tests/debuginfod-subr.sh | 14 +- > tests/run-srcfiles-self.sh | 77 +++++++++- > 8 files changed, 394 insertions(+), 54 deletions(-) > > diff --git a/NEWS b/NEWS > index 0420d3b8..3391d6a1 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,8 @@ > +Version 0.191 (after 0.189) > + > +srcfiles: Can now fetch the source files of a DWARF/ELF file and > + place them into a zip. > + > Version 0.190 "Woke!" > > CONTRIBUTING: Switch from real name policy to known identity policy. > @@ -9,6 +14,9 @@ libelf: Add RELR support. > > libdw: Recognize .debug_[ct]u_index sections > > +srcfiles: added srcfiles tool that lists all the source files of a given > + DWARF/ELF file. > + > readelf: Support readelf -Ds, --use-dynamic --symbol. > Support .gdb_index version 9 > > diff --git a/configure.ac b/configure.ac > index af5b6bf7..ddb79b83 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -841,7 +841,7 @@ AM_CONDITIONAL([LIBDEBUGINFOD],[test > "x$enable_libdebuginfod" = "xyes" || test " > AM_CONDITIONAL([DUMMY_LIBDEBUGINFOD],[test "x$enable_libdebuginfod" = > "xdummy"]) > AC_CHECK_HEADERS([execinfo.h]) > > -# Look for libmicrohttpd, libarchive, sqlite for debuginfo server > +# Look for libmicrohttpd, libarchive, sqlite for debuginfo server and > srcfiles tool > # minimum versions as per rhel7. > AC_ARG_ENABLE([debuginfod],AS_HELP_STRING([--enable-debuginfod], [Build > debuginfod server])) > AS_IF([test "x$enable_debuginfod" != "xno"], [ > @@ -853,11 +853,12 @@ AS_IF([test "x$enable_debuginfod" != "xno"], [ > AC_MSG_ERROR([need libdebuginfod (or dummy), use --disable-debuginfod > to disable.]) > fi > enable_debuginfod=yes # presume success > + AC_DEFINE([HAVE_LIBARCHIVE], [1], [Define to 1 if libarchive is > available]) # presume success > PKG_PROG_PKG_CONFIG > PKG_CHECK_MODULES([libmicrohttpd],[libmicrohttpd >= > 0.9.33],[],[enable_debuginfod=no]) > PKG_CHECK_MODULES([oldlibmicrohttpd],[libmicrohttpd < > 0.9.51],[old_libmicrohttpd=yes],[old_libmicrohttpd=no]) > PKG_CHECK_MODULES([sqlite3],[sqlite3 >= > 3.7.17],[],[enable_debuginfod=no]) > - PKG_CHECK_MODULES([libarchive],[libarchive >= > 3.1.2],[],[enable_debuginfod=no]) > + PKG_CHECK_MODULES([libarchive],[libarchive >= > 3.1.2],[],[enable_debuginfod=no], AC_DEFINE([HAVE_LIBARCHIVE], [0], [Define > to 0 if libarchive is not available])) > if test "x$enable_debuginfod" = "xno"; then > AC_MSG_ERROR([dependencies not found, use --disable-debuginfod to > disable.]) > fi > diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx > index 524be948..6b21f46f 100644 > --- a/debuginfod/debuginfod.cxx > +++ b/debuginfod/debuginfod.cxx > @@ -2996,8 +2996,6 @@ dwarf_extract_source_paths (Elf *elf, set<string>& > debug_sourcefiles) > > if (comp_dir[0] == '\0' && cuname[0] != '/') > { > - // This is a common symptom for dwz-compressed debug files, > - // where the altdebug file cannot be resolved. > if (verbose > 3) > obatched(clog) << "skipping cu=" << cuname << " due to empty > comp_dir" << endl; > continue; > diff --git a/doc/srcfiles.1 b/doc/srcfiles.1 > index 6149c21b..a7cde664 100644 > --- a/doc/srcfiles.1 > +++ b/doc/srcfiles.1 > @@ -21,15 +21,18 @@ > eu-srcfiles \- Lists the source files of a DWARF/ELF file. > > .SH "SYNOPSIS" > -eu-srcfiles [\fB\-0\fR|\fB\-\-null\fR] [\fB\-c\fR|\fB\-\-cu\-only\fR] > [\fB\-v\fR|\fB\-\-verbose\fR] INPUT > +eu-srcfiles [\fB\-0\fR|\fB\-\-null\fR] [\fB\-c\fR|\fB\-\-cu\-only\fR] > [\fB\-v\fR|\fB\-\-verbose\fR] [\fB\-z\fR|\fB\-\-zip\fR] INPUT > > .SH "DESCRIPTION" > -\fBeu-srcfiles\fR lists the source files of a given \s-DWARF/ELF\s0 > +\fBeu-srcfiles\fR lists all the source files of a given DWARF/ELF > file. This list is based on a search of the DWARF debuginfo, which > may be automatically fetched by debuginfod if applicable. The target > file may be an executable, a coredump, a process, or even the running > -kernel. The default is the file 'a.out'. The source file names are > -made unique and printed to standard output. > +kernel. The default input is the file 'a.out'. The source file names are > +made unique by prepending the full path name and then printed to standard > output. The source files can be > +placed in a zip file that is sent to stdout. > + > +Note that all occurrences of '/./' and '/../' in the path name are > canonicalized. > > .SH "INPUT OPTIONS" > The long and short forms of options, shown here as alternatives, are > @@ -82,12 +85,20 @@ Separate items by a null instead of a newline. > > .TP > \fB\-c, \-\-cu\-only\fR > -Only list the CU names. > +Only list the (compilation unit) CU names. I think this should be either "compilation unit (CU)" or CU (compilation unit)". > .TP > \fB\-v, \-\-verbose\fR > Increase verbosity of logging messages. > > +.TP > +\fB\-z, \-\-zip\fR > +Zip all the source files and send to stdout. > +Files may be automatically fetched by > +debuginfod (if applicable) or locally as a > +backup. Any source files that were not found > +will not be archived. > + > > .SH EXAMPLES > > @@ -119,6 +130,11 @@ List the source files of a kernel image. > eu-srcfiles -e /boot/vmlinuz-`uname -r` > .ESAMPLE > > +Zip all the source files for a binary. > +.SAMPLE > +eu-srcfiles -z -e /bin/ls > ls.zip > +.ESAMPLE > + > > .SH "AUTHOR" > Written by Housam Alamour. > diff --git a/src/Makefile.am b/src/Makefile.am > index d3d9d408..5fa8df3d 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -21,7 +21,7 @@ DEFS += $(YYDEBUG) -DDEBUGPRED=@DEBUGPRED@ \ > -DSRCDIR=\"$(shell cd $(srcdir);pwd)\" -DOBJDIR=\"$(shell pwd)\" > AM_CPPFLAGS += -I$(srcdir)/../libelf -I$(srcdir)/../libebl \ > -I$(srcdir)/../libdw -I$(srcdir)/../libdwelf \ > - -I$(srcdir)/../libdwfl -I$(srcdir)/../libasm > + -I$(srcdir)/../libdwfl -I$(srcdir)/../libasm -I../debuginfod > > AM_LDFLAGS = -Wl,-rpath-link,../libelf:../libdw $(STACK_USAGE_NO_ERROR) > > @@ -50,6 +50,11 @@ libelf = ../libelf/libelf.so > endif > libebl = ../libebl/libebl.a ../backends/libebl_backends.a ../libcpu/libcpu.a > libeu = ../lib/libeu.a > +if LIBDEBUGINFOD > +libdebuginfod = ../debuginfod/libdebuginfod.so > +else > +libdebuginfod = > +endif > > if DEMANGLE > demanglelib = -lstdc++ > @@ -85,7 +90,7 @@ stack_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) > $(argp_LDADD) $(demanglelib) > elfcompress_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD) > elfclassify_LDADD = $(libelf) $(libdw) $(libeu) $(argp_LDADD) > srcfiles_SOURCES = srcfiles.cxx > -srcfiles_LDADD = $(libdw) $(libelf) $(libeu) $(argp_LDADD) > +srcfiles_LDADD = $(libdw) $(libelf) $(libeu) $(argp_LDADD) > $(libarchive_LIBS) $(libdebuginfod) > > installcheck-binPROGRAMS: $(bin_PROGRAMS) > bad=0; pid=$$$$; list="$(bin_PROGRAMS)"; for p in $$list; do \ > diff --git a/src/srcfiles.cxx b/src/srcfiles.cxx > index 3c7afdc4..0e4f756f 100644 > --- a/src/srcfiles.cxx > +++ b/src/srcfiles.cxx > @@ -15,6 +15,21 @@ > > You should have received a copy of the GNU General Public License > along with this program. If not, see <http://www.gnu.org/licenses/>. */ > + > + > +/* In case we have a bad fts we include this before config.h because it > + can't handle _FILE_OFFSET_BITS. > + Everything we need here is fine if its declarations just come first. > + Also, include sys/types.h before fts. On some systems fts.h is not self > + contained. */ > +#ifdef BAD_FTS > + #include <sys/types.h> > + #include <fts.h> > +#endif > + > +#ifdef HAVE_CONFIG_H > +# include <config.h> > +#endif > > #include "printversion.h" > #include <dwarf.h> > @@ -23,23 +38,50 @@ > #include <set> > #include <string> > #include <cassert> > -#include <config.h> > +#include <gelf.h> > +#include <memory> > + > +#ifdef ENABLE_LIBDEBUGINFOD > + #include "debuginfod.h" > +#endif > > #include <libdwfl.h> > #include <fcntl.h> > #include <iostream> > #include <libdw.h> > +#include <sstream> > +#include <vector> > + > +/* Libraries for use by the --zip option */ > +#ifdef HAVE_LIBARCHIVE > + #include <archive.h> > + #include <archive_entry.h> > +#endif > + > +/* If fts.h is included before config.h, its indirect inclusions may not > + give us the right LFS aliases of these functions, so map them manually. */ > +#ifdef BAD_FTS > + #ifdef _FILE_OFFSET_BITS > + #define open open64 > + #define fopen fopen64 > + #endif > +#else > + #include <sys/types.h> > + #include <fts.h> > +#endif > > using namespace std; > > -/* Name and version of program. */ > +/* Name and version of program. */ FYI it's common to leave two spaces after a period. But if you prefer just one space that's fine too. > ARGP_PROGRAM_VERSION_HOOK_DEF = print_version; > > -/* Bug report address. */ > +/* Bug report address. */ > ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT; > > -/* Definitions of arguments for argp functions. */ > -static const struct argp_option options[] = > +constexpr size_t BUFFER_SIZE = 8192; > + > +/* Definitions of arguments for argp functions. */ > +static const struct argp_option options[] = > { > { NULL, 0, NULL, OPTION_DOC, N_("Output options:"), 1 }, > { "null", '0', NULL, 0, > @@ -47,21 +89,30 @@ static const struct argp_option options[] = > { "verbose", 'v', NULL, 0, > N_ ("Increase verbosity of logging messages."), 0 }, > { "cu-only", 'c', NULL, 0, N_ ("Only list the CU names."), 0 }, > - { NULL, 0, NULL, 0, NULL, 0 } > + #ifdef HAVE_LIBARCHIVE > + { "zip", 'z', NULL, 0, N_ ("Zip all the source files and send to stdout." > + "Cannot be used with the null option"), 0 }, > + #ifdef ENABLE_LIBDEBUGINFOD > + { "no-backup", 'b', NULL, 0, N_ ("Disables local source file search when > " > + "debuginfod fails to fetch files. This option is only applicable" There should be a space after "send to stdout." and "is only applicable". I would emphasize in the -z help text and man page entry that debuginfod queries, if enabled, take priority over local source files by default. I would also keep -z and -b options even when srcfiles isn't built with libarchive/libdebuginfod. If -z or -b is given in these cases, we can just emit a warning that the option isn't supported and will be ignored. This helps prevent immediate error exits due to unrecognized options, which can be inconvenient when srcfiles is used in a script. Also it looks like the man page is missing an entry for -b. > + "when fetching and zipping files."), 0 }, > + #endif > + #endif > + { NULL, 0, NULL, 0, NULL, 0 } > }; > > -/* Short description of program. */ > +/* Short description of program. */ > static const char doc[] = N_("Lists the source files of a DWARF/ELF file. > The default input is the file 'a.out'."); > > -/* Strings for arguments in help texts. */ > +/* Strings for arguments in help texts. */ > static const char args_doc[] = N_("INPUT"); > > -/* Prototype for option handler. */ > +/* Prototype for option handler. */ > static error_t parse_opt (int key, char *arg, struct argp_state *state); > > -static struct argp_child argp_children[2]; /* [0] is set in main. */ > +static struct argp_child argp_children[2]; /* [0] is set in main. */ > > -/* Data structure to communicate with argp functions. */ > +/* Data structure to communicate with argp functions. */ > static const struct argp argp = > { > options, parse_opt, args_doc, doc, argp_children, NULL, NULL > @@ -73,8 +124,19 @@ static bool verbose; > static bool null_arg; > /* Only print compilation unit names. */ > static bool CU_only; > - > -/* Handle program arguments. */ > +#ifdef HAVE_LIBARCHIVE > + /* Zip all the source files and send to stdout. */ > + static bool zip; > + > + #ifdef ENABLE_LIBDEBUGINFOD > + /* Disables local source file search when debuginfod fails to fetch them. > + This option is only applicable when fetching and zipping files.*/ > + static bool no_backup; > + #endif > +#endif > + > +/* Handle program arguments. Note null arg and zip > + cannot be combined due to warnings raised when unzipping. */ > static error_t > parse_opt (int key, char *arg, struct argp_state *state) > { > @@ -97,6 +159,18 @@ parse_opt (int key, char *arg, struct argp_state *state) > case 'c': > CU_only = true; > break; > + > + #ifdef HAVE_LIBARCHIVE > + case 'z': > + zip = true; > + break; > + > + #ifdef ENABLE_LIBDEBUGINFOD > + case 'b': > + no_backup = true; > + break; > + #endif > + #endif > > default: > return ARGP_ERR_UNKNOWN; > @@ -104,11 +178,40 @@ parse_opt (int key, char *arg, struct argp_state *state) > return 0; > } > > +/* Remove the "/./" , "../" and the preceding directory > + that some paths include which raise errors during unzip. */ > +string canonicalize_path(string path) > +{ > + stringstream ss(path); > + string token; > + vector<string> tokens; > + /* Extract each directory of the path and place into a vector. */ > + while (getline(ss, token, '/')) { > + /* Ignore any empty //, or /./ dirs. */ > + if (token == "" || token == ".") > + continue; > + /* When /.. is encountered, remove the most recent directory from the > vector. */ > + else if (token == "..") { > + if (!tokens.empty()) > + tokens.pop_back(); > + } else > + tokens.push_back(token); > + } > + stringstream result; > + if (tokens.empty()) > + return "/"; > + /* Reconstruct the path from the extracted directories. */ > + for (const string &t : tokens) { > + result << '/' << t; > + } > + return result.str(); > +} > > -/* Global list of collected source files. Normally, it'll contain > - the sources of just one named binary, but the '-K' option can cause > - multiple dwfl modules to be loaded, thus listed. */ > - set<string> debug_sourcefiles; > +/* Global list of collected source files and their respective module. > + Normally, it'll contain the sources of just one named binary, but > + the '-K' option can cause multiple dwfl modules to be loaded, thus > + listed. */ > +set<pair<string, Dwfl_Module*>> debug_sourcefiles; > > static int > collect_sourcefiles (Dwfl_Module *dwflmod, > @@ -118,15 +221,14 @@ collect_sourcefiles (Dwfl_Module *dwflmod, > void *arg __attribute__ ((unused))) > { > Dwarf *dbg; > - Dwarf_Addr bias; /* ignored - for addressing purposes only */ > - > + Dwarf_Addr bias; /* ignored - for addressing purposes only. */ > + > dbg = dwfl_module_getdwarf (dwflmod, &bias); > > Dwarf_Off offset = 0; > Dwarf_Off old_offset; > size_t hsize; > - > - /* Traverse all CUs of this module. */ > + /* Traverse all CUs of this module. */ > while (dwarf_nextcu (dbg, old_offset = offset, &offset, &hsize, NULL, > NULL, NULL) == 0) > { > Dwarf_Die cudie_mem; > @@ -141,7 +243,7 @@ collect_sourcefiles (Dwfl_Module *dwflmod, > if (dwarf_getsrcfiles (cudie, &files, &nfiles) != 0) > continue; > > - /* extract DW_AT_comp_dir to resolve relative file names */ > + /* extract DW_AT_comp_dir to resolve relative file names. */ > const char *comp_dir = ""; > const char *const *dirs; > size_t ndirs; > @@ -152,11 +254,19 @@ collect_sourcefiles (Dwfl_Module *dwflmod, > comp_dir = ""; > > if (verbose) > - std::clog << "searching for sources for cu=" << cuname > + clog << "searching for sources for cu=" << cuname > << " comp_dir=" << comp_dir << " #files=" << nfiles > << " #dirs=" << ndirs << endl; > - > - for (size_t f = 1; f < nfiles; f++) > + > + if (comp_dir[0] == '\0' && cuname[0] != '/') > + { > + /* This is a common symptom for dwz-compressed debug files, > + where the altdebug file cannot be resolved. */ > + if (verbose) > + clog << "skipping cu=" << cuname << " due to empty comp_dir" << > endl; > + continue; > + } > + for (size_t f = 1; f < nfiles; ++f) > { > const char *hat; > if (CU_only) > @@ -172,7 +282,7 @@ collect_sourcefiles (Dwfl_Module *dwflmod, > continue; > > if (string(hat).find("<built-in>") > - != std::string::npos) /* gcc intrinsics, don't bother record > */ > + != string::npos) /* gcc intrinsics, don't bother recording */ > continue; > > string waldo; > @@ -180,23 +290,137 @@ collect_sourcefiles (Dwfl_Module *dwflmod, > waldo = (string (hat)); > else if (comp_dir[0] != '\0') /* comp_dir relative */ > waldo = (string (comp_dir) + string ("/") + string (hat)); > - debug_sourcefiles.insert (waldo); > + else > + { > + if (verbose) > + clog << "skipping file=" << hat << " due to empty comp_dir" << > endl; > + continue; > + } > + waldo = canonicalize_path (waldo); > + debug_sourcefiles.insert (make_pair(waldo, dwflmod)); > } > } > - > return DWARF_CB_OK; > } > > +#ifdef HAVE_LIBARCHIVE > + void zip_files() You don't have to indent within preprocessor conditionals. Especially since the lines of code in this function are already quite long. > + { > + struct archive *a = archive_write_new(); > + struct stat st; > + char buff[BUFFER_SIZE]; > + int len; > + int fd; > + #ifdef ENABLE_LIBDEBUGINFOD > + /* Initialize a debuginfod client. */ > + static unique_ptr <debuginfod_client, void (*)(debuginfod_client*)> > + client (debuginfod_begin(), &debuginfod_end); > + #endif > + > + archive_write_set_format_zip(a); > + archive_write_open_fd(a, STDOUT_FILENO); > + > + int missing_files = 0; > + for (const auto &pair : debug_sourcefiles) > + { > + fd = -1; > + const std::string &file_path = pair.first; > + > + /* Attempt to query debuginfod client to fetch source files. */ > + #ifdef ENABLE_LIBDEBUGINFOD > + Dwfl_Module* dwflmod = pair.second; > + /* Obtain source file's build ID. */ > + const unsigned char *bits; > + GElf_Addr vaddr; > + int bits_length = dwfl_module_build_id(dwflmod, &bits, &vaddr); > + /* Ensure successful client and build ID acquisition. */ > + if (client.get() != NULL && bits_length > 0) > + { > + fd = debuginfod_find_source(client.get(), > + bits, bits_length, > + file_path.c_str(), NULL); > + } > + else > + { > + if (client.get() == NULL) > + cerr << "Error: Failed to initialize debuginfod client." << > endl; > + else > + cerr << "Error: Invalid build ID length (" << bits_length << > ")." << endl; > + } > + > + #endif > + if (!no_backup) > + /* Files could not be located using debuginfod, search locally */ > + if (fd < 0) > + fd = open(file_path.c_str(), O_RDONLY); > + if (fd < 0) > + { > + missing_files++; > + continue; > + } > + > + /* Create an entry for each file including file information to be > placed in the zip. */ > + if (fstat(fd, &st) == -1) > + { > + missing_files++; > + if (verbose) > + cerr << "Error: Failed to get file status for " << file_path << ": > " << strerror(errno) << endl; > + continue; > + } > + struct archive_entry *entry = archive_entry_new(); > + /* Removing first "/"" to make the path "relative" before zipping, > otherwise warnings are raised when unzipping. */ > + string entry_name = file_path.substr(file_path.find_first_of('/') + 1); > + archive_entry_set_pathname(entry, entry_name.c_str()); > + archive_entry_copy_stat(entry, &st); > + if (archive_write_header(a, entry) != ARCHIVE_OK) > + { > + missing_files++; > + if (verbose) > + cerr << "Error: failed to write header for " << file_path << ": " > << archive_error_string(a) << endl; > + continue; > + } > + > + /* Write the file to the zip. */ > + len = read(fd, buff, sizeof(buff)); > + if (len == -1) > + { > + missing_files++; > + if (verbose) > + cerr << "Error: Failed to open file: " << file_path << ": " << > strerror(errno) <<endl; > + continue; > + } > + if (verbose && len > 0) > + clog << "Writing to zip: " << file_path << endl; > + while (len > 0) > + { > + if (archive_write_data(a, buff, len) < ARCHIVE_OK) > + { > + if (verbose) > + cerr << "Error: Failed to read from the file: " << file_path << > ": " << strerror(errno) << endl; > + break; > + } > + len = read(fd, buff, sizeof(buff)); > + } > + close(fd); > + archive_entry_free(entry); > + } > + if (verbose && missing_files > 0 ) > + cerr << "WARNING: " << missing_files << " files could not be found." > << endl; I think we should also print the names of the missing source files. This information could help the user decide if the missing files are a problem for them. Also just a small detail, but I'd remove "WARNING" and just say: N files could not be found: file1 file2 ... A file might not be found for a variety of mundane reasons but the all-caps warning suggests to me that something is critically wrong. > + > + archive_write_close(a); > + archive_write_free(a); > + } > +#endif > > int > main (int argc, char *argv[]) > { > int remaining; > - > - /* Parse and process arguments. This includes opening the modules. */ > + > + /* Parse and process arguments. This includes opening the modules. */ > argp_children[0].argp = dwfl_standard_argp (); > argp_children[0].group = 1; > - > + > Dwfl *dwfl = NULL; > (void) argp_parse (&argp, argc, argv, 0, &remaining, &dwfl); > assert (dwfl != NULL); > @@ -204,14 +428,23 @@ main (int argc, char *argv[]) > (void) dwfl_getmodules (dwfl, &collect_sourcefiles, NULL, 0); > > if (!debug_sourcefiles.empty ()) > - for (const string &element : debug_sourcefiles) > + { > + #ifdef HAVE_LIBARCHIVE > + if (zip) > + zip_files(); > + else > + #endif > { > - std::cout << element; > - if (null_arg) > - std::cout << '\0'; > - else > - std::cout << '\n'; > + for (const auto &pair : debug_sourcefiles) > + { > + cout << pair.first; > + if (null_arg) > + cout << '\0'; > + else > + cout << '\n'; > + } > } > + } > > dwfl_end (dwfl); > return 0; > diff --git a/tests/debuginfod-subr.sh b/tests/debuginfod-subr.sh > index 108dff74..c3b0603d 100755 > --- a/tests/debuginfod-subr.sh > +++ b/tests/debuginfod-subr.sh > @@ -79,12 +79,12 @@ errfiles() { > # So we gather the LD_LIBRARY_PATH with this cunning trick: > ldpath=`testrun sh -c 'echo $LD_LIBRARY_PATH'` > > -wait_ready() > +wait_ready4() > { > port=$1; > what=$2; > value=$3; > - timeout=20; > + timeout=$4; > > echo "Wait $timeout seconds on $port for metric $what to change to $value" > while [ $timeout -gt 0 ]; do > @@ -105,6 +105,16 @@ wait_ready() > fi > } > > +wait_ready() > +{ > + port=$1; > + what=$2; > + value=$3; > + timeout=20; > + wait_ready4 "$port" "$what" "$value" "$timeout" > +} > + > + > archive_test() { > __BUILDID=$1 > __SOURCEPATH=$2 > diff --git a/tests/run-srcfiles-self.sh b/tests/run-srcfiles-self.sh > index 0e64dd2b..1ee460f6 100755 > --- a/tests/run-srcfiles-self.sh > +++ b/tests/run-srcfiles-self.sh > @@ -1,4 +1,4 @@ > -#! /bin/sh > +#! /usr/bin/env bash > # Copyright (C) 2023 Red Hat, Inc. > # This file is part of elfutils. > # > @@ -15,7 +15,16 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > -. $srcdir/test-subr.sh > +. $srcdir/debuginfod-subr.sh > + > +# prereqs > +type unzip 2>/dev/null || exit 77 We could check for unzip when it's actually needed. This way we can at least run the tests that don't rely on unzip. > + > +# for test case debugging, uncomment: > +# set -x > +set -e > +base=14000 > +get_ports > > # Test different command line combinations on the srcfiles binary itself. > ET_EXEC="${abs_top_builddir}/src/srcfiles" > @@ -26,11 +35,16 @@ SRC_NAME="srcfiles.cxx" > # Ensure the output contains the expected source file srcfiles.cxx > testrun $ET_EXEC -e $ET_EXEC | grep $SRC_NAME > /dev/null > > +# Check if zip option is available (only available if libarchive is > available. > +# Debuginfod optional to fetch source files from debuginfod federation.) > +$ET_EXEC --help | grep -q zip && zip=true || zip=false If -z is changed to be always a valid option even when support is missing (as suggested above), then this check for -z support will have to change. > + > for null_arg in --null ""; do > for verbose_arg in --verbose ""; do > + echo "Test with options $null_arg $verbose_arg" > testrun $ET_EXEC $null_arg $verbose_arg -p $ET_PID > /dev/null > > - # Ensure that the output contains srclines.cxx > + # Ensure that the output contains srcfiles.cxx > cu_only=$(testrun $ET_EXEC $null_arg $verbose_arg -c -e $ET_EXEC) > default=$(testrun $ET_EXEC $null_arg $verbose_arg -e $ET_EXEC) > result1=$(echo "$cu_only" | grep "$SRC_NAME") > @@ -40,9 +54,64 @@ for null_arg in --null ""; do > exit 1 > fi > > - # Ensure that the output with the cu-only option contains less source > files > + # Ensure that the output with the cu-only option contains fewer source > files > if [ $(echo "$cu_only" | wc -m) -gt $(echo "$default" | wc -m) ]; then > exit 1 > fi > + > + if $zip; then > + # Zip option tests > + testrun $ET_EXEC $verbose_arg -z -e $ET_EXEC > test.zip > + tempfiles test.zip > + > + unzip -v test.zip > + unzip -t test.zip > + > + # Ensure unzipped srcfiles.cxx and its contents are the same as the > original source file > + unzip -j test.zip "*/$SRC_NAME" > + diff "$SRC_NAME" $abs_srcdir/../src/$SRC_NAME > + rm -f test.zip $SRC_NAME > + fi > done > done > + > +# Debuginfod source file downloading test. > +# Start debuginfod server on the elfutils build directory. > +if [ -x ${abs_builddir}/../debuginfod/debuginfod ] && $zip; then > + LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../debuginfod/debuginfod -vvvv -d > debuginfod.sqlite3 -F -p $PORT1 ${abs_top_builddir}/src > debuginfod.log 2>&1 > & > + PID1=$! > + tempfiles debuginfod.sqlite3 debuginfod.log > + wait_ready $PORT1 'ready' 1 > + wait_ready $PORT1 'thread_work_total{role="traverse"}' 1 > + wait_ready $PORT1 'thread_work_pending{role="scan"}' 0 > + wait_ready4 $PORT1 'thread_busy{role="scan"}' 0 300 # lots of source files > may be slow to index with $db on NFS > + > + export DEBUGINFOD_URLS="http://localhost:${PORT1}/"; > + export DEBUGINFOD_VERBOSE=1 > + testrun $ET_EXEC -z -b -e $ET_EXEC > test.zip > + tempfiles test.zip > + > + unzip -v test.zip > + unzip -t test.zip > + > + # Extract the zip. > + mkdir extracted > + unzip test.zip -d extracted > + > + # Ensure that source files for this tool have been archived. > + source_files="srcfiles.cxx libdwfl.h gelf.h" > + extracted_files=$(find extracted -type f) > + for file in $source_files; do > + echo "$extracted_files" | grep -q "$file" > /dev/null > + done > + > + # Compare between the extracted file and the actual source file > srcfiles.cxx. > + extracted_file=$(find extracted -name $SRC_NAME) > + diff "$extracted_file" $abs_srcdir/../src/$SRC_NAME > + > + rm -rf extracted > + > + kill $PID1 > + wait > + PID1=0 > +fi > -- > 2.43.0 > Nice, the tests pass for me even if libarchive and/or debuginfod is missing. Aaron
