Gentoo Archives: gentoo-portage-dev

From: Zac Medico <zmedico@g.o>
To: gentoo-portage-dev@l.g.o, "Michał Górny" <mgorny@g.o>
Subject: Re: [gentoo-portage-dev] [PATCH] Support disabling docompress for binary package builds
Date: Sat, 03 Nov 2018 02:35:27
Message-Id: a4a27587-f755-8af0-40c1-605c9b4997db@gentoo.org
In Reply to: Re: [gentoo-portage-dev] [PATCH] Support disabling docompress for binary package builds by "Michał Górny"
1 On 11/02/2018 12:35 PM, Michał Górny wrote:
2 > On Fri, 2018-11-02 at 12:22 -0700, Zac Medico wrote:
3 >> On 11/02/2018 12:16 PM, Michał Górny wrote:
4 >>> On Fri, 2018-11-02 at 11:49 -0700, Zac Medico wrote:
5 >>>> On 11/01/2018 02:50 AM, Michał Górny wrote:
6 >>>>> diff --git a/lib/portage/package/ebuild/doebuild.py b/lib/portage/package/ebuild/doebuild.py
7 >>>>> index 9706de422..d7b535698 100644
8 >>>>> --- a/lib/portage/package/ebuild/doebuild.py
9 >>>>> +++ b/lib/portage/package/ebuild/doebuild.py
10 >>>>> @@ -674,7 +674,7 @@ def doebuild(myebuild, mydo, _unused=DeprecationWarning, settings=None, debug=0,
11 >>>>> "config", "info", "setup", "depend", "pretend",
12 >>>>> "fetch", "fetchall", "digest",
13 >>>>> "unpack", "prepare", "configure", "compile", "test",
14 >>>>> - "install", "rpm", "qmerge", "merge",
15 >>>>> + "install", "instprep", "rpm", "qmerge", "merge",
16 >>>>> "package", "unmerge", "manifest", "nofetch"]
17 >>>>>
18 >>>>> if mydo not in validcommands:
19 >>>>> @@ -1402,6 +1402,7 @@ def _spawn_actionmap(settings):
20 >>>>> "compile": {"cmd":ebuild_sh, "args":{"droppriv":droppriv, "free":nosandbox, "sesandbox":sesandbox, "fakeroot":0}},
21 >>>>> "test": {"cmd":ebuild_sh, "args":{"droppriv":droppriv, "free":nosandbox, "sesandbox":sesandbox, "fakeroot":0}},
22 >>>>> "install": {"cmd":ebuild_sh, "args":{"droppriv":0, "free":0, "sesandbox":sesandbox, "fakeroot":fakeroot}},
23 >>>>> +"instprep": {"cmd":misc_sh, "args":{"droppriv":0, "free":0, "sesandbox":sesandbox, "fakeroot":fakeroot}},
24 >>>>> "rpm": {"cmd":misc_sh, "args":{"droppriv":0, "free":0, "sesandbox":0, "fakeroot":fakeroot}},
25 >>>>> "package": {"cmd":misc_sh, "args":{"droppriv":0, "free":0, "sesandbox":0, "fakeroot":fakeroot}},
26 >>>>> }
27 >>>>
28 >>>> The feature seems find but the instprep phase is not needed if we use
29 >>>> MiscFunctionsProcess instead of EbuildPhase. Here's a list of odd things
30 >>>> about the instprep ebuild phase implementation as it is:
31 >>>>
32 >>>> 1) You can call instprep explicitly via the ebuild(1) command, but I
33 >>>> doubt that we really want or need to allow that.
34 >>>
35 >>> I was actually wondering about that. I suppose it might be useful for
36 >>> debugging purposes.
37
38 Sure. In fact, we could split out separate instcompress and inststrip
39 phases. I suppose we could split instprep into multiple phases in the
40 future, if we find a use for it.
41
42 >>> However, since I wasn't able to figure out how to
43 >>> fully integrate it with the phase machinery, I went for the minimal set
44 >>> of code that wouldn't be definitive either way.
45 >>>
46 >>>> 2) Unlinke other ebuild phases, the doebuild function doesn't created a
47 >>>> .instpreped file to indicate when the instprep phase has executed.
48 >>>
49 >>> I suppose I can try to do that if that's desirable.
50
51 Note that a .instpreped file could be useful as a means to prevent
52 people from triggering a QA notice about pre-compressed or pre-stripped
53 files if they happen to pass an instprep argument to the ebuild(1)
54 command more than once. Maybe this case is unlikely enough that we can
55 safely neglect it?
56
57 >>>> 3) Currently instprep executes when FEATURES=binpkg-docompress is
58 >>>> enabled, even though it does nothing of value. I think we should instead
59 >>>> generate a relevant list of MiscFunctionsProcess commands for the
60 >>>> enabled FEATURES, and only start a MiscFunctionsProcess instance if the
61 >>>> list of commands is non-empty.
62 >>>
63 >>> That sounds like premature optimization with a bit of going against
64 >>> principle of least surprise. Given it's a phase function with specific
65 >>> implementation that could be extended in the future, I'd rather avoid
66 >>> hiding additional conditions for running it elsewhere, as it opens
67 >>> a strong possibility that somebody adds additional functionality but
68 >>> forgets to update the condition resulting either in immediate WTF
69 >>> or the new code being skipped only for some users, with even harder-to-
70 >>> debug WTF.
71 >>
72 >> I wasn't really sure that instprep deserved to be a full-fledged phase
73 >> at this point. I guess you're planning to add more stuff there?
74 >
75 > At least stripping. But as usually happens, others may also have more
76 > ideas. I suppose the main use case would be stuff that needs to happen
77 > after install but isn't strictly necessary for building binary packages.
78
79 Sure, delayed strip will be a useful feature.
80 --
81 Thanks,
82 Zac

Attachments

File name MIME type
signature.asc application/pgp-signature

Replies