[gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[Mike Gilbert]

Closes: https://bugs.gentoo.org/691478
---
 eclass/acct-user.eclass | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/eclass/acct-user.eclass b/eclass/acct-user.eclass
index 60009643c144..077f85417ce8 100644
--- a/eclass/acct-user.eclass
+++ b/eclass/acct-user.eclass
@@ -334,8 +334,12 @@ acct-user_pkg_preinst() {
 		if [[ -z ${ACCT_USER_HOME_OWNER} ]]; then
 			ACCT_USER_HOME_OWNER=${ACCT_USER_NAME}:${ACCT_USER_GROUPS[0]}
 		fi
-		fowners "${ACCT_USER_HOME_OWNER}" "${ACCT_USER_HOME}"
-		fperms "${ACCT_USER_HOME_PERMS}" "${ACCT_USER_HOME}"
+		# Path might be missing due to INSTALL_MASK, etc.
+		# https://bugs.gentoo.org/691478
+		if [[ -e "${ED}/${ACCT_USER_HOME#/}" ]]; then
+			fowners "${ACCT_USER_HOME_OWNER}" "${ACCT_USER_HOME}"
+			fperms "${ACCT_USER_HOME_PERMS}" "${ACCT_USER_HOME}"
+		fi
 	fi
 }
 
-- 
2.23.0.rc1

Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[Michał Górny]

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

On Wed, 2019-08-14 at 19:54 -0400, Mike Gilbert wrote:
> Closes: https://bugs.gentoo.org/691478
> ---
>  eclass/acct-user.eclass | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/eclass/acct-user.eclass b/eclass/acct-user.eclass
> index 60009643c144..077f85417ce8 100644
> --- a/eclass/acct-user.eclass
> +++ b/eclass/acct-user.eclass
> @@ -334,8 +334,12 @@ acct-user_pkg_preinst() {
>  		if [[ -z ${ACCT_USER_HOME_OWNER} ]]; then
>  			ACCT_USER_HOME_OWNER=${ACCT_USER_NAME}:${ACCT_USER_GROUPS[0]}
>  		fi
> -		fowners "${ACCT_USER_HOME_OWNER}" "${ACCT_USER_HOME}"
> -		fperms "${ACCT_USER_HOME_PERMS}" "${ACCT_USER_HOME}"
> +		# Path might be missing due to INSTALL_MASK, etc.
> +		# https://bugs.gentoo.org/691478
> +		if [[ -e "${ED}/${ACCT_USER_HOME#/}" ]]; then
> +			fowners "${ACCT_USER_HOME_OWNER}" "${ACCT_USER_HOME}"
> +			fperms "${ACCT_USER_HOME_PERMS}" "${ACCT_USER_HOME}"
> +		fi
>  	fi
>  }
>  

LGTM.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[Ulrich Mueller]

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

>>>>> On Thu, 15 Aug 2019, Mike Gilbert wrote:

> +		# Path might be missing due to INSTALL_MASK, etc.
> +		# https://bugs.gentoo.org/691478
> +		if [[ -e "${ED}/${ACCT_USER_HOME#/}" ]]; then
> +			fowners "${ACCT_USER_HOME_OWNER}" "${ACCT_USER_HOME}"
> +			fperms "${ACCT_USER_HOME_PERMS}" "${ACCT_USER_HOME}"
> +		fi

IIUC, this would result in an entry for the directory in /etc/passwd,
but the directory wouldn't exist on the system?

I don't think that's a sane situation, so maybe the eclass should just
die here? (Basically, there are two possibilities: Either, things will
break if the dir is missing, then dying might be the best option.
Or, everything works without the dir, then the ebuild should set it to
/dev/null, in the first place.)

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[Michael Orlitzky]

On 8/15/19 3:19 AM, Ulrich Mueller wrote:
> 
> I don't think that's a sane situation, so maybe the eclass should just
> die here? (Basically, there are two possibilities: Either, things will
> break if the dir is missing, then dying might be the best option.
> Or, everything works without the dir, then the ebuild should set it to
> /dev/null, in the first place.)
> 

That's my feeling as well. In 100% of cases so far, this has been a
useful QA tool. Can we wait until that's not the case to change it?

Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[Mike Gilbert]

On Thu, Aug 15, 2019 at 8:33 AM Michael Orlitzky <mjo@gentoo.org> wrote:
>
> On 8/15/19 3:19 AM, Ulrich Mueller wrote:
> >
> > I don't think that's a sane situation, so maybe the eclass should just
> > die here? (Basically, there are two possibilities: Either, things will
> > break if the dir is missing, then dying might be the best option.
> > Or, everything works without the dir, then the ebuild should set it to
> > /dev/null, in the first place.)
> >
>
> That's my feeling as well. In 100% of cases so far, this has been a
> useful QA tool. Can we wait until that's not the case to change it?

I think an explicit die with a useful error message for the user would
be better than the status-quo.

Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst

[Jaco Kroon]

Hi,

On 2019/08/15 16:47, Mike Gilbert wrote:

> On Thu, Aug 15, 2019 at 8:33 AM Michael Orlitzky <mjo@gentoo.org> wrote:
>> On 8/15/19 3:19 AM, Ulrich Mueller wrote:
>>> I don't think that's a sane situation, so maybe the eclass should just
>>> die here? (Basically, there are two possibilities: Either, things will
>>> break if the dir is missing, then dying might be the best option.
>>> Or, everything works without the dir, then the ebuild should set it to
>>> /dev/null, in the first place.)
>>>
>> That's my feeling as well. In 100% of cases so far, this has been a
>> useful QA tool. Can we wait until that's not the case to change it?
> I think an explicit die with a useful error message for the user would
> be better than the status-quo.

I agree.


Kind Regards,
Jaco