Re: [gentoo-portage-dev] [PATCH] Add @changed-subslot package set

[Zac Medico]

On 1/18/21 8:42 PM, Alec Warner wrote:
> On Mon, Jan 18, 2021 at 8:09 PM Zac Medico <zmedico@g.o> wrote:
>>
>> On 1/18/21 6:07 PM, Alec Warner wrote:
>>> On Fri, Jan 15, 2021 at 6:47 PM Matt Turner <mattst88@g.o> wrote:
>>>>
>>>> This set is the upgradable packages for which the highest visible
>>>> version has a different subslot than the currently installed version.
>>>>
>>>> The primary purpose of this feature is for use in catalyst builds. We
>>>> update the "seed" stage3 before using it to build a new stage1.
>>>>
>>>> Updating the entire stage is expensive and unnecessary (since we're
>>>> going to build the latest packages in stage1 and then rebuild everything
>>>> in stage3).
>>>>
>>>> What we definitely do need to update in the original stage3 however, is
>>>> any package that would trigger a subslot rebuild.
>>>>
>>>> For example: gcc links with libmpfr.so from dev-libs/mpfr. mpfr's SONAME
>>>> changes from libmpfr.so.4 (SLOT="0/4") to libmpfr.so.6 (SLOT="0/6"). If
>>>> the seed stage's dev-libs/mpfr is not updated before emerging gcc, gcc
>>>> will link with libmpfr.so.4, but the latest version of dev-libs/mpfr
>>>> will be built and libmpfr.so.6 included into the stage1. Since the old
>>>> libmpfr.so.4 is not included in the stage1, gcc will not work, breaking
>>>> subsequent stage builds.
>>>>
>>>> Our current options to update the seed are too large a hammer (e.g.,
>>>> "--update --deep --newuse @world" or "--update --deep --newuse
>>>> --complete-graph --rebuild-if-new-ver gcc") and spend too much time
>>>> updating seed stages for no gain beyond updating only packages for whom
>>>> the subslot has changed.
>>>>
>>>> With this set, catalyst will likely use
>>>>
>>>>         emerge @changed-subslot --ignore-built-slot-operator-deps y
>>>>
>>>> to update the seed stage.
>>>>
>>>> Thank you to Zac Medico for showing me how to do this.
>>>>
>>>> Bug: https://bugs.gentoo.org/739004
>>>> Signed-off-by: Matt Turner <mattst88@g.o>
>>>> ---
>>>>  cnf/sets/portage.conf      |  5 +++++
>>>>  lib/portage/_sets/dbapi.py | 39 +++++++++++++++++++++++++++++++++++++-
>>>>  2 files changed, 43 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/cnf/sets/portage.conf b/cnf/sets/portage.conf
>>>> index 22f0fa3a5..5651a9c53 100644
>>>> --- a/cnf/sets/portage.conf
>>>> +++ b/cnf/sets/portage.conf
>>>> @@ -84,6 +84,11 @@ exclude-files = /usr/bin/Xorg
>>>>  [rebuilt-binaries]
>>>>  class = portage.sets.dbapi.RebuiltBinaries
>>>>
>>>> +# Installed packages for which the subslot of the highest visible ebuild
>>>> +# version is different than the currently installed version.
>>>> +[changed-subslot]
>>>> +class = portage.sets.dbapi.SubslotChangedSet
>>>> +
>>>>  # Installed packages for which the highest visible ebuild
>>>>  # version is lower than the currently installed version.
>>>>  [downgrade]
>>>> diff --git a/lib/portage/_sets/dbapi.py b/lib/portage/_sets/dbapi.py
>>>> index 52367c4a6..46ba5c17d 100644
>>>> --- a/lib/portage/_sets/dbapi.py
>>>> +++ b/lib/portage/_sets/dbapi.py
>>>> @@ -15,7 +15,7 @@ from portage._sets import SetConfigError, get_boolean
>>>>  import portage
>>>>
>>>>  __all__ = ["CategorySet", "ChangedDepsSet", "DowngradeSet",
>>>> -       "EverythingSet", "OwnerSet", "VariableSet"]
>>>> +       "EverythingSet", "OwnerSet", "SubslotChangedSet", "VariableSet"]
>>>>
>>>>  class EverythingSet(PackageSet):
>>>>         _operations = ["merge"]
>>>> @@ -167,6 +167,43 @@ class VariableSet(EverythingSet):
>>>>
>>>>         singleBuilder = classmethod(singleBuilder)
>>>>
>>>> +class SubslotChangedSet(PackageSet):
>>>> +
>>>> +       _operations = ["merge", "unmerge"]
>>>> +
>>>> +       description = "Package set which contains all packages " + \
>>>> +               "for which the subslot of the highest visible ebuild is " + \
>>>> +               "different than the currently installed version."
>>>
>>> description = ("string1",
>>>                        "string2",
>>>                        "string3")
>>>
>>> vs concat + \ for line continuation?
>>>
>>> -A
>>
>> We also got flak on irc about the classmethod(singleBuilder) usage as
>> opposed to @classmethod. This package set is formatted exactly like
>> others in the file, so it's just a copy / paste thing.
>
> Ack, I can send a patch. I don't watch all these CLs.
>
> PEP8 is fairly clear about line continuations (\) and whey are
> appropriate (e.g. not here.)
> I don't think the pep cares about classmethod() vs @classmethod (and I
> don't really care either.)
>
> I personally find:
>
> a = "foo" + " bar" + \
>     "baz"
>
> jarring; as compared to:
>
> a = ("foo", "bar",
>     "baz")
>

Me too, parenthesis look much better.

> but then you have to know that python will implicitly join these
> strings together, which is unfortunate, butl I attempt to write python
> assuming readers know python (so I'm not worried about readers knowing
> implicit string join.) There are probably 5 other ways to write this
> block (""" strings, ''.join(), string.format(), some other stuff.)

Implicit join works for me. Let's assume readers know python or will
learn it.

>>
>> On the topic of python formatting, maybe we should use something like
>> https://github.com/psf/black to automate it?
>
> I've used black before, my recollection was that you can't disable
> stuff, and this leads to situations where black and other tools
> disagree about a thing, and then you have to stop the tools from
> fighting.
> This is mentioned in the black readme (e.g. black often fights with
> isort.)

I think black is more valuable than isort, so in the worst case I'd
choose to disable isort (and we're not even using it yet).

> It also means we need to send a giant set of patches to format
> with black...I'd like to think we have enough test coverage for this
> (after the previous linting changes.)

Yeah I think the test coverage is fine for this.

> I'm not opposed, just understand
> that as the human doing most of the merges, you might end up being the
> human also making the tools not fight ;)

I'm willing to struggle with it a bit, but yeah if it becomes too
painful then my plan would be to disable isort.
-- 
Thanks,
Zac