ClojureScript

Dependency on Java 7 introduced by patch for CLJS-674

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

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.

Activity

Hide
Tim Visher added a comment -

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.

Show
Tim Visher added a comment - 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.
Tim Visher made changes -
Field Original Value New Value
Attachment fix-694.patch [ 12476 ]
Hide
Tim Visher added a comment -

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
Show
Tim Visher added a comment - 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
Hide
David Nolen added a comment -

Excellent

Show
David Nolen added a comment - Excellent
Hide
David Powell added a comment -

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

Show
David Powell added a comment - 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...
Hide
Tim Visher added a comment -

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

Show
Tim Visher added a comment - @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.
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 (1)

Dates

  • Created:
    Updated:
    Resolved: