daxctl: link against libndctl, in case its use doesn't get pruned
diff mbox series

Message ID 20190801010044.56927-1-kilobyte@angband.pl
State New
Headers show
Series
  • daxctl: link against libndctl, in case its use doesn't get pruned
Related show

Commit Message

Adam Borowski Aug. 1, 2019, 1 a.m. UTC
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(-)

Comments

Verma, Vishal L Aug. 2, 2019, 5:15 p.m. UTC | #1
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
Adam Borowski Aug. 2, 2019, 6:36 p.m. UTC | #2
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?


喵!

Patch
diff mbox series

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