-
Notifications
You must be signed in to change notification settings - Fork 119
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
Don't mask rpm2cpio failure in Pkg._extract_rpm() #1266
Changes from all commits
42cdb0d
088ced3
c404f3d
ffe4b2c
cd5f890
ce08677
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -621,7 +621,7 @@ | |
subprocess.check_output('rpm2archive - | tar -xz && chmod -R +rX .', shell=True, env=ENGLISH_ENVIRONMENT, | ||
stderr=stderr, stdin=rpm_data) | ||
else: | ||
command_str = f'rpm2cpio {quote(str(filename))} | cpio -id ; chmod -R +rX .' | ||
command_str = f'rpm2cpio {quote(str(filename))} | cpio -id && chmod -R +rX .' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes the tests fails on tumbleweed:
This is because It's true that this is hiding the real problem, but it could be good to fix the tests at the same time we do this change. I need to investigate a bit more about it. These are the broken test packages right now:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. Any idea how to fix the problem on Tumbleweed? Also, I forgot to add context about how I discovered this problem: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into the failures and it seems that cpio tries to extract a file to a directory that doesn't have the
I believe rpm workarounds this by setting file perms after they're all extracted. I think all these test failures should be marked as expected failures. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that should be the expected result. So I think this is correct, but I want to fix those tests that are failing. Migrate to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option is just to skip these tests with I'm working on the tests migration, with the help of GSoC intern, so it's something that I'll need to check soon. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed additional commits that modify the tests. EDIT: the last commit will definitely require some changes, mocking |
||
subprocess.check_output(command_str, shell=True, env=ENGLISH_ENVIRONMENT, stderr=stderr) | ||
self.extracted = True | ||
return dirname | ||
|
Check warning
Code scanning / CodeQL
Unsafe shell command constructed from library input Medium