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

quoting fix (continue on #1115, tested on Windows) #1116

Conversation

davidlatwe
Copy link
Contributor

As title, this should fix #1114 based on #1115, and tested on my Windows machine.

___________________________________________________ TestShells.test_rez_env_output ____________________________________________________

self = <rez.tests.test_shells.TestShells testMethod=test_rez_env_output>

    @per_available_shell()
    @install_dependent()
    def test_rez_env_output(self):
        def _test(txt):
            # Assumes that the shell has an echo command, build-in or alias
            binpath = os.path.join(system.rez_bin_path, "rez-env")
            args = [binpath, "--", "echo", txt]

            process = subprocess.Popen(
                args, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
                stderr=subprocess.PIPE, universal_newlines=True
            )
            sh_out = process.communicate()
            self.assertEqual(sh_out[0].strip(), txt)

        # please note - it's no coincidence that there are no substrings like
        # '$you' here. These would expand to the equivalent env-var (as
        # intended), which would be an empty string. We're not testing that
        # here though.
        #

        _test("hey")  # simple case
>       _test("hey you")  # with a space

test_shells.py:205:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
test_shells.py:196: in _test
    self.assertEqual(sh_out[0].strip(), txt)
E   AssertionError: '"hey you"' != 'hey you'
E   - "hey you"
E   ? -       -
E   + hey you
E    (in shell 'cmd')
-------------------------------------------------------- Captured stdout call ---------------------------------------------------------

testing in shell: cmd...
======================================================= short test summary info =======================================================
FAILED test_shells.py::TestShells::test_rez_env_output - AssertionError: '"hey you"' != 'hey you'
========================================================== 1 failed in 1.28s ==========================================================

@nerdvegas
Copy link
Contributor

Hey David, this isn't working, see https://github.com/nerdvegas/rez/pull/1116/checks?check_run_id=3245129015.
Question is, why did the test succeed? Actually it looks like pytest-based rez-selftest is exiting with 0, that'll have to be fixed(!!).

In any case this doesn't work because the output should not match the shlex-joined output.

Example (bash):

]$ echo "hey you"
hey you

When rez creates the runtime to run a command, it writes shell script out to a rez-shell.sh file. So, it needs to literally print echo "hey you" into that script, which is why shlex_join is used. However, the output should be hey you, not "hey you".

I am assuming that the required escaping is different in cmd, it's picking up the double quotes as part of the string to be printed, which is unintentional.

I may get a look in at 1117 today.

#1117

Cheers
A

@nerdvegas nerdvegas closed this Aug 4, 2021
@davidlatwe davidlatwe deleted the issue_1114-command-quoting branch August 5, 2021 06:07
@davidlatwe davidlatwe mentioned this pull request Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug in command quoting
2 participants