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

[Alec Warner]

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")

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.)

>
> 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.) 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.) 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 ;)

-A

> --
> Thanks,
> Zac
>