[gentoo-portage-dev] [PATCH v3] install-qa-check: New QA check/cleanup for empty directories
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 0..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) + + if [[ ${warn_dirs[@]} ]]; then + eqawarn "One or more empty directories installed to /var:" + eqawarn + for d in "${warn_dirs[@]}"; do + eqawarn " ${d#${ED%/}}" + 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 diff --git a/man/make.conf.5 b/man/make.conf.5 index a81b497bd..cb0f00237 100644 --- a/man/make.conf.5 +++ b/man/make.conf.5 @@ -623,6 +623,10 @@ see \fBinstallsources\fR. Have portage react strongly to conditions that have the potential to be dangerous (like missing or incorrect digests for ebuilds). .TP +.B strict-keepdir +Have portage strictly require keepdir calls in ebuilds. Empty +directories installed without explicit keepdir will be removed. +.TP .B stricter Have portage react strongly to conditions that may conflict with system security provisions (for example textrels, executable stack). Read about diff --git a/pym/portage/const.py b/pym/portage/const.py index e5fa4b67c..655be82b1 100644 --- a/pym/portage/const.py +++ b/pym/portage/const.py @@ -184,6 +184,7 @@ SUPPORTED_FEATURES = frozenset([ "split-elog", "split-log", "strict", + "strict-keepdir", "stricter", "suidctl", "test", -- 2.16.1
Re: [gentoo-portage-dev] Re: [PATCH] emerge: add --changed-deps-report option (bug 645780)
On 01/29/2018 07:29 PM, Duncan wrote: > Zac Medico posted on Sun, 28 Jan 2018 22:21:48 -0800 as excerpted: > >> On 01/28/2018 09:49 PM, Zac Medico wrote: 3) Show a NOTE telling users about --changed-deps=y >>> >>> This is in the HINT section, which is displayed if both --changed-deps >>> and --dynamic-deps are disabled in PATCH v2. >> >> Actually, the whole report should be suppressed if either --changed-deps >> or --dynamic-deps is enabled, so I'll send PATCH v4 for that. > > This is shaping up quite nicely and by (1) dramatically shortening the > original "wall of text" report and (2) aborting the report if no affected > packages are in the graph, it's vastly improved from the original. > > I definitely expect it to be rather helpful here, since I have both > --dynamic-deps and --changed-deps off by default, and seeing that list > could be /quite/ helpful. Looking forward to it! =:^) Great! > My remaining concern, and I'm not sure there's a solution, is that for > routine 30-day-plus updaters, the warning could quickly become "routine > noise", due to valid no-r-bump exceptions such as the llvm example mgorny > provided, which very well /could/ happen often enough to trigger the > warning nearly every time for 30-day-plus updaters. Then when it really > counts and could help, people will likely be ignoring it. Until we invent something better, people will have to set EMERGE_DEFAULT_OPTS="--changed-deps-report=n" if it bothers them too much. This is acceptable to me because my main goal is simply to make people aware of --changed-deps when they need it most. If the set --changed-deps-report=n then it's their responsibility to know when to use --changed-deps. > Maybe someone else has an idea, but as I said it's already vastly > improved from the original, and I believe usable as-is, now, while I'd > have found the original quite irritating by about the third time I saw > it, even if also helpful. Great, thanks for the feedback! -- Thanks, Zac signature.asc Description: OpenPGP digital signature
[gentoo-portage-dev] Re: [PATCH] emerge: add --changed-deps-report option (bug 645780)
Zac Medico posted on Sun, 28 Jan 2018 22:21:48 -0800 as excerpted: > On 01/28/2018 09:49 PM, Zac Medico wrote: >>> 3) Show a NOTE telling users about --changed-deps=y >> >> This is in the HINT section, which is displayed if both --changed-deps >> and --dynamic-deps are disabled in PATCH v2. > > Actually, the whole report should be suppressed if either --changed-deps > or --dynamic-deps is enabled, so I'll send PATCH v4 for that. This is shaping up quite nicely and by (1) dramatically shortening the original "wall of text" report and (2) aborting the report if no affected packages are in the graph, it's vastly improved from the original. I definitely expect it to be rather helpful here, since I have both --dynamic-deps and --changed-deps off by default, and seeing that list could be /quite/ helpful. Looking forward to it! =:^) My remaining concern, and I'm not sure there's a solution, is that for routine 30-day-plus updaters, the warning could quickly become "routine noise", due to valid no-r-bump exceptions such as the llvm example mgorny provided, which very well /could/ happen often enough to trigger the warning nearly every time for 30-day-plus updaters. Then when it really counts and could help, people will likely be ignoring it. Maybe someone else has an idea, but as I said it's already vastly improved from the original, and I believe usable as-is, now, while I'd have found the original quite irritating by about the third time I saw it, even if also helpful. -- Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman
Re: [gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack
Dnia 29 stycznia 2018 09:07:47 CET, Michael Haubenwallner napisał(a): >On 01/25/2018 10:11 AM, Michał Górny wrote: >> W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael >> Haubenwallner napisał: >>> Hi, >>> >>> ${Subject} ringing a bell here: >>> >>> dev-db/oracle-instantclient is fetch restricted. As a binary package >with >>> multiple USE options there's a bunch of files to download - even for >>> multiple archs when multilib is active. >>> >>> So in pkg_nofetch() I'm telling the user whether a file to download >is >>> "already here" or "still absent", by testing if $A exists in >$DISTDIR. >>> >>> With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch >too. >>> >> >> You're doing the wrong thing then. DISTDIR is not allowed >> in pkg_nofetch(). > >Is there a supported way to tell the user exactly which files are still >missing? Sounds like a featurereq for Portage itself. > >> Furthermore, you're touching files whose hashes have >> not been verified which is twice wrong. > >Well - does portage actually provide unverified files in the shadow >DISTDIR? No. The phases in which the directory is present are only run if Manifest verification succeeds. > >Thanks! >/haubi/ -- Best regards, Michał Górny (by phone)
[gentoo-portage-dev] Re: [PATCH v2 3/3] _emerge.Ebuild*: delay creating DISTDIR shadow until src_unpack
On 01/25/2018 10:11 AM, Michał Górny wrote: > W dniu czw, 25.01.2018 o godzinie 10∶07 +0100, użytkownik Michael > Haubenwallner napisał: >> Hi, >> >> ${Subject} ringing a bell here: >> >> dev-db/oracle-instantclient is fetch restricted. As a binary package with >> multiple USE options there's a bunch of files to download - even for >> multiple archs when multilib is active. >> >> So in pkg_nofetch() I'm telling the user whether a file to download is >> "already here" or "still absent", by testing if $A exists in $DISTDIR. >> >> With ${Subject}, I'm wondering if DISTDIR is created for pkg_nofetch too. >> > > You're doing the wrong thing then. DISTDIR is not allowed > in pkg_nofetch(). Is there a supported way to tell the user exactly which files are still missing? > Furthermore, you're touching files whose hashes have > not been verified which is twice wrong. Well - does portage actually provide unverified files in the shadow DISTDIR? Thanks! /haubi/