elbepack: aptpkgutils: implement recursive package blacklisting in diet builds #453
Conversation
elbepack/aptpkgutils.py
Outdated
|
|
||
|
|
||
| def getalldeps(c, pkgname): | ||
| def getalldeps(c, pkgname, blacklist=[]): |
There was a problem hiding this comment.
Mutable default arguments are problematic, as any mutation will persist between function invocations.
While this won't happen here, it is still a red flag.
Use an empty tuple () instead.
elbepack/schema/dbsfed.xsd
Outdated
| <element name="target" type="rfs:pkg-list" minOccurs="0" maxOccurs="unbounded"> | ||
| <annotation> | ||
| <documentation> | ||
| avoid installing the specified packages into the target |
There was a problem hiding this comment.
Please mention that it only works for diet builds.
|
|
||
| if xml.tgt.has('diet'): | ||
| if xml.has('target/pkg-blacklist/'): | ||
| blacklist = [p.et.text for p in xml.node('target/pkg-blacklist/target')] |
There was a problem hiding this comment.
Instead of silently ignoring the entry for non-diet builds, please throw an error instead.
| for p in pkglist: | ||
| deps = cache.get_dependencies(p) | ||
| if p in blacklist: | ||
| continue |
There was a problem hiding this comment.
Instead of having multiple places checking the blacklist, IMO it would be cleaner to move all the blacklist checking into cache.get_dependencies() by also having it return the original package and drop the withdeps += [p] below.
|
Hello @t-8ch, thank you for the review. Sadly tighten remove all dependencies for all packages. My use case is specifically for packages coming from an unofficial source (ROS2 packages). Those packages may be incorrectly written and, in my case, tend to use the This result in the fact that downloading a ROS package for a Lidar will download all the useless debugging and visualization tools for it, Qt packages to run those apps and a bunch of display related packages. Using the tighten directive would force me to recursively search by hand for each and every needed dependencies for all my official packages which would be a painful step just to fix a bad package. |
|
I see. On the other hand having to maintain special logic in ELBE only to account for those broken packages is undesirable from my perspective. |
Understandable, however I think that my use case is one of a long list of potential ones for this feature.
Indeed, I didn't report the issue, it is probably linked to the way ROS automatically creates debian packages. |
This reverts commit 98a0c58.
Taumille
force-pushed
the
package_blacklisting
branch
2 times, most recently
from
cbcc124 to
53c4e2c
Compare
So far non of them have come up :-)
Implementing the feature won't be that big of a problem. Having to maintain it however, I'd like to avoid.
Please do report it. |
Ok, I understand, so do you want me to close the PR ? or are you interested in the feature ?
I will, (first I need to understand in depth why it exists) |
Leave it open for now, please.
Maintaining it properly would require the addition of new testcases. And these need to be written and then run all the time. |
If needed, I can spend some times creating a test case and include it in the PR |
…et builds Implements package blacklisting for diet builds, this allows the user to create a list of packages to exclude from the target. This will recursively avoid installing a package and all its dependencies on the target even if this package comes as a dependency of another package. This must be carefully used as it can create packages with lacking dependencies. Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
Taumille
force-pushed
the
package_blacklisting
branch
from
53c4e2c to
da20aba
Compare
2 participants
Implements package blacklisting for diet builds, this allows the user to create a list of packages to exclude from the target. This will recursively avoid installing a package and all its dependencies on the target even if this package comes as a dependency of another package.
This must be carefully used as it can create packages with lacking dependencies.
This pull-request conflict with #451 that removes the part of the documentation on target package blacklisting