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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion mock/py/mockbuild/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

success_file = os.path.join(resultdir, 'success')
build_ret_code = 0
try:
Expand Down
16 changes: 11 additions & 5 deletions mock/py/mockbuild/buildroot.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def wrapper(*args, **kwargs):


class Buildroot(object):
# pylint: disable=too-many-public-methods,too-many-instance-attributes
@traceLog()
def __init__(self, config, uid_manager, state, plugins, bootstrap_buildroot=None, is_bootstrap=False):
self.config = config
Expand Down Expand Up @@ -213,13 +214,18 @@ def _setup_basedir(self):
os.chmod(self.basedir, 0o775)

@traceLog()
def _setup_result_dir(self):
def create_resultdir(self):
"""
(re)create self.resultdir directory with appropriate permissions
"""
self._setup_basedir()
with self.uid_manager:
try:
file_util.mkdirIfAbsent(self.resultdir)
except Error:
raise ResultDirNotAccessible(ResultDirNotAccessible.__doc__ % self.resultdir)
except Error as err:
raise ResultDirNotAccessible(
ResultDirNotAccessible.__doc__ % self.resultdir
) from err


@traceLog()
Expand Down Expand Up @@ -300,7 +306,7 @@ def _init(self, prebuild):
# intentionally we do not call bootstrap hook here - it does not have sense
self._setup_nosync()
self.chroot_was_initialized = self.chroot_is_initialized()
self._setup_result_dir()
self.create_resultdir()
getLog().info("calling preinit hooks")
self.plugins.call_hooks('preinit')
# intentionally we do not call bootstrap hook here - it does not have sense
Expand Down Expand Up @@ -629,7 +635,7 @@ def resetLogging(self, force=False):
return
self.logging_initialized = True

self._setup_result_dir()
self.create_resultdir()

with self.uid_manager:
# attach logs to log files.
Expand Down
4 changes: 2 additions & 2 deletions mock/py/mockbuild/plugins/chroot_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

# our imports
from mockbuild.trace_decorator import getLog, traceLog
from mockbuild import file_util, util
from mockbuild import util

requires_api_version = "1.1"

Expand Down Expand Up @@ -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

count = 0
logger = getLog()
logger.debug("chroot_scan: Starting scan of %s", chroot)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Several internal code locations attempt to ensure that the result directory
exists, creating it if necessary. However, these locations handled it
inconsistently, sometimes neglecting to [change the ownership][issue#1467] of
the result directory. Now, all locations use a single method dedicated to
result directory preparation.
Loading