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

[Michał Górny]

W dniu wto, 30.01.2018 o godzinie 10∶39 -0800, użytkownik Zac Medico
napisał:
> On 01/30/2018 10:18 AM, Zac Medico wrote:
> > On 01/29/2018 11:23 PM, 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 | 42 +++++++++++++++++++++++++++++++++++++
> > >  man/make.conf.5                     |  4 ++++
> > >  pym/portage/const.py                |  1 +
> > >  3 files changed, 47 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..0d06b278d
> > > --- /dev/null
> > > +++ b/bin/install-qa-check.d/95empty-dirs
> > > @@ -0,0 +1,42 @@
> > > +# 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 striparg=
> > > +
> > > +	[[ ${FEATURES} == *strict-keepdir* ]] && striparg=-delete
> > > +
> > > +	while IFS= read -r -d $'\0' d; do
> > > +		[[ ${d} == ${ED%/}/var/* ]] && warn_dirs+=( "${d}" )
> > > +	done < <(find "${ED}" -depth -mindepth 1 -type d -empty -print0 ${striparg} | sort -z)
> >
> > Are you sure that this sort call is guaranteed to produce the correct
> > order? Comparison of '-' characters with '/' characters can lead to odd
> > results like this:
> >
> > $ printf 'foo/bar\nfoo-bar/baz\nfoo/bar/baz\n' | sort
> > foo/bar
> > foo-bar/baz
> > foo/bar/baz
>
> The sort is only for display purposes, maybe use LC_ALL=C for locale
> independence?

That's what I wanted to suggest. Well, LC_COLLATE=C should be enough.

-- 
Best regards,
Michał Górny