Re: [gentoo-portage-dev] [PATCH] Support disabling docompress for binary package builds

[Zac Medico]

On 11/01/2018 02:50 AM, Michał Górny wrote:
> Add FEATURES=binpkg-docompress that can be used whether docompress
> compression is performed before or after creating binary packages.  With
> the feature enabled (the default), the current behavior of storing
> compressed files in binpkg is preserved.  With it disabled, uncompressed
> files are stored inside binary package and are compressed when
> installing.
>
> Storing uncompressed files in binary packages has two advantages:
>
> 1. Avoids the double-compression penalty, effectively improving binary
>    package compression speed and compression ratio.
>
> 2. Allows the same packages to be reused on systems with different
>    docompress configurations.
>
> The option is roughly backwards compatible.  Old Portage versions will
> install packages created with FEATURES=-binpkg-docompress correctly,
> albeit without compression.  Portage with FEATURES=binpkg-docompress
> should install old binpackages semi-correctly, potentially recompressing
> them (and throwing already-compressed warnings on format mismatch).
> The new behavior is left off by default to avoid those problems.
>
> Signed-off-by: Michał Górny <mgorny@g.o>
> ---
>  bin/misc-functions.sh                  | 34 +++++++++++++++++++++++---
>  cnf/make.globals                       |  2 +-
>  lib/portage/const.py                   |  1 +
>  lib/portage/dbapi/vartree.py           | 12 +++++++++
>  lib/portage/package/ebuild/doebuild.py |  3 ++-
>  man/make.conf.5                        |  6 +++++
>  6 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
> index db7aaed5a..96aa66611 100755
> --- a/bin/misc-functions.sh
> +++ b/bin/misc-functions.sh
> @@ -109,10 +109,13 @@ install_qa_check() {
>
>  	[[ -d ${ED%/}/usr/share/info ]] && prepinfo
>
> -	# Apply compression.
> -	"${PORTAGE_BIN_PATH}"/ecompress --queue "${PORTAGE_DOCOMPRESS[@]}"
> -	"${PORTAGE_BIN_PATH}"/ecompress --ignore "${PORTAGE_DOCOMPRESS_SKIP[@]}"
> -	"${PORTAGE_BIN_PATH}"/ecompress --dequeue
> +	# If binpkg-docompress is enabled, apply compression before creating
> +	# the binary package.
> +	if has binpkg-docompress ${FEATURES}; then
> +		"${PORTAGE_BIN_PATH}"/ecompress --queue "${PORTAGE_DOCOMPRESS[@]}"
> +		"${PORTAGE_BIN_PATH}"/ecompress --ignore "${PORTAGE_DOCOMPRESS_SKIP[@]}"
> +		"${PORTAGE_BIN_PATH}"/ecompress --dequeue
> +	fi
>
>  	export STRIP_MASK
>  	if ___eapi_has_dostrip; then
> @@ -160,6 +163,29 @@ install_qa_check() {
>  	rm -f "${ED%/}"/usr/share/info/dir{,.gz,.bz2} || die "rm failed!"
>  }
>
> +__dyn_instprep() {
> +	if has chflags ${FEATURES}; then
> +		# Save all the file flags for restoration afterwards.
> +		mtree -c -p "${ED}" -k flags > "${T}/bsdflags.mtree"
> +		# Remove all the file flags so that we can do anything necessary.
> +		chflags -R noschg,nouchg,nosappnd,nouappnd "${ED}"
> +		chflags -R nosunlnk,nouunlnk "${ED}" 2>/dev/null
> +	fi
> +
> +	# If binpkg-docompress is disabled, we need to apply compression
> +	# before installing.
> +	if ! has binpkg-docompress ${FEATURES}; then
> +		"${PORTAGE_BIN_PATH}"/ecompress --queue "${PORTAGE_DOCOMPRESS[@]}"
> +		"${PORTAGE_BIN_PATH}"/ecompress --ignore "${PORTAGE_DOCOMPRESS_SKIP[@]}"
> +		"${PORTAGE_BIN_PATH}"/ecompress --dequeue
> +	fi
> +
> +	if has chflags ${FEATURES}; then
> +		# Restore all the file flags that were saved earlier on.
> +		mtree -U -e -p "${ED}" -k flags < "${T}/bsdflags.mtree" &> /dev/null
> +	fi
> +}
> +
>  preinst_qa_check() {
>  	postinst_qa_check preinst
>  }
> diff --git a/cnf/make.globals b/cnf/make.globals
> index 04a708af8..72b567e98 100644
> --- a/cnf/make.globals
> +++ b/cnf/make.globals
> @@ -50,7 +50,7 @@ RESUMECOMMAND_SSH=${FETCHCOMMAND_SSH}
>  FETCHCOMMAND_SFTP="bash -c \"x=\\\${2#sftp://} ; host=\\\${x%%/*} ; port=\\\${host##*:} ; host=\\\${host%:*} ; [[ \\\${host} = \\\${port} ]] && port= ; eval \\\"declare -a ssh_opts=(\\\${3})\\\" ; exec sftp \\\${port:+-P \\\${port}} \\\"\\\${ssh_opts[@]}\\\" \\\"\\\${host}:/\\\${x#*/}\\\" \\\"\\\$1\\\"\" sftp \"\${DISTDIR}/\${FILE}\" \"\${URI}\" \"\${PORTAGE_SSH_OPTS}\""
>
>  # Default user options
> -FEATURES="assume-digests binpkg-logs
> +FEATURES="assume-digests binpkg-docompress binpkg-logs
>            config-protect-if-modified distlocks ebuild-locks
>            fixlafiles merge-sync multilib-strict news
>            parallel-fetch preserve-libs protect-owned
> diff --git a/lib/portage/const.py b/lib/portage/const.py
> index 7f84bf0e9..a343fc040 100644
> --- a/lib/portage/const.py
> +++ b/lib/portage/const.py
> @@ -122,6 +122,7 @@ EBUILD_PHASES            = (
>  )
>  SUPPORTED_FEATURES       = frozenset([
>  	"assume-digests",
> +	"binpkg-docompress",
>  	"binpkg-logs",
>  	"binpkg-multi-instance",
>  	"buildpkg",
> diff --git a/lib/portage/dbapi/vartree.py b/lib/portage/dbapi/vartree.py
> index c16fdfe88..fd8aaeb8e 100644
> --- a/lib/portage/dbapi/vartree.py
> +++ b/lib/portage/dbapi/vartree.py
> @@ -3719,6 +3719,7 @@ class dblink(object):
>
>  		This function does the following:
>
> +		calls doebuild(mydo=instprep)
>  		calls get_ro_checker to retrieve a function for checking whether Portage
>  		will write to a read-only filesystem, then runs it against the directory list
>  		calls self._preserve_libs if FEATURES=preserve-libs
> @@ -3768,6 +3769,17 @@ class dblink(object):
>  				level=logging.ERROR, noiselevel=-1)
>  			return 1
>
> +		# run instprep internal phase
> +		doebuild_environment(myebuild, "instprep",
> +			settings=self.settings, db=mydbapi)
> +		phase = EbuildPhase(background=False, phase="instprep",
> +			scheduler=self._scheduler, settings=self.settings)
> +		phase.start()
> +		if phase.wait() != os.EX_OK:
> +			showMessage(_("!!! instprep failed\n"),
> +				level=logging.ERROR, noiselevel=-1)
> +			return 1
> +
>  		is_binpkg = self.settings.get("EMERGE_FROM") == "binary"
>  		slot = ''
>  		for var_name in ('CHOST', 'SLOT'):
> diff --git a/lib/portage/package/ebuild/doebuild.py b/lib/portage/package/ebuild/doebuild.py
> index 9706de422..d7b535698 100644
> --- a/lib/portage/package/ebuild/doebuild.py
> +++ b/lib/portage/package/ebuild/doebuild.py
> @@ -674,7 +674,7 @@ def doebuild(myebuild, mydo, _unused=DeprecationWarning, settings=None, debug=0,
>  	                "config", "info", "setup", "depend", "pretend",
>  	                "fetch", "fetchall", "digest",
>  	                "unpack", "prepare", "configure", "compile", "test",
> -	                "install", "rpm", "qmerge", "merge",
> +	                "install", "instprep", "rpm", "qmerge", "merge",
>  	                "package", "unmerge", "manifest", "nofetch"]
>
>  	if mydo not in validcommands:
> @@ -1402,6 +1402,7 @@ def _spawn_actionmap(settings):
>  "compile":  {"cmd":ebuild_sh, "args":{"droppriv":droppriv, "free":nosandbox, "sesandbox":sesandbox, "fakeroot":0}},
>  "test":     {"cmd":ebuild_sh, "args":{"droppriv":droppriv, "free":nosandbox, "sesandbox":sesandbox, "fakeroot":0}},
>  "install":  {"cmd":ebuild_sh, "args":{"droppriv":0,        "free":0,         "sesandbox":sesandbox, "fakeroot":fakeroot}},
> +"instprep": {"cmd":misc_sh,   "args":{"droppriv":0,        "free":0,         "sesandbox":sesandbox, "fakeroot":fakeroot}},
>  "rpm":      {"cmd":misc_sh,   "args":{"droppriv":0,        "free":0,         "sesandbox":0,         "fakeroot":fakeroot}},
>  "package":  {"cmd":misc_sh,   "args":{"droppriv":0,        "free":0,         "sesandbox":0,         "fakeroot":fakeroot}},
>  		}

The feature seems find but the instprep phase is not needed if we use
MiscFunctionsProcess instead of EbuildPhase. Here's a list of odd things
about the instprep ebuild phase implementation as it is:

1) You can call instprep explicitly via the ebuild(1) command, but I
doubt that we really want or need to allow that.

2) Unlinke other ebuild phases, the doebuild function doesn't created a
.instpreped file to indicate when the instprep phase has executed.

3) Currently instprep executes when FEATURES=binpkg-docompress is
enabled, even though it does nothing of value. I think we should instead
generate a relevant list of MiscFunctionsProcess commands for the
enabled FEATURES, and only start a MiscFunctionsProcess instance if the
list of commands is non-empty.
-- 
Thanks,
Zac