1 |
On Fri, 2018-11-02 at 12:22 -0700, Zac Medico wrote: |
2 |
> On 11/02/2018 12:16 PM, Michał Górny wrote: |
3 |
> > On Fri, 2018-11-02 at 11:49 -0700, Zac Medico wrote: |
4 |
> > > On 11/01/2018 02:50 AM, Michał Górny wrote: |
5 |
> > > > Add FEATURES=binpkg-docompress that can be used whether docompress |
6 |
> > > > compression is performed before or after creating binary packages. With |
7 |
> > > > the feature enabled (the default), the current behavior of storing |
8 |
> > > > compressed files in binpkg is preserved. With it disabled, uncompressed |
9 |
> > > > files are stored inside binary package and are compressed when |
10 |
> > > > installing. |
11 |
> > > > |
12 |
> > > > Storing uncompressed files in binary packages has two advantages: |
13 |
> > > > |
14 |
> > > > 1. Avoids the double-compression penalty, effectively improving binary |
15 |
> > > > package compression speed and compression ratio. |
16 |
> > > > |
17 |
> > > > 2. Allows the same packages to be reused on systems with different |
18 |
> > > > docompress configurations. |
19 |
> > > > |
20 |
> > > > The option is roughly backwards compatible. Old Portage versions will |
21 |
> > > > install packages created with FEATURES=-binpkg-docompress correctly, |
22 |
> > > > albeit without compression. Portage with FEATURES=binpkg-docompress |
23 |
> > > > should install old binpackages semi-correctly, potentially recompressing |
24 |
> > > > them (and throwing already-compressed warnings on format mismatch). |
25 |
> > > > The new behavior is left off by default to avoid those problems. |
26 |
> > > > |
27 |
> > > > Signed-off-by: Michał Górny <mgorny@g.o> |
28 |
> > > > --- |
29 |
> > > > bin/misc-functions.sh | 34 +++++++++++++++++++++++--- |
30 |
> > > > cnf/make.globals | 2 +- |
31 |
> > > > lib/portage/const.py | 1 + |
32 |
> > > > lib/portage/dbapi/vartree.py | 12 +++++++++ |
33 |
> > > > lib/portage/package/ebuild/doebuild.py | 3 ++- |
34 |
> > > > man/make.conf.5 | 6 +++++ |
35 |
> > > > 6 files changed, 52 insertions(+), 6 deletions(-) |
36 |
> > > > |
37 |
> > > > diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh |
38 |
> > > > index db7aaed5a..96aa66611 100755 |
39 |
> > > > --- a/bin/misc-functions.sh |
40 |
> > > > +++ b/bin/misc-functions.sh |
41 |
> > > > @@ -109,10 +109,13 @@ install_qa_check() { |
42 |
> > > > |
43 |
> > > > [[ -d ${ED%/}/usr/share/info ]] && prepinfo |
44 |
> > > > |
45 |
> > > > - # Apply compression. |
46 |
> > > > - "${PORTAGE_BIN_PATH}"/ecompress --queue "${PORTAGE_DOCOMPRESS[@]}" |
47 |
> > > > - "${PORTAGE_BIN_PATH}"/ecompress --ignore "${PORTAGE_DOCOMPRESS_SKIP[@]}" |
48 |
> > > > - "${PORTAGE_BIN_PATH}"/ecompress --dequeue |
49 |
> > > > + # If binpkg-docompress is enabled, apply compression before creating |
50 |
> > > > + # the binary package. |
51 |
> > > > + if has binpkg-docompress ${FEATURES}; then |
52 |
> > > > + "${PORTAGE_BIN_PATH}"/ecompress --queue "${PORTAGE_DOCOMPRESS[@]}" |
53 |
> > > > + "${PORTAGE_BIN_PATH}"/ecompress --ignore "${PORTAGE_DOCOMPRESS_SKIP[@]}" |
54 |
> > > > + "${PORTAGE_BIN_PATH}"/ecompress --dequeue |
55 |
> > > > + fi |
56 |
> > > > |
57 |
> > > > export STRIP_MASK |
58 |
> > > > if ___eapi_has_dostrip; then |
59 |
> > > > @@ -160,6 +163,29 @@ install_qa_check() { |
60 |
> > > > rm -f "${ED%/}"/usr/share/info/dir{,.gz,.bz2} || die "rm failed!" |
61 |
> > > > } |
62 |
> > > > |
63 |
> > > > +__dyn_instprep() { |
64 |
> > > > + if has chflags ${FEATURES}; then |
65 |
> > > > + # Save all the file flags for restoration afterwards. |
66 |
> > > > + mtree -c -p "${ED}" -k flags > "${T}/bsdflags.mtree" |
67 |
> > > > + # Remove all the file flags so that we can do anything necessary. |
68 |
> > > > + chflags -R noschg,nouchg,nosappnd,nouappnd "${ED}" |
69 |
> > > > + chflags -R nosunlnk,nouunlnk "${ED}" 2>/dev/null |
70 |
> > > > + fi |
71 |
> > > > + |
72 |
> > > > + # If binpkg-docompress is disabled, we need to apply compression |
73 |
> > > > + # before installing. |
74 |
> > > > + if ! has binpkg-docompress ${FEATURES}; then |
75 |
> > > > + "${PORTAGE_BIN_PATH}"/ecompress --queue "${PORTAGE_DOCOMPRESS[@]}" |
76 |
> > > > + "${PORTAGE_BIN_PATH}"/ecompress --ignore "${PORTAGE_DOCOMPRESS_SKIP[@]}" |
77 |
> > > > + "${PORTAGE_BIN_PATH}"/ecompress --dequeue |
78 |
> > > > + fi |
79 |
> > > > + |
80 |
> > > > + if has chflags ${FEATURES}; then |
81 |
> > > > + # Restore all the file flags that were saved earlier on. |
82 |
> > > > + mtree -U -e -p "${ED}" -k flags < "${T}/bsdflags.mtree" &> /dev/null |
83 |
> > > > + fi |
84 |
> > > > +} |
85 |
> > > > + |
86 |
> > > > preinst_qa_check() { |
87 |
> > > > postinst_qa_check preinst |
88 |
> > > > } |
89 |
> > > > diff --git a/cnf/make.globals b/cnf/make.globals |
90 |
> > > > index 04a708af8..72b567e98 100644 |
91 |
> > > > --- a/cnf/make.globals |
92 |
> > > > +++ b/cnf/make.globals |
93 |
> > > > @@ -50,7 +50,7 @@ RESUMECOMMAND_SSH=${FETCHCOMMAND_SSH} |
94 |
> > > > 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}\"" |
95 |
> > > > |
96 |
> > > > # Default user options |
97 |
> > > > -FEATURES="assume-digests binpkg-logs |
98 |
> > > > +FEATURES="assume-digests binpkg-docompress binpkg-logs |
99 |
> > > > config-protect-if-modified distlocks ebuild-locks |
100 |
> > > > fixlafiles merge-sync multilib-strict news |
101 |
> > > > parallel-fetch preserve-libs protect-owned |
102 |
> > > > diff --git a/lib/portage/const.py b/lib/portage/const.py |
103 |
> > > > index 7f84bf0e9..a343fc040 100644 |
104 |
> > > > --- a/lib/portage/const.py |
105 |
> > > > +++ b/lib/portage/const.py |
106 |
> > > > @@ -122,6 +122,7 @@ EBUILD_PHASES = ( |
107 |
> > > > ) |
108 |
> > > > SUPPORTED_FEATURES = frozenset([ |
109 |
> > > > "assume-digests", |
110 |
> > > > + "binpkg-docompress", |
111 |
> > > > "binpkg-logs", |
112 |
> > > > "binpkg-multi-instance", |
113 |
> > > > "buildpkg", |
114 |
> > > > diff --git a/lib/portage/dbapi/vartree.py b/lib/portage/dbapi/vartree.py |
115 |
> > > > index c16fdfe88..fd8aaeb8e 100644 |
116 |
> > > > --- a/lib/portage/dbapi/vartree.py |
117 |
> > > > +++ b/lib/portage/dbapi/vartree.py |
118 |
> > > > @@ -3719,6 +3719,7 @@ class dblink(object): |
119 |
> > > > |
120 |
> > > > This function does the following: |
121 |
> > > > |
122 |
> > > > + calls doebuild(mydo=instprep) |
123 |
> > > > calls get_ro_checker to retrieve a function for checking whether Portage |
124 |
> > > > will write to a read-only filesystem, then runs it against the directory list |
125 |
> > > > calls self._preserve_libs if FEATURES=preserve-libs |
126 |
> > > > @@ -3768,6 +3769,17 @@ class dblink(object): |
127 |
> > > > level=logging.ERROR, noiselevel=-1) |
128 |
> > > > return 1 |
129 |
> > > > |
130 |
> > > > + # run instprep internal phase |
131 |
> > > > + doebuild_environment(myebuild, "instprep", |
132 |
> > > > + settings=self.settings, db=mydbapi) |
133 |
> > > > + phase = EbuildPhase(background=False, phase="instprep", |
134 |
> > > > + scheduler=self._scheduler, settings=self.settings) |
135 |
> > > > + phase.start() |
136 |
> > > > + if phase.wait() != os.EX_OK: |
137 |
> > > > + showMessage(_("!!! instprep failed\n"), |
138 |
> > > > + level=logging.ERROR, noiselevel=-1) |
139 |
> > > > + return 1 |
140 |
> > > > + |
141 |
> > > > is_binpkg = self.settings.get("EMERGE_FROM") == "binary" |
142 |
> > > > slot = '' |
143 |
> > > > for var_name in ('CHOST', 'SLOT'): |
144 |
> > > > diff --git a/lib/portage/package/ebuild/doebuild.py b/lib/portage/package/ebuild/doebuild.py |
145 |
> > > > index 9706de422..d7b535698 100644 |
146 |
> > > > --- a/lib/portage/package/ebuild/doebuild.py |
147 |
> > > > +++ b/lib/portage/package/ebuild/doebuild.py |
148 |
> > > > @@ -674,7 +674,7 @@ def doebuild(myebuild, mydo, _unused=DeprecationWarning, settings=None, debug=0, |
149 |
> > > > "config", "info", "setup", "depend", "pretend", |
150 |
> > > > "fetch", "fetchall", "digest", |
151 |
> > > > "unpack", "prepare", "configure", "compile", "test", |
152 |
> > > > - "install", "rpm", "qmerge", "merge", |
153 |
> > > > + "install", "instprep", "rpm", "qmerge", "merge", |
154 |
> > > > "package", "unmerge", "manifest", "nofetch"] |
155 |
> > > > |
156 |
> > > > if mydo not in validcommands: |
157 |
> > > > @@ -1402,6 +1402,7 @@ def _spawn_actionmap(settings): |
158 |
> > > > "compile": {"cmd":ebuild_sh, "args":{"droppriv":droppriv, "free":nosandbox, "sesandbox":sesandbox, "fakeroot":0}}, |
159 |
> > > > "test": {"cmd":ebuild_sh, "args":{"droppriv":droppriv, "free":nosandbox, "sesandbox":sesandbox, "fakeroot":0}}, |
160 |
> > > > "install": {"cmd":ebuild_sh, "args":{"droppriv":0, "free":0, "sesandbox":sesandbox, "fakeroot":fakeroot}}, |
161 |
> > > > +"instprep": {"cmd":misc_sh, "args":{"droppriv":0, "free":0, "sesandbox":sesandbox, "fakeroot":fakeroot}}, |
162 |
> > > > "rpm": {"cmd":misc_sh, "args":{"droppriv":0, "free":0, "sesandbox":0, "fakeroot":fakeroot}}, |
163 |
> > > > "package": {"cmd":misc_sh, "args":{"droppriv":0, "free":0, "sesandbox":0, "fakeroot":fakeroot}}, |
164 |
> > > > } |
165 |
> > > |
166 |
> > > The feature seems find but the instprep phase is not needed if we use |
167 |
> > > MiscFunctionsProcess instead of EbuildPhase. Here's a list of odd things |
168 |
> > > about the instprep ebuild phase implementation as it is: |
169 |
> > > |
170 |
> > > 1) You can call instprep explicitly via the ebuild(1) command, but I |
171 |
> > > doubt that we really want or need to allow that. |
172 |
> > |
173 |
> > I was actually wondering about that. I suppose it might be useful for |
174 |
> > debugging purposes. However, since I wasn't able to figure out how to |
175 |
> > fully integrate it with the phase machinery, I went for the minimal set |
176 |
> > of code that wouldn't be definitive either way. |
177 |
> > |
178 |
> > > 2) Unlinke other ebuild phases, the doebuild function doesn't created a |
179 |
> > > .instpreped file to indicate when the instprep phase has executed. |
180 |
> > |
181 |
> > I suppose I can try to do that if that's desirable. |
182 |
> > |
183 |
> > > 3) Currently instprep executes when FEATURES=binpkg-docompress is |
184 |
> > > enabled, even though it does nothing of value. I think we should instead |
185 |
> > > generate a relevant list of MiscFunctionsProcess commands for the |
186 |
> > > enabled FEATURES, and only start a MiscFunctionsProcess instance if the |
187 |
> > > list of commands is non-empty. |
188 |
> > |
189 |
> > That sounds like premature optimization with a bit of going against |
190 |
> > principle of least surprise. Given it's a phase function with specific |
191 |
> > implementation that could be extended in the future, I'd rather avoid |
192 |
> > hiding additional conditions for running it elsewhere, as it opens |
193 |
> > a strong possibility that somebody adds additional functionality but |
194 |
> > forgets to update the condition resulting either in immediate WTF |
195 |
> > or the new code being skipped only for some users, with even harder-to- |
196 |
> > debug WTF. |
197 |
> |
198 |
> I wasn't really sure that instprep deserved to be a full-fledged phase |
199 |
> at this point. I guess you're planning to add more stuff there? |
200 |
|
201 |
At least stripping. But as usually happens, others may also have more |
202 |
ideas. I suppose the main use case would be stuff that needs to happen |
203 |
after install but isn't strictly necessary for building binary packages. |
204 |
|
205 |
-- |
206 |
Best regards, |
207 |
Michał Górny |