[PATCH] Set also the group of installed files

[PATCH] Set also the group of installed files

[Ulrich Mueller]

Currently the files in the lisp and etc directories are being
installed using a pair of tars, followed by a recursive chown.
This however adjusts only the user, but not the group ownership.

When building Emacs in a sandboxed environment (as it is the case
for Gentoo), the installed files will afterwards belong to root but
to the group of the build user. Note that this also doesn't match the
behaviour of install(1) which sets both user and group ownership.

See patch included below.

Ulrich


From 851904d043ce268878444140df7eec8ff5de9a17 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Thu, 21 Jun 2018 01:27:44 +0200
Subject: [PATCH] Set also the group of installed files

* Makefile.in (set_installuser): Also set the group, in order
to match install(1) behavior.
---
 Makefile.in | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile.in b/Makefile.in
index 52d44d9708..d659d7a56f 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -518,7 +518,10 @@ INSTALL_ARCH_INDEP_EXTRA = @INSTALL_ARCH_INDEP_EXTRA@
 set_installuser=for installuser in $${LOGNAME} $${USERNAME} $${USER} \
 	  `id -un 2> /dev/null`; do \
 	  [ -n "$${installuser}" ] && break ; \
-	done
+	done; \
+	installgroup=$$(id -gn 2>/dev/null); \
+	[ -n "$${installgroup}" ] && \
+	  installuser="$${installuser}:$${installgroup}"
 
 ### Install the files that are machine-independent.
 ### Most of them come straight from the distribution; the exception is
-- 
2.17.1

Re: [PATCH] Set also the group of installed files

[Eli Zaretskii]

> Date: Thu, 21 Jun 2018 16:31:34 +0200
> From: Ulrich Mueller <ulm@gentoo.org>
> 
> diff --git a/Makefile.in b/Makefile.in
> index 52d44d9708..d659d7a56f 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -518,7 +518,10 @@ INSTALL_ARCH_INDEP_EXTRA = @INSTALL_ARCH_INDEP_EXTRA@
>  set_installuser=for installuser in $${LOGNAME} $${USERNAME} $${USER} \
>  	  `id -un 2> /dev/null`; do \
>  	  [ -n "$${installuser}" ] && break ; \
> -	done
> +	done; \
> +	installgroup=$$(id -gn 2>/dev/null); \
> +	[ -n "$${installgroup}" ] && \
> +	  installuser="$${installuser}:$${installgroup}"

Is 'id' sufficiently portable to use it here?

Thanks.

Re: [PATCH] Set also the group of installed files

[Ulrich Mueller]

>>>>> On Thu, 21 Jun 2018, Eli Zaretskii wrote:

>> --- a/Makefile.in
>> +++ b/Makefile.in
>> @@ -518,7 +518,10 @@ INSTALL_ARCH_INDEP_EXTRA = @INSTALL_ARCH_INDEP_EXTRA@
>> set_installuser=for installuser in $${LOGNAME} $${USERNAME} $${USER} \
>> `id -un 2> /dev/null`; do \
>> [ -n "$${installuser}" ] && break ; \
>> -	done
>> +	done; \
>> +	installgroup=$$(id -gn 2>/dev/null); \
>> +	[ -n "$${installgroup}" ] && \
>> +	  installuser="$${installuser}:$${installgroup}"

> Is 'id' sufficiently portable to use it here?

The -g and -n options are specified by POSIX. I have only tested it on
GNU/Linux and FreeBSD though.

Also it should fall back to the current behaviour if "id" errors
out for some reason.

Re: [PATCH] Set also the group of installed files

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On 06/21/2018 08:27 AM, Ulrich Mueller wrote:
> Also it should fall back to the current behaviour if "id" errors
> out for some reason.

Yes, that sort of thing should work. Thanks for the patch. I tweaked it 
a bit to make it more robust if 'id' is missing or exits with nonzero 
status or if the shell doesn't grok $(...) or the installation process 
is running without a password or group entry so the -n option fails (!), 
and installed the attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-group-when-installing-too.patch --]
[-- Type: text/x-patch; name="0001-Set-group-when-installing-too.patch", Size: 1399 bytes --]

From 6b8a9891a197512b76abd04e80f444b6885d2943 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 21 Jun 2018 13:29:15 -0700
Subject: [PATCH] Set group when installing, too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From a patch by Ulrich Mueller in:
https://lists.gnu.org/r/emacs-devel/2018-06/msg00687.html
* Makefile.in (set_installuser): Also set the group, in order
to match install(1) behavior.  Also, don’t clutter stderr
with a diagnostic if ‘id’ is missing.
---
 Makefile.in | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 52d44d9708..4d7627ba09 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -516,9 +516,11 @@ INSTALL_ARCH_INDEP_EXTRA =
 ## https://lists.gnu.org/r/emacs-devel/2007-10/msg01672.html
 ## Needs to be the user running install, so configure can't set it.
 set_installuser=for installuser in $${LOGNAME} $${USERNAME} $${USER} \
-	  `id -un 2> /dev/null`; do \
+	  `(id -u) 2> /dev/null`; do \
 	  [ -n "$${installuser}" ] && break ; \
-	done
+	done; \
+	installgroup=`(id -g) 2>/dev/null` && [ -n "$$installgroup" ] && \
+	  installuser=$$installuser:$$installgroup
 
 ### Install the files that are machine-independent.
 ### Most of them come straight from the distribution; the exception is
-- 
2.17.1