-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
elbepack: cdroms: use source name of pkgs with on_src_cd attribute #419
base: master
Are you sure you want to change the base?
elbepack: cdroms: use source name of pkgs with on_src_cd attribute #419
Conversation
6ca1810
to
cbe69cf
Compare
elbepack/cdroms.py
Outdated
@@ -56,6 +57,12 @@ def mk_source_cdrom(components, codename, | |||
for component in components.keys(): | |||
rfs, cache, pkg_lst = components[component] | |||
logging.info('Adding %s component', component) | |||
|
|||
forbiddenSrcPackages = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a set()
then the duplicate entry check is not necessary
elbepack/cdroms.py
Outdated
@@ -56,6 +57,12 @@ def mk_source_cdrom(components, codename, | |||
for component in components.keys(): | |||
rfs, cache, pkg_lst = components[component] | |||
logging.info('Adding %s component', component) | |||
|
|||
forbiddenSrcPackages = [] | |||
for name, _ in cache.get_corresponding_source_packages(forbiddenPackages): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem correct.
If multiple packages share the same Build-Using
and only one of the binary packages is excluded from the source cdrom, then that exclusion will override all others.
This is especially problematic if some base Debian packages have been used to build some excluded package.
The correct solution seems to need to go away from "on_src_cd" to some external explicit list.
However as a good-enough solution it should be fine to add a (keyword only) parameter to get_corresponding_source_packages()
to inhibit Build-Using
handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I didn't had in mind, that the build dependencies are then also excluded.
And I agree a separate list would make the things more visible.
I'm not 100% sure if I understand the proposed good-enough solution correctly.
You mean something like the following approach?
forbidden_src_packages = set()
for name, _ in cache.get_corresponding_source_packages(forbidden_packages, parse_build_depends=False):
forbidden_src_packages.add(name)
def get_corresponding_source_packages(cache, pkg_lst=None, parse_build_depends=True):
if pkg_lst is None:
pkg_lst = {p.name for p in cache if p.is_installed}
src_set = set()
for pkg in pkg_lst:
if isinstance(pkg, str):
version = cache[pkg].installed or cache[pkg.candidate]
elif isinstance(pkg, PackageBase):
version = cache[pkg.name].versions[pkg.installed_version]
src_set.add((version.source_name, version.source_version))
if parse_build_depends:
for name, ver in parse_built_using(version.record.get('Built-Using')):
src_set.add((name, ver))
return list(src_set)
def get_corresponding_source_packages(self, pkg_lst=None, parse_build_depends=True):
return get_corresponding_source_packages(self.cache, pkg_lst, parse_build_depends)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean something like the following approach?
Yes, but:
- Name the new argument
include_built_using
- Make it kwargs only
(self, pkg_lst=None, *, include_built_using=True)
- The new method shouldn't be necessary
elbepack/cdroms.py
Outdated
@@ -56,6 +57,12 @@ def mk_source_cdrom(components, codename, | |||
for component in components.keys(): | |||
rfs, cache, pkg_lst = components[component] | |||
logging.info('Adding %s component', component) | |||
|
|||
forbiddenSrcPackages = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you are at it, please rename it to snake_case.
Please also add |
Resolve the corresponding source name of the binary packages where the XML attribute on_src_cd="False" is set. Until now the binary name was used, which doesn't work when the source name differs to the binary name of the package. The forbidden_packages list, containing the binary package names will be resolved and the corresponding source package names are stored in the forbidden_src_packages list which is then used for the comparison inside the add_source_pkg function. Signed-off-by: Martin Hoehn <[email protected]>
cbe69cf
to
4688847
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I'll also test this a bit and include it in the next release.
Great thanks! |
Resolve the corresponding source name of the binary packages where the XML attribute on_src_cd="False" is set.
Until now the binary name was used, which doesn't work when the source name differs to the binary name of the package. The forbiddenPackages list, containing the binary package names will be resolved and the corresponding source package names are stored in the forbiddenSrcPackages list which is then used for the comparison inside the add_source_pkg function.