public inbox for linux-nvdimm@lists.01.org
 help / color / mirror / Atom feed
From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	"kilobyte@angband.pl" <kilobyte@angband.pl>
Subject: Re: [PATCH] daxctl: link against libndctl, in case its use doesn't get pruned
Date: Fri, 2 Aug 2019 17:15:21 +0000	[thread overview]
Message-ID: <47f7555596eae0f7daef0d6a0a9ce0bae96f6af4.camel@intel.com> (raw)
In-Reply-To: <20190801010044.56927-1-kilobyte@angband.pl>

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

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

  reply	other threads:[~2019-08-02 17:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-01  1:00 [PATCH] daxctl: link against libndctl, in case its use doesn't get pruned Adam Borowski
2019-08-02 17:15 ` Verma, Vishal L [this message]
2019-08-02 18:36   ` Adam Borowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47f7555596eae0f7daef0d6a0a9ce0bae96f6af4.camel@intel.com \
    --to=vishal.l.verma@intel.com \
    --cc=kilobyte@angband.pl \
    --cc=linux-nvdimm@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox