diff options
22 files changed, 132 insertions, 87 deletions
@@ -98,9 +98,6 @@ disable=##################################### # Messages that we would like to enable at some point # ####################################################### - # Overriden methods don't actually override but redefine - arguments-differ, - duplicate-code, # Some invalid names are alright, we should configure pylint diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py index cdef589b7..dce63e14f 100644 --- a/src/buildstream/_frontend/widget.py +++ b/src/buildstream/_frontend/widget.py @@ -253,8 +253,10 @@ class LogFile(Widget): self._err_profile = err_profile self._logdir = context.logdir - def render(self, message, abbrev=True): + def render(self, message): + return self.render_abbrev(message) + def render_abbrev(self, message, abbrev=True): if message.logfile and message.scheduler: logfile = message.logfile @@ -285,7 +287,7 @@ class MessageOrLogFile(Widget): def render(self, message): # Show the log file only in the main start/success messages if message.logfile and message.scheduler and \ - message.message_type in [MessageType.START, MessageType.SUCCESS]: + message.message_type in [MessageType.START, MessageType.SUCCESS]: text = self._logfile_widget.render(message) else: text = self._message_widget.render(message) @@ -709,7 +711,7 @@ class LogLine(Widget): elif self._log_lines > 0: text += self._indent + self._err_profile.fmt("Printing the last {} lines from log file:" .format(self._log_lines)) + '\n' - text += self._indent + self._logfile_widget.render(message, abbrev=False) + '\n' + text += self._indent + self._logfile_widget.render_abbrev(message, abbrev=False) + '\n' text += self._indent + self._err_profile.fmt("=" * 70) + '\n' log_content = self._read_last_lines(message.logfile) diff --git a/src/buildstream/_fuse/fuse.py b/src/buildstream/_fuse/fuse.py index 5c97cc466..41e126ef5 100644 --- a/src/buildstream/_fuse/fuse.py +++ b/src/buildstream/_fuse/fuse.py @@ -453,6 +453,9 @@ class FUSE(object): This gives you access to direct_io, keep_cache, etc. ''' + # Note that in BuildStream we're assuming that raw_fi is always False. + assert not raw_fi, "raw_fi is not supported in BuildStream." + self.operations = operations self.raw_fi = raw_fi self.encoding = encoding @@ -754,15 +757,14 @@ class FUSE(object): fi = fip.contents path = path.decode(self.encoding) - if self.raw_fi: - return self.operations('create', path, mode, fi) - else: - # This line is different from upstream to fix issues - # reading file opened with O_CREAT|O_RDWR. - # See issue #143. - fi.fh = self.operations('create', path, mode, fi.flags) - # END OF MODIFICATION - return 0 + assert not self.raw_fi + + # This line is different from upstream to fix issues + # reading file opened with O_CREAT|O_RDWR. + # See issue #143. + fi.fh = self.operations('create', path, mode, fi.flags) + # END OF MODIFICATION + return 0 def ftruncate(self, path, length, fip): if self.raw_fi: @@ -838,14 +840,7 @@ class Operations(object): def chown(self, path, uid, gid): raise FuseOSError(EROFS) - def create(self, path, mode, fi=None): - ''' - When raw_fi is False (default case), fi is None and create should - return a numerical file handle. - - When raw_fi is True the file handle should be set directly by create - and return 0. - ''' + def create(self, path, mode, flags): raise FuseOSError(EROFS) diff --git a/src/buildstream/_fuse/hardlinks.py b/src/buildstream/_fuse/hardlinks.py index ff2e81eea..798e1c816 100644 --- a/src/buildstream/_fuse/hardlinks.py +++ b/src/buildstream/_fuse/hardlinks.py @@ -100,9 +100,9 @@ class SafeHardlinkOps(Operations): ########################################################### # Fuse Methods # ########################################################### - def access(self, path, mode): + def access(self, path, amode): full_path = self._full_path(path) - if not os.access(full_path, mode): + if not os.access(full_path, amode): raise FuseOSError(errno.EACCES) def chmod(self, path, mode): @@ -161,20 +161,22 @@ class SafeHardlinkOps(Operations): 'f_ffree', 'f_files', 'f_flag', 'f_frsize', 'f_namemax')) def unlink(self, path): - return os.unlink(self._full_path(path)) + os.unlink(self._full_path(path)) - def symlink(self, name, target): - return os.symlink(target, self._full_path(name)) + def symlink(self, target, source): + 'creates a symlink `target -> source` (e.g. ln -s source target)' + return os.symlink(source, self._full_path(target)) def rename(self, old, new): return os.rename(self._full_path(old), self._full_path(new)) - def link(self, target, name): + def link(self, target, source): + 'creates a hard link `target -> source` (e.g. ln source target)' # When creating a hard link here, should we ensure the original # file is not a hardlink itself first ? # - return os.link(self._full_path(name), self._full_path(target)) + return os.link(self._full_path(source), self._full_path(target)) def utimens(self, path, times=None): return os.utime(self._full_path(path), times) @@ -195,13 +197,13 @@ class SafeHardlinkOps(Operations): self._ensure_copy(full_path) return os.open(full_path, flags, mode) - def read(self, path, length, offset, fh): + def read(self, path, size, offset, fh): os.lseek(fh, offset, os.SEEK_SET) - return os.read(fh, length) + return os.read(fh, size) - def write(self, path, buf, offset, fh): + def write(self, path, data, offset, fh): os.lseek(fh, offset, os.SEEK_SET) - return os.write(fh, buf) + return os.write(fh, data) def truncate(self, path, length, fh=None): full_path = self._full_path(path) @@ -214,5 +216,5 @@ class SafeHardlinkOps(Operations): def release(self, path, fh): return os.close(fh) - def fsync(self, path, fdatasync, fh): + def fsync(self, path, datasync, fh): return self.flush(path, fh) diff --git a/src/buildstream/_gitsourcebase.py b/src/buildstream/_gitsourcebase.py index 7f02af40d..1f539d820 100644 --- a/src/buildstream/_gitsourcebase.py +++ b/src/buildstream/_gitsourcebase.py @@ -118,7 +118,7 @@ class _GitMirror(SourceFetcher): fail_temporarily=True, cwd=self.mirror) - def fetch(self, alias_override=None): + def fetch(self, alias_override=None): # pylint: disable=arguments-differ # Resolve the URL for the message resolved_url = self.source.translate_url(self.url, alias_override=alias_override, @@ -470,8 +470,8 @@ class _GitSourceBase(Source): def get_ref(self): return self.mirror.ref, self.mirror.tags - def set_ref(self, ref_data, node): - if not ref_data: + def set_ref(self, ref, node): + if not ref: self.mirror.ref = None if 'ref' in node: del node['ref'] @@ -479,8 +479,8 @@ class _GitSourceBase(Source): if 'tags' in node: del node['tags'] else: - ref, tags = ref_data - node['ref'] = self.mirror.ref = ref + actual_ref, tags = ref + node['ref'] = self.mirror.ref = actual_ref self.mirror.tags = tags if tags: node['tags'] = [] @@ -493,7 +493,7 @@ class _GitSourceBase(Source): if 'tags' in node: del node['tags'] - def track(self): + def track(self): # pylint: disable=arguments-differ # If self.tracking is not specified it's not an error, just silently return if not self.tracking: diff --git a/src/buildstream/_options/optionarch.py b/src/buildstream/_options/optionarch.py index 3117b8273..e7735eaa2 100644 --- a/src/buildstream/_options/optionarch.py +++ b/src/buildstream/_options/optionarch.py @@ -40,7 +40,7 @@ class OptionArch(OptionEnum): OPTION_TYPE = 'arch' def load(self, node): - super().load(node, allow_default_definition=False) + super().load_special(node, allow_default_definition=False) def load_default_value(self, node): arch = Platform.get_host_arch() diff --git a/src/buildstream/_options/optioneltmask.py b/src/buildstream/_options/optioneltmask.py index 507dc7070..178999fa1 100644 --- a/src/buildstream/_options/optioneltmask.py +++ b/src/buildstream/_options/optioneltmask.py @@ -33,7 +33,7 @@ class OptionEltMask(OptionFlags): def load(self, node): # Ask the parent constructor to disallow value definitions, # we define those automatically only. - super().load(node, allow_value_definitions=False) + super().load_special(node, allow_value_definitions=False) # Here we want all valid elements as possible values, # but we'll settle for just the relative filenames diff --git a/src/buildstream/_options/optionenum.py b/src/buildstream/_options/optionenum.py index f214d779d..889db965c 100644 --- a/src/buildstream/_options/optionenum.py +++ b/src/buildstream/_options/optionenum.py @@ -30,7 +30,14 @@ class OptionEnum(Option): OPTION_TYPE = 'enum' - def load(self, node, allow_default_definition=True): + def __init__(self, name, definition, pool): + self.values = None + super().__init__(name, definition, pool) + + def load(self, node): + self.load_special(node) + + def load_special(self, node, allow_default_definition=True): super().load(node) valid_symbols = OPTION_SYMBOLS + ['values'] diff --git a/src/buildstream/_options/optionflags.py b/src/buildstream/_options/optionflags.py index ba16244ba..eba3a8dd5 100644 --- a/src/buildstream/_options/optionflags.py +++ b/src/buildstream/_options/optionflags.py @@ -30,7 +30,14 @@ class OptionFlags(Option): OPTION_TYPE = 'flags' - def load(self, node, allow_value_definitions=True): + def __init__(self, name, definition, pool): + self.values = None + super().__init__(name, definition, pool) + + def load(self, node): + self.load_special(node) + + def load_special(self, node, allow_value_definitions=True): super().load(node) valid_symbols = OPTION_SYMBOLS + ['default'] diff --git a/src/buildstream/_options/optionos.py b/src/buildstream/_options/optionos.py index 6263a05a2..fcf4552f5 100644 --- a/src/buildstream/_options/optionos.py +++ b/src/buildstream/_options/optionos.py @@ -29,7 +29,7 @@ class OptionOS(OptionEnum): OPTION_TYPE = 'os' def load(self, node): - super().load(node, allow_default_definition=False) + super().load_special(node, allow_default_definition=False) def load_default_value(self, node): return platform.uname().system diff --git a/src/buildstream/storage/_casbaseddirectory.py b/src/buildstream/storage/_casbaseddirectory.py index 94e6e09c7..29aee7ad3 100644 --- a/src/buildstream/storage/_casbaseddirectory.py +++ b/src/buildstream/storage/_casbaseddirectory.py @@ -394,16 +394,16 @@ class CasBasedDirectory(Directory): return result - def import_single_file(self, srcpath): + def import_single_file(self, external_pathspec): result = FileListResult() - if self._check_replacement(os.path.basename(srcpath), - os.path.dirname(srcpath), + if self._check_replacement(os.path.basename(external_pathspec), + os.path.dirname(external_pathspec), result): - self._add_file(os.path.dirname(srcpath), - os.path.basename(srcpath), - modified=os.path.basename(srcpath) + self._add_file(os.path.dirname(external_pathspec), + os.path.basename(external_pathspec), + modified=os.path.basename(external_pathspec) in result.overwritten) - result.files_written.append(srcpath) + result.files_written.append(external_pathspec) return result def set_deterministic_mtime(self): @@ -507,10 +507,25 @@ class CasBasedDirectory(Directory): if i and i.modified: yield p - def list_relative_paths(self, relpath=""): + def list_relative_paths(self): """Provide a list of all relative paths. - Return value: List(str) - list of all paths + Yields: + (List(str)) - list of all files with relative paths. + + """ + yield from self._list_prefixed_relative_paths() + + def _list_prefixed_relative_paths(self, prefix=""): + """Provide a list of all relative paths. + + Arguments: + prefix (str): an optional prefix to the relative paths, this is + also emitted by itself. + + Yields: + (List(str)) - list of all files with relative paths. + """ file_list = list(filter(lambda i: i[1].type != _FileType.DIRECTORY, @@ -518,15 +533,15 @@ class CasBasedDirectory(Directory): directory_list = filter(lambda i: i[1].type == _FileType.DIRECTORY, self.index.items()) - if relpath != "": - yield relpath + if prefix != "": + yield prefix for (k, v) in sorted(file_list): - yield os.path.join(relpath, k) + yield os.path.join(prefix, k) for (k, v) in sorted(directory_list): subdir = v.get_directory(self) - yield from subdir.list_relative_paths(relpath=os.path.join(relpath, k)) + yield from subdir._list_prefixed_relative_paths(prefix=os.path.join(prefix, k)) def get_size(self): digest = self._get_digest() diff --git a/src/buildstream/storage/_filebaseddirectory.py b/src/buildstream/storage/_filebaseddirectory.py index 90911da86..233b56a09 100644 --- a/src/buildstream/storage/_filebaseddirectory.py +++ b/src/buildstream/storage/_filebaseddirectory.py @@ -112,13 +112,13 @@ class FileBasedDirectory(Directory): os.utime(os.path.join(self.external_directory, f), times=(cur_time, cur_time)) return import_result - def import_single_file(self, srcpath): - dstpath = os.path.join(self.external_directory, os.path.basename(srcpath)) + def import_single_file(self, external_pathspec): + dstpath = os.path.join(self.external_directory, os.path.basename(external_pathspec)) result = FileListResult() if os.path.exists(dstpath): result.ignored.append(dstpath) else: - shutil.copyfile(srcpath, dstpath, follow_symlinks=False) + shutil.copyfile(external_pathspec, dstpath, follow_symlinks=False) return result def _mark_changed(self): @@ -153,23 +153,23 @@ class FileBasedDirectory(Directory): # First, it sorts the results of os.listdir() to ensure the ordering of # the files in the archive is the same. Second, it sets a fixed # timestamp for each entry. See also https://bugs.python.org/issue24465. - def export_to_tar(self, tf, dir_arcname, mtime=_magic_timestamp): + def export_to_tar(self, tarfile, destination_dir, mtime=_magic_timestamp): # We need directories here, including non-empty ones, # so list_relative_paths is not used. for filename in sorted(os.listdir(self.external_directory)): source_name = os.path.join(self.external_directory, filename) - arcname = os.path.join(dir_arcname, filename) - tarinfo = tf.gettarinfo(source_name, arcname) + arcname = os.path.join(destination_dir, filename) + tarinfo = tarfile.gettarinfo(source_name, arcname) tarinfo.mtime = mtime if tarinfo.isreg(): with open(source_name, "rb") as f: - tf.addfile(tarinfo, f) + tarfile.addfile(tarinfo, f) elif tarinfo.isdir(): - tf.addfile(tarinfo) - self.descend(*filename.split(os.path.sep)).export_to_tar(tf, arcname, mtime) + tarfile.addfile(tarinfo) + self.descend(*filename.split(os.path.sep)).export_to_tar(tarfile, arcname, mtime) else: - tf.addfile(tarinfo) + tarfile.addfile(tarinfo) def is_empty(self): it = os.scandir(self.external_directory) diff --git a/src/buildstream/testing/_utils/junction.py b/src/buildstream/testing/_utils/junction.py index ca059eb8b..2bf53ac7c 100644 --- a/src/buildstream/testing/_utils/junction.py +++ b/src/buildstream/testing/_utils/junction.py @@ -60,7 +60,10 @@ class _SimpleGit(Repo): universal_newlines=True, ).stdout.strip() - def source_config(self, ref=None, checkout_submodules=None): + def source_config(self, ref=None): + return self.source_config_extra(ref) + + def source_config_extra(self, ref=None, checkout_submodules=None): config = { 'kind': 'git', 'url': 'file://' + self.repo, diff --git a/src/buildstream/testing/runcli.py b/src/buildstream/testing/runcli.py index 8b3185143..9efff4593 100644 --- a/src/buildstream/testing/runcli.py +++ b/src/buildstream/testing/runcli.py @@ -513,6 +513,16 @@ class CliIntegration(Cli): # run() # + # This supports the same arguments as Cli.run(), see run_project_config(). + # + def run(self, configure=True, project=None, silent=False, env=None, + cwd=None, options=None, args=None, binary_capture=False): + return self.run_project_config( + configure=configure, project=project, silent=silent, env=env, + cwd=cwd, options=options, args=args, binary_capture=binary_capture) + + # run_project_config() + # # This supports the same arguments as Cli.run() and additionally # it supports the project_config keyword argument. # @@ -524,7 +534,7 @@ class CliIntegration(Cli): # be a dictionary of additional project configuration options, and # will be composited on top of the already loaded project.conf # - def run(self, *args, project_config=None, **kwargs): + def run_project_config(self, *, project_config=None, **kwargs): # First load the project.conf and substitute {project_dir} # @@ -575,7 +585,7 @@ class CliIntegration(Cli): with open(project_filename, 'w') as f: f.write(config) - return super().run(*args, **kwargs) + return super().run(**kwargs) class CliRemote(CliIntegration): diff --git a/tests/frontend/consistencyerror/plugins/consistencybug.py b/tests/frontend/consistencyerror/plugins/consistencybug.py index 3c6fb46f3..c60d81ea0 100644 --- a/tests/frontend/consistencyerror/plugins/consistencybug.py +++ b/tests/frontend/consistencyerror/plugins/consistencybug.py @@ -23,7 +23,7 @@ class ConsistencyBugSource(Source): def set_ref(self, ref, node): pass - def fetch(self): + def fetch(self, **kwargs): pass def stage(self, directory): diff --git a/tests/frontend/consistencyerror/plugins/consistencyerror.py b/tests/frontend/consistencyerror/plugins/consistencyerror.py index fe2037fc6..bcbd1b5e9 100644 --- a/tests/frontend/consistencyerror/plugins/consistencyerror.py +++ b/tests/frontend/consistencyerror/plugins/consistencyerror.py @@ -24,7 +24,7 @@ class ConsistencyErrorSource(Source): def set_ref(self, ref, node): pass - def fetch(self): + def fetch(self, **kwargs): pass def stage(self, directory): diff --git a/tests/frontend/init.py b/tests/frontend/init.py index fa4623c63..0eac5d528 100644 --- a/tests/frontend/init.py +++ b/tests/frontend/init.py @@ -110,7 +110,7 @@ def test_element_path_interactive(cli, tmp_path, monkeypatch, element_path): def create(cls, *args, **kwargs): return DummyInteractiveApp(*args, **kwargs) - def _init_project_interactive(self, *args, **kwargs): + def _init_project_interactive(self, *args, **kwargs): # pylint: disable=arguments-differ return ('project_name', '0', element_path) monkeypatch.setattr(App, 'create', DummyInteractiveApp.create) diff --git a/tests/frontend/mirror.py b/tests/frontend/mirror.py index f7427cf88..09beb38b8 100644 --- a/tests/frontend/mirror.py +++ b/tests/frontend/mirror.py @@ -427,7 +427,7 @@ def test_mirror_fallback_git_only_submodules(cli, tmpdir, datafiles): element = { 'kind': 'import', 'sources': [ - main_repo.source_config(ref=main_ref, checkout_submodules=True) + main_repo.source_config_extra(ref=main_ref, checkout_submodules=True) ] } element_name = 'test.bst' @@ -522,7 +522,7 @@ def test_mirror_fallback_git_with_submodules(cli, tmpdir, datafiles): element = { 'kind': 'import', 'sources': [ - upstream_main_repo.source_config(ref=upstream_main_ref, checkout_submodules=True) + upstream_main_repo.source_config_extra(ref=upstream_main_ref, checkout_submodules=True) ] } element['sources'][0]['url'] = aliased_repo diff --git a/tests/integration/build-uid.py b/tests/integration/build-uid.py index 2ebf230a1..26f1bd2d4 100644 --- a/tests/integration/build-uid.py +++ b/tests/integration/build-uid.py @@ -30,7 +30,8 @@ def test_build_uid_overridden(cli, datafiles): } } - result = cli.run(project=project, project_config=project_config, args=['build', element_name]) + result = cli.run_project_config( + project=project, project_config=project_config, args=['build', element_name]) assert result.exit_code == 0 @@ -48,7 +49,8 @@ def test_build_uid_in_project(cli, datafiles): } } - result = cli.run(project=project, project_config=project_config, args=['build', element_name]) + result = cli.run_project_config( + project=project, project_config=project_config, args=['build', element_name]) assert result.exit_code == 0 diff --git a/tests/integration/shell.py b/tests/integration/shell.py index 868064d5b..d0c9f1f99 100644 --- a/tests/integration/shell.py +++ b/tests/integration/shell.py @@ -33,7 +33,8 @@ DATA_DIR = os.path.join( # def execute_shell(cli, project, command, *, config=None, mount=None, element='base.bst', isolate=False): # Ensure the element is built - result = cli.run(project=project, project_config=config, args=['build', element]) + result = cli.run_project_config( + project=project, project_config=config, args=['build', element]) assert result.exit_code == 0 args = ['shell'] @@ -44,7 +45,8 @@ def execute_shell(cli, project, command, *, config=None, mount=None, element='ba args += ['--mount', host_path, target_path] args += [element, '--', *command] - return cli.run(project=project, project_config=config, args=args) + return cli.run_project_config( + project=project, project_config=config, args=args) # Test running something through a shell, allowing it to find the diff --git a/tests/sources/git.py b/tests/sources/git.py index e9cc369d7..e01477c49 100644 --- a/tests/sources/git.py +++ b/tests/sources/git.py @@ -128,7 +128,7 @@ def test_submodule_fetch_source_enable_explicit(cli, tmpdir, datafiles): element = { 'kind': 'import', 'sources': [ - repo.source_config(ref=ref, checkout_submodules=True) + repo.source_config_extra(ref=ref, checkout_submodules=True) ] } _yaml.dump(element, os.path.join(project, 'target.bst')) @@ -167,7 +167,7 @@ def test_submodule_fetch_source_disable(cli, tmpdir, datafiles): element = { 'kind': 'import', 'sources': [ - repo.source_config(ref=ref, checkout_submodules=False) + repo.source_config_extra(ref=ref, checkout_submodules=False) ] } _yaml.dump(element, os.path.join(project, 'target.bst')) @@ -206,7 +206,7 @@ def test_submodule_fetch_submodule_does_override(cli, tmpdir, datafiles): element = { 'kind': 'import', 'sources': [ - repo.source_config(ref=ref, checkout_submodules=False) + repo.source_config_extra(ref=ref, checkout_submodules=False) ] } _yaml.dump(element, os.path.join(project, 'target.bst')) @@ -250,7 +250,7 @@ def test_submodule_fetch_submodule_individual_checkout(cli, tmpdir, datafiles): element = { 'kind': 'import', 'sources': [ - repo.source_config(ref=ref, checkout_submodules=True) + repo.source_config_extra(ref=ref, checkout_submodules=True) ] } _yaml.dump(element, os.path.join(project, 'target.bst')) @@ -295,7 +295,7 @@ def test_submodule_fetch_submodule_individual_checkout_explicit(cli, tmpdir, dat element = { 'kind': 'import', 'sources': [ - repo.source_config(ref=ref, checkout_submodules=True) + repo.source_config_extra(ref=ref, checkout_submodules=True) ] } _yaml.dump(element, os.path.join(project, 'target.bst')) diff --git a/tests/testutils/repo/git.py b/tests/testutils/repo/git.py index 0ed0590be..ba1590273 100644 --- a/tests/testutils/repo/git.py +++ b/tests/testutils/repo/git.py @@ -72,7 +72,10 @@ class Git(Repo): self._run_git('commit', '-m', 'Removing {}'.format(path)) return self.latest_commit() - def source_config(self, ref=None, checkout_submodules=None): + def source_config(self, ref=None): + return self.source_config_extra(ref) + + def source_config_extra(self, ref=None, checkout_submodules=None): config = { 'kind': 'git', 'url': 'file://' + self.repo, |