[2.24!] Don't install the internal header grp-merge.h.

Message ID 1468528111-4565-1-git-send-email-zackw@panix.com
State Committed, archived
Headers

Commit Message

Zack Weinberg July 14, 2016, 8:28 p.m. UTC
  grp-merge.h was introduced in Stephen Gallagher's patch adding the
"group merging" feature to NSS.  It declares two functions, __copy_grp
and __merge_grp, both of which are tagged 'internal_function', which
means that nobody can even compile the contents of the header without
access to libc-symbols.h, which is not installed.  (Also, these
functions are GLIBC_PRIVATE exports from libc.so.)  Hence I believe
grp-merge.h should not be installed either.

This really needs to be in 2.24, so that no released version of the
library installs this header.

I hope that what I did to the ChangeLog diff will allow it to be
applied without hassle.

zw

---
 ChangeLog    | 4 ++++
 grp/Makefile | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)
  

Comments

Stephen Gallagher July 15, 2016, 1:41 a.m. UTC | #1
> On Jul 14, 2016, at 4:28 PM, Zack Weinberg <zackw@panix.com> wrote:
> 
> grp-merge.h was introduced in Stephen Gallagher's patch adding the
> "group merging" feature to NSS.  It declares two functions, __copy_grp
> and __merge_grp, both of which are tagged 'internal_function', which
> means that nobody can even compile the contents of the header without
> access to libc-symbols.h, which is not installed.  (Also, these
> functions are GLIBC_PRIVATE exports from libc.so.)  Hence I believe
> grp-merge.h should not be installed either.
> 
> This really needs to be in 2.24, so that no released version of the
> library installs this header.
> 
> I hope that what I did to the ChangeLog diff will allow it to be
> applied without hassle.
> 
> zw

Sorry, this is indeed a purely internal header and should not be installed. My apologies. Thanks for the patch!

One question: will this change cause the header to disappear from the tarball? Because we do need it for building.


> 
> ---
> ChangeLog    | 4 ++++
> grp/Makefile | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 234a3cc..5655319 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,0 +1,4 @@
> +2016-07-14  Zack Weinberg  <zackw@panix.com>
> +
> +    * grp/Makefile: Don't install the internal header grp-merge.h.
> +
> diff --git a/grp/Makefile b/grp/Makefile
> index b4d52e2..3807bfa 100644
> --- a/grp/Makefile
> +++ b/grp/Makefile
> @@ -22,7 +22,7 @@ subdir    := grp
> 
> include ../Makeconfig
> 
> -headers := grp.h grp-merge.h
> +headers := grp.h
> 
> routines := fgetgrent initgroups setgroups \
>        getgrent getgrgid getgrnam putgrent \
> -- 
> 2.8.1
>
  
Zack Weinberg July 15, 2016, 11:06 a.m. UTC | #2
On Thu, Jul 14, 2016 at 9:41 PM, Stephen Gallagher <sgallagh@redhat.com> wrote:
>> On Jul 14, 2016, at 4:28 PM, Zack Weinberg <zackw@panix.com> wrote:
...
>> I believe grp-merge.h should not be installed either.
>>
>> This really needs to be in 2.24, so that no released version of the
>> library installs this header.
>
> Sorry, this is indeed a purely internal header and should not be installed.
> My apologies. Thanks for the patch!

You're welcome.  I'm going to need someone to check it in for me.

> One question: will this change cause the header to disappear from the
> tarball? Because we do need it for building.

'make dist' just runs 'git archive', and it's in there:

$ tar tzf ../glibc/glibc-2.23-565-gdbc9ea5.tar.gz | grep grp-merge
glibc-2.23-565-gdbc9ea5/grp/grp-merge.c
glibc-2.23-565-gdbc9ea5/grp/grp-merge.h
glibc-2.23-565-gdbc9ea5/include/grp-merge.h

If this is not the proper way of generating tarballs then I don't know
how to do it.

zw
  
Adhemerval Zanella Netto July 18, 2016, 12:28 p.m. UTC | #3
On 14/07/2016 22:41, Stephen Gallagher wrote:
> 
> 
>> On Jul 14, 2016, at 4:28 PM, Zack Weinberg <zackw@panix.com> wrote:
>>
>> grp-merge.h was introduced in Stephen Gallagher's patch adding the
>> "group merging" feature to NSS.  It declares two functions, __copy_grp
>> and __merge_grp, both of which are tagged 'internal_function', which
>> means that nobody can even compile the contents of the header without
>> access to libc-symbols.h, which is not installed.  (Also, these
>> functions are GLIBC_PRIVATE exports from libc.so.)  Hence I believe
>> grp-merge.h should not be installed either.
>>
>> This really needs to be in 2.24, so that no released version of the
>> library installs this header.
>>
>> I hope that what I did to the ChangeLog diff will allow it to be
>> applied without hassle.
>>
>> zw
> 
> Sorry, this is indeed a purely internal header and should not be installed. My apologies. Thanks for the patch!

Right, I think this patch should be included in 2.24.  I will commit
it today.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 234a3cc..5655319 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,0 +1,4 @@ 
+2016-07-14  Zack Weinberg  <zackw@panix.com>
+
+	* grp/Makefile: Don't install the internal header grp-merge.h.
+
diff --git a/grp/Makefile b/grp/Makefile
index b4d52e2..3807bfa 100644
--- a/grp/Makefile
+++ b/grp/Makefile
@@ -22,7 +22,7 @@  subdir	:= grp
 
 include ../Makeconfig
 
-headers := grp.h grp-merge.h
+headers := grp.h
 
 routines := fgetgrent initgroups setgroups \
 	    getgrent getgrgid getgrnam putgrent \