Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: Add a QA check for installing xattrs
On Mon, 2021-09-27 at 21:09 -0400, Mike Gilbert wrote: > On Mon, Sep 27, 2021 at 1:20 PM Michał Górny wrote: > > + eqawarn > > + eqawarn "It is impossible to reliably guarantee that the > > extended attributes" > > + eqawarn "will be reliably preserved while merging. Please > > ensure that any" > > + eqawarn "extended metadata necessary is applied in > > pkg_postinst() phase," > > + eqawarn "and that the implementation includes a fallback if > > necessary." > > This message suggests that applying xattrs in pkg_postinst is > acceptable. However, your patch offers no way to disable the QA > warning for ebuilds that do so. We'll cross that bridge when we get there. Ideally, we wouldn't need to silence the check because no packages would do that. If they do, then we'll probably want to work on an eclass like fcaps.eclas. -- Best regards, Michał Górny
Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: Add a QA check for installing xattrs
On Mon, 2021-09-27 at 21:03 -0400, Mike Gilbert wrote: > On Mon, Sep 27, 2021 at 1:20 PM Michał Górny wrote: > > > > Warn the developers if ebuilds install files with xattrs to ${ED}. > > The xattrs may or may not be preserved when installing the package, > > making them unreliable on one hand, and somewhat suprising in other > > cases (e.g. when they unintentionally leak from developer's system). > > > > This is the first step towards restoring PMS compliance and *not* > > preserving extended metadata. > > How does preserving xattrs conflict with PMS? The PMS doesn't specify that xattrs, ACLs, caps etc. are preserved. By doing that, Portage allows developers to commit ebuilds that are not going to work reliably without even realizing it. In fact, this can't even work reliably inside Portage itself, depending on the filesystem used for $D. Furthermore, doexe preserving stuff goes contrary to common sense. Why would helpers preserve xattrs when they are supposed to reset things like mode and ownership by design? > Is there a bug report you could reference? It starts with https://bugs.gentoo.org/814857. -- Best regards, Michał Górny
Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: Add a QA check for installing xattrs
On Mon, Sep 27, 2021 at 1:20 PM Michał Górny wrote: > + eqawarn > + eqawarn "It is impossible to reliably guarantee that the > extended attributes" > + eqawarn "will be reliably preserved while merging. Please > ensure that any" > + eqawarn "extended metadata necessary is applied in > pkg_postinst() phase," > + eqawarn "and that the implementation includes a fallback if > necessary." This message suggests that applying xattrs in pkg_postinst is acceptable. However, your patch offers no way to disable the QA warning for ebuilds that do so.
Re: [gentoo-portage-dev] [PATCH] install-qa-check.d: Add a QA check for installing xattrs
On Mon, Sep 27, 2021 at 1:20 PM Michał Górny wrote: > > Warn the developers if ebuilds install files with xattrs to ${ED}. > The xattrs may or may not be preserved when installing the package, > making them unreliable on one hand, and somewhat suprising in other > cases (e.g. when they unintentionally leak from developer's system). > > This is the first step towards restoring PMS compliance and *not* > preserving extended metadata. How does preserving xattrs conflict with PMS? Is there a bug report you could reference?
[gentoo-portage-dev] [PATCH] install-qa-check.d: Add a QA check for installing xattrs
Warn the developers if ebuilds install files with xattrs to ${ED}. The xattrs may or may not be preserved when installing the package, making them unreliable on one hand, and somewhat suprising in other cases (e.g. when they unintentionally leak from developer's system). This is the first step towards restoring PMS compliance and *not* preserving extended metadata. Signed-off-by: Michał Górny --- bin/install-qa-check.d/95xattr | 54 ++ 1 file changed, 54 insertions(+) create mode 100644 bin/install-qa-check.d/95xattr diff --git a/bin/install-qa-check.d/95xattr b/bin/install-qa-check.d/95xattr new file mode 100644 index 0..07d8042a8 --- /dev/null +++ b/bin/install-qa-check.d/95xattr @@ -0,0 +1,54 @@ +# Check for xattrs. + +xattr_check() { + type -P getfattr >/dev/null || return + + pushd "${ED}" >/dev/null || die + local x file= keys + local -A data=() + while read -r x; do + case ${x} in + "# file: "*) + file=${x#*: } + file=/${file#.} + ;; + btrfs.*) + # ignore btrfs xattrs, they're implicit fs metadata + ;; + security.capability) + # don't report caps if we have fcaps.eclass inherited + if ! has fcaps ${INHERITED}; then + data[${file}]+=" ${x}" + fi + ;; + ?*) + data[${file}]+=" ${x}" + ;; + esac + done < <(getfattr -R -h -m - . 2>/dev/null) + popd >/dev/null || die + + if [[ ${data[@]} ]]; then + eqawarn "One or more files in \${ED} include extended attributes." + eqawarn + + for file in "${!data[@]}"; do + keys=( ${data[${file}]} ) + for x in "${keys[@]}"; do + eqatag xattr "key=${x}" "${file}" + done + eqawarn " ${file} (${keys[*]})" + done + + eqawarn + eqawarn "It is impossible to reliably guarantee that the extended attributes" + eqawarn "will be reliably preserved while merging. Please ensure that any" + eqawarn "extended metadata necessary is applied in pkg_postinst() phase," + eqawarn "and that the implementation includes a fallback if necessary." + fi +} + +xattr_check +: # guarantee successful exit + +# vim:ft=sh -- 2.33.0
Re: [gentoo-portage-dev] [PATCH v2] eend: Output QA notice when called without argument
> On Sat, 04 Sep 2021, Ulrich Müller wrote: > PMS says about eend: "Takes one fixed argument, which is a numeric > return code, and an optional message in all subsequent arguments." Pushed. signature.asc Description: PGP signature
Re: [gentoo-portage-dev] [PATCH v3] Copy files/* into the work tree instead of symlinking it
On Mon, 2021-09-27 at 12:49 +0200, Ulrich Mueller wrote: > > > > > > On Sun, 26 Sep 2021, Michał Górny wrote: > > > Symlinking FILESDIR into the work tree has the unintended > > consequence > > of preserving all original file metadata, including system-specific > > ACLs > > and so on. When these files are installed, this could lead to > > unintentionally copying this metadata to the system and/or binary > > packages. > > > Let's copy all files instead and drop metadata in the process. > > Since > > FILESDIR is expected to be small by design, this shouldn't cause any > > major trouble. It is also easier and less likely to cause > > regressions > > than making sure stuff is not preserved when installing. > > > Unfortunately, a similar problem applies to DISTDIR. However, > > installing files from DISTDIR is rarer than from FILESDIR, so I > > guess > > we'll cross that bridge when we get to it. > > Sorry for the late reply, but this looks like the wrong solution to > me. > > Looking at the installation helpers (doins, doexe, etc.), they don't > preserve the normal permission bits, but reset them to a defined > state. > So why would they preserve xattrs? > > I don't see anything in PMS that would mandate that behaviour (on the > contrary, in section 13.3.1 there is "Other file attributes may be > discarded"). How do the other package managers handle this? Yes, I agree this is the wrong long-term solution but it's a fix that works today and shouldn't cause regressions. I'd like to revisit how files are installed long-term but this requires more time, and most importantly making sure we won't accidentally break stuff. I'd like to establish whether we have any ebuilds relying e.g. on caps or ACLs being preserved from src_install(), and get alternative solutions to that first (fcaps.eclass-alike?). Apparently PaX will also require special fixes but I'm not sure if it's worth fixing at this point. -- Best regards, Michał Górny
Re: [gentoo-portage-dev] [PATCH v3] Copy files/* into the work tree instead of symlinking it
> On Sun, 26 Sep 2021, Michał Górny wrote: > Symlinking FILESDIR into the work tree has the unintended consequence > of preserving all original file metadata, including system-specific ACLs > and so on. When these files are installed, this could lead to > unintentionally copying this metadata to the system and/or binary > packages. > Let's copy all files instead and drop metadata in the process. Since > FILESDIR is expected to be small by design, this shouldn't cause any > major trouble. It is also easier and less likely to cause regressions > than making sure stuff is not preserved when installing. > Unfortunately, a similar problem applies to DISTDIR. However, > installing files from DISTDIR is rarer than from FILESDIR, so I guess > we'll cross that bridge when we get to it. Sorry for the late reply, but this looks like the wrong solution to me. Looking at the installation helpers (doins, doexe, etc.), they don't preserve the normal permission bits, but reset them to a defined state. So why would they preserve xattrs? I don't see anything in PMS that would mandate that behaviour (on the contrary, in section 13.3.1 there is "Other file attributes may be discarded"). How do the other package managers handle this? Ulrich signature.asc Description: PGP signature