Chrome looks for dist/dist/out/foo.cljs instead of dist/out/foo.cljs
You can manually fix it by opening dist/main.map.js and changing all the dist/out to out.
I think the general solution is that all the paths must be relative to the parent of :source-map

Attachments

Activity

:source-map should probably only allow a file name, not a path - it should always be placed in the same directory as the single generated JS file. Before attempting to address we should probably consider limiting what is supported - I'm inclined to say that all paths must be relative to :output-to for simplicity.

David Nolen
added a comment - 10/Nov/13 2:05 PM:source-map should probably only allow a file name, not a path - it should always be placed in the same directory as the single generated JS file. Before attempting to address we should probably consider limiting what is supported - I'm inclined to say that all paths must be relative to :output-to for simplicity.

This patch (fix-674.patch on 15/NOV/13) allows users to specify output-dir or not (defaulting to "out", as usual), and relativizes paths consistently between both the intermediary compilation phase and the final source-map output.

Tim Visher
added a comment - 15/Nov/13 5:23 PM - edited This patch (fix-674.patch on 15/NOV/13) allows users to specify output-dir or not (defaulting to "out", as usual), and relativizes paths consistently between both the intermediary compilation phase and the final source-map output.

I would think that the more sensible approach would be that `:output-to` and `:source-map` would be names, and `:output-dir` would be a required entry in the options map. That way all output goes to the `:output-dir` (which make the name make sense to me), and the file resulting from compilation would reside at its top level, as well as the source map, if specified.

In either case, I wonder if we should issue some sort of warning when using source maps that they are alpha and subject to change, even though that maybe should already be obvious.

As a more general thought, most of why I did it the way I did so far was to avoid breaking people's extant code. Do we feel like this is an alpha enough feature that we're ok with breaking it for people? I suppose that thread you (David) started was silent enough that people don't seem to have much of an opinion.

Tim Visher
added a comment - 17/Nov/13 11:56 AM I would think that the more sensible approach would be that `:output-to` and `:source-map` would be names, and `:output-dir` would be a required entry in the options map. That way all output goes to the `:output-dir` (which make the name make sense to me), and the file resulting from compilation would reside at its top level, as well as the source map, if specified.
In either case, I wonder if we should issue some sort of warning when using source maps that they are alpha and subject to change, even though that maybe should already be obvious.
As a more general thought, most of why I did it the way I did so far was to avoid breaking people's extant code. Do we feel like this is an alpha enough feature that we're ok with breaking it for people? I suppose that thread you (David) started was silent enough that people don't seem to have much of an opinion.

Tim, I thought about it some more. I think my idea of only supporting names is probably unwise because of the resulting breakage for lein-cljsbuild etc. I think your approach is ok, however I note that some combinations result in confusing exceptions for example:

David Nolen
added a comment - 17/Nov/13 12:06 PM - edited Tim, I thought about it some more. I think my idea of only supporting names is probably unwise because of the resulting breakage for lein-cljsbuild etc. I think your approach is ok, however I note that some combinations result in confusing exceptions for example:

In the latest patch the how directories and files are detected is unlikely to work across platforms. For the next release I would like to resolve outstanding issues for Windows users that want source maps.

David Nolen
added a comment - 18/Nov/13 11:49 AM Can we delete old patches please?
In the latest patch the how directories and files are detected is unlikely to work across platforms. For the next release I would like to resolve outstanding issues for Windows users that want source maps.

I put together a patch that would follow some of the intransigents we're defining here (output-dir must be a relative directory, output-to and source-map can only specify a single file). I like this approach as the other option, I think, basically requires special configuration on the server.

I also have it output warnings if it needed to muck with your configuration at all so that people aren't silently confused.

Also, because we sanitize input before hand, it allows the code further in to be a bit simpler.

@David: With your comments I tried out your example with the current patch and it throws, so I'll need to dig in further. I'll keep this up for historical benefit but it doesn't look ready yet.

Tim Visher
added a comment - 18/Nov/13 11:52 AM I put together a patch that would follow some of the intransigents we're defining here (output-dir must be a relative directory, output-to and source-map can only specify a single file). I like this approach as the other option, I think, basically requires special configuration on the server.
I also have it output warnings if it needed to muck with your configuration at all so that people aren't silently confused.
Also, because we sanitize input before hand, it allows the code further in to be a bit simpler.
@David: With your comments I tried out your example with the current patch and it throws, so I'll need to dig in further. I'll keep this up for historical benefit but it doesn't look ready yet.
fix-674-follow-assumptions-about-nature-of-output-dir-output-to-source-map.patch @ 18/Nov/13

David Nolen
added a comment - 18/Nov/13 12:04 PM Tim, I prefer the earlier approach + validation. Anything else will likely just be a source of confusion. CLJS-683 solves the server configuration issue already.
For validation purposes, I think we should just enforce that :output-dir and :source-map be in the same directory as :output-to and call it a day.

1. output-to must be a relative file
2. output-dir must be a relative directory
3. output-to's .getParent must be or contain output-dir
4. if you have named a source-map, its .getParent must be output-to's .getParent
5. if you have named a source-map, it must be a relative file

Tim Visher
added a comment - 18/Nov/13 2:08 PM fix-674.patch @ 18/NOV/13
Adds in assertions for the following conditions:
1. output-to must be a relative file
2. output-dir must be a relative directory
3. output-to's .getParent must be or contain output-dir
4. if you have named a source-map, its .getParent must be output-to's .getParent
5. if you have named a source-map, it must be a relative file

I don't follow what the `:source-map-path` option is getting us over and above `:output-dir`. Anyway, fix-674.patch @ 18/NOV/13 works in all the contexts we've listed so far without this, but I'm just not following if we're trying to use `:source-map-path` in other parts of the code that I'm not thinking of.

Tim Visher
added a comment - 18/Nov/13 2:16 PM - edited Looks like I stomped on source-map-path from CLJS-683. I'll need to refactor. :\
What's the semantics of the options at this point?
A full config might look like:

I don't follow what the `:source-map-path` option is getting us over and above `:output-dir`. Anyway, fix-674.patch @ 18/NOV/13 works in all the contexts we've listed so far without this, but I'm just not following if we're trying to use `:source-map-path` in other parts of the code that I'm not thinking of.

I don't have time to do it right now, but I'm getting a little concerned by the number of knobs we're introducing to support source maps. I think a simpler design is possible. I'd like to whip together a listing of a couple of example configurations and their resulting output before developing this much further as I think the design is changing faster than I (at least) am implementing it.

I need to be offline for the next few hours but I'll try do this before the end of the night so that when I or whoever else finishes this out, we can start from solid ground.

@David: Specifically, what I was asking about was what the prefix attached to the paths in the source-map file would be? In that case would it be `maps/`?

Tim Visher
added a comment - 18/Nov/13 3:32 PM I don't have time to do it right now, but I'm getting a little concerned by the number of knobs we're introducing to support source maps. I think a simpler design is possible. I'd like to whip together a listing of a couple of example configurations and their resulting output before developing this much further as I think the design is changing faster than I (at least) am implementing it.
I need to be offline for the next few hours but I'll try do this before the end of the night so that when I or whoever else finishes this out, we can start from solid ground.
@David: Specifically, what I was asking about was what the prefix attached to the paths in the source-map file would be? In that case would it be `maps/`?

:source-map-path does something not supported by any of the other source map flags, nor does your patch address the problem it's intended to solve. Currently if you specify some relative path, the entire relative path will prefixed to the path of every compiled file. So instead of having to deal with "js/out/foo.js" you will have to configure your web server to understand "public/resources/js/out/foo.js". The :source-map-path option gives you a way to avoid this.

David Nolen
added a comment - 18/Nov/13 3:48 PM:source-map-path does something not supported by any of the other source map flags, nor does your patch address the problem it's intended to solve. Currently if you specify some relative path, the entire relative path will prefixed to the path of every compiled file. So instead of having to deal with "js/out/foo.js" you will have to configure your web server to understand "public/resources/js/out/foo.js". The :source-map-path option gives you a way to avoid this.

This may be a non sequitur, but I'm seeing some concern re: lein-cljsbuild configuration breakage wagging the dog of how the compiler should structure its options. I'm largely a spectator around source maps, but my suggestion (/me puts on his cljsbuild maintainer hat) is to make the CLJS compiler how it should be, and let the tooling work itself out. Taking on debt around configuration / API changes to satisfy tools written to match a known-insufficient existing "schema" doesn't make sense. If downstream coordinating changes are necessary, we can make that happen.

That said, source map configuration is a compiler option, which lein-cljsbuild passes through unmodified AFAIK. Unless I'm wrong there, then we're just talking about breakage of existing user config...for which I'd take the same perspective as above. If there's a right way, make it so now; later will hurt more.

Chas Emerick
added a comment - 18/Nov/13 3:51 PM This may be a non sequitur, but I'm seeing some concern re: lein-cljsbuild configuration breakage wagging the dog of how the compiler should structure its options. I'm largely a spectator around source maps, but my suggestion (/me puts on his cljsbuild maintainer hat) is to make the CLJS compiler how it should be, and let the tooling work itself out. Taking on debt around configuration / API changes to satisfy tools written to match a known-insufficient existing "schema" doesn't make sense. If downstream coordinating changes are necessary, we can make that happen.
That said, source map configuration is a compiler option, which lein-cljsbuild passes through unmodified AFAIK. Unless I'm wrong there, then we're just talking about breakage of existing user config...for which I'd take the same perspective as above. If there's a right way, make it so now; later will hurt more.

I think making :output-dir and :source-map relative to :output-to is more, not less confusing. People already understand making the path relative to where the JVM started up. What I think is correct path forward for this ticket - a) fix the broken cases, b) validate inputs so the user understands what is required.

David Nolen
added a comment - 18/Nov/13 4:20 PM I think making :output-dir and :source-map relative to :output-to is more, not less confusing. People already understand making the path relative to where the JVM started up. What I think is correct path forward for this ticket - a) fix the broken cases, b) validate inputs so the user understands what is required.

I'll attempt to outline the design I think makes the most sense so people can agree or disagree with that specifically.

I believe we need 3 keys: :output-to, :output-dir (not crazy about that name but can't come up with anything better…), and :source-map.

I want to require :output-to and :output-dir. Requiring :output-dir is not strictly necessary, but I think it's sufficiently useful in a wide variety of workflows that it should be called out as required just to teach people about it. Alternatively, it could simple emit a warning if the default value, out, is being used.

:output-to should name a file. It can be a relative path, in the case of a typical ring served app, or absolute path, in the case of a file that should be served out of some pre-existing server.

:output-dir should name a directory. It can be a relative or absolute path, depending on the goal. It will hold the compiled intermediary files as well as all duplicated source files.

:source-map is not required, but if it is specified, it should denote a relative or absolute file. It will contain relativized references to source files in :output-dir.

:output-dir and :output-to must be independent of one another to support the widest variety of workflows. Specifically, you must be able to :output-to a separate directory from :output-dir in order to support using build to prepare a directory for directoly syncing to a server so that it contains nothing but the desired output.

Because :output-dir and :output-to must already be independent of one another, I believe it makes sense to specify :source-map as indepedent of both of them, again to support the widest variety of workflows. Some of those workflows might not make sense, but at this level it probably pays to be permissive rather than prescriptive.

The pro I see of making all three of these configuration inputs independent of one another is that it supports a wide variety of workflows not possible without further scripting when you tie any of them to each other.

For example, if :output-dir was the forced parent of all build output, then a workflow which intended to run a build and then sync the output to a server would have to specify an exclude option or do some further munging.

The biggest con is that there is a lot of repetition. I think the repetition can be forgiven given the explicitness of the configuration (explicit first, later by convention?) and the wide variety of workflows that it supports.

I'll now list out examples of configurations and their build output to illustrate why I named the intransigents I listed above.

;; A production build intended to support bare syncing of a directory to a server
{:output-to "sync/app.js"
:output-dir "target/advanced"}
=>
./sync/app.js
./target/advanced/goog/base.js
./target/advanced/goog/…

Tim Visher
added a comment - 18/Nov/13 9:08 PM - edited I'll attempt to outline the design I think makes the most sense so people can agree or disagree with that specifically.
I believe we need 3 keys: :output-to, :output-dir (not crazy about that name but can't come up with anything better…), and :source-map.
I want to require :output-to and :output-dir. Requiring :output-dir is not strictly necessary, but I think it's sufficiently useful in a wide variety of workflows that it should be called out as required just to teach people about it. Alternatively, it could simple emit a warning if the default value, out, is being used.
:output-to should name a file. It can be a relative path, in the case of a typical ring served app, or absolute path, in the case of a file that should be served out of some pre-existing server.
:output-dir should name a directory. It can be a relative or absolute path, depending on the goal. It will hold the compiled intermediary files as well as all duplicated source files.
:source-map is not required, but if it is specified, it should denote a relative or absolute file. It will contain relativized references to source files in :output-dir.
:output-dir and :output-to must be independent of one another to support the widest variety of workflows. Specifically, you must be able to :output-to a separate directory from :output-dir in order to support using build to prepare a directory for directoly syncing to a server so that it contains nothing but the desired output.
Because :output-dir and :output-to must already be independent of one another, I believe it makes sense to specify :source-map as indepedent of both of them, again to support the widest variety of workflows. Some of those workflows might not make sense, but at this level it probably pays to be permissive rather than prescriptive.
The pro I see of making all three of these configuration inputs independent of one another is that it supports a wide variety of workflows not possible without further scripting when you tie any of them to each other.
For example, if :output-dir was the forced parent of all build output, then a workflow which intended to run a build and then sync the output to a server would have to specify an exclude option or do some further munging.
The biggest con is that there is a lot of repetition. I think the repetition can be forgiven given the explicitness of the configuration (explicit first, later by convention?) and the wide variety of workflows that it supports.
I'll now list out examples of configurations and their build output to illustrate why I named the intransigents I listed above.

;; A production build intended to support bare syncing of a directory to a server
{:output-to "sync/app.js"
:output-dir "target/advanced"}
=>
./sync/app.js
./target/advanced/goog/base.js
./target/advanced/goog/…

This is as I expected. I no longer think the patch is The Right Thing To Do™, but I think it addresses all the concerns we listed before my little design session, and it doesn't need source-map-path.

Am I missing something?

FWIW, the name source-map-path doesn't indicate to me that it's the path that will be prepended, in alternation to output-dir, to the relative source listings inside the source map. Others may feel differently, but that's my opinion. Maybe a better name would be source-file-request-path-parent (which is pretty bad, I know) to specifically denote that it's to specify something for the requests against the server.

This is as I expected. I no longer think the patch is The Right Thing To Do™, but I think it addresses all the concerns we listed before my little design session, and it doesn't need source-map-path.
Am I missing something?
FWIW, the name source-map-path doesn't indicate to me that it's the path that will be prepended, in alternation to output-dir, to the relative source listings inside the source map. Others may feel differently, but that's my opinion. Maybe a better name would be source-file-request-path-parent (which is pretty bad, I know) to specifically denote that it's to specify something for the requests against the server.

@David: Good catch regarding cross platform detection of directories and files. I read up a little bit on java.io.File and there's some goodness I can use from there pretty easily. I don't want to do too much more development till the design is locked down, but it shouldn't be any trouble to make it work cross-platform.

Tim Visher
added a comment - 18/Nov/13 9:30 PM @David: Good catch regarding cross platform detection of directories and files. I read up a little bit on java.io.File and there's some goodness I can use from there pretty easily. I don't want to do too much more development till the design is locked down, but it shouldn't be any trouble to make it work cross-platform.

The problem with your new proposal - the directory contains both transient compiler artifacts and the final output source. This is undesirable, unless I hear more compelling arguments we should keep :source-map-path and move on.

David Nolen
added a comment - 19/Nov/13 10:11 AM - edited The problem with your new proposal - the directory contains both transient compiler artifacts and the final output source. This is undesirable, unless I hear more compelling arguments we should keep :source-map-path and move on.

fix-674.patch @ 19/NOV/13 should address all the concerns listed above, sans the functionality I was hoping for.

Specifically, it checks the following intransigents in a Host independent fashion.

Key-spec:

:output-to
:output-dir
[:source-map
:source-map-path]

Relationship Rules:

1. :output-to names an absolute or relative file
2. :output-dir names an absolute or relative directory, which must be or be contained by the directory containing :output-to
3. :source-map and :source-map-path are optional but must be specified together if at all.
4. If specified, :source-map must name a file that is a sibling to :output-to.
5. If :source-map is specified, :source-map-path must also be specified. Because of the behavior of it, I can't figure out a good way to verify that it's 'correct'. It essentially has to be the unique portion of output-dir against output-to.

I have no new ways to articulate why I think this is a bad direction and so I'll drop the issue and submit the patch that I believe fulfills all of David's requirements. One last time, specifically:

1. I can't figure out a way to make this support creating a one-stop sync dir
2. Because we require :output-dir to be within the same directory as :output-to, build output will be mixed with the source code as a rule.
3. Much more repetition is required amongst all the configuration options (you must specify the same directory prefix 3 times for a source-map)
4. the :source-map-path option, confusingly, requires you to leave off the common directory and only specify the unique portion of the :output-dir option, which is something we could easily have derived.

Anyway, I'm sure I'm just missing something, which would make the patch perhaps not correct. I just want something that support relative source-maps to make it in!

Tim Visher
added a comment - 19/Nov/13 3:17 PM - edited fix-674.patch @ 19/NOV/13 should address all the concerns listed above, sans the functionality I was hoping for.
Specifically, it checks the following intransigents in a Host independent fashion.
Key-spec:

:output-to
:output-dir
[:source-map
:source-map-path]

Relationship Rules:
1. :output-to names an absolute or relative file
2. :output-dir names an absolute or relative directory, which must be or be contained by the directory containing :output-to
3. :source-map and :source-map-path are optional but must be specified together if at all.
4. If specified, :source-map must name a file that is a sibling to :output-to.
5. If :source-map is specified, :source-map-path must also be specified. Because of the behavior of it, I can't figure out a good way to verify that it's 'correct'. It essentially has to be the unique portion of output-dir against output-to.
I have no new ways to articulate why I think this is a bad direction and so I'll drop the issue and submit the patch that I believe fulfills all of David's requirements. One last time, specifically:
1. I can't figure out a way to make this support creating a one-stop sync dir
2. Because we require :output-dir to be within the same directory as :output-to, build output will be mixed with the source code as a rule.
3. Much more repetition is required amongst all the configuration options (you must specify the same directory prefix 3 times for a source-map)
4. the :source-map-path option, confusingly, requires you to leave off the common directory and only specify the unique portion of the :output-dir option, which is something we could easily have derived.
Anyway, I'm sure I'm just missing something, which would make the patch perhaps not correct. I just want something that support relative source-maps to make it in!

Because otherwise we'll just prepend the relative paths with the value of :output-dir which is almost certainly the wrong behavior (it's where we were before this patch, prepending things like "a/relative/out/dir/" to the relative path). Unless we also want to dedup the resulting relative source file with the value of source-map or something, which wouldn't be that hard to add in to the patch.

Tim Visher
added a comment - 19/Nov/13 3:59 PM Because otherwise we'll just prepend the relative paths with the value of :output-dir which is almost certainly the wrong behavior (it's where we were before this patch, prepending things like "a/relative/out/dir/" to the relative path). Unless we also want to dedup the resulting relative source file with the value of source-map or something, which wouldn't be that hard to add in to the patch.

This doesn't follow given the restrictions. Since :output-dir must always be located in the same directory as :source-map, it will simply be the last component of the :output-dir path. :source-map-path is just about overriding that behavior.

David Nolen
added a comment - 19/Nov/13 4:08 PM - edited This doesn't follow given the restrictions. Since :output-dir must always be located in the same directory as :source-map, it will simply be the last component of the :output-dir path. :source-map-path is just about overriding that behavior.

Good point. All that's left on this then is fixing the intransigents (specifically removing the requirement of specifying a :source-map-path if a :source-map is specified) and then also finishing the relativizing of the paths is source_map.clj. I'll be offline for a few hours but I should be able to finish this out tonight.

Tim Visher
added a comment - 19/Nov/13 4:25 PM Good point. All that's left on this then is fixing the intransigents (specifically removing the requirement of specifying a :source-map-path if a :source-map is specified) and then also finishing the relativizing of the paths is source_map.clj. I'll be offline for a few hours but I should be able to finish this out tonight.
I promise I'm not trying to be obtuse! Thanks for your patience.

Tim Visher
added a comment - 19/Nov/13 5:07 PM As a matter of fact, if you're not specifying :source-map then we don't need the constraint of :output-dir being in the same directory as :output-to, which would make setting up a prod sync very easy.
So

Specifically, it checks the following intransigents in a Host independent fashion.

Key-spec:

:output-to
:output-dir
[:source-map]
[:source-map-path]

Relationship Rules:

1. :output-to names an absolute or relative file
2. :output-dir names an absolute or relative directory, which must be or be contained by the directory containing :output-to if :source-map is also specified.
3. :source-map is optional. If specified, it must name a file in the same directory as :output-to
4. :source-map-path is optional. If specified, it must name a directory.

This relaxes the requirement that :output-dir name a relative directory only so that you can set up a prod sync system easily while keeping it (the requirement) if you also want source maps.

Tim Visher
added a comment - 20/Nov/13 7:49 AM fix-674.patch @ 20/NOV/13 I think is the final version!
Specifically, it checks the following intransigents in a Host independent fashion.
Key-spec:

:output-to
:output-dir
[:source-map]
[:source-map-path]

Relationship Rules:
1. :output-to names an absolute or relative file
2. :output-dir names an absolute or relative directory, which must be or be contained by the directory containing :output-to if :source-map is also specified.
3. :source-map is optional. If specified, it must name a file in the same directory as :output-to
4. :source-map-path is optional. If specified, it must name a directory.
This relaxes the requirement that :output-dir name a relative directory only so that you can set up a prod sync system easily while keeping it (the requirement) if you also want source maps.
IMO, the error messages are quite informative.
/me crosses fingers.

The enforcement that :output-to provide a file is not correct, leaving it out means that the output will be sent to standard out. Otherwise looks promising, fingers crossed it addresses our Windows issues too

David Nolen
added a comment - 20/Nov/13 10:06 AM - edited The enforcement that :output-to provide a file is not correct, leaving it out means that the output will be sent to standard out. Otherwise looks promising, fingers crossed it addresses our Windows issues too

1. :output-to is optional. If specified, it names an absolute or relative file
2. :output-dir is required if :output-to is specified. If specified, it names an absolute or relative directory, which must be or be contained by the directory containing :output-to if :source-map is also specified.
3. :source-map is optional. If specified, it must name a file in the same directory as :output-to, and :output-to and :output-dir must also be specified.
4. :source-map-path is optional. If specified, it must name a directory, and :source-map, :output-to, and :output-dir must also be specified.

I'll move ahead with this understanding but I wanted to give everyone the opportunity to correct me.

Tim Visher
added a comment - 20/Nov/13 10:43 AM Interesting. Hadn't considered that use case.
So, at this point is the following more accurate?
Key-spec:

[:output-to
:output-dir
[:source-map]
[:source-map-path]]

Relationship Rules:
1. :output-to is optional. If specified, it names an absolute or relative file
2. :output-dir is required if :output-to is specified. If specified, it names an absolute or relative directory, which must be or be contained by the directory containing :output-to if :source-map is also specified.
3. :source-map is optional. If specified, it must name a file in the same directory as :output-to, and :output-to and :output-dir must also be specified.
4. :source-map-path is optional. If specified, it must name a directory, and :source-map, :output-to, and :output-dir must also be specified.
I'll move ahead with this understanding but I wanted to give everyone the opportunity to correct me.

LOL. That's good because that's what I coded. (I'm in the wrong business...)

Anyway, fix-674.patch @ 20/NOV/13 should address all the issues. :output-to is no longer required. The cascade goes:

If :output-to is specified, check that it's a file (using string?).

If :output-dir is specified, check that it's a directory (using string?) and that :output-to has been specified.

If :source-map is specified, check that it's a sibling file to :output-to and that :output-dir is specified and names a directory within the parent of :output-to. :output-to and :output-dir must have also been specified.

If :source-map-path is specified, check that it's a file (using string?) and ensure that :output-to, :output-dir, and :source-map have also been specified.

Tim Visher
added a comment - 20/Nov/13 11:22 AM LOL. That's good because that's what I coded. (I'm in the wrong business...)
Anyway, fix-674.patch @ 20/NOV/13 should address all the issues. :output-to is no longer required. The cascade goes:
If :output-to is specified, check that it's a file (using string?).
If :output-dir is specified, check that it's a directory (using string?) and that :output-to has been specified.
If :source-map is specified, check that it's a sibling file to :output-to and that :output-dir is specified and names a directory within the parent of :output-to. :output-to and :output-dir must have also been specified.
If :source-map-path is specified, check that it's a file (using string?) and ensure that :output-to, :output-dir, and :source-map have also been specified.

David Nolen
added a comment - 20/Nov/13 12:03 PM The patch does not work for me, I get an assertion about :output-to needing to be a file still. Please make sure to run the tests ./script/test and verify that the REPL works ./script/repljs.

Tim Visher
added a comment - 20/Nov/13 12:17 PM Interesting. It works for me at repl running (build "samples/hello/cljs" {:optimizations :whitespace}).
I'm trying to get up and running with ./script/test now (need to install spidermonkey, v8, and jsc).
Are you running that or did you run something directly that you could paste in?

David Nolen
added a comment - 20/Nov/13 12:26 PM - edited I always run ./script/test and ./script/repljs before pushing a commit. Note you don't need to setup every single JS engine - v8 is enough for this.

Tim Visher
added a comment - 20/Nov/13 12:36 PM - edited Thanks for the tip.
So it's been a long time since I was manually messing with the CLASSPATH at the command line… I'm running into issues with tools.reader not being present as well as clojure.instant.
Reading the scripts, it looks like it expects everything to be in lib but the version of Clojure there is quite old and doesn't have clojure.instant, and tools.reader isn't even there.
Also, testing this on master doesn't produce a different result.
Anyone able to help bootstrap me here? :\

Tim Visher
added a comment - 20/Nov/13 12:57 PM - edited fix-674.patch @ 20/NOV/13. Go!
This fixes the {:output-to :print} case.
I get a couple failures (ethel count, integer?, etc.) but they don't seem to be at all related to anything I've done and fail on master as well.

Tim Visher
added a comment - 21/Nov/13 9:50 AM Not completely, no.
What version of Java do we support? I'm sure I can rework this in terms of older versions if that's necessary. Can we settle on Java 6 since that's what Clojure supports at this point?