-
Notifications
You must be signed in to change notification settings - Fork 89
import_srpm: check command rpm2cpio|cpio #742
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
base: master
Are you sure you want to change the base?
Conversation
`rpm2cpio` might not exist on the system, and `cpio` return the error "premature end of archive" in that case. This doesn't check if `rpm2cpio` returns an error but it's probably ok to lean on `cpio` returning an error. Also, check early if `rpm2cpio` actually exist on the system. Signed-off-by: Anthony PERARD <[email protected]>
69af6cc to
4ab8a7a
Compare
|
|
||
| os.chdir('SOURCES') | ||
| os.system('rpm2cpio "%s" | cpio -idmv' % source_rpm_abs) | ||
| call_process(['sh', '-c', 'rpm2cpio "%s" | cpio -idmv' % source_rpm_abs]) |
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.
| call_process(['sh', '-c', 'rpm2cpio "%s" | cpio -idmv' % source_rpm_abs]) | |
| call_process(['sh', '-o', pipefail', '-c', 'rpm2cpio "%s" | cpio -idmv' % source_rpm_abs]) |
otherwise it won't fail if rpm2cpio fails.
Or we can use #743 — I didn't see there was already a PR for that problem 😅
It's in pure Python, but it's a much bigger change.
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'm sorry, pipefail is a bash option, it doesn't exist on posix shell. I've actually look before writing my commit message, saying that this ignores error from rpm2cpio.
We don't know what implementation is used for sh, it might be dash on Debian, or bash on Arch Linux.
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.
Sorry, I didn't read your comment carefully enough.
Too bad, I was liking your version better than mine, for its conciseness.
It's probably better to go with #743 then.
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 you're calling sh explicitly anyway you can just replace with /bin/bash right? Unless calling out bash is unwanted.
| try: | ||
| call_process(['sh', '-c', 'command -v rpm2cpio']) | ||
| except subprocess.CalledProcessError: |
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.
| try: | |
| call_process(['sh', '-c', 'command -v rpm2cpio']) | |
| except subprocess.CalledProcessError: | |
| if shutil.which('rpm2cpio') is None: |
just the same, but with just python 🙂
|
@tperard It seems to me that there are unsolved comments left on this PR of yours. |
rpm2cpiomight not exist on the system, andcpioreturn the error "premature end of archive" in that case.This doesn't check if
rpm2cpioreturns an error but it's probably ok to lean oncpioreturning an error.Also, check early if
rpm2cpioactually exist on the system.