ClojureScript

Source maps in advanced mode don't work when compiled on Windows

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

See the difference between:

https://github.com/djpowell/cstest2/blob/linux/out/main-map.js
and
https://github.com/djpowell/cstest2/blob/windows/out/main-map.js

Note that windows has an entry for "out/" in sources and is missing all of the cljs entries.

Might be something to do with path munging and backslashes on Windows?

Activity

Hide
David Powell added a comment -

One problem is that file URLs converted using url.getPath() are of the form:
/C:/Temp/some/file.js

whereas the keys in @env/::compiled-cljs/* are absolute paths, such as:
C:\Temp\some\file.js

so these files get dropped by cljs.closure/optimize because they can't be found in @env.

A second problem, that shows up when :output-dir isn't specified, is that the cljs.source-maps/relativize-path returns things like:
/C:/Temp/cstest2/target/cljsbuild-compiler-0//goog/base.js
from the browser, this doesn't resolve properly. We need a better implementation of relativize here - perhaps the one in cljs.closure/path-relative-to could be used?

It is a bit unclear in the code what is supposed to refer to a file, and what is supposed to be a relative uri. Windows paths are particularly sensitive to the difference.

Show
David Powell added a comment - One problem is that file URLs converted using url.getPath() are of the form: /C:/Temp/some/file.js whereas the keys in @env/::compiled-cljs/* are absolute paths, such as: C:\Temp\some\file.js so these files get dropped by cljs.closure/optimize because they can't be found in @env. A second problem, that shows up when :output-dir isn't specified, is that the cljs.source-maps/relativize-path returns things like: /C:/Temp/cstest2/target/cljsbuild-compiler-0//goog/base.js from the browser, this doesn't resolve properly. We need a better implementation of relativize here - perhaps the one in cljs.closure/path-relative-to could be used? It is a bit unclear in the code what is supposed to refer to a file, and what is supposed to be a relative uri. Windows paths are particularly sensitive to the difference.
Hide
David Nolen added a comment -

It probably makes sense for the the lookup to be based on URL and not file paths as the source map needs relative paths anyway. I think if we switch the keys in ::compiled-clj to URLs we should be able to avoid host path issues?

Show
David Nolen added a comment - It probably makes sense for the the lookup to be based on URL and not file paths as the source map needs relative paths anyway. I think if we switch the keys in ::compiled-clj to URLs we should be able to avoid host path issues?
Hide
David Nolen added a comment -

Can we verify that this is resolved by CLJS-674? You can install ClojureScript master locally by running ./script/build in a git checkout of the project. Thanks!

Show
David Nolen added a comment - Can we verify that this is resolved by CLJS-674? You can install ClojureScript master locally by running ./script/build in a git checkout of the project. Thanks!
Hide
David Powell added a comment -

No, this still doesn't fix it on windows.
I'm now getting content like this at the start of my sourcemaps when I specify :output-dir "out" -

{"version":3,
"file":"main-map.js",
"sources":
["out", "out\\goog
base.js", "out\\goog\\string
string.js",
"out\\goog\\debug
error.js", "out\\goog\\asserts
asserts.js",
"out\\goog\\array
array.js", "out\\goog\\object
object.js",
"out\\goog\\string
stringbuffer.js", "out
constants_table.cljs"],
"lineCount":3609,
...

  • backslashes instead of forward slashes
  • "out" instead of the expected cljs files

It looks similar to before, but now with backslashes instead of forward slashes.
I haven't checked, but I assume that the format of paths in ::compiled-clj isn't matching what is being looked up again.

Show
David Powell added a comment - No, this still doesn't fix it on windows. I'm now getting content like this at the start of my sourcemaps when I specify :output-dir "out" - {"version":3, "file":"main-map.js", "sources": ["out", "out\\goog
base.js", "out\\goog\\string
string.js", "out\\goog\\debug
error.js", "out\\goog\\asserts
asserts.js", "out\\goog\\array
array.js", "out\\goog\\object
object.js", "out\\goog\\string
stringbuffer.js", "out
constants_table.cljs"], "lineCount":3609, ...
  • backslashes instead of forward slashes
  • "out" instead of the expected cljs files
It looks similar to before, but now with backslashes instead of forward slashes. I haven't checked, but I assume that the format of paths in ::compiled-clj isn't matching what is being looked up again.
Hide
Tim Visher added a comment -

I don't have any more time tonight but I believe the patch for CLJS-694 corrects at least the backslash issue. Probably won't correct more than that, but the source maps being relativized as URIs rather than files appears to produce the right output if the URI is in the same directory as the one being compared to, which should cover our cases here.

BTW, you can use modern.ie VMs to test this stuff out! Who knew, right? (Windows 8 is… Different.)

Show
Tim Visher added a comment - I don't have any more time tonight but I believe the patch for CLJS-694 corrects at least the backslash issue. Probably won't correct more than that, but the source maps being relativized as URIs rather than files appears to produce the right output if the URI is in the same directory as the one being compared to, which should cover our cases here. BTW, you can use modern.ie VMs to test this stuff out! Who knew, right? (Windows 8 is… Different.)
Hide
Tim Visher added a comment -

I just tested this on Windows 8 and the backslashes at least are gone. Maybe I'll have some time this weekend to get in there with a little trace action and see what else can be done to get the non-jar file dependencies correctly output.

Show
Tim Visher added a comment - I just tested this on Windows 8 and the backslashes at least are gone. Maybe I'll have some time this weekend to get in there with a little trace action and see what else can be done to get the non-jar file dependencies correctly output.
Hide
Tim Visher added a comment -

fix-681.patch @ 22/NOV/13 fixes this for me on Windows and continues to pass on OS X.

Should be more thoroughly tested and impact analyzed, I'm sure, but I wanted to keep the discussion going.

Show
Tim Visher added a comment - fix-681.patch @ 22/NOV/13 fixes this for me on Windows and continues to pass on OS X. Should be more thoroughly tested and impact analyzed, I'm sure, but I wanted to keep the discussion going.
Tim Visher made changes -
Field Original Value New Value
Attachment fix-681.patch [ 12479 ]
Hide
David Powell added a comment -

fix-681 is working for me with either :output-dir as "out" or unset.

Show
David Powell added a comment - fix-681 is working for me with either :output-dir as "out" or unset.
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: