-
-
Notifications
You must be signed in to change notification settings - Fork 574
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
Add dependencies to rpm #4084
base: develop
Are you sure you want to change the base?
Add dependencies to rpm #4084
Conversation
f9c5a17
to
7b69733
Compare
@pombredanne please review this PR, also all test cases have been successfully executed and passed. |
1c0e2b1
to
9bb48db
Compare
Added dependencies to rpm. Reference: aboutcode-org#649 Signed-off-by: Alok Kumar <[email protected]> Signed-off-by: Alok Kumar <[email protected]>
9bb48db
to
e16372a
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 for this effort. Please see comments for feedback:
- This code cannot and does not work as it is: .rpm archives and installed RPM dbs are not the same.
- There is a lot of code duplication
- A package cannot be its own dependency
- Please start by crafting proper tests first so you can set expectations with carefully reviewed cases.
) | ||
|
||
# Prepare the dependent package model | ||
dependencies.append( |
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.
Since when there is a single dependency?
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.
@pombredanne Can you suggest how I proceed to find multiple dependencies, I tried by Adding RPMTAG_REQUIRES and RPMTAG_REQUIREVERSION in RPMtags in pyrpm.py but in RPMTAG_REQUIRES I got this like eg: ['/bin/sh', '/bin/sh', '/bin/sh', '/bin/sh', '/bin/sh', 'rpmlib(PayloadFilesHavePrefix)', 'rpmlib(CompressedFileNames)', 'rpmlib(PayloadIsBzip2)']
corresponding I got require_version=[None, None, None, None, None, '4.0-1', '3.0.4-1', '3.0.5-1']
source: http://ftp.rpm.org/max-rpm/ch-queryformat-tags.html (For rpm tages)
can you tell how I proceed to find out their package name and their version, in rmp_requires, these represent capabilities or libraries, not actual packages
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.
Since when there is a single dependency?
I do for single dependency only.
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.
I do for single dependency only.
This cannot work. We need all of them.
- You can use "get_rpm_tags" only for a .rpm file, but then you do not need to call it because this is already called
- for installed RPM databases, you can use exclusively the data in
parse_rpm_xmlish
https://github.com/aboutcode-org/scancode-toolkit/pull/4084/files#diff-17fc898047e57c63a40599213e50fb03f8d853b60ea7790133ace1c6ab2b9709R170 and if it does not return what you need,parse_rpm_xmlish
needs to be updated in https://github.com/aboutcode-org/scancode-toolkit/blob/develop/src/packagedcode/rpm_installed.py. Look at require*, provide* and conflict* for instance in https://github.com/aboutcode-org/scancode-toolkit/edit/develop/tests/packagedcode/data/rpm_installed/distro-xmlish/rhel-rpms.xmlish
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.
@alok1304 you cannot use get_rpm_tags for installed packages. And its dope not make sense to collect "self" as a dependency and to further consider that there is only one dependency which is the package itself.
See also my comments inline
) | ||
|
||
# Prepare the dependent package model | ||
dependencies.append( |
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.
I do for single dependency only.
This cannot work. We need all of them.
- You can use "get_rpm_tags" only for a .rpm file, but then you do not need to call it because this is already called
- for installed RPM databases, you can use exclusively the data in
parse_rpm_xmlish
https://github.com/aboutcode-org/scancode-toolkit/pull/4084/files#diff-17fc898047e57c63a40599213e50fb03f8d853b60ea7790133ace1c6ab2b9709R170 and if it does not return what you need,parse_rpm_xmlish
needs to be updated in https://github.com/aboutcode-org/scancode-toolkit/blob/develop/src/packagedcode/rpm_installed.py. Look at require*, provide* and conflict* for instance in https://github.com/aboutcode-org/scancode-toolkit/edit/develop/tests/packagedcode/data/rpm_installed/distro-xmlish/rhel-rpms.xmlish
Yes, I got it. I will sure see this and get some idea of this. |
@pombredanne I checked .rpm(rpm_archive) for dependencies, there are four things in dependencies.
what I do, I add this in RPMTAGS in pyrmp.py
is this right? |
Added dependencies to rpm.
Reference: #649
Fixes #649
Tasks
Run tests locally to check for errors.
Signed-off-by: Alok Kumar [email protected]