From 9f71900637d41b8a82c27501ee13a1ad049d1a7e Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 12 Jul 2020 12:39:33 +0100 Subject: [PATCH 01/11] add debugging code --- bin/run_tests.py | 3 ++- test/test_environment.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/run_tests.py b/bin/run_tests.py index 17f06aa55..96813ae64 100755 --- a/bin/run_tests.py +++ b/bin/run_tests.py @@ -17,4 +17,5 @@ subprocess.check_call(unit_test_args) # run the integration tests - subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test']) + # TODO: REMOVE 'overridden' + subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test', '-k', 'overridden']) diff --git a/test/test_environment.py b/test/test_environment.py index 03bf3bd2f..7f1c641c9 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -70,4 +70,7 @@ def test_overridden_path(tmp_path, capfd): assert len(os.listdir(output_dir)) == 0 captured = capfd.readouterr() + # TODO: REMOVE THIS PRINT + print('captured.out', captured.out) + print('captured.err', captured.err) assert "python available on PATH doesn't match our installed instance" in captured.err From dc446d3cac4d8ca49cb2b010ce686aa346145c79 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 12 Jul 2020 12:45:59 +0100 Subject: [PATCH 02/11] moar debug --- cibuildwheel/bashlex_eval.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index b5e478c84..4cc9c0537 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -69,6 +69,8 @@ def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> # remove the None letters and concat value = ''.join(letters) + print('node value:', value) + # apply bash-like quotes/whitespace treatment return ' '.join(word.strip() for word in shlex.split(value)) From 9fb748bfb0411dbdfbdd7d204eeb232ac3172e23 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 12 Jul 2020 21:45:46 +0100 Subject: [PATCH 03/11] Add unit test for this failure mode --- unit_test/environment_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/unit_test/environment_test.py b/unit_test/environment_test.py index 4e10e07c8..5a0cf613c 100644 --- a/unit_test/environment_test.py +++ b/unit_test/environment_test.py @@ -103,3 +103,14 @@ def test_operators_inside_eval(): environment_dict = environment_recipe.as_dictionary(os.environ.copy()) assert environment_dict.get('SOMETHING') == 'a\nb\nc' + + +def test_substitution_with_backslash(): + environment_recipe = parse_environment('PATH2="somewhere_else;$PATH1"') + + # pass the existing process env so PATH is available + environment_dict = environment_recipe.as_dictionary(prev_environment={ + 'PATH1': 'c:\\folder\\' + }) + + assert environment_dict.get('PATH2') == 'somewhere_else;c:\\folder\\' From 08eddeff8bc0e2ca7699a0c1af71b15f12615132 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 15 Jul 2020 21:07:33 +0100 Subject: [PATCH 04/11] Fix test and add another more awkward example --- cibuildwheel/bashlex_eval.py | 26 +++++++++++--------------- unit_test/environment_test.py | 9 +++++++++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index 4cc9c0537..122b50b30 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -51,28 +51,24 @@ def evaluate_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: - word_start = node.pos[0] - word_end = node.pos[1] - word_string = context.input[word_start:word_end] - letters = list(word_string) + value = node.word for part in node.parts: - part_start = part.pos[0] - word_start - part_end = part.pos[1] - word_start + part_string = context.input[part.pos[0]:part.pos[1]] + part_value = evaluate_node(part, context=context) - # Set all the characters in the part to None - for i in range(part_start, part_end): - letters[i] = '' + if part_string not in value: + raise RuntimeError( + 'bash parse failed. part "{}" not found in "{}". Word was "{}". Full input was "{}"'.format( + part_string, value, node.word, context.input, + ) + ) - letters[part_start] = evaluate_node(part, context=context) - - # remove the None letters and concat - value = ''.join(letters) + value = value.replace(part_string, part_value, 1) print('node value:', value) - # apply bash-like quotes/whitespace treatment - return ' '.join(word.strip() for word in shlex.split(value)) + return value def evaluate_command_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: diff --git a/unit_test/environment_test.py b/unit_test/environment_test.py index 5a0cf613c..0827cc0d4 100644 --- a/unit_test/environment_test.py +++ b/unit_test/environment_test.py @@ -114,3 +114,12 @@ def test_substitution_with_backslash(): }) assert environment_dict.get('PATH2') == 'somewhere_else;c:\\folder\\' + + +def test_awkwardly_quoted_variable(): + environment_recipe = parse_environment('VAR2=something"like this""$VAR1"$VAR1$(echo "theres more")"$(echo "and more!")"') + + # pass the existing process env so PATH is available + environment_dict = environment_recipe.as_dictionary({'VAR1': 'but wait'}) + + assert environment_dict.get('VAR2') == 'somethinglike thisbut waitbut waittheres moreand more!' From 69fe085ba3b88c220558f6e5a2347f2151fa52c0 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 15 Jul 2020 21:25:29 +0100 Subject: [PATCH 05/11] more back-slash hilarity --- test/test_environment.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_environment.py b/test/test_environment.py index 7f1c641c9..3bcb6e3ab 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -65,7 +65,9 @@ def test_overridden_path(tmp_path, capfd): (new_path / 'python').touch(mode=0o777) utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ - 'CIBW_ENVIRONMENT': f'''PATH="{new_path}{os.pathsep}$PATH"''', + # use single-quotes for new_path, because on windows, paths with backslashes + # are interpreted by bash as escape sequences! + 'CIBW_ENVIRONMENT': f'''PATH='{new_path}'{os.pathsep}"$PATH"''', }) assert len(os.listdir(output_dir)) == 0 From 5df97a4121312ab5f7af98c119767ca1c12bad65 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 15 Jul 2020 21:34:30 +0100 Subject: [PATCH 06/11] Correct quoting of semicolon --- test/test_environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_environment.py b/test/test_environment.py index 3bcb6e3ab..04d4d3d98 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -67,7 +67,7 @@ def test_overridden_path(tmp_path, capfd): utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ # use single-quotes for new_path, because on windows, paths with backslashes # are interpreted by bash as escape sequences! - 'CIBW_ENVIRONMENT': f'''PATH='{new_path}'{os.pathsep}"$PATH"''', + 'CIBW_ENVIRONMENT': f'''PATH='{new_path}{os.pathsep}'$PATH''', }) assert len(os.listdir(output_dir)) == 0 From 56cd6c965d4835d12530b2be97171e1c7063e1dc Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 15 Jul 2020 22:02:20 +0100 Subject: [PATCH 07/11] Avoid backslash-in-env-var entirely --- test/test_environment.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/test_environment.py b/test/test_environment.py index 04d4d3d98..6ff4dbd15 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -65,9 +65,8 @@ def test_overridden_path(tmp_path, capfd): (new_path / 'python').touch(mode=0o777) utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ - # use single-quotes for new_path, because on windows, paths with backslashes - # are interpreted by bash as escape sequences! - 'CIBW_ENVIRONMENT': f'''PATH='{new_path}{os.pathsep}'$PATH''', + 'NEW_PATH': new_path, + 'CIBW_ENVIRONMENT': f'''PATH="$NEW_PATH{os.pathsep}$PATH''', }) assert len(os.listdir(output_dir)) == 0 From b90c4035b381c9bdb09f12203c2470b8cbca1c24 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Thu, 16 Jul 2020 07:56:15 +0100 Subject: [PATCH 08/11] Update test_environment.py --- test/test_environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_environment.py b/test/test_environment.py index 6ff4dbd15..7a6088363 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -65,7 +65,7 @@ def test_overridden_path(tmp_path, capfd): (new_path / 'python').touch(mode=0o777) utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ - 'NEW_PATH': new_path, + 'NEW_PATH': str(new_path), 'CIBW_ENVIRONMENT': f'''PATH="$NEW_PATH{os.pathsep}$PATH''', }) From af2a1719946f1723d19af61447e29893405f76dc Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 17 Jul 2020 09:09:02 +0100 Subject: [PATCH 09/11] Fix quotes --- test/test_environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_environment.py b/test/test_environment.py index 7a6088363..c9b66d0cf 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -66,7 +66,7 @@ def test_overridden_path(tmp_path, capfd): utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ 'NEW_PATH': str(new_path), - 'CIBW_ENVIRONMENT': f'''PATH="$NEW_PATH{os.pathsep}$PATH''', + 'CIBW_ENVIRONMENT': f'''PATH="$NEW_PATH{os.pathsep}$PATH"''', }) assert len(os.listdir(output_dir)) == 0 From 684745485091cb7922132e0d82fba380ae53f8b8 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 17 Jul 2020 09:31:47 +0100 Subject: [PATCH 10/11] Remove debug code --- bin/run_tests.py | 1 - cibuildwheel/bashlex_eval.py | 2 -- test/test_environment.py | 3 --- 3 files changed, 6 deletions(-) diff --git a/bin/run_tests.py b/bin/run_tests.py index 96813ae64..316f9a543 100755 --- a/bin/run_tests.py +++ b/bin/run_tests.py @@ -17,5 +17,4 @@ subprocess.check_call(unit_test_args) # run the integration tests - # TODO: REMOVE 'overridden' subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test', '-k', 'overridden']) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index 122b50b30..57791b2c6 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -66,8 +66,6 @@ def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> value = value.replace(part_string, part_value, 1) - print('node value:', value) - return value diff --git a/test/test_environment.py b/test/test_environment.py index c9b66d0cf..dd2de0ea9 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -71,7 +71,4 @@ def test_overridden_path(tmp_path, capfd): assert len(os.listdir(output_dir)) == 0 captured = capfd.readouterr() - # TODO: REMOVE THIS PRINT - print('captured.out', captured.out) - print('captured.err', captured.err) assert "python available on PATH doesn't match our installed instance" in captured.err From 7d983cdf8c720a86490744718bcc342d95b1fcba Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 17 Jul 2020 09:39:22 +0100 Subject: [PATCH 11/11] Reenable all tests --- bin/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/run_tests.py b/bin/run_tests.py index 316f9a543..17f06aa55 100755 --- a/bin/run_tests.py +++ b/bin/run_tests.py @@ -17,4 +17,4 @@ subprocess.check_call(unit_test_args) # run the integration tests - subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test', '-k', 'overridden']) + subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test'])