Re: [gentoo-portage-dev] [PATCH v2] install-qa-check: New QA check/cleanup for empty directories

[Zac Medico]

On 01/28/2018 01:50 AM, Michał Górny wrote:
> Warn about empty directories installed to /var in install-qa-check phase
> (that were not "filled" using keepdir), to help developers stop relying
> upon Portage preserving them. Those directories are rather unlikely to
> be false positives.
>
> Furthermore, remove all the empty directories if FEATURES=strict-keepdir
> is used to catch even more problems (intended for developers). Here
> warnings are not really suitable since there will be a high number
> of false positives.
>
> The PMS specifies the behavior upon merging empty directories
> as undefined, and specifically prohibits ebuilds from attempting
> to install empty directories. However, ebuilds occasionally still fall
> into the trap of relying on 'dodir' preserving the directory. Make
> the Portage behavior more strict in order to prevent that.
> ---
>  bin/install-qa-check.d/95empty-dirs | 43 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 bin/install-qa-check.d/95empty-dirs
>
> diff --git a/bin/install-qa-check.d/95empty-dirs b/bin/install-qa-check.d/95empty-dirs
> new file mode 100644
> index 000000000..a9cda7fe1
> --- /dev/null
> +++ b/bin/install-qa-check.d/95empty-dirs
> @@ -0,0 +1,43 @@
> +# Warn about and/or remove empty directories installed by ebuild.
> +
> +# Rationale: PMS prohibits ebuilds from installing empty directories.
> +# Cleaning them up from the installation image provides an easy way
> +# to make sure that ebuilds are not relying on it while making it easy
> +# for users to override this if they need to.
> +#
> +# The ebuilds that need to preserve empty directories should use keepdir
> +# as documented e.g.:
> +# https://devmanual.gentoo.org/function-reference/install-functions/index.html
> +#
> +# For now, we emit QA warnings for empty directories in /var.
> +# Additionally, if FEATURES=strict-keepdir is enabled we explicitly
> +# remove *all* empty directories to trigger breakage.
> +
> +find_empty_dirs() {
> +	local warn_dirs=()
> +	local d strip=
> +
> +	[[ ${FEATURES} == *strict-keepdir* ]] && strip=1

strict-keepdir has to be added to portage.const.SUPPORTED_FEATURES.
Maybe also document it in man/make.conf.5?

> +
> +	while IFS= read -r -d $'\0' d; do
> +		warn_dirs+=( "${d}" )
> +	done < <(find "${ED}" -mindepth 1 -type d -empty -print0)
> +
> +	if [[ ${warn_dirs[@]} ]]; then
> +		eqawarn "One or more empty directories installed to /var:"
> +		eqawarn
> +		for d in "${warn_dirs[@]}"; do
> +			eqawarn "  ${d#${ED%/}}"
> +			[[ ${strip} ]] && rmdir "${d}"
Do you also want the strip logic to traverse upwards, removing
directories that contain only empty directories?

> +		done
> +		eqawarn
> +		eqawarn "If those directories need to be preserved, please make sure to create"
> +		eqawarn "or mark them for keeping using 'keepdir'. Future versions of Portage"
> +		eqawarn "will strip empty directories from installation image."
> +	fi
> +}
> +
> +find_empty_dirs
> +: # guarantee successful exit
> +
> +# vim:ft=sh
>


-- 
Thanks,
Zac