Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst
Hi, On 2019/08/15 16:47, Mike Gilbert wrote: > On Thu, Aug 15, 2019 at 8:33 AM Michael Orlitzky 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
Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst
On Thu, Aug 15, 2019 at 8:33 AM Michael Orlitzky 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
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
> 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
signature.asc
Description: PGP signature
Re: [gentoo-dev] [PATCH] acct-user.eclass: ignore missing directory in preinst
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
signature.asc
Description: This is a digitally signed message part
