<< Back to previous view

[CLJS-1437] Recompile dependents broken when files are not inside one directory Created: 30/Aug/15  Updated: 05/Oct/15

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: build

Attachments: Text File cljs-1437-3a.patch     Text File cljs-1437-3b.patch    


When a file is changed files dependent on that file are marked for recompile. This doesn't seem to work when the changed file is not in the directory which is passed to build and the directory where dependent namespace is.

Looks like this caused by how build and compile-file work:

  • Build calls -compile -> compile-dir -> compile-root which sorts the files in dependency order and calls -compile for the files in dependency order
    • core ns is compiled
  • Build calls add-depedencies -> add-dependencies -> get-compiled-cljs -> -compile -> compile-file
    • test ns is compiled, core ns is marked as dependent on test ns but as core ns is already compiled this doesn't matter

I believe this effectively breaks :recompile-dependents in cases all cases but one where all the sources are inside one directory. In multiple directory case order of directories passed to cljs.build.api/inputs affects the order on which compile is called so it's possible that some dependent namespaces are compiled before namespaces they depend on are compiled.

Original problem was that added tests were not picked up by run-tests but I have since reproduced this without cljs.test.

The minimal way to reproduce this needs two namespaces, where one depends on another. To display that the dependent namespace is not recompiled, it will print some compile time metadata from the another namespace.


(ns hello.core
  (:require hello-world.test))


(println (:foo (meta #'hello.test/a)))


(ns hello.test)

(def ^{:foo :bar} a "foobar")

To build:

(require '[cljs.build.api :refer [build]])

(def opts {:main "hello.core"
           :output-to "out/main.js"
           :output-dir "out"
           :optimizations :none
           :verbose true})

(defn broken []
  (build "src" opts))

(defn working []
  " Note: ordering of the dirs matters
  (build (inputs "src2" "src" opts)))

To trigger the problem, the metadata needs to be changed after running the build once. Changing test ns doesn't cause core ns to recompiled and changing the metadata doesn't have any effect to println on core ns.

Comment by Juho Teperi [ 02/Sep/15 5:55 PM ]

First version for review.

Still missing support for finding sources for any build source parameter. Previously this was provided through Compilable interface but that doesn't make sense anymore as we don't want to compile them but find the namespaces.

Function naming could use some thinking.

This includes minimally changed alternative build function: build-new. I think future improvements to recompile-dependents would be limited to compile-sources where it would be possible to mark all dependent namespaces for recompile.

Comment by Juho Teperi [ 09/Sep/15 2:02 PM ]

Separate patches for deps bits and another for alternative build implementation.

Comment by Juho Teperi [ 22/Sep/15 1:05 PM ]

A patch contains the new functions.
B modifies build to use new pipeline.


To support different kind of source parameters, I added new method to Compilable protocol which only finds the source files in given source (dir, file, url...). There is still problem with supporting giving the source as list or vector, as I'm not sure if it's possible to create IJavaScript map from already read source.

Compile-file is still doing the recompile-dependents logic which is not optimal, but it's probably best to implement those changes separately.

Comment by Juho Teperi [ 22/Sep/15 1:14 PM ]

I noticed I forgot to update build function to use new -find-sources method. Fixed in 3a and 3b patches.

Comment by David Nolen [ 22/Sep/15 10:23 PM ]

Please remove all patches that don't just implement the new pipeline. Thanks! It just makes figuring out what patch to look at that much more complicated.

Comment by Juho Teperi [ 23/Sep/15 12:38 PM ]

Removed old patches.

Comment by Juho Teperi [ 29/Sep/15 2:57 PM ]

I noticed I had accidentally committed automatic version changes in second patch. Fixed now.

[CLJ-1509] Some clojure namespaces not AOT-compiled and included in the clojure jar Created: 20/Aug/14  Updated: 20/Aug/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: build

Patch: Code
Approval: Triaged


There is a list of namespaces to AOT in build.xml and several namespaces are missing from that list, thus no .class files for those namespaces are created or included in the standard clojure jar file as part of the build.

Missing namespaces include:

  • clojure.core.reducers
  • clojure.instant
  • clojure.parallel
  • clojure.uuid

Proposal: Attached patch sorts the ns list alphabetically (for easier maintenance) and adds clojure.instant and clojure.uuid to the compiled namespaces. clojure.parallel is deprecated and requires the JSR-166 jar so was not included (perhaps it's a separate ticket to remove this). clojure.core.reducers uses a compile-time check to choose the fork/join packages to use so cannot be compiled early.

Patch: clj-1509.diff

Screened by:

Comment by Alex Miller [ 20/Aug/14 1:06 PM ]

Looking at this a bit further, clojure.core.reducers uses the compile-if macro to determine what version of fork/join is available so AOT-compiling this namespace would fix that decision at build time rather than runtime, so it cannot be included.

[CLJ-1268] Require Java 1.6 as minimum for Clojure Created: 28/Sep/13  Updated: 25/Oct/13  Resolved: 25/Oct/13

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: Release 1.6

Type: Enhancement Priority: Blocker
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: build

Attachments: Text File clj-1268.patch    
Patch: Code
Approval: Ok


In Clojure 1.6, we plan to move to JDK 1.6 as the minimum JDK.

Patch: clj-1268.patch

This patch changes the build configurations for both Maven and Ant to assume JDK 1.6 as the "source" and "target" runtimes.

Configuration changes will be necessary on Hudson. We already build Clojure and contrib libraries on JDK 1.6 by default, but we will need to remove matrix test builds for JDK 1.5. See for example clojure-test-matrix and data.csv-test-matrix – coordinate with Stuart Sierra for this change.

Comment by Stuart Sierra [ 04/Oct/13 8:45 AM ]


I have verified that both the Ant and Maven builds still work (running on JDK 1.7) and that the output .class files contain the bytecode header for JDK 1.6.

[CLJ-250] debug builds Created: 27/Jan/10  Updated: 23/Oct/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.2
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: build

Approval: Vetted
Waiting On: Rich Hickey


This ticket includes two patches:

  1. a patch to set assert when clojure.lang.RT loads, based on the presence of system property clojure.debug
  2. expand error messages in assert to include local-bindings</code> (a new macro which wraps the implicit <code>&env)

Things to consider before approving these patches:

  1. should there be an easy Clojure-level way to query if debug is enabled? (checking assert isn't the same, as debug should eventually drive other features)
  2. assertions will now be off by default – this is a change!
  3. is the addition of the name local-bindings to clojure.core cool?

Comment by Assembla Importer [ 24/Aug/10 6:05 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/250
add-clojure-debug-flag.patch - https://www.assembla.com/spaces/clojure/documents/aUWn50c64r35E-eJe5aVNr/download/aUWn50c64r35E-eJe5aVNr
assert-report-locals.patch - https://www.assembla.com/spaces/clojure/documents/aUWqLSc64r35E-eJe5aVNr/download/aUWqLSc64r35E-eJe5aVNr

Comment by Stuart Halloway [ 07/Dec/10 8:23 PM ]

Ignore the old patches. Considering the following implementation, please review and then move ticket to waiting on Stu:

  1. RT will check system property "clojure.debug", default to false
  2. property will set the root binding for the current *assert*, plus a new *debug* flag. (Debug builds can and will drive other things than just asserts.)
  3. does Compile.java need to push *assert* or *debug* as thread local bindings, or can they be root-bound only when compiling clojure?
  4. will add *debug* binding to clojure.main/with-bindings. Anywhere else?
  5. build.xml should not have to change – system properties will flow through (and build.xml may not be around much longer anyway)
  6. once we agree on the approach, I will ping maven plugin and lein owners so that they flow the setting through
  7. better assertion messages will be a separate ticket
  8. what is the interaction between *debug* and *unchecked-math*? Change checks to (and *unchecked-math* (not *debug*))}?
Comment by Rich Hickey [ 08/Dec/10 11:00 AM ]

#3 - root bound only
#4 - should not be in with-bindings for same reason as #3 - we don't want people to set! *debug* nor *assert*
#8 - yes, wrapping that in a helper fn

#6 - my biggest reservation is that this isn't yet informed by maven best practices

Comment by Stuart Sierra [ 08/Dec/10 2:09 PM ]

System properties can be passed through Maven, so I do not anticipate this being a problem.

However, I would prefer *assert* to remain true by default.

Comment by Chas Emerick [ 09/Dec/10 7:19 AM ]

SS is correct about this approach not posing any issue for Maven. In addition, the build could easily be set up to always emit two jars, one "normal", one "debug".

I'd suggest that, while clojure.debug might have broad effect, additional properties should be available to provide fine-grained control over each of the additional "debug"-related parameterizations that might become available in the future.

I'd like to raise a couple of potentially tangential concerns (after now thinking about assertions for a bit in the above context), some or all of which may simply be a result of my lack of understanding in various areas.

Looking at where assert is used in core.clj (only two places AFAICT: validating arguments to derive and checking pre- and post-conditions in fn), it would seem unwise to make it false by default. i.e. non-Named values would be able to get into hierarchies, and pre- and post-conditions would simply be ignored.

It's my understanding that assertions (talking here of the JVM construct, from which Clojure reuses AssertionError) should not be used to validate arguments to public API functions, or used to validate any aspect of a function's normal operation (i.e. "where not to use assertions"). That would imply that derive should throw IllegalArugmentException when necessary, and fn pre- and post-conditions should perhaps throw IllegalStateException – or, in any case, something other than AssertionError via assert. This would match up far better with most functions in core using assert-args rather than assert, the former throwing IllegalArgumentException rather than AssertionError.

That leads me to the question: is assert (and *assert*) intended to be a Clojure construct, or a quasi-interop form?

If the former, then it can roughly have whatever semantics we want, but then it seems like it should not be throwing AssertionError.

If the latter, then AssertionError is appropriate on the JVM, but then we need to take care that assertions can be enabled and disabled at runtime (without having to switch around different builds of Clojure), ideally using the host-defined switches (e.g. -ea and friends) and likely not anything like *assert*. I don't know if this is possible or practical at this point (I presume this would require nontrivial compiler changes).

Hopefully the above is not water under the bridge at this point. Thank you in advance for your forbearance.

Comment by Rich Hickey [ 09/Dec/10 8:08 AM ]

Thanks for the useful input Chas. Nothing is concluded yet. I think we should step back and look at the objective here, before moving forward with a solution. Being a dynamic language, there are many things we might want to validate about our programs, where the cost of checking is something we are unwilling to pay in production.

Being a macro, assert has the nice property that, should *assert* not be true during compilation, it generates nil, no conditional test at all. Thus right now it is more like static conditional compilation.

Java assert does have runtime toggle-ability, via -ea as you say. I haven't looked at the bytecode generated for Java asserts, but it might be possible for Clojure assert to do something similar, if the runtime overhead is negligible. It is quite likely that HotSpot has special code for eliding the assertion bytecode given a single check of some flag. I'm just not sure that flag is Class.desiredAssertionStatus.

Whether this turns into changes in assert or pre/post conditions, best practices etc is orthogonal and derived. Currently we don't have a facility to run without the checks. We need to choose between making them disappear during compilation (debug build) or runtime (track -ea) or both. Then we can look at how that is reflected in assert/pre-post and re-examine existing use of both. The "where not to use assertions" doc deals with them categorically, but in not considering their cost, seems unrealistic IMO.

I'd appreciate it if someone could look into how assert is generated and optimized by Java itself.

Comment by Chas Emerick [ 09/Dec/10 5:04 PM ]

Bytecode issues continue to be above my pay grade, unfortunately…

A few additional thoughts in response that you may or may not be juggling already:

assert being a macro makes total sense for what it's doing. Trouble is, "compile-time" is a tricky concept in Clojure: there's code-loading-time, AOT-compile-time, and transitively-AOT-compile-time. Given that, it's entirely possible for an application deployed to a production environment that contains a patchwork of code or no code produced by assert usages in various libraries and namespaces depending upon when those libs and ns' were loaded, AOT-compiled, or their dependents AOT-compiled, and the value of *assert* at each of those times. Of course, this is the case for all such macros whose results are dependent upon context-dependent state (I think this was a big issue with clojure.contrib.logging, making it only usable with log4j for a while).

What's really attractive about the JVM assertion mechanism is that it can be parameterized for a given runtime on a per-package basis, if desired. Reimplementing that concept so that assert can be *ns*-sensitive seems like it'd be straightforward, but the compile-time complexity already mentioned remains, and the idea of having two independently-controlled assertion facilities doesn't sound fun.

I know nearly nothing about the CLR, but it would appear that it doesn't provide for anything like runtime-controllable assertions.

Comment by Stuart Halloway [ 29/Dec/10 3:17 PM ]

The best (dated) evidence I could find says that the compiler sets a special class static final field $assertionsDisabled based on the return of desiredAssertionStatus. HotSpot doesn't do anything special with this, dead code elimination simply makes it go away. The code indeed compiles this way:

11: getstatic #6; //Field $assertionsDisabled:Z
14: ifne 33
17: lload_1
18: lconst_0
19: lcmp
20: ifeq 33
23: new #7; //class java/lang/AssertionError
26: dup
27: ldc #8; //String X should be zero
29: invokespecial #9; //Method java/lang/AssertionError."<init>":(Ljava/lang/Object;)V
32: athrow

Even if we were 100% sure that assertion removal was total, I would still vote for a separate Clojure-level switch, for the following reasons:

  1. I have a real and pressing need to disable some assertions, and I don't need the Java interop at all. Arguably others will be in the same boat.
  2. there will be multiple debugging facilities over time, and having a top-level debug switch is convenient for Clojure users.
  3. Java dis/enabling via command line flags is still possible as a separate feature. We could add this later as a (small) breaking change to our assert, or have a separate java-assert interop form. I am on the fence about which way to go here.
  4. I believe it is perfectly fine to throw an AssertionError from a non-Java-assertion-form. We don't believe in a world of a static exception hierarchy, and an assertion in production is a critical failure no matter what you call it. Even Scala does it http://daily-scala.blogspot.com/2010/03/assert-require-assume.html

Rich: awaiting your blessing to move forward on this.

Comment by Rich Hickey [ 07/Jan/11 9:42 AM ]

The compiler sets $assertionsDisabled when, in static init code? Is there special support in the classloader for it? Is there a link to the best dated evidence you found?

Comment by Stuart Halloway [ 07/Jan/11 9:51 AM ]
  1. Yes, in static init code
  2. There is no special support in the classloader, per Brian Goetz (private correspondence) last week. But dead code elimination is great: "The run-time cost of disabled assertions should indeed be zero for compiled code"
Comment by Stuart Halloway [ 07/Jan/11 9:57 AM ]

Link: Google "java assert shirazi". (Not posting link because I can't tell in 10 secs whether it includes my session information.)

Comment by Alexander Kiel [ 14/Mar/13 1:28 PM ]

Is there anything new on this issue? I also look for a convenient way to disable assertions in production.

Comment by Michał Łopuszyński [ 11/Oct/13 4:21 AM ]

I am also interested in any news on this issue.
Convenient way to enable/disable assertions at runtime (preferably via -ea/-da options) would be a great feature!

Comment by Michał Łopuszyński [ 23/Oct/13 3:12 AM ]

Btw. there is a library for runtime-toggable assertions available via clojars
This helped me a great deal.

Generated at Wed Oct 07 01:37:44 CDT 2015 using JIRA 4.4#649-r158309.