Re: [PATCH] libebl: Don't install libebl.a, libebl.h and remove backends from spec.
Hi Dmitry, On Thu, 2020-01-02 at 03:02 +0300, Dmitry V. Levin wrote: > On Thu, Aug 29, 2019 at 11:43:58PM +0200, Mark Wielaard wrote: > > All archive members from libebl.a are now in libdw.a. We don't generate > > separate backend shared libraries anymore. So remove them from the > > elfutils.spec file. > > > > diff --git a/libebl/ChangeLog b/libebl/ChangeLog > > index 6ba3a02b..4da7eeeb 100644 > > --- a/libebl/ChangeLog > > +++ b/libebl/ChangeLog > > @@ -1,3 +1,8 @@ > > +2019-08-29 Mark Wielaard > > + > > + * Makefile.am (noinst_LIBRARIES): Add libebl.a. > > + (noinst_HEADERS): Add libebl.h. > > (pkginclude_HEADERS): Remove. Ah, yes, sorry missed that one. > > @@ -62,6 +59,6 @@ libebl_a_SOURCES = eblopenbackend.c > > eblclosebackend.c eblreloctypename.c \ > > libebl_pic_a_SOURCES = > > am_libebl_pic_a_OBJECTS = $(libebl_a_SOURCES:.c=.os) > > > > -noinst_HEADERS = libeblP.h ebl-hooks.h > > +noinst_HEADERS = libebl.h libeblP.h ebl-hooks.h > > > > MOSTLYCLEANFILES = $(am_libebl_pic_a_OBJECTS) > > After this part of the change libebl.h is no longer installed, but > it's included by libasm.h which is still installed. > > This has to be fixed somehow, but I'm not sure whether libebl.h should be > reinstated, libasm.h should stop to include it, or libasm.h should stop > to be installed. Urgh. Missed that too. Even if you could use libasm.h, linking with libasm always was kind of useless since there was no supported way to create an Ebl handle. Unless you statically linked with libebl, which was never supported and might still not work since it might conflict with changes in the libebl linked to by libasm/libdw. In fact our own elfutils.spec doesn't even install it. But sadly some distros (including Fedora... my fault) do install it. Short term I propose the attached patch. But longer term we need new asm_begin and disasm_begin functions that don't depend on unsupportable libebl features. Cheers, Mark From 5cf9865639fdedc22e06f09bfa3f7713b8b65d95 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Wed, 8 Jan 2020 15:04:50 +0100 Subject: [PATCH] libasm.h: Don't include libebl.h. Define an opaque Ebl handle. Using libasm isn't really usable without a way to create an Ebl handle. But we don't support libebl.h (and libebl itself). Just define the Ebl handle as an opaque struct. Code that uses it needs to figure out how to instantiate one itself (they cannot in any supportable way...) Signed-off-by: Mark Wielaard --- libasm/ChangeLog | 5 + libasm/libasm.h | 2 +- libasm/libasmP.h | 1 + tests/ChangeLog | 4 tests/asm-tst1.c | 1 + tests/asm-tst2.c | 1 + tests/asm-tst3.c | 1 + tests/asm-tst4.c | 1 + tests/asm-tst5.c | 1 + tests/asm-tst6.c | 1 + tests/asm-tst7.c | 1 + tests/asm-tst8.c | 1 + tests/asm-tst9.c | 1 + 13 files changed, 20 insertions(+), 1 deletion(-) diff --git a/libasm/ChangeLog b/libasm/ChangeLog index a1abac88..7b0d3df3 100644 --- a/libasm/ChangeLog +++ b/libasm/ChangeLog @@ -1,3 +1,8 @@ +2020-01-08 Mark Wielaard + + * libasm.h: Don't include libebl.h. Define an opaque Ebl handle. + * libasmP.h: Do include libebl.h. + 2019-08-28 Mark Wielaard * Makefile.am (libasm_so_DEPS): Replace libebl.a with libebl_pic.a. diff --git a/libasm/libasm.h b/libasm/libasm.h index 5c612243..a45c9fa3 100644 --- a/libasm/libasm.h +++ b/libasm/libasm.h @@ -32,7 +32,7 @@ #include #include -#include +typedef struct ebl Ebl; /* Opaque type for the assembler context descriptor. */ diff --git a/libasm/libasmP.h b/libasm/libasmP.h index 54460cf9..a4703fc3 100644 --- a/libasm/libasmP.h +++ b/libasm/libasmP.h @@ -31,6 +31,7 @@ #include +#include "libebl.h" #include #include "libdwelf.h" diff --git a/tests/ChangeLog b/tests/ChangeLog index 7103cf51..2ed36c6c 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,7 @@ +2020-01-08 Mark Wielaard + + * asm-test?.c: include libebl.h. + 2019-12-04 Frank Ch. Eigler * run-debuinfod-find.sh: Test $DEBUGINFOD_PROGRESS. diff --git a/tests/asm-tst1.c b/tests/asm-tst1.c index 9afc676b..cdf2a921 100644 --- a/tests/asm-tst1.c +++ b/tests/asm-tst1.c @@ -20,6 +20,7 @@ #endif #include +#include ELFUTILS_HEADER(ebl) #include ELFUTILS_HEADER(asm) #include #include diff --git a/tests/asm-tst2.c b/tests/asm-tst2.c index 2556d0c4..9e88b70c 100644 --- a/tests/asm-tst2.c +++ b/tests/asm-tst2.c @@ -20,6 +20,7 @@ #endif #include +#include ELFUTILS_HEADER(ebl) #include ELFUTILS_HEADER(asm) #include #include diff --git a/tests/asm-tst3.c b/tests/asm-tst3.c index e52cfbe1..39c1d90c 100644 --- a/tests/asm-tst3.c +++ b/tests/asm-tst3.c @@ -20,6 +20,7 @@ #endif #include +#include ELFUTILS_HEADER(ebl) #include ELFUTILS_HEADER(asm) #include #include diff --git a/tests/asm-tst4.c b/tests/asm-tst4.c index 52e9e20b..5114938b 100644 --- a/tests/asm-tst4.c +++ b/tests/asm-tst4.c @@ -20,6 +20,7 @@ #endif #include +#include ELFUTILS_HEADER(ebl) #include ELFUTILS_HEADER(asm) #include #
Re: [PATCH] Do not install libdebuginfod.pc unless debuginfod is enabled
On Thu, 2020-01-02 at 02:42 +0300, Dmitry V. Levin wrote:
> Fixes: 288f6b199 ("debuginfod 1/2: client side")
> Signed-off-by: Dmitry V. Levin
> [...]
> +2020-01-01 Dmitry V. Levin
> +
> + * Makefile.am (pkgconfig_DATA): Conditionalize libdebuginfod.pc
> + on DEBUGINFOD.
That looks correct. Pushed to master.
Thanks,
Mark
Re: rfc/patch: user-agent distro-description for debuginfod http traffic
Hi Frank, On Mon, 2020-01-06 at 04:53 -0500, Frank Ch. Eigler wrote: > +debuginfod-client-useragent.h: > + if type uname 2>/dev/null; then \ > + echo '#define DEBUGINFOD_CLIENT_USERAGENT_1 "'`uname -sm | sed > -e 's, ,/,g'`'"' > $@; \ > + else \ > + echo '#define DEBUGINFOD_CLIENT_USERAGENT_1 ""' > $@; \ > + fi > + if type lsb_release 2>/dev/null; then \ > + echo '#define DEBUGINFOD_CLIENT_USERAGENT_2 "'`lsb_release -sir > | sed -e 's, ,/,g'`'"' >> $@; \ > + else \ > + echo '#define DEBUGINFOD_CLIENT_USERAGENT_2 ""' > $@; \ > + fi > + echo '#define DEBUGINFOD_CLIENT_USERAGENT DEBUGINFOD_CLIENT_USERAGENT_1 > "," DEBUGINFOD_CLIENT_USERAGENT_2' >> $@ Eep. We really should pick up this info during runtime instead of during build time. We do want a reproducible build. And this will most likely produce wrong information if the package build server is on a different OS or release than the OS/release it is producing packages for. uname -sm might be "stable", but probably not when cross- compiling. But where does lsb_release come from? I don't have that on my systems. Cheers, Mark >
Re: rfc/patch: user-agent distro-description for debuginfod http traffic
Hi - > Eep. We really should pick up this info during runtime instead of > during build time. That's what I thought at first too. However, doing it at run time means doing work - a popen() etc. - over and over or saving in a locked global. Since on a normal machine, the distro doesn't change without elfutils also changing, it's practically a literal. > We do want a reproducible build. Can you give an example? Bootstrapping a new distro version/arch? > And this will most likely produce wrong information if the package > build server is on a different OS or release than the OS/release it > is producing packages for. uname -sm might be "stable", but probably > not when cross- compiling. I wonder if the cross-compiled debuginfod-client case is worth worrying about. > But where does lsb_release come from? I don't have that on my > systems. BuildRequires: /usr/bin/lsb_release :-) It's a different package on different distros. And running at run time would make a Require: rather than a one-time BuildRequire:. - FChE
Re: rfc/patch: user-agent distro-description for debuginfod http traffic
Hi Frank,
On Wed, Jan 08, 2020 at 10:11:25AM -0500, Frank Ch. Eigler wrote:
> > Eep. We really should pick up this info during runtime instead of
> > during build time.
>
> That's what I thought at first too. However, doing it at run time
> means doing work - a popen() etc. - over and over or saving in a
> locked global. Since on a normal machine, the distro doesn't change
> without elfutils also changing, it's practically a literal.
We are already opening some files to scan for cached files, open a
socket, download data, etc. I don't think one or two extra syscalls
are that much overhead.
> > We do want a reproducible build.
>
> Can you give an example? Bootstrapping a new distro version/arch?
Probably best explained at https://reproducible-builds.org/ most
distributions are participating.
> > And this will most likely produce wrong information if the package
> > build server is on a different OS or release than the OS/release it
> > is producing packages for. uname -sm might be "stable", but probably
> > not when cross- compiling.
>
> I wonder if the cross-compiled debuginfod-client case is worth
> worrying about.
I think it is.
> > But where does lsb_release come from? I don't have that on my
> > systems.
>
> BuildRequires: /usr/bin/lsb_release :-) It's a different package on
> different distros. And running at run time would make a Require:
> rather than a one-time BuildRequire:.
It looks like the new standard (since about 8 years) is os-release:
http://man7.org/linux/man-pages/man5/os-release.5.html
It looks like an easily parsable file format. The attached produces
some reasonable looking identification strings on a couple of my
systems out of the box:
Linux/i686 4.19.0-6-686-pae
debian/10
Linux/x86_64 3.10.0-1062.9.1.el7.x86_64
rhel/7.7
Linux/x86_64 4.19.0-5-amd64
pureos/9.0
Maybe something like that is usable?
Cheers,
Mark#include
#include
#include
#include
int
main ()
{
struct utsname uts;
if (uname (&uts) == 0)
printf ("%s/%s %s\n", uts.sysname, uts.machine, uts.release);
FILE *f;
f = fopen ("/etc/os-release", "r");
if (f == NULL)
f = fopen ("/usr/lib/os-release", "r");
char *id = NULL;
char *version = NULL;
if (f != NULL)
{
while (id == NULL || version == NULL)
{
char buf[128];
char *s = &buf[0];
if (fgets (s, 128, f) == NULL)
break;
int len = strlen (s);
if (len < 3)
continue;
if (s[len - 1] == '\n')
{
s[len - 1] = '\0';
len--;
}
char *v = strchr (s, '=');
if (v == NULL || strlen (v) < 2)
continue;
/* Split var and value. */
*v = '\0';
v++;
/* Remove optional quotes around value string. */
if (*v == '"' || *v == '\'')
{
v++;
s[len - 1] = '\0';
}
if (strcmp (s, "ID") == 0)
id = strdup (v);
if (strcmp (s, "VERSION_ID") == 0)
version = strdup (v);
}
fclose (f);
}
printf ("%s/%s\n", id, version);
free (id);
free (version);
return 0;
}
Re: [PATCH] libebl: Don't install libebl.a, libebl.h and remove backends from spec.
Hi Mark, On Wed, Jan 08, 2020 at 03:09:32PM +0100, Mark Wielaard wrote: > On Thu, 2020-01-02 at 03:02 +0300, Dmitry V. Levin wrote: > > On Thu, Aug 29, 2019 at 11:43:58PM +0200, Mark Wielaard wrote: > > > All archive members from libebl.a are now in libdw.a. We don't generate > > > separate backend shared libraries anymore. So remove them from the > > > elfutils.spec file. > > > > > > diff --git a/libebl/ChangeLog b/libebl/ChangeLog > > > index 6ba3a02b..4da7eeeb 100644 > > > --- a/libebl/ChangeLog > > > +++ b/libebl/ChangeLog > > > @@ -1,3 +1,8 @@ > > > +2019-08-29 Mark Wielaard > > > + > > > + * Makefile.am (noinst_LIBRARIES): Add libebl.a. > > > + (noinst_HEADERS): Add libebl.h. > > > > (pkginclude_HEADERS): Remove. > > Ah, yes, sorry missed that one. > > > > @@ -62,6 +59,6 @@ libebl_a_SOURCES = eblopenbackend.c > > > eblclosebackend.c eblreloctypename.c \ > > > libebl_pic_a_SOURCES = > > > am_libebl_pic_a_OBJECTS = $(libebl_a_SOURCES:.c=.os) > > > > > > -noinst_HEADERS = libeblP.h ebl-hooks.h > > > +noinst_HEADERS = libebl.h libeblP.h ebl-hooks.h > > > > > > MOSTLYCLEANFILES = $(am_libebl_pic_a_OBJECTS) > > > > After this part of the change libebl.h is no longer installed, but > > it's included by libasm.h which is still installed. > > > > This has to be fixed somehow, but I'm not sure whether libebl.h should be > > reinstated, libasm.h should stop to include it, or libasm.h should stop > > to be installed. > > Urgh. Missed that too. > > Even if you could use libasm.h, linking with libasm always was kind of > useless since there was no supported way to create an Ebl handle. > Unless you statically linked with libebl, which was never supported and > might still not work since it might conflict with changes in the libebl > linked to by libasm/libdw. > > In fact our own elfutils.spec doesn't even install it. But sadly some > distros (including Fedora... my fault) do install it. > > Short term I propose the attached patch. Thanks, it works: the packaging check that used to complain about compilation error in installed libasm.h turns green when this patch is applied. -- ldv
