From 767cc21f9f2f75ef5cf62fc6ea8d2343530287ed Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sun, 11 Sep 2022 17:14:25 -0600 Subject: Tweak Copy() changes: Simplify check for copy-a-list case: just try the os.makedirs, catch the exception, and raise a BuildError which seems to fit better. Add a second test for copying lists in test/Copy-Action: now tests both a list of explicit strings and a list of Nodes. Signed-off-by: Mats Wichmann --- CHANGES.txt | 4 ++-- SCons/Defaults.py | 11 +++++------ test/Copy-Action.py | 5 +++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index e895b29..addc5f7 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -29,8 +29,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER From Mats Wichmann: - A list argument as the source to the Copy() action function is now - correctly handled by converting elements to string. Copy bails if - asked to copy a list to an existing non-directory destination. + correctly handled by converting elements to string. Copy errors out + if asked to copy a list to an existing non-directory destination. Both the implementation and the strfunction which prints the progress message were adjusted. Fixes #3009. diff --git a/SCons/Defaults.py b/SCons/Defaults.py index 218eb10..32b924f 100644 --- a/SCons/Defaults.py +++ b/SCons/Defaults.py @@ -276,14 +276,13 @@ def copy_func(dest, src, symlinks=True) -> int: SCons.Node.FS.invalidate_node_memos(dest) if SCons.Util.is_List(src): - if not os.path.exists(dest): + # this fails only if dest exists and is not a dir + try: os.makedirs(dest, exist_ok=True) - elif not os.path.isdir(dest): - # is Python's NotADirectoryError more appropriate? - raise SCons.Errors.UserError( - f'Copy() called with list src but dest "{dest}" is not a directory' + except FileExistsError: + raise SCons.Errors.BuildError( + errstr=f'Error: Copy() called with list src but "{dest}" is not a directory' ) - for file in src: shutil.copy2(file, dest) return 0 diff --git a/test/Copy-Action.py b/test/Copy-Action.py index e2e01ef..1b1356e 100644 --- a/test/Copy-Action.py +++ b/test/Copy-Action.py @@ -41,6 +41,8 @@ Execute(Copy('f1.out', 'f1.in')) Execute(Copy(File('d2.out'), 'd2.in')) Execute(Copy('d3.out', File('f3.in'))) # Issue #3009: make sure it's not mangled if src is a list. +# make sure both list-of-str and list-of-Node work +Execute(Copy('d7.out', ['f10.in', 'f11.in'])) Execute(Copy('d7.out', Glob('f?.in'))) def cat(env, source, target): @@ -104,6 +106,7 @@ expect = test.wrap_stdout( Copy("f1.out", "f1.in") Copy("d2.out", "d2.in") Copy("d3.out", "f3.in") +Copy("d7.out", ["f10.in", "f11.in"]) Copy("d7.out", ["f1.in", "f3.in", "f4.in", "f6.in", "f7.in", "f8.in", "f9.in"]) """, build_str="""\ @@ -128,6 +131,7 @@ test.must_not_exist('f1.out') test.must_not_exist('d2.out') test.must_not_exist(os.path.join('d3.out', 'f3.in')) test.must_not_exist(os.path.join('d7.out', 'f7.in')) +test.must_not_exist(os.path.join('d7.out', 'f11.in')) test.must_not_exist('f4.out') test.must_not_exist('d5.out') test.must_not_exist(os.path.join('d6.out', 'f6.in')) @@ -147,6 +151,7 @@ test.must_match('f1.out', "f1.in\n", mode='r') test.must_match(['d2.out', 'file'], "d2.in/file\n", mode='r') test.must_match(['d3.out', 'f3.in'], "f3.in\n", mode='r') test.must_match(['d7.out', 'f7.in'], "f7.in\n", mode='r') +test.must_match(['d7.out', 'f11.in'], "f11.in\n", mode='r') test.must_match('f4.out', "f4.in\n", mode='r') test.must_match(['d5.out', 'file'], "d5.in/file\n", mode='r') test.must_match(['d6.out', 'f6.in'], "f6.in\n", mode='r') -- cgit v0.12