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

[Michał Górny]

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 <***@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}},
}
diff --git a/man/make.conf.5 b/man/make.conf.5
index a33929143..ec03c93ca 100644
--- a/man/make.conf.5
+++ b/man/make.conf.5
@@ -275,6 +275,12 @@ existing digest, the digest will be regenerated regardless of whether or
not \fIassume\-digests\fR is enabled. The \fBebuild\fR(1) \fBdigest\fR command
has a \fB\-\-force\fR option that can be used to force regeneration of digests.
.TP
+.B binpkg\-docompress
+Perform \fBdocompress\fR (controllable file compression) before creating binary
+package. When this option is enabled (the default), documentation files are
+already compressed inside binary packages. When it is disabled, binary packages
+contain uncompressed documentation and Portage compresses it before installing.
+.TP
.B binpkg\-logs
Keep logs from successful binary package merges. This is relevant only when
\fBPORT_LOGDIR\fR is set.

[Zac Medico]

Post by Michał Górny
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.
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.
---
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 --dequeue
+ # If binpkg-docompress is enabled, apply compression before creating
+ # the binary package.
+ if has binpkg-docompress ${FEATURES}; then
+ "${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 --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}
# 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
+ 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
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()
+ showMessage(_("!!! instprep failed\n"),
+ level=logging.ERROR, noiselevel=-1)
+ return 1
+
is_binpkg = self.settings.get("EMERGE_FROM") == "binary"
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"]
"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.

[Michał Górny]

Post by Zac Medico

Post by Michał Górny
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.
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.
---
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 --dequeue
+ # If binpkg-docompress is enabled, apply compression before creating
+ # the binary package.
+ if has binpkg-docompress ${FEATURES}; then
+ "${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 --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}
# 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
+ 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
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()
+ showMessage(_("!!! instprep failed\n"),
+ level=logging.ERROR, noiselevel=-1)
+ return 1
+
is_binpkg = self.settings.get("EMERGE_FROM") == "binary"
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"]
"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
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.

Post by Zac Medico
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.

Post by Zac Medico
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.

[Zac Medico]

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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.
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.
---
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 --dequeue
+ # If binpkg-docompress is enabled, apply compression before creating
+ # the binary package.
+ if has binpkg-docompress ${FEATURES}; then
+ "${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 --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}
# 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
+ 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
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()
+ showMessage(_("!!! instprep failed\n"),
+ level=logging.ERROR, noiselevel=-1)
+ return 1
+
is_binpkg = self.settings.get("EMERGE_FROM") == "binary"
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"]
"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
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.

Post by Zac Medico
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.

Post by Zac Medico
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?

[Michał Górny]

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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.
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.
---
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 --dequeue
+ # If binpkg-docompress is enabled, apply compression before creating
+ # the binary package.
+ if has binpkg-docompress ${FEATURES}; then
+ "${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 --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}
# 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
+ 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
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()
+ showMessage(_("!!! instprep failed\n"),
+ level=logging.ERROR, noiselevel=-1)
+ return 1
+
is_binpkg = self.settings.get("EMERGE_FROM") == "binary"
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"]
"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
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.

Post by Zac Medico
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.

Post by Zac Medico
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?

At least stripping. But as usually happens, others may also have more
ideas. I suppose the main use case would be stuff that needs to happen
after install but isn't strictly necessary for building binary packages.

[Zac Medico]

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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"]
"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
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.

Sure. In fact, we could split out separate instcompress and inststrip
phases. I suppose we could split instprep into multiple phases in the
future, if we find a use for it.

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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.

Post by Zac Medico
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.

Note that a .instpreped file could be useful as a means to prevent
people from triggering a QA notice about pre-compressed or pre-stripped
files if they happen to pass an instprep argument to the ebuild(1)
command more than once. Maybe this case is unlikely enough that we can
safely neglect it?

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico
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?

At least stripping. But as usually happens, others may also have more
ideas. I suppose the main use case would be stuff that needs to happen
after install but isn't strictly necessary for building binary packages.

Sure, delayed strip will be a useful feature.

[Michał Górny]

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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"]
"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
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.

Ok, I've rechecked it and ebuild(1) currently rejects it as not a valid
phase name. So we can either ignore the problem until somebody finds it
useful to call it directly, or work on it. Would the existing patch be
ok, provided that ebuild(1) doesn't allow calling prepinst manually?

Post by Zac Medico
Sure. In fact, we could split out separate instcompress and inststrip
phases. I suppose we could split instprep into multiple phases in the
future, if we find a use for it.

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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.

Post by Zac Medico
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.

Note that a .instpreped file could be useful as a means to prevent
people from triggering a QA notice about pre-compressed or pre-stripped
files if they happen to pass an instprep argument to the ebuild(1)
command more than once. Maybe this case is unlikely enough that we can
safely neglect it?

That should be easy enough to implement. What was definitely harder for
me to comprehend, are phase dependencies. I wasn't sure whether adding
them to actionmap wouldn't make it possible for 'preinst' to be called
automatically before 'package'.

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico
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?

At least stripping. But as usually happens, others may also have more
ideas. I suppose the main use case would be stuff that needs to happen
after install but isn't strictly necessary for building binary packages.

Sure, delayed strip will be a useful feature.

[Zac Medico]

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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"]
"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
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.

Ok, I've rechecked it and ebuild(1) currently rejects it as not a valid
phase name.

I have your patches applied, and for me it treats 'instprep' as a valid
phase which I'm able to call. It also lists it in the list of available
phases if I choose an invalid phase named 'foo', since 'instprep' is
listed in validcommands:

!!! doebuild: 'foo' is not one of the following valid commands:
!!! clean cleanrm compile config configure depend
!!! digest fetch fetchall help info install
!!! instprep manifest merge nofetch package postinst
!!! postrm preinst prepare prerm pretend qmerge
!!! rpm setup test unmerge unpack

Post by Michał Górny
So we can either ignore the problem until somebody finds it
useful to call it directly, or work on it. Would the existing patch be
ok, provided that ebuild(1) doesn't allow calling prepinst manually?

Lets clarify our intention here, since apparently it works for me but
not for you when you tested. I think it's fine as-is, but let's make
sure that the behavior is what we collectively intend.

Post by Michał Górny

Post by Zac Medico
Sure. In fact, we could split out separate instcompress and inststrip
phases. I suppose we could split instprep into multiple phases in the
future, if we find a use for it.

Post by Michał Górny
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.

Post by Zac Medico
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.

Note that a .instpreped file could be useful as a means to prevent
people from triggering a QA notice about pre-compressed or pre-stripped
files if they happen to pass an instprep argument to the ebuild(1)
command more than once. Maybe this case is unlikely enough that we can
safely neglect it?

That should be easy enough to implement. What was definitely harder for
me to comprehend, are phase dependencies. I wasn't sure whether adding
them to actionmap wouldn't make it possible for 'preinst' to be called
automatically before 'package'.

The actionmap_deps map is just the edges of a directed dependency graph,
and it currently only covers up to the 'install' phase. The 'package'
phase currently depends on 'install', which should remain as-is, right?

This change will cause dependencies of 'instprep' to be invoked
automatically:

--- a/lib/portage/package/ebuild/doebuild.py
+++ b/lib/portage/package/ebuild/doebuild.py
@@ -656,6 +656,7 @@ def doebuild(myebuild, mydo,
_unused=DeprecationWarning, settings=None, debug=0,
"compile":["configure"],
"test": ["compile"],
"install":["test"],
+ "instprep":["install"],
"rpm": ["install"],
"package":["install"],
"merge" :["install"],

There's no need to make 'merge' depend on 'instprep', since treewalk
implicitly invokes 'instprep' when 'merge' is called. Adding a
dependency on 'instprep' would cause it to be invoked *twice* (once by
spawnebuild and again by treewalk).

[Zac Medico]

Post by Zac Medico

Post by Michał Górny

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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"]
"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
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.

Ok, I've rechecked it and ebuild(1) currently rejects it as not a valid
phase name.

I have your patches applied, and for me it treats 'instprep' as a valid
phase which I'm able to call. It also lists it in the list of available
phases if I choose an invalid phase named 'foo', since 'instprep' is
!!! clean cleanrm compile config configure depend
!!! digest fetch fetchall help info install
!!! instprep manifest merge nofetch package postinst
!!! postrm preinst prepare prerm pretend qmerge
!!! rpm setup test unmerge unpack

Post by Michał Górny
So we can either ignore the problem until somebody finds it
useful to call it directly, or work on it. Would the existing patch be
ok, provided that ebuild(1) doesn't allow calling prepinst manually?

Lets clarify our intention here, since apparently it works for me but
not for you when you tested. I think it's fine as-is, but let's make
sure that the behavior is what we collectively intend.

If we allow the 'instprep' phase to be directly invoked, then we should
also document it in man/ebuild.1.

[Michał Górny]

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico

Post by Michał Górny
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"]
"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
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.

Ok, I've rechecked it and ebuild(1) currently rejects it as not a valid
phase name.

I have your patches applied, and for me it treats 'instprep' as a valid
phase which I'm able to call. It also lists it in the list of available
phases if I choose an invalid phase named 'foo', since 'instprep' is
!!! clean cleanrm compile config configure depend
!!! digest fetch fetchall help info install
!!! instprep manifest merge nofetch package postinst
!!! postrm preinst prepare prerm pretend qmerge
!!! rpm setup test unmerge unpack

Post by Michał Górny
So we can either ignore the problem until somebody finds it
useful to call it directly, or work on it. Would the existing patch be
ok, provided that ebuild(1) doesn't allow calling prepinst manually?

Lets clarify our intention here, since apparently it works for me but
not for you when you tested. I think it's fine as-is, but let's make
sure that the behavior is what we collectively intend.

My mistake. I typed 'prepinst' instead of 'instprep' ;-).

Post by Zac Medico

Post by Michał Górny

Post by Zac Medico
Sure. In fact, we could split out separate instcompress and inststrip
phases. I suppose we could split instprep into multiple phases in the
future, if we find a use for it.

Post by Michał Górny
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.

Post by Zac Medico
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.

Note that a .instpreped file could be useful as a means to prevent
people from triggering a QA notice about pre-compressed or pre-stripped
files if they happen to pass an instprep argument to the ebuild(1)
command more than once. Maybe this case is unlikely enough that we can
safely neglect it?

That should be easy enough to implement. What was definitely harder for
me to comprehend, are phase dependencies. I wasn't sure whether adding
them to actionmap wouldn't make it possible for 'preinst' to be called
automatically before 'package'.

The actionmap_deps map is just the edges of a directed dependency graph,
and it currently only covers up to the 'install' phase. The 'package'
phase currently depends on 'install', which should remain as-is, right?
This change will cause dependencies of 'instprep' to be invoked
--- a/lib/portage/package/ebuild/doebuild.py
+++ b/lib/portage/package/ebuild/doebuild.py
@@ -656,6 +656,7 @@ def doebuild(myebuild, mydo,
_unused=DeprecationWarning, settings=None, debug=0,
"compile":["configure"],
"test": ["compile"],
"install":["test"],
+ "instprep":["install"],
"rpm": ["install"],
"package":["install"],
"merge" :["install"],
There's no need to make 'merge' depend on 'instprep', since treewalk
implicitly invokes 'instprep' when 'merge' is called. Adding a
dependency on 'instprep' would cause it to be invoked *twice* (once by
spawnebuild and again by treewalk).

Ok, that makes sense. I'll update the patch with this and ebuild.1
as requested in followup mail.