Hi, Ryan -
Sorry it's been taking time. These are some notes for what I believe
is the newest snapshot in git: users/rgoldber/try-metadata_query.
Overall things look fine. The gist of it (for the sake of audience)
is the introduction of a new webapi to query debuginfod servers for
metadata about files matching a glob pattern. So, fetching
/metadata/GLOBPATTERN
would return a json document with an array of metadata kind of like
[ { "atype": "e", "buildid":
"f17a29b5a25bd4960531d82aa6b07c8abe84fa66", "mtime": 98298348,
"stype": "R", "source0": "/foo/D/hithere_1.0-1_amd64.deb",
"source1": "/usr/bin/hithere", "artifactsrc": "", "source0ref": ""
} ]
based on the current index of the debuginfod server, plus all of its
upstream federated peers. So it'd be a really easy way of finding all
known versions of "/lib*/libc.so.6" for example. Corresponding C api
& cli included. Consumers can use this to map from path names to
buildids (such as for a forthcoming systemtap feature).
Quibble 1: the webapi quoting is a bit clumsy. The testcase says:
http://127.0.0.1:$PORT2/metadata/%2Fusr%2Abin%2Ahi%2A
Let's switch to something like:
http://127.0.0.1:$PORT2/metadata?glob=/usr/bin/*hi*
because this leaves room for future expandability (beyond "glob="),
and does not require quite as much unnatural urlencoding.
Quibble 2: the C api has to be super clear about the lifetime of
the returned string: the m parameter here.
+int debuginfod_find_metadata (debuginfod_client *c,
+ const char* p, char** m) { return -ENOSYS; }
In the other _find_ routines, the result is written to the *m
pointer (if non-NULL!) and as a copied string that the caller is expected
to free. That's fine here too, but then debuginfod_find_metadata
cannot possibly just set *metadata = "[ ]"; which is not a free'able
copied string.
Quibble 3: autoconf json-c can just feed HAVE_JSON_C rather than
ENABLE_FIND_METADATA.
Quibble 4: in the client, metadata_callback() could be simplified,
as realloc can take NULL first parameter, so the
if (data->*data == NULL)
malloc()
else
realloc()
isn't needed. Just use realloc. (I see the same code in header_callback,
so same simplification can work there too.)
Quibble 5: In debuginfod-find, call PATH a GLOB, or something like that
Quibble 6: in debuginfod.cxx handle_metadata(), you don't really need
three separate sqlite queries. One can combine the three via
select XYZ from ABC
union all
select XYZ from DEF
union all
select XYZ from GHI
Why the order by?
Quibble 7: Same function, bottom. In case the metadata queries are
not supported, it's better not to give fake data "[ ]" back. Older
servers will just reject the query anyway with a 503 or similar via an
exception. It's okay to do the same here.
Quibble 8: Server, handler_cb(). There is no need for "after you"
type synchronization for metadata. These queries are not worth
delaying, for "after you" purposes, because near-concurrent query
results r or prefetches are not retained in a cache.
Quibble 9: debuginfod.h doc for new function, spell out that the
**metadata must be non-NULL to receive the string. Don't bother spell
out that things are "ESCAPED", it's just a normal json array string.
Quibble 10. We need to decide how much of the individual json
metadata objects to document, i.e., which ones we promise to apps to
expect. We can leave some fuzziness too, like "extra fields may
appear". Absent fields are probably better than empty-string fields.
We should document whether there is any duplicate elimination provided
by the client, I think the answer is no, but let's be clear.
Quibble 11. The test case test-compiles a C file to look for json-c.
It could probably just run `pkg-config json-c` to see if suceeds or
fails. And heck the main autoconf could use
PKG_CHECK_MODULE([json-c]...) also actually. No big deal.
Quibble 12. The test case should process the resulting files with jq
(really easy to extract exactly all the buildids from a json
array-of-objects), or else get some guarantee from debuginfod-find
that the output is formatted in a specific way and then grep or
whatever. The test case shouldn't rely on undocumented formatting
accidents from debuginfod.
- FChE
