diff mbox

Disable rest of sunrpc, too [V3]

Message ID 20170224142058.GA12862@suse.de
State New
Headers show

Commit Message

Thorsten Kukuk Feb. 24, 2017, 2:20 p.m. UTC
Ok, here is V3 of the patch.

Comments

Zack Weinberg Feb. 24, 2017, 2:31 p.m. UTC | #1
On Fri, Feb 24, 2017 at 9:20 AM, Thorsten Kukuk <kukuk@suse.de> wrote:
>
> Ok, here is V3 of the patch.

LGTM.

Regarding /etc/rpc and (rpc/)netdb.h, does your software (or TIRPC
proper) include an implementation of getrpcby(name|number|ent)?  With
or without nsswitch.conf support?  I see no particular harm in keeping
these around in libc even if we do ever get to being able to *remove*
the rest of Sun RPC, but since they're not of interest to any program
that doesn't use Sun RPC it would be cleaner if we could get rid of
them too.

zw
Thorsten Kukuk Feb. 24, 2017, 2:40 p.m. UTC | #2
On Fri, Feb 24, Zack Weinberg wrote:

> Regarding /etc/rpc and (rpc/)netdb.h, does your software (or TIRPC
> proper) include an implementation of getrpcby(name|number|ent)? 

No, especially not one which is NSS capable. That's why this functions
are still enabled and the header file still installed.
I remembered meanwhile an older discussion, that we wanted to keep that
functions in glibc, since this is no real RPC code but glibc macros.

  Thorsten
Zack Weinberg Feb. 24, 2017, 2:50 p.m. UTC | #3
On Fri, Feb 24, 2017 at 9:40 AM, Thorsten Kukuk <kukuk@suse.de> wrote:
> On Fri, Feb 24, Zack Weinberg wrote:
>
>> Regarding /etc/rpc and (rpc/)netdb.h, does your software (or TIRPC
>> proper) include an implementation of getrpcby(name|number|ent)?
>
> No, especially not one which is NSS capable. That's why this functions
> are still enabled and the header file still installed.
> I remembered meanwhile an older discussion, that we wanted to keep that
> functions in glibc, since this is no real RPC code but glibc macros.

Yeah, I came to the same conclusion myself just now.

Given the history of these patches, I suspect nobody else is going to
react. Please wait another week or two for objections, and if there
aren't any, go ahead and commit them.

zw
Joseph Myers Feb. 24, 2017, 5:07 p.m. UTC | #4
On Fri, 24 Feb 2017, Zack Weinberg wrote:

> or without nsswitch.conf support?  I see no particular harm in keeping
> these around in libc even if we do ever get to being able to *remove*
> the rest of Sun RPC, but since they're not of interest to any program

I hope we can get to removing sunrpc / libnsl *for new architectures / 
ABIs* (that is, for such architectures and ABIs not building the code even 
as compat symbols and not supporting the configure options to enable it, 
so it doesn't form part of the ABI baselines for those configurations).  
For existing architectures / ABIs it needs to stay (as compat symbols) for 
ABI compatibility (in the absence of SONAME changes).
Carlos O'Donell Feb. 24, 2017, 8:38 p.m. UTC | #5
On 02/24/2017 09:20 AM, Thorsten Kukuk wrote:
> 0001-If-sunrpc-code-is-disabled-no-RPC-header-files-rpcge.patch

Thorsten,

High level:

- This looks good and makes sense overall.

- Have you run this through SUSE Tumbleweed to see if anything fails to build?

At the details:

- I'm suggesting a slightly different NEWS entry. Please see below.

- Could you also write up something for the distribution maintainers regarding
  any packaging changes this might cause? Text mentioning which headers are now
  going to _not_ be installed would be great, and how they might fix that if
  possible e.g. install X or port to Y.

  https://sourceware.org/glibc/wiki/Release/2.26#Packaging_Changes
  https://sourceware.org/glibc/wiki/EditorGroup

  You can see example text here:
  https://sourceware.org/glibc/wiki/Release/2.25#Packaging_Changes

> From a1e62f6a31c06cfc9d0c8430b48b0bb0c10b36a6 Mon Sep 17 00:00:00 2001
> From: Thorsten Kukuk <kukuk@thkukuk.de>
> Date: Fri, 24 Feb 2017 15:18:39 +0100
> Subject: [PATCH 1/1] If sunrpc code is disabled, no RPC header files, rpcgen
>  nor librpcsvc.a should be installed, too.
> 
> 	* sunrpc/Makefile: don't build and install header files,
> 	rpcgen and librpcsvc.a by default.
> 
> Signed-off-by: Thorsten Kukuk <kukuk@thkukuk.de>
> ---
>  NEWS            |  5 +++++
>  sunrpc/Makefile | 15 +++++++++------
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index d87e9ce..4eedffa 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -11,6 +11,11 @@ Version 2.26
>    transliteration tables are all updated to Unicode 9.0.0, using
>    generator scripts contributed by Mike FABIAN (Red Hat).
>  
> +* rpcgen, librpcsvc and related headers will now only be built and
> +  installed when glibc is configured with --enable-obsolete-rpc.
> +  Alternative sources can be found at
> +  https://github.com/thkukuk/rpcsvc-proto.

I'm going to suggest the following text:

* The rpcgen, librpcsvc and related headers will only be built and
  installed when glibc is configured with --enable-obsolete-rpc.
  This allows alternative RPC implementations, like TIRPC, to be used
  by default. Applications needing features missing from TIRPC should
  consider the rpcsvc-proto project developed by Thorsten Kukuk (SUSE).

OK with that?

> +
>  Security related changes:
>  
>    [Add security related changes here]
> diff --git a/sunrpc/Makefile b/sunrpc/Makefile
> index 0c1e612..305bee6 100644
> --- a/sunrpc/Makefile
> +++ b/sunrpc/Makefile
> @@ -48,11 +48,10 @@ include ../Makeconfig
>  rpcsvc = bootparam_prot.x nlm_prot.x rstat.x \
>  	 yppasswd.x klm_prot.x rex.x sm_inter.x mount.x \
>  	 rusers.x spray.x nfs_prot.x rquota.x key_prot.x
> -headers-in-tirpc = $(addprefix rpc/,auth.h auth_unix.h clnt.h pmap_clnt.h \
> -				    pmap_prot.h pmap_rmt.h rpc.h rpc_msg.h \
> -				    svc.h svc_auth.h types.h xdr.h auth_des.h \
> -				    des_crypt.h)
> -headers-not-in-tirpc = $(addprefix rpc/,key_prot.h rpc_des.h) \

OK.

> +headers-sunrpc = $(addprefix rpc/,auth.h auth_unix.h clnt.h pmap_clnt.h \
> +				  pmap_prot.h pmap_rmt.h rpc.h rpc_msg.h \
> +				  svc.h svc_auth.h types.h xdr.h auth_des.h \
> +				  des_crypt.h key_prot.h rpc_des.h) \

OK.

>  		       $(rpcsvc:%=rpcsvc/%) rpcsvc/bootparam.h
>  headers = rpc/netdb.h
>  install-others = $(inst_sysconfdir)/rpc
> @@ -61,7 +60,7 @@ generated += $(rpcsvc:%.x=rpcsvc/%.h) $(rpcsvc:%.x=x%.c) $(rpcsvc:%.x=x%.stmp) \
>  generated-dirs += rpcsvc
>  
>  ifeq ($(link-obsolete-rpc),yes)
> -headers += $(headers-in-tirpc) $(headers-not-in-tirpc)
> +headers += $(headers-sunrpc)

OK.

>  endif
>  
>  ifeq ($(build-shared),yes)
> @@ -86,12 +85,14 @@ shared-only-routines = $(routines)
>  endif
>  endif
>  
> +ifeq ($(link-obsolete-rpc),yes)
>  install-bin := rpcgen
>  rpcgen-objs = rpc_main.o rpc_hout.o rpc_cout.o rpc_parse.o \
>  	      rpc_scan.o rpc_util.o rpc_svcout.o rpc_clntout.o \
>  	      rpc_tblout.o rpc_sample.o
>  extra-objs = $(rpcgen-objs) $(addprefix cross-,$(rpcgen-objs))
>  others += rpcgen
> +endif
>  

OK.

>  tests = tst-xdrmem tst-xdrmem2 test-rpcent
>  xtests := tst-getmyaddr
> @@ -105,12 +106,14 @@ rpcgen-tests := $(objpfx)bug20790.out
>  tests-special += $(rpcgen-tests)
>  endif
>  
> +ifeq ($(link-obsolete-rpc),yes)
>  headers += $(rpcsvc:%.x=rpcsvc/%.h)
>  extra-libs := librpcsvc
>  extra-libs-others := librpcsvc # Make it in `others' pass, not `lib' pass.
>  librpcsvc-routines = $(rpcsvc:%.x=x%)
>  librpcsvc-inhibit-o = .os # Build no shared rpcsvc library.
>  omit-deps = $(librpcsvc-routines)
> +endif

OK.

>  
>  ifeq (yes,$(build-shared))
>  rpc-compat-routines = $(addprefix compat-,$(need-export-routines))
> -- 1.8.5.6
Thorsten Kukuk Feb. 27, 2017, 9:41 a.m. UTC | #6
Hi,

On Fri, Feb 24, Carlos O'Donell wrote:

> On 02/24/2017 09:20 AM, Thorsten Kukuk wrote:
> > 0001-If-sunrpc-code-is-disabled-no-RPC-header-files-rpcge.patch

> - Have you run this through SUSE Tumbleweed to see if anything fails to build?

That's part of a SUSE Tumbleweed project, yes. Nothing is using rpcsvc,
neither the header files nor the static library (everything else would
have surprised me).
One package is using rpcgen, a new buildrequires was necessary. That's
all.
Of course only if you run it in a project already converted from sunrpc
to libtirpc.

> - Could you also write up something for the distribution maintainers regarding
>   any packaging changes this might cause? Text mentioning which headers are now
>   going to _not_ be installed would be great, and how they might fix that if
>   possible e.g. install X or port to Y.

I really doubt that package maintainers will see any affect of this.
I haven't seen anything except one package out of 3000 calling rpcgen.
I think the rpcsvc code is dead code since day one.

> > +* rpcgen, librpcsvc and related headers will now only be built and
> > +  installed when glibc is configured with --enable-obsolete-rpc.
> > +  Alternative sources can be found at
> > +  https://github.com/thkukuk/rpcsvc-proto.
> 
> I'm going to suggest the following text:
> 
> * The rpcgen, librpcsvc and related headers will only be built and
>   installed when glibc is configured with --enable-obsolete-rpc.
>   This allows alternative RPC implementations, like TIRPC, to be used
>   by default. Applications needing features missing from TIRPC should
>   consider the rpcsvc-proto project developed by Thorsten Kukuk (SUSE).
> 
> OK with that?

Fine with me.

  Thorsten
Stefan Liebler March 13, 2017, 8:49 a.m. UTC | #7
On 02/27/2017 10:41 AM, Thorsten Kukuk wrote:
>
> Hi,
>
> On Fri, Feb 24, Carlos O'Donell wrote:
>
>> On 02/24/2017 09:20 AM, Thorsten Kukuk wrote:
>>> 0001-If-sunrpc-code-is-disabled-no-RPC-header-files-rpcge.patch
>
>> - Have you run this through SUSE Tumbleweed to see if anything fails to build?
>
> That's part of a SUSE Tumbleweed project, yes. Nothing is using rpcsvc,
> neither the header files nor the static library (everything else would
> have surprised me).
> One package is using rpcgen, a new buildrequires was necessary. That's
> all.
> Of course only if you run it in a project already converted from sunrpc
> to libtirpc.
>
>> - Could you also write up something for the distribution maintainers regarding
>>   any packaging changes this might cause? Text mentioning which headers are now
>>   going to _not_ be installed would be great, and how they might fix that if
>>   possible e.g. install X or port to Y.
>
> I really doubt that package maintainers will see any affect of this.
> I haven't seen anything except one package out of 3000 calling rpcgen.
> I think the rpcsvc code is dead code since day one.
>
>>> +* rpcgen, librpcsvc and related headers will now only be built and
>>> +  installed when glibc is configured with --enable-obsolete-rpc.
>>> +  Alternative sources can be found at
>>> +  https://github.com/thkukuk/rpcsvc-proto.
>>
>> I'm going to suggest the following text:
>>
>> * The rpcgen, librpcsvc and related headers will only be built and
>>   installed when glibc is configured with --enable-obsolete-rpc.
>>   This allows alternative RPC implementations, like TIRPC, to be used
>>   by default. Applications needing features missing from TIRPC should
>>   consider the rpcsvc-proto project developed by Thorsten Kukuk (SUSE).
>>
>> OK with that?
>
> Fine with me.
>
>   Thorsten
>

Hi Thorsten,

I get a testsuite fail:
FAIL: sunrpc/bug20790

This is a "rpcgen-test". As rpcgen isn't available anymore, the test fails:
<build>/sunrpc/rpcgen -c bug20790.x -o <build>/sunrpc/bug20790.out

Can you disable those tests if rpcgen isn't built?

Bye
Stefan
Thorsten Kukuk March 13, 2017, 4:29 p.m. UTC | #8
On Mon, Mar 13, Stefan Liebler wrote:

> Hi Thorsten,
> 
> I get a testsuite fail:
> FAIL: sunrpc/bug20790
> 
> This is a "rpcgen-test". As rpcgen isn't available anymore, the test fails:
> <build>/sunrpc/rpcgen -c bug20790.x -o <build>/sunrpc/bug20790.out
> 
> Can you disable those tests if rpcgen isn't built?

Thanks for spotting this, fixed.

  Thorsten
Joseph Myers March 13, 2017, 4:40 p.m. UTC | #9
On Mon, 13 Mar 2017, Thorsten Kukuk wrote:

> On Mon, Mar 13, Stefan Liebler wrote:
> 
> > Hi Thorsten,
> > 
> > I get a testsuite fail:
> > FAIL: sunrpc/bug20790
> > 
> > This is a "rpcgen-test". As rpcgen isn't available anymore, the test fails:
> > <build>/sunrpc/rpcgen -c bug20790.x -o <build>/sunrpc/bug20790.out
> > 
> > Can you disable those tests if rpcgen isn't built?
> 
> Thanks for spotting this, fixed.

ChangeLog entries are needed for both these commits.
diff mbox

Patch

From a1e62f6a31c06cfc9d0c8430b48b0bb0c10b36a6 Mon Sep 17 00:00:00 2001
From: Thorsten Kukuk <kukuk@thkukuk.de>
Date: Fri, 24 Feb 2017 15:18:39 +0100
Subject: [PATCH 1/1] If sunrpc code is disabled, no RPC header files, rpcgen
 nor librpcsvc.a should be installed, too.

	* sunrpc/Makefile: don't build and install header files,
	rpcgen and librpcsvc.a by default.

Signed-off-by: Thorsten Kukuk <kukuk@thkukuk.de>
---
 NEWS            |  5 +++++
 sunrpc/Makefile | 15 +++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index d87e9ce..4eedffa 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,11 @@  Version 2.26
   transliteration tables are all updated to Unicode 9.0.0, using
   generator scripts contributed by Mike FABIAN (Red Hat).
 
+* rpcgen, librpcsvc and related headers will now only be built and
+  installed when glibc is configured with --enable-obsolete-rpc.
+  Alternative sources can be found at
+  https://github.com/thkukuk/rpcsvc-proto.
+
 Security related changes:
 
   [Add security related changes here]
diff --git a/sunrpc/Makefile b/sunrpc/Makefile
index 0c1e612..305bee6 100644
--- a/sunrpc/Makefile
+++ b/sunrpc/Makefile
@@ -48,11 +48,10 @@  include ../Makeconfig
 rpcsvc = bootparam_prot.x nlm_prot.x rstat.x \
 	 yppasswd.x klm_prot.x rex.x sm_inter.x mount.x \
 	 rusers.x spray.x nfs_prot.x rquota.x key_prot.x
-headers-in-tirpc = $(addprefix rpc/,auth.h auth_unix.h clnt.h pmap_clnt.h \
-				    pmap_prot.h pmap_rmt.h rpc.h rpc_msg.h \
-				    svc.h svc_auth.h types.h xdr.h auth_des.h \
-				    des_crypt.h)
-headers-not-in-tirpc = $(addprefix rpc/,key_prot.h rpc_des.h) \
+headers-sunrpc = $(addprefix rpc/,auth.h auth_unix.h clnt.h pmap_clnt.h \
+				  pmap_prot.h pmap_rmt.h rpc.h rpc_msg.h \
+				  svc.h svc_auth.h types.h xdr.h auth_des.h \
+				  des_crypt.h key_prot.h rpc_des.h) \
 		       $(rpcsvc:%=rpcsvc/%) rpcsvc/bootparam.h
 headers = rpc/netdb.h
 install-others = $(inst_sysconfdir)/rpc
@@ -61,7 +60,7 @@  generated += $(rpcsvc:%.x=rpcsvc/%.h) $(rpcsvc:%.x=x%.c) $(rpcsvc:%.x=x%.stmp) \
 generated-dirs += rpcsvc
 
 ifeq ($(link-obsolete-rpc),yes)
-headers += $(headers-in-tirpc) $(headers-not-in-tirpc)
+headers += $(headers-sunrpc)
 endif
 
 ifeq ($(build-shared),yes)
@@ -86,12 +85,14 @@  shared-only-routines = $(routines)
 endif
 endif
 
+ifeq ($(link-obsolete-rpc),yes)
 install-bin := rpcgen
 rpcgen-objs = rpc_main.o rpc_hout.o rpc_cout.o rpc_parse.o \
 	      rpc_scan.o rpc_util.o rpc_svcout.o rpc_clntout.o \
 	      rpc_tblout.o rpc_sample.o
 extra-objs = $(rpcgen-objs) $(addprefix cross-,$(rpcgen-objs))
 others += rpcgen
+endif
 
 tests = tst-xdrmem tst-xdrmem2 test-rpcent
 xtests := tst-getmyaddr
@@ -105,12 +106,14 @@  rpcgen-tests := $(objpfx)bug20790.out
 tests-special += $(rpcgen-tests)
 endif
 
+ifeq ($(link-obsolete-rpc),yes)
 headers += $(rpcsvc:%.x=rpcsvc/%.h)
 extra-libs := librpcsvc
 extra-libs-others := librpcsvc # Make it in `others' pass, not `lib' pass.
 librpcsvc-routines = $(rpcsvc:%.x=x%)
 librpcsvc-inhibit-o = .os # Build no shared rpcsvc library.
 omit-deps = $(librpcsvc-routines)
+endif
 
 ifeq (yes,$(build-shared))
 rpc-compat-routines = $(addprefix compat-,$(need-export-routines))
-- 
1.8.5.6