On 11/02/2018 12:16 PM, Michał Górny wrote:
> On Fri, 2018-11-02 at 11:49 -0700, Zac Medico wrote:
>> 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 <mgo...@gentoo.org>
>>> ---
>>> 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.
>
> I was actually wondering about that. I suppose it might be useful for
> debugging purposes. However, since I wasn't able to figure out how to
> fully integrate it with the phase machinery, I went for the minimal set
> of code that wouldn't be definitive either way.
>
>> 2) Unlinke other ebuild phases, the doebuild function doesn't created a
>> .instpreped file to indicate when the instprep phase has executed.
>
> I suppose I can try to do that if that's desirable.
>
>> 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.
>
> That sounds like premature optimization with a bit of going against
> principle of least surprise. Given it's a phase function with specific
> implementation that could be extended in the future, I'd rather avoid
> hiding additional conditions for running it elsewhere, as it opens
> a strong possibility that somebody adds additional functionality but
> forgets to update the condition resulting either in immediate WTF
> or the new code being skipped only for some users, with even harder-to-
> debug WTF.I wasn't really sure that instprep deserved to be a full-fledged phase at this point. I guess you're planning to add more stuff there? -- Thanks, Zac
signature.asc
Description: OpenPGP digital signature
