[gentoo-portage-dev] [PATCH 0/4] Proper fix for colliding (un)compressed files

[Michał Górny]

Hi,

Here's a proper fix for the problem, as promised. I've also taken
the liberty of reverting the ugly hack that is no longer necessary,
and reintroducing parallel compression as requested earlier.

I've also increased verbosity of the messages. If precompressed files
are found, Portage now explicitly prints the list of them. If colliding
files are found, Portage explicitly prints which files collide and asks
the ebuild maintainer to fix that.

It no longer tries to silently ignore this major issue and/or discard
some of the files but explicitly fails, given that there are no valid
circumstances for ebuilds to install colliding files. It only indicates
a serious issue with the ebuild, and Portage should explicitly force
the developer to fix it.

--
Best regards,
Michał Górny


Michał Górny (4):
Revert "ecompress-file: de-duplicate filtered_args (bug 667072)"
ecompress: Run compression in parallel
ecompress: Detect and report colliding (un)compressed files
ecompress: Fix duplicate pre-compressed file warning

bin/ecompress | 44 ++++++++++++++++++++++++++++++++++++++++++--
bin/ecompress-file | 22 ++++++----------------
2 files changed, 48 insertions(+), 18 deletions(-)

[Michał Górny]

Reverts: 1fc311ce0afeef9f982213e43220d079a4ffec26
Signed-off-by: Michał Górny <***@gentoo.org>
---
bin/ecompress-file | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bin/ecompress-file b/bin/ecompress-file
index e65b21ee4..18269c91b 100755
--- a/bin/ecompress-file
+++ b/bin/ecompress-file
@@ -13,7 +13,7 @@ compress_file() {
done
set +f
mask_ext_re="^(${mask_ext_re:1})\$"
- local -A filtered_args
+ local filtered_args=()
local had_precompressed=
for x in "$@" ; do
[[ ${x##*.} =~ $mask_ext_re ]] && continue
@@ -35,10 +35,10 @@ compress_file() {
had_precompressed=1;;
esac

- filtered_args[${x}]=
+ filtered_args+=( "$x" )
done
[[ ${#filtered_args[@]} -eq 0 ]] && return 0
- set -- "${!filtered_args[@]}"
+ set -- "${filtered_args[@]}"

if [[ ${had_precompressed} ]]; then
eqawarn "One or more compressed files were found in docompress-ed directories."

[Michał Górny]

Signed-off-by: Michał Górny <***@gentoo.org>
Reviewed-by: Zac Medico <***@gentoo.org>
Closes: https://github.com/gentoo/portage/pull/365
---
bin/ecompress | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bin/ecompress b/bin/ecompress
index 434456f0c..36bdb585b 100755
--- a/bin/ecompress
+++ b/bin/ecompress
@@ -139,8 +139,8 @@ fix_symlinks() {
}

export PORTAGE_COMPRESS PORTAGE_COMPRESS_FLAGS
-find "${ED}" -name '*.ecompress' -delete \
- -exec "${PORTAGE_BIN_PATH}"/ecompress-file {} +
+find "${ED}" -name '*.ecompress' -delete -print0 |
+ ___parallel_xargs -0 "${PORTAGE_BIN_PATH}"/ecompress-file
ret=${?}

fix_symlinks

[Michał Górny]

Whenever the install directory contains files that would collide upon
(re)compressing, report them explicitly and fail. This indicates
a serious problem in ebuild and since we don't know which of the files
is correct, we should not attempt to choose between them.

To reduce performance impact, the check is only done whenever compressed
files are found. This is sufficient since for issue to occur there must
be at least one compressed variant.

Bug: https://bugs.gentoo.org/667072
Signed-off-by: Michał Górny <***@gentoo.org>
---
bin/ecompress | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/bin/ecompress b/bin/ecompress
index 36bdb585b..bc1f5e08a 100755
--- a/bin/ecompress
+++ b/bin/ecompress
@@ -49,6 +49,30 @@ while [[ $# -gt 0 ]] ; do
find_args+=( -size "+${PORTAGE_DOCOMPRESS_SIZE_LIMIT}c" )

while IFS= read -d '' -r path; do
+ # detect the horrible posibility of the ebuild installing
+ # colliding compressed and/or uncompressed variants
+ # and fail hard (bug #667072)
+ #
+ # note: to save time, we need to do this only if there's
+ # at least one compressed file
+ case ${path} in
+ *.Z|*.gz|*.bz2|*.lzma|*.xz)
+ vpath=${path%.*}
+ for comp in '' .Z .gz .bz2 .lzma .xz; do
+ if [[ ${vpath}${comp} != ${path} && \
+ -e ${vpath}${comp} ]]; then
+ eerror "Colliding files found for ecompress:"
+ eerror
+ eerror " ${path#${D%/}}"
+ eerror " ${vpath#${D%/}}${comp}"
+ eerror
+ eerror "Please remove the incorrect of those files."
+ die "Aborting due to colliding compressed files."
+ fi
+ done
+ ;;
+ esac
+

"${path}.ecompress" || die

done < <(find "${find_args[@]}" -print0 || die)
fi

[Zac Medico]

Post by Michał Górny
Whenever the install directory contains files that would collide upon
(re)compressing, report them explicitly and fail. This indicates
a serious problem in ebuild and since we don't know which of the files
is correct, we should not attempt to choose between them.
To reduce performance impact, the check is only done whenever compressed
files are found. This is sufficient since for issue to occur there must
be at least one compressed variant.
Bug: https://bugs.gentoo.org/667072
---
bin/ecompress | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/bin/ecompress b/bin/ecompress
index 36bdb585b..bc1f5e08a 100755
--- a/bin/ecompress
+++ b/bin/ecompress
@@ -49,6 +49,30 @@ while [[ $# -gt 0 ]] ; do
find_args+=( -size "+${PORTAGE_DOCOMPRESS_SIZE_LIMIT}c" )
while IFS= read -d '' -r path; do
+ # detect the horrible posibility of the ebuild installing
+ # colliding compressed and/or uncompressed variants
+ # and fail hard (bug #667072)
+ #
+ # note: to save time, we need to do this only if there's
+ # at least one compressed file
+ case ${path} in
+ *.Z|*.gz|*.bz2|*.lzma|*.xz)
+ vpath=${path%.*}
+ for comp in '' .Z .gz .bz2 .lzma .xz; do
+ if [[ ${vpath}${comp} != ${path} && \
+ -e ${vpath}${comp} ]]; then
+ eerror "Colliding files found for ecompress:"
+ eerror
+ eerror " ${path#${D%/}}"
+ eerror " ${vpath#${D%/}}${comp}"
+ eerror
+ eerror "Please remove the incorrect of those files."
+ die "Aborting due to colliding compressed files."
+ fi
+ done
+ ;;
+ esac
+

"${path}.ecompress" || die

fi

This breaks compatibility, so let's use eqawarn and simply leave the
compressed files in place for now, then make it die in the next EAPI.

[Michał Górny]

Post by Zac Medico

Post by Michał Górny
Whenever the install directory contains files that would collide upon
(re)compressing, report them explicitly and fail. This indicates
a serious problem in ebuild and since we don't know which of the files
is correct, we should not attempt to choose between them.
To reduce performance impact, the check is only done whenever compressed
files are found. This is sufficient since for issue to occur there must
be at least one compressed variant.
Bug: https://bugs.gentoo.org/667072
---
bin/ecompress | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/bin/ecompress b/bin/ecompress
index 36bdb585b..bc1f5e08a 100755
--- a/bin/ecompress
+++ b/bin/ecompress
@@ -49,6 +49,30 @@ while [[ $# -gt 0 ]] ; do
find_args+=( -size "+${PORTAGE_DOCOMPRESS_SIZE_LIMIT}c" )
while IFS= read -d '' -r path; do
+ # detect the horrible posibility of the ebuild installing
+ # colliding compressed and/or uncompressed variants
+ # and fail hard (bug #667072)
+ #
+ # note: to save time, we need to do this only if there's
+ # at least one compressed file
+ case ${path} in
+ *.Z|*.gz|*.bz2|*.lzma|*.xz)
+ vpath=${path%.*}
+ for comp in '' .Z .gz .bz2 .lzma .xz; do
+ if [[ ${vpath}${comp} != ${path} && \
+ -e ${vpath}${comp} ]]; then
+ eerror "Colliding files found for ecompress:"
+ eerror
+ eerror " ${path#${D%/}}"
+ eerror " ${vpath#${D%/}}${comp}"
+ eerror
+ eerror "Please remove the incorrect of those files."
+ die "Aborting due to colliding compressed files."
+ fi
+ done
+ ;;
+ esac
+

"${path}.ecompress" || die

fi

This breaks compatibility, so let's use eqawarn and simply leave the
compressed files in place for now, then make it die in the next EAPI.

Compatibility with what? With one ebuild where maintainer failed to
handle recent eqawarn? Given that he couldn't have expected any sane
behavior previously?

[Zac Medico]

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
Whenever the install directory contains files that would collide upon
(re)compressing, report them explicitly and fail. This indicates
a serious problem in ebuild and since we don't know which of the files
is correct, we should not attempt to choose between them.
To reduce performance impact, the check is only done whenever compressed
files are found. This is sufficient since for issue to occur there must
be at least one compressed variant.
Bug: https://bugs.gentoo.org/667072
---
bin/ecompress | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/bin/ecompress b/bin/ecompress
index 36bdb585b..bc1f5e08a 100755
--- a/bin/ecompress
+++ b/bin/ecompress
@@ -49,6 +49,30 @@ while [[ $# -gt 0 ]] ; do
find_args+=( -size "+${PORTAGE_DOCOMPRESS_SIZE_LIMIT}c" )
while IFS= read -d '' -r path; do
+ # detect the horrible posibility of the ebuild installing
+ # colliding compressed and/or uncompressed variants
+ # and fail hard (bug #667072)
+ #
+ # note: to save time, we need to do this only if there's
+ # at least one compressed file
+ case ${path} in
+ *.Z|*.gz|*.bz2|*.lzma|*.xz)
+ vpath=${path%.*}
+ for comp in '' .Z .gz .bz2 .lzma .xz; do
+ if [[ ${vpath}${comp} != ${path} && \
+ -e ${vpath}${comp} ]]; then
+ eerror "Colliding files found for ecompress:"
+ eerror
+ eerror " ${path#${D%/}}"
+ eerror " ${vpath#${D%/}}${comp}"
+ eerror
+ eerror "Please remove the incorrect of those files."
+ die "Aborting due to colliding compressed files."
+ fi
+ done
+ ;;
+ esac
+

"${path}.ecompress" || die

fi

This breaks compatibility, so let's use eqawarn and simply leave the
compressed files in place for now, then make it die in the next EAPI.

Compatibility with what? With one ebuild where maintainer failed to
handle recent eqawarn? Given that he couldn't have expected any sane
behavior previously?

How do you know how many ebuilds are affected, either in the gentoo
repository or other repository? Making it die can potentially introduce
a burden on users if the ebuilds that they rely on will no longer build.
It's much easier for users if the ebuild maintainers have an opportunity
to fix issues before the ebuilds begin to die.

[Michał Górny]

Fix the pre-compressed file warning to be reported only once, now that
files are processed in parallel. Also, print the list of precompressed
files verbosely.

Signed-off-by: Michał Górny <***@gentoo.org>
---
bin/ecompress | 16 ++++++++++++++++
bin/ecompress-file | 16 +++-------------
2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/bin/ecompress b/bin/ecompress
index bc1f5e08a..38b91a121 100755
--- a/bin/ecompress
+++ b/bin/ecompress
@@ -70,6 +70,7 @@ while [[ $# -gt 0 ]] ; do
die "Aborting due to colliding compressed files."
fi
done
+ echo "${path}" >> "${T}"/.ecompress_had_precompressed
;;
esac

@@ -167,6 +168,21 @@ find "${ED}" -name '*.ecompress' -delete -print0 |
___parallel_xargs -0 "${PORTAGE_BIN_PATH}"/ecompress-file
ret=${?}

+if [[ -f ${T}/.ecompress_had_precompressed ]]; then
+ eqawarn "One or more compressed files were found in docompress-ed directories."
+ eqawarn "Please fix the ebuild not to install compressed files (manpages,"
+ eqawarn "documentation) when automatic compression is used:"
+ eqawarn
+ n=0
+ while read -r f; do
+ eqawarn " ${f#${D%/}}"
+ if [[ $(( n++ )) -eq 10 ]]; then
+ eqawarn " ..."
+ break
+ fi
+ done <"${T}"/.ecompress_had_precompressed
+fi
+
fix_symlinks
: $(( ret |= ${?} ))
[[ $ret -ne 0 ]] && __helpers_die "${0##*/} failed"
diff --git a/bin/ecompress-file b/bin/ecompress-file
index 18269c91b..bc8fe5451 100755
--- a/bin/ecompress-file
+++ b/bin/ecompress-file
@@ -14,7 +14,6 @@ compress_file() {
set +f
mask_ext_re="^(${mask_ext_re:1})\$"
local filtered_args=()
- local had_precompressed=
for x in "$@" ; do
[[ ${x##*.} =~ $mask_ext_re ]] && continue
[[ -s ${x} ]] || continue
@@ -23,16 +22,13 @@ compress_file() {
case ${x} in
*.gz|*.Z)
gunzip -f "${x}" || __helpers_die "gunzip failed"
- x=${x%.*}
- had_precompressed=1;;
+ x=${x%.*};;
*.bz2)
bunzip2 -f "${x}" || __helpers_die "bunzip2 failed"
- x=${x%.bz2}
- had_precompressed=1;;
+ x=${x%.bz2};;
*.lzma|*.xz)
unxz -f "${x}" || __helpers_die "unxz failed"
- x=${x%.*}
- had_precompressed=1;;
+ x=${x%.*};;
esac

filtered_args+=( "$x" )
@@ -40,12 +36,6 @@ compress_file() {
[[ ${#filtered_args[@]} -eq 0 ]] && return 0
set -- "${filtered_args[@]}"

- if [[ ${had_precompressed} ]]; then
- eqawarn "One or more compressed files were found in docompress-ed directories."
- eqawarn "Please fix the ebuild not to install compressed files (manpages,"
- eqawarn "documentation) when automatic compression is used."
- fi
-
# If a compressed version of the file already exists, simply
# delete it so that the compressor doesn't whine (bzip2 will
# complain and skip, gzip will prompt for input)