<< Back to previous view

[CLJS-681] Source maps in advanced mode don't work when compiled on Windows Created: 15/Nov/13  Updated: 22/Nov/13  Resolved: 22/Nov/13

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Powell Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix-681.patch    

 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?



 Comments   
Comment by David Powell [ 16/Nov/13 11:06 AM ]

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.

Comment by David Nolen [ 18/Nov/13 8:06 AM ]

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?

Comment by David Nolen [ 20/Nov/13 1:29 PM ]

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!

Comment by David Powell [ 20/Nov/13 7:36 PM ]

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.

Comment by Tim Visher [ 21/Nov/13 9:44 PM ]

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.)

Comment by Tim Visher [ 22/Nov/13 12:06 PM ]

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.

Comment by Tim Visher [ 22/Nov/13 2:32 PM ]

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.

Comment by David Powell [ 22/Nov/13 3:55 PM ]

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

Comment by David Nolen [ 22/Nov/13 4:20 PM ]

fixed,https://github.com/clojure/clojurescript/commit/c51caf64611177d3c7ad3e25ea8b054c5a838719

Generated at Wed Apr 23 09:45:15 CDT 2014 using JIRA 4.4#649-r158309.