Message ID | 20190801010044.56927-1-kilobyte@angband.pl (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | daxctl: link against libndctl, in case its use doesn't get pruned | expand |
On Thu, 2019-08-01 at 03:00 +0200, Adam Borowski wrote: > util/json.c uses libndctl symbols, and is included by daxctl. These > functions should then get pruned as unused, but on some platforms the > toolchain fails to do so. > > These platforms are ia64, hppa and 32-bit powerpc. It's generally a > waste of our time to build ndctl there, but as the lack of a > build-dependency requires annoying workarounds higher in the stack, > this has been requested in https://bugs.debian.org/914348 -- and is > trivially fixable on the ndctl side. > > Thanks to -Wl,--as-needed there's no harm to architectures where the > pruning works, thus let's not bother with detection. > > As daxctl and libdaxctl are separate, there's no circular dependency. > > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > daxctl/Makefile.am | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Hi Adam, Thanks for the report and the patch - however, historically, we've avoided linking from daxctl to libndctl. I think we can still avoid this by moving the libndctl users in util/json.c and util/filter.c into respective ndctl/util/ files. The same goes for libdaxctl users in those files - they move into daxctl/util/ I think that would be a cleaner approach, and I can take a shot at making the split next week, however we're close to a v66 release, and it will have to be after that happens. Perhaps the debian package can temporarily carry your patch for the archs that fail? Thanks, -Vishal > > diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am > index 94f73f9..a5487d6 100644 > --- a/daxctl/Makefile.am > +++ b/daxctl/Makefile.am > @@ -21,4 +21,5 @@ daxctl_LDADD =\ > lib/libdaxctl.la \ > ../libutil.a \ > $(UUID_LIBS) \ > - $(JSON_LIBS) > + $(JSON_LIBS) \ > + ../ndctl/lib/libndctl.la
On Fri, Aug 02, 2019 at 05:15:21PM +0000, Verma, Vishal L wrote: > On Thu, 2019-08-01 at 03:00 +0200, Adam Borowski wrote: > > util/json.c uses libndctl symbols, and is included by daxctl. These > > functions should then get pruned as unused, but on some platforms the > > toolchain fails to do so. > > this has been requested in https://bugs.debian.org/914348 > Thanks for the report and the patch - however, historically, we've > avoided linking from daxctl to libndctl. > > I think we can still avoid this by moving the libndctl users in > util/json.c and util/filter.c into respective ndctl/util/ files. > > The same goes for libdaxctl users in those files - they move into > daxctl/util/ > > I think that would be a cleaner approach, and I can take a shot at > making the split next week, however we're close to a v66 release, and it > will have to be after that happens. > > Perhaps the debian package can temporarily carry your patch for the > archs that fail? Sounds like a plan. CCing the bug. Breno: could you please take this patch for v65 or v66, until it gets done a better (but more intrusive) way? 喵!
diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am index 94f73f9..a5487d6 100644 --- a/daxctl/Makefile.am +++ b/daxctl/Makefile.am @@ -21,4 +21,5 @@ daxctl_LDADD =\ lib/libdaxctl.la \ ../libutil.a \ $(UUID_LIBS) \ - $(JSON_LIBS) + $(JSON_LIBS) \ + ../ndctl/lib/libndctl.la
util/json.c uses libndctl symbols, and is included by daxctl. These functions should then get pruned as unused, but on some platforms the toolchain fails to do so. These platforms are ia64, hppa and 32-bit powerpc. It's generally a waste of our time to build ndctl there, but as the lack of a build-dependency requires annoying workarounds higher in the stack, this has been requested in https://bugs.debian.org/914348 -- and is trivially fixable on the ndctl side. Thanks to -Wl,--as-needed there's no harm to architectures where the pruning works, thus let's not bother with detection. As daxctl and libdaxctl are separate, there's no circular dependency. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- daxctl/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)