Skip to content
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

Fix the permissions for (re)created resultdir #1468

Merged

Conversation

praiskup
Copy link
Member

Fixes: #1467

This is already handled by the preceding resetLogging() call.

Relates: rpm-software-management#1467
@praiskup praiskup force-pushed the praiskup-resultdir-permissions branch from 48217f1 to d690bca Compare September 26, 2024 08:11
To make the fix possible, we needed to make the result directory
creation method public.

Fixes: rpm-software-management#1467
@praiskup praiskup force-pushed the praiskup-resultdir-permissions branch from d690bca to a0155c5 Compare September 26, 2024 08:18
@praiskup
Copy link
Member Author

I plan to merge this early and wrap the release (once the CI gets green). Please comment ASAP.

@praiskup praiskup added the release-blocker This needs to be shipped in the next release label Sep 26, 2024
@@ -477,7 +477,6 @@ def chain(self, args, options, buildroot):
resultdir = os.path.join(self.config['local_repo_dir'], pdn)
self.buildroot.resultdir = resultdir
self.buildroot.resetLogging(force=True)
file_util.mkdirIfAbsent(resultdir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uff. I'm not sure about this. But yeah, in theory it should not be needed. Let be brave and do it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# ensure we dont attach the handlers multiple times.
if self.logging_initialized and not force:
return
self.logging_initialized = True
self._setup_result_dir()

This is the force=True case, I think we are safe.

Copy link
Member

@xsuchy xsuchy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@praiskup praiskup mentioned this pull request Sep 26, 2024
@praiskup praiskup merged commit aa9a5b0 into rpm-software-management:main Sep 26, 2024
21 checks passed
@@ -56,7 +56,7 @@ def __scanChroot(self):
regexstr = "|".join(self.scan_opts['regexes'])
regex = re.compile(regexstr)
chroot = self.buildroot.make_chroot_path()
file_util.mkdirIfAbsent(self.resultdir)
self.buildroot.create_resultdir()
Copy link
Member Author

@praiskup praiskup Sep 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh! While it was causing issues before, now it is broken even more -> because self.resultdir is not serl.buildroot.resultdir, nah. chroot_scan is broken in 5.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker This needs to be shipped in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mock resultdir created by chroot_scan plugin has weird permissions
2 participants