On 11/02/2018 12:35 PM, Michał Górny wrote:
> On Fri, 2018-11-02 at 12:22 -0700, Zac Medico wrote:
>> 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:
>>>>> 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.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. >>> 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. 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? >>>> 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. -- Thanks, Zac
signature.asc
Description: OpenPGP digital signature
