Use a Unix-compatible list2cmdline in jstests.py

RESOLVED FIXED in mozilla18

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

unspecified
mozilla18
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozbase])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Sadly, subprocess.list2cmdline only seems to do something sensible for Windows, which I find rather bizarre.
(Assignee)

Comment 1

5 years ago
Created attachment 661628 [details] [diff] [review]
Use a Unix-compatible list2cmdline

This patch fixes the problem where -s can give you something like

  ./js -e option(\'allow_xml\'); -f ...

which is totally invalid in bash.
Attachment #661628 - Flags: review?(terrence)
Comment on attachment 661628 [details] [diff] [review]
Use a Unix-compatible list2cmdline

Review of attachment 661628 [details] [diff] [review]:
-----------------------------------------------------------------

That is indeed totally bizarre.

::: js/src/tests/lib/results.py
@@ +5,5 @@
> +if (sys.platform.startswith('linux') or
> +    sys.platform.startswith('darwin')
> +   ):
> +    import pipes
> +    list2cmdline = lambda args: ' '.join([ pipes.quote(a) for a in args ])

Neat trick!  I had no idea pipes even existed.  If quote works correct on Windows as well, just use this variant unconditionally.
Attachment #661628 - Flags: review?(terrence) → review+
(Assignee)

Comment 3

5 years ago
(In reply to Terrence Cole [:terrence] from comment #2)
> Neat trick!  I had no idea pipes even existed.

Me neither, but Our Benevolent Overlord Google does.

> If quote works correct on Windows as well, just use this variant unconditionally.

That's a good point. It seems to, since MozillaBuild gives you a bash shell anyway. I didn't have a working shell on Windows, but it generally seemed to work, so I'll do that. Thanks. It can't be any worse than what it does now.
(Assignee)

Comment 4

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/ea2e059c896b
https://hg.mozilla.org/mozilla-central/rev/ea2e059c896b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Comment 6

5 years ago
we may want to put this in e.g. mozfile at this point
Whiteboard: [mozbase]
You need to log in before you can comment on or make changes to this bug.