From d14d9694f925a04f033e6f0119f36f5fbfbc73f3 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Mon, 19 Dec 2022 20:32:18 -0800 Subject: Fixed using --diskcheck=none from command line. It was always broken. SetOption('diskcheck','none') has been working all along. Also refactored the DiskChecker class to have more meaningful properties and not shadow default python objects (list, dir).. --- CHANGES.txt | 2 ++ RELEASE.txt | 2 ++ SCons/Node/FS.py | 41 ++++++++++++++++++++++++----------------- SCons/Script/SConsOptions.py | 5 ++++- test/diskcheck.py | 17 ++++++++++++----- 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 04ce9e7..c46f9b6 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,6 +34,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER read packaging/etc/README.txt if you are interested. - Added --experimental=tm_v2, which enables Andrew Morrow's new NewParallel Job implementation. This should scale much better for highly parallel builds. You can also enable this via SetOption(). + - Fixed setting diskcheck to 'none' via command line works --diskcheck=none. Apparently it has never worked + even though SetOption('diskcheck','none') did work. From Dan Mezhiborsky: - Add newline to end of compilation db (compile_commands.json). diff --git a/RELEASE.txt b/RELEASE.txt index 81ee612..40852fe 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -64,6 +64,8 @@ FIXES - Ninja: Fix execution environment sanitation for launching ninja. Previously if you set an execution environment variable set to a python list it would crash. Now it will create a string joining the list with os.pathsep +- Fixed setting diskcheck to 'none' via command line works --diskcheck=none. Apparently it has never worked + even though SetOption('diskcheck','none') did work. IMPROVEMENTS ------------ diff --git a/SCons/Node/FS.py b/SCons/Node/FS.py index f5642e2..aadcba5 100644 --- a/SCons/Node/FS.py +++ b/SCons/Node/FS.py @@ -378,20 +378,26 @@ else: return x.upper() - class DiskChecker: - def __init__(self, type, do, ignore): - self.type = type - self.do = do - self.ignore = ignore - self.func = do + """ + This Class will hold various types of logic for checking if a file/dir on disk matches + the type which is expected. And allow Options to decide to enable or disable said check + """ + def __init__(self, disk_check_type, do_check_function, ignore_check_function): + self.disk_check_type = disk_check_type + self.do_check_function = do_check_function + self.ignore_check_function = ignore_check_function + self.func = do_check_function + def __call__(self, *args, **kw): return self.func(*args, **kw) - def set(self, list): - if self.type in list: - self.func = self.do + + def set(self, disk_check_type_list): + if self.disk_check_type in disk_check_type_list: + self.func = self.do_check_function else: - self.func = self.ignore + self.func = self.ignore_check_function + def do_diskcheck_match(node, predicate, errorfmt): result = predicate() @@ -409,24 +415,25 @@ def do_diskcheck_match(node, predicate, errorfmt): if result: raise TypeError(errorfmt % node.get_abspath()) + def ignore_diskcheck_match(node, predicate, errorfmt): pass - diskcheck_match = DiskChecker('match', do_diskcheck_match, ignore_diskcheck_match) diskcheckers = [ diskcheck_match, ] -def set_diskcheck(list): + +def set_diskcheck(enabled_checkers): for dc in diskcheckers: - dc.set(list) + dc.set(enabled_checkers) -def diskcheck_types(): - return [dc.type for dc in diskcheckers] +def diskcheck_types(): + return [dc.disk_check_type for dc in diskcheckers] class EntryProxy(SCons.Util.Proxy): @@ -2403,7 +2410,7 @@ class RootDir(Dir): return Base.must_be_same(self, klass) - def _lookup_abs(self, p, klass, create=1): + def _lookup_abs(self, p, klass, create=True): """ Fast (?) lookup of a *normalized* absolute path. @@ -2428,7 +2435,7 @@ class RootDir(Dir): raise SCons.Errors.UserError(msg) # There is no Node for this path name, and we're allowed # to create it. - dir_name, file_name = p.rsplit('/',1) + dir_name, file_name = p.rsplit('/', 1) dir_node = self._lookup_abs(dir_name, Dir) result = klass(file_name, dir_node, self.fs) diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py index a3e3ea8..8391d62 100644 --- a/SCons/Script/SConsOptions.py +++ b/SCons/Script/SConsOptions.py @@ -54,7 +54,10 @@ def diskcheck_convert(value): if v == 'all': result = diskcheck_all elif v == 'none': - result = [] + # Don't use an empty list here as that fails the normal check + # to see if an optparse parser of if parser.argname: + # Changed to ['none'] as diskcheck expects a list value + result = ['none'] elif v in diskcheck_all: result.append(v) else: diff --git a/test/diskcheck.py b/test/diskcheck.py index 36cfa4e..c07dc6b 100644 --- a/test/diskcheck.py +++ b/test/diskcheck.py @@ -1,6 +1,8 @@ #!/usr/bin/env python # -# __COPYRIGHT__ +# MIT License +# +# Copyright The SCons Foundation # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -20,9 +22,7 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -# -__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" """ Test that the --diskcheck option and SetOption('diskcheck') correctly @@ -41,16 +41,23 @@ test.write('file', "file\n") test.write('SConstruct', """ -SetOption('diskcheck', 'none') + +if GetOption('diskcheck') == ['match'] or ARGUMENTS.get('setoption_none',0): + SetOption('diskcheck', 'none') File('subdir') """) -test.run() +test.run(status=2, stderr=None) +test.must_contain_all_lines(test.stderr(), ["found where file expected"]) test.run(arguments='--diskcheck=match', status=2, stderr=None) test.must_contain_all_lines(test.stderr(), ["found where file expected"]) +# Test that setting --diskcheck to none via command line also works. +test.run(arguments='--diskcheck=none') +# Test that SetOption('diskcheck','none') works to override default as well +test.run(arguments='setoption_none=1') test.pass_test() -- cgit v0.12 From d2569dc26751464f9e42df9c4723d53458a86ed2 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Tue, 20 Dec 2022 10:36:47 -0800 Subject: Address feedback from mwichmann in PR --- CHANGES.txt | 4 ++-- RELEASE.txt | 4 ++-- SCons/Node/FS.py | 15 +++++++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index c46f9b6..bbc2800 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,8 +34,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER read packaging/etc/README.txt if you are interested. - Added --experimental=tm_v2, which enables Andrew Morrow's new NewParallel Job implementation. This should scale much better for highly parallel builds. You can also enable this via SetOption(). - - Fixed setting diskcheck to 'none' via command line works --diskcheck=none. Apparently it has never worked - even though SetOption('diskcheck','none') did work. + - Fixed command line argument --diskcheck: previously a value of 'none' was ignored. + SetOption('diskcheck','none') is unaffected, as it did not have the problem. From Dan Mezhiborsky: - Add newline to end of compilation db (compile_commands.json). diff --git a/RELEASE.txt b/RELEASE.txt index 40852fe..32094bf 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -64,8 +64,8 @@ FIXES - Ninja: Fix execution environment sanitation for launching ninja. Previously if you set an execution environment variable set to a python list it would crash. Now it will create a string joining the list with os.pathsep -- Fixed setting diskcheck to 'none' via command line works --diskcheck=none. Apparently it has never worked - even though SetOption('diskcheck','none') did work. +- Fixed command line argument --diskcheck: previously a value of 'none' was ignored. + SetOption('diskcheck','none') is unaffected, as it did not have the problem. IMPROVEMENTS ------------ diff --git a/SCons/Node/FS.py b/SCons/Node/FS.py index aadcba5..67e1ff6 100644 --- a/SCons/Node/FS.py +++ b/SCons/Node/FS.py @@ -380,8 +380,10 @@ else: class DiskChecker: """ - This Class will hold various types of logic for checking if a file/dir on disk matches - the type which is expected. And allow Options to decide to enable or disable said check + Implement disk check variation. + + This Class will hold functions to determine what this particular disk + checking implementation should do when enabled or disabled. """ def __init__(self, disk_check_type, do_check_function, ignore_check_function): self.disk_check_type = disk_check_type @@ -392,7 +394,12 @@ class DiskChecker: def __call__(self, *args, **kw): return self.func(*args, **kw) - def set(self, disk_check_type_list): + def enable(self, disk_check_type_list): + """ + If the current object's disk_check_type matches any in the list passed + :param disk_check_type_list: List of disk checks to enable + :return: + """ if self.disk_check_type in disk_check_type_list: self.func = self.do_check_function else: @@ -429,7 +436,7 @@ diskcheckers = [ def set_diskcheck(enabled_checkers): for dc in diskcheckers: - dc.set(enabled_checkers) + dc.enable(enabled_checkers) def diskcheck_types(): -- cgit v0.12