<< Back to previous view

[CLJS-694] Dependency on Java 7 introduced by patch for CLJS-674 Created: 21/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: Tim Visher Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix-694.patch    

 Description   

I introduced a call to .toPath, which is defined in Java 7. Need to refactor to avoid this so we can support down to Java 6.



 Comments   
Comment by Tim Visher [ 21/Nov/13 10:47 AM ]

fix-694.patch @ 21/NOV/13

I'm using URI's for relativizing and basic string comparison for directory comparison. URIs have a bug where they can't produce ../../ style relative paths, but since we require that your :output-dir be in the same directory as :source-map, this shouldn't bite us.

Can someone (David Powell?) who has easy access to Java 6 test this?

All tests pass.

Comment by Tim Visher [ 21/Nov/13 3:31 PM ]

I was able to get 1.6 installed:

java version "1.6.0_65"
Java(TM) SE Runtime Environment (build 1.6.0_65-b14-462-11M4609)
Java HotSpot(TM) 64-Bit Server VM (build 20.65-b04-462, mixed mode)

And run the test prior to my patch:

$ JAVA_HOME=/Library/Java/Home V8_HOME=/usr/local/bin SPIDERMONKEY_HOME=/usr/local/bin JSC_HOME=/usr/local/bin ./script/test
java version "1.6.0_65"
Java(TM) SE Runtime Environment (build 1.6.0_65-b14-462-11M4609)
Java HotSpot(TM) 64-Bit Server VM (build 20.65-b04-462, mixed mode)
Reflection warning, cljs/source_map.clj:166:41 - reference to field toPath can't be resolved.
Reflection warning, cljs/source_map.clj:166:41 - reference to field toAbsolutePath can't be resolved.
Reflection warning, cljs/source_map.clj:170:41 - reference to field toPath can't be resolved.
Reflection warning, cljs/source_map.clj:170:41 - reference to field toAbsolutePath can't be resolved.
Reflection warning, cljs/source_map.clj:170:41 - reference to field getParent can't be resolved.
Reflection warning, cljs/source_map.clj:175:18 - call to relativize can't be resolved.
Testing with V8
Tests completed without exception


Testing with SpiderMonkey
out/core-advanced-test.js:463: Error: Assert failed: (not (integer? 1e+308))
Testing with JavaScriptCore
Tests completed without exception


Tested with 3 out of 3 possible js targets

and after the patch:

$ JAVA_HOME=/Library/Java/Home V8_HOME=/usr/local/bin SPIDERMONKEY_HOME=/usr/local/bin JSC_HOME=/usr/local/bin ./script/test
Testing with V8
Tests completed without exception


Testing with SpiderMonkey
out/core-advanced-test.js:463: Error: Assert failed: (not (integer? 1e+308))
Testing with JavaScriptCore
Tests completed without exception


Tested with 3 out of 3 possible js targets
Comment by David Nolen [ 21/Nov/13 3:38 PM ]

Excellent

Comment by David Powell [ 22/Nov/13 12:58 AM ]

Prior to this patch, I have seen cljsbuild emit sourcemaps that refer to files from the /target directory, which if you set :output-dir to "out", is going to require an implementation of relativize() that is able to emit paths starting with ../target

So I'm not sure if something wrong is happening there? I'll have to check that again once the rest of my windows issues are sorted.

I'm still looking at CLJS-681 - the main issue is it omitting files entirely from the sourcemap - I'm getting closer...

Comment by Tim Visher [ 22/Nov/13 6:09 AM ]

@David Powell: After CLJS-674 though, we've restricted what :output-dir can be when using :source-map to ensure that :output-dir will be relative to :source-map's parent so that URI relativization should work fine.

Comment by David Nolen [ 22/Nov/13 7:39 AM ]

fixed, https://github.com/clojure/clojurescript/commit/023b49280a347979548cecc9b555f717b65b884f

Generated at Thu Dec 18 21:20:23 CST 2014 using JIRA 4.4#649-r158309.