[Fix #143] Inline dependencies #183

benedekfazekas · 2015-04-08T08:28:44Z

use mranderson to inline dependencies at package time
simple build script to help working with mranderson
bit of explanation in readme
travis file modified accordingly

tests pass, not yet tested the built middleware tho (will do) but ready for a review I suppose.

expez · 2015-04-08T08:34:29Z

README.md

+
+`lein with-profile +plugin.mranderson/config test`
+
+note the plus sign before the leiningen profile. For this leining profile to work **you need leiningen version 2.5.0+!**


typo: leining

benedekfazekas · 2015-04-08T08:43:09Z

build failure: I guess I messed up building mranderson with java8 and not setting target/source version etc. will repackage this evening. pls check the oracle java8 build for now

bbatsov · 2015-04-08T16:37:39Z

project.clj

@@ -1,25 +1,22 @@
 (def VERSION "0.9.0-SNAPSHOT")

-(def VERSION-FORM `(do (require 'cider-nrepl.plugin)


Aren't you breaking the plugin by removing this?

i deliberately just removed this as this deffo caused the problems i struggled with (see last comments on #143). If I understood project.clj right this magic was only used in dev profile... is that right? why do you need this in dev? what is the usual dev workflow which needs this trick?

We read in plugin.clj. It's a somewhat lame setup and I've been meaning to change this for a while.

bbatsov · 2015-04-10T05:30:49Z

You'll need to rebase this.

bbatsov · 2015-04-10T05:33:04Z

README.md

+
+`lein do clean, source-deps`
+
+this creates the munged local dependencies in target/srcdeps directory


this -> This

Add the missing . to the sentence.

Wrap target/srcdeps in backticks.

bbatsov · 2015-04-10T05:37:21Z

This has to be rebased.

Btw, if you don't use the mranderson profiles nothing will be changed right? Meaning you can develop cider-nrepl without thinking about it all? It's important simply to release the package using inlined deps.

benedekfazekas · 2015-04-10T08:15:55Z

will rebase and also squash commits.

dev workflow: in short yes. in more details you have two options. You start up your REPL as usual and work as usual, you might want to make sure that you run lein clean before to remove any possible leftover of mrandersoned namespaces in target/srcdeps. The other option is to start up mrandersoned repl and modify namespaces under target/srcdeps.

That said it is important to test the code after it is mrandersoned specially if you changed/upgraded dependencies. Also note that the final jar will be much bigger than before as it is practically an uberjar only the deps are in a deeply nested directory structure.

- use mranderson (v0.4.2) to inline dependencies at package time - simple build script to help working with mranderson - exit if any build step fails - bit of explanation in readme - travis file modified accordingly - do **not** use mranderson for code check jobs - explicitly add the project file to the jar and read it from the plugin

benedekfazekas · 2015-04-12T21:04:31Z

all rebased and squashed.

couple of notes:

I modified the travis file so all the tests run with mranderson apart from the code checks (eastwood, etc). they did not run with mranderson and I have not investigated tbh
no idea why the coverage decreased
I modified the location of the project.clj in the jar file, could not make the META-INF path work for some reason
tested the jar locally, by invoking some basic functionality like M ., completion and the like: worked ok

bbatsov · 2015-04-14T05:59:38Z

I modified the travis file so all the tests run with mranderson apart from the code checks (eastwood, etc). they did not run with mranderson and I have not investigated tbh

Don't you have such checks in refactor-nrepl as well?

no idea why the coverage decreased

The difference is tiny so we can ignore it.

I modified the location of the project.clj in the jar file, could not make the META-INF path work for some reason

That's odd, although if the lein plugin is still working properly I guess all is fine.

benedekfazekas · 2015-04-14T06:17:05Z

no, at least not automated with travis. running them with mranderson would mean to check your dependencies' code too, so not sure that makes much sense

the plugin works on the same principals but the file location in the jar is different

[Fix #143] Inline dependencies

bbatsov · 2015-04-14T06:22:38Z

👍 Let's rock!

bbatsov · 2015-04-14T06:25:44Z

Hmm, the tests are failing for me:

ERROR in (breakpoint) (core_deftype.clj:541)
expected: (= (d/breakpoint 10 {}) 10)
  actual: java.lang.IllegalArgumentException: No implementation of method: :send of protocol: #'clojure.tools.nrepl.transport/Transport found for class: nil
 at clojure.core$_cache_protocol_fn.invoke (core_deftype.clj:541)
    clojure.tools.nrepl.transport$eval340$fn__341$G__329__348.invoke (transport.clj:16)
    cider.nrepl.middleware.debug$breakpoint.invoke (debug.clj:44)
    cider.nrepl.middleware.debug_test/fn (debug_test.clj:9)
    clojure.test$test_var$fn__7145.invoke (test.clj:701)
    clojure.test$test_var.invoke (test.clj:701)
    clojure.test$test_all_vars$fn__7149$fn__7156.invoke (test.clj:717)
    clojure.test$default_fixture.invoke (test.clj:671)
    clojure.test$test_all_vars$fn__7149.invoke (test.clj:717)
    clojure.test$default_fixture.invoke (test.clj:671)
    clojure.test$test_all_vars.invoke (test.clj:713)
    clojure.test$test_ns.invoke (test.clj:736)
    clojure.core$map$fn__4207.invoke (core.clj:2487)
    clojure.lang.LazySeq.sval (LazySeq.java:42)
    clojure.lang.LazySeq.seq (LazySeq.java:60)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.next (RT.java:598)
    clojure.core$next.invoke (core.clj:64)
    clojure.core$reduce1.invoke (core.clj:896)
    clojure.core$reduce1.invoke (core.clj:887)
    clojure.core$merge_with.doInvoke (core.clj:2702)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invoke (core.clj:619)
    clojure.test$run_tests.doInvoke (test.clj:751)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.core$apply.invoke (core.clj:617)
    user$eval85$fn__140$fn__171.invoke (form-init322449120891752959.clj:1)
    user$eval85$fn__140$fn__141.invoke (form-init322449120891752959.clj:1)
    user$eval85$fn__140.invoke (form-init322449120891752959.clj:1)
    user$eval85.invoke (form-init322449120891752959.clj:1)
    clojure.lang.Compiler.eval (Compiler.java:6619)
    clojure.lang.Compiler.eval (Compiler.java:6609)
    clojure.lang.Compiler.load (Compiler.java:7064)
    clojure.lang.Compiler.loadFile (Compiler.java:7020)
    clojure.main$load_script.invoke (main.clj:294)
    clojure.main$init_opt.invoke (main.clj:299)
    clojure.main$initialize.invoke (main.clj:327)
    clojure.main$null_opt.invoke (main.clj:362)
    clojure.main$main.doInvoke (main.clj:440)
    clojure.lang.RestFn.invoke (RestFn.java:421)
    clojure.lang.Var.invoke (Var.java:419)
    clojure.lang.AFn.applyToHelper (AFn.java:163)
    clojure.lang.Var.applyTo (Var.java:532)
    clojure.main.main (main.java:37)

lein test cider.nrepl.middleware.format-test

lein test cider.nrepl.middleware.info-test

lein test cider.nrepl.middleware.inspect-test

lein test cider.nrepl.middleware.macroexpand-test

lein test cider.nrepl.middleware.ns-test

lein test cider.nrepl.middleware.pprint-test

lein test cider.nrepl.middleware.resource-test

lein test cider.nrepl.middleware.stacktrace-test

lein test cider.nrepl.middleware.trace-test

lein test cider.nrepl.middleware.undef-test

lein test cider.nrepl.middleware.util.instrument-test

lein test cider.nrepl.middleware.util.java-test
clojure/lang/PersistentHashMap.java:1089: warning: '_' used as an identifier
    Box _ = new Box(null);
        ^
  (use of '_' as an identifier might not be supported in releases after Java SE 8)
clojure/lang/PersistentHashMap.java:1092: warning: '_' used as an identifier
        .assoc(edit, shift, key1hash, key1, val1, _)
                                                  ^
  (use of '_' as an identifier might not be supported in releases after Java SE 8)
clojure/lang/PersistentHashMap.java:1093: warning: '_' used as an identifier
        .assoc(edit, shift, key2hash, key2, val2, _);
                                                  ^
  (use of '_' as an identifier might not be supported in releases after Java SE 8)
clojure/lang/PersistentHashMap.java:1100: warning: '_' used as an identifier
    Box _ = new Box(null);
        ^
  (use of '_' as an identifier might not be supported in releases after Java SE 8)
clojure/lang/PersistentHashMap.java:1102: warning: '_' used as an identifier
        .assoc(edit, shift, key1hash, key1, val1, _)
                                                  ^
  (use of '_' as an identifier might not be supported in releases after Java SE 8)
clojure/lang/PersistentHashMap.java:1103: warning: '_' used as an identifier
        .assoc(edit, shift, key2hash, key2, val2, _);
                                                  ^
  (use of '_' as an identifier might not be supported in releases after Java SE 8)

lein test cider.nrepl.middleware.util.misc-test

lein test cider.nrepl.test-session

lein test cider.nrepl.test-transport

Ran 87 tests containing 305 assertions.
0 failures, 1 errors.
Tests failed.
Error encountered performing task 'test' with profile(s): 'base,system,user,provided,dev,test-clj,1.6,config'
Tests failed.
FAILED

/cc @Malabarba

This happened when I ran build.sh deploy clojars.

bbatsov · 2015-04-14T06:29:08Z

The travis build also failed. @benedekfazekas guess your branch was not up-to-date or something. Please, look into this.

benedekfazekas · 2015-04-14T06:58:16Z

will do

benedekfazekas · 2015-04-14T08:42:16Z

i see a different error both locally and on travis:

Exception in thread "main" java.io.FileNotFoundException: Could not locate debugger/time__init.class or debugger/time.clj on classpath: , compiling:(cider/nrepl/middleware/debug_test.clj:1:1)
    at clojure.lang.Compiler.load(Compiler.java:7142)
    at clojure.lang.RT.loadResourceScript(RT.java:370)
    at clojure.lang.RT.loadResourceScript(RT.java:361)

the cause of this error is that debug_test.clj requires a dependency of cider-nrepl (debugger.time and debugger.config). since it is a dependency its namespace got changed by mranderson.

the reason of this not showing up before that @Malabarba just moved this test ns to the right place lately.

possible solutions include i guess

making sure that you don't require deps namespaces in your tests (may not be a realistic approach)
i look for a way so mranderson processes the test files too to change the ns references

bbatsov · 2015-04-14T10:44:37Z

I like the second option better. :-)

benedekfazekas · 2015-04-14T10:54:37Z

I guessed so actually ;) I will try to find time to look at this asap

swaroopch · 2015-04-16T14:41:58Z

build.sh

+
+# runs source-deps and tests and then provided lein target with mranderson
+
+function check_result {


set -e would make this simpler? http://unix.stackexchange.com/a/166053/3398

looks like it. although what do you think of http://mywiki.wooledge.org/BashFAQ/105

btw @bbatsov surely will wlc a PR ;)

http://mywiki.wooledge.org/BashFAQ/105

Yikes!

expez reviewed Apr 8, 2015
View reviewed changes

bbatsov reviewed Apr 8, 2015
View reviewed changes

bbatsov reviewed Apr 10, 2015
View reviewed changes

bbatsov added a commit that referenced this pull request Apr 14, 2015

Merge pull request #183 from benedekfazekas/master

e2b6f40

[Fix #143] Inline dependencies

bbatsov merged commit e2b6f40 into clojure-emacs:master Apr 14, 2015

bbatsov mentioned this pull request Apr 14, 2015

Update 'With Immutant/WildFly' section #192

Merged

benedekfazekas mentioned this pull request Apr 15, 2015

tests which require mrandersoned dependencies of a project fail benedekfazekas/mranderson#5

Open

swaroopch reviewed Apr 16, 2015
View reviewed changes

bbatsov mentioned this pull request Jun 18, 2015

Isolate runtime deps xsc/lein-ancient#56

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix #143] Inline dependencies #183

[Fix #143] Inline dependencies #183

benedekfazekas commented Apr 8, 2015

expez Apr 8, 2015

benedekfazekas commented Apr 8, 2015

bbatsov Apr 8, 2015

benedekfazekas Apr 8, 2015

bbatsov Apr 10, 2015

bbatsov commented Apr 10, 2015

bbatsov Apr 10, 2015

bbatsov commented Apr 10, 2015

benedekfazekas commented Apr 10, 2015

benedekfazekas commented Apr 12, 2015

bbatsov commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

bbatsov commented Apr 14, 2015

bbatsov commented Apr 14, 2015

bbatsov commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

bbatsov commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

swaroopch Apr 16, 2015

benedekfazekas Apr 16, 2015

swaroopch Apr 16, 2015


		`lein with-profile +plugin.mranderson/config test`

		note the plus sign before the leiningen profile. For this leining profile to work you need leiningen version 2.5.0+!

		@@ -1,25 +1,22 @@
		(def VERSION "0.9.0-SNAPSHOT")

		(def VERSION-FORM `(do (require 'cider-nrepl.plugin)


		`lein do clean, source-deps`

		this creates the munged local dependencies in target/srcdeps directory


		# runs source-deps and tests and then provided lein target with mranderson

		function check_result {

[Fix #143] Inline dependencies #183

[Fix #143] Inline dependencies #183

Conversation

benedekfazekas commented Apr 8, 2015

Choose a reason for hiding this comment

benedekfazekas commented Apr 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbatsov commented Apr 10, 2015

Choose a reason for hiding this comment

bbatsov commented Apr 10, 2015

benedekfazekas commented Apr 10, 2015

benedekfazekas commented Apr 12, 2015

bbatsov commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

bbatsov commented Apr 14, 2015

bbatsov commented Apr 14, 2015

bbatsov commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

bbatsov commented Apr 14, 2015

benedekfazekas commented Apr 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment