[Buildroot] [PATCH 2/3] graph-depends: split off get_version/get_depends into pkgutil.py
arnout at mind.be
Thu Nov 8 08:01:17 UTC 2018
On 07/11/18 23:45, Yann E. MORIN wrote:
> Arnout, All,
> On 2018-11-07 23:26 +0100, Arnout Vandecappelle spake thusly:
>> On 07/11/18 20:39, Yann E. MORIN wrote:
>>> I'm changing the way the dependency tree is extracted from the Makefile
>>> data, and the function now has this API (function name yet to be
>>> bike-shedded about):
>> Let me take this opportunity to bikeshed on this right away :-)
> If that were not you, then who else? ;-)
>>> dict_deps, dict_types = get_dependency_tree(direction)
>> I like the function name! However...
> Oh come on, that *is* the most important thing! ;-)
>>> - direction is either string 'forward' or 'back',
>> I would make this a boolean argument:
>> def get_dependency_tree(backward: bool = True) -> Dict[str, Package]:
> Just for my own understanding: what syntax is that? I was trying to write
> the protoype of the function, but in Python there is no prototype where
> you can explain the return type. Yours seems like it is what I was
> looking for. Is it described somewhere?
Those are type annotations. They don't do anything by themselves, but they can
be used by other tools/libraries to do static type checking.
Of course, they can't be used in Python 2.ancient so you can't actually use
them in graph-depends, but it's a good way of documenting the intention.
For documentation, I'll refer to PEP 484 .
> (See how I'm trying to sidetrack you into not bikeshedding? ;-))
>> Note however that it is trivial to populate the backward dependencies once you
>> have the forward ones, so I'm not sure its worth it to make the distinction.
> I was also thinking about that too.
>>> - dict_deps is a dictionnary, which keys are the package names, and
>>> which values are lists of packages that are direct dependencies of
>>> the key package (basically, what get_all_depends() currently
>>> returns, but the whole dependency tree)
>>> - dict_types is a dictionnary, which keys are the package names, and
>>> the values are string representing whtehr the packages are target or
>>> host, and virtual or not, e.g.: 'target', 'target-virtual', 'host',
>> I would take this opportunity to create a class Package with members:
>> name: str # Package name including hsot- prefix
>> dependencies: Set[Package] # Forward dependencies
>> dependencies_backwards: Set[Package] # Backward dependencies
>> virtual: bool # If true, it's a virtual package
>> host: bool # If true, it's a host package
>> version: str # Package version
> Meh, I was thinking about exactly the same thing, but it is quite an
> invasive change.
I would think that the invasive change you're about to do is the ideal occasion
to do this one :-)
> But let me bikeshed in turn: I would make your proposed
> 'host' field an enum instead,
Python enums require an external package that we can't assume is installed. But
I suppose you didn't mean an actual enum :-P
> because we don't have only packages, but
> also rootfs. So I would have 'kind' (or 'type' if that's not reserved?)
> which could be one of 'target','host', or 'rootfs'.
Sounds good to me. 'kind' is better than 'type' IMHO.
>> The __init__ would set name, virtual, host, version. The dependencies are added
>> dynamically, and whenever you add one you do it in both directions.
> Dang, I'm always getting side-tracked into side topics. All I wanted was
> a faster graph-depends, and it's already there now! Sob... ;-)
Feel free to ignore my feedback! I'll take progress over perfection any time.
More information about the buildroot