Closed Bug 510553 Opened 10 years ago Closed 10 years ago

TM: Windows 'make check' is busted in new trace-test harness


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
status1.9.2 --- beta1-fixed


(Reporter: dmandelin, Assigned: dmandelin)



(1 file, 1 obsolete file)

This is caused by yucky differences in shell quoting and path separators.
Attached patch Patch (obsolete) — Splinter Review
Attachment #394547 - Flags: review?(jorendorff)
>+    p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=close_fds)

I think what we really want is to omit shell=True here...

>+    # On Windows, we want the path to use slashes, because JS will be able
>+    # to handle it and it means less string quote escaping.
>+    if os.sep == '\\':
>+        libdir_var = libdir_var.replace('\\', '/')
>+    expr = "const platform='%s'; const libdir='%s';"%(sys.platform, libdir_var)
>+    cmd = '%s -j -e "%s" -f %s -f %s'%(
>         JS, expr, os.path.join(lib_dir, 'prolog.js'), path)

and write:

    expr = "const platform=%r; const libdir=%r;" % (sys.platform, libdir_var)
    cmd = [JS, '-j', '-e', expr, '-f', os.path.join(lib_dir, 'prolog.js'),
           '-f', path]
Attached patch Patch 2Splinter Review
Attachment #394547 - Attachment is obsolete: true
Attachment #394547 - Flags: review?(jorendorff)
Comment on attachment 394548 [details] [diff] [review]
Patch 2

This works on Windows and Mac (and presumably linux).
Attachment #394548 - Flags: review?(jorendorff)
Attachment #394548 - Flags: review?(jorendorff) → review+
Pushed to TM as d8e4676dc10e.
Backed out due to tjss failures.
Oops, wrong bug for comment 6. But something bad did happen here: these changes got nuked, probably when got copied over with
Yep. Sorry!

changeset:   31574:12a9bea2d331
user:        Jason Orendorff <>
date:        Sat Aug 15 07:14:45 2009 -0500
summary:     Re-apply d8e4676dc10e (bug 510553) to Changeset c3648b2ea86c inadvertently reverted these changes because I rebased it across d8e4676dc10e and rebase is dumb. r=sea of orange
Closed: 10 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.