Fork me on GitHub
#cider
<
2018-05-07
>
bozhidar04:05:06

@bhauman Consider it done!

bozhidar05:05:11

@arrdem Seems something’s wrong with the content-type middleware - I did some tests before cutting 0.17 and I don’t get any responses from the middleware when evaluating some images.

dominicm05:05:23

│   └── cider
│       └── nrepl
│           └── test
│               ├── AnotherTestClass.class
│               ├── TestClass.class
│               └── YetAnotherTest.class
Tests are in the cider release

bozhidar05:05:54

Now I’m wondering whether to cut 0.17 regardless of this.

dominicm05:05:56

It's worth having a quick poke at https://github.com/clojure-emacs/cider-nrepl/pull/524 fixing this at the root perhaps.

dominicm05:05:34

@bozhidar images that worked before? or things that weren't originally checked for?

bozhidar05:05:59

I just assumed they’d work, as @arrdem had tested those. 🙂

bozhidar05:05:27

Then I noticed one ticket where @stardiviner mentioned they don’t work for him and I decided to try those myself - same result.

bozhidar05:05:08

As for your other comment - I’m not sure what exactly am I supposed to do here.

bozhidar05:05:01

I see that the artefact is not properly generated, but I don’t know why, and I think this has been like this for ages.

dominicm05:05:43

it was right in 0.16 from what I can tell, except that we didn't have the main.

dominicm05:05:05

I suspect we're "doing" profiles wrong, but with no real reason to that.

dominicm05:05:52

definitely to do with using profiles.

bozhidar05:05:18

Nothing in project.clj seems off to me. Maybe @gonewest818 would have more insight into this.

gonewest81818:05:08

sorry… I was completely offline handling a family matter last week and through the weekend. Looks like this got resolved though.

bozhidar05:05:52

Seems this release is truly cursed. 😄 Makes me wonder I should cut it even somewhat broken just to be done with it…

dominicm05:05:40

It's because we're now using +. I think leiningen's profiles have always been a little bit wonky around with-profile and later tasks, because how can it get the right profile set?

dominicm05:05:30

Changing install to:

install: .source-deps
	lein with-profile plugin.mranderson/config install
makes the generated pom.xml have the correct scoping for tools.nrepl

bozhidar05:05:48

> because how can it get the right profile set? I don’t quite follow. Aren’t we supposed to be providing multiple profiles with + normally? I can certainly issue the final release manually without the Clojure version profile, though.

dominicm05:05:50

There's two syntaxes that with-profile takes. +profile and profile. The + syntax is "in addition to defaults". Unless the task pipeline is run in reverse, I've never really understood what the defaults would be in cases like lein with-profile +blah do clean, deploy Would it be the defaults of clean or deploy?

dominicm05:05:48

I'll have a fiddle with this

dominicm05:05:29

OH, I've got it

bozhidar05:05:30

Ah, got it. That’s pretty messed up. I keep dreaming of simpler tools. 😄

dominicm05:05:59

@bozhidar Remove tools.nrepl from the :dev profile.

bozhidar05:05:59

OK, I’ll do this in a bit.

dominicm05:05:03

❯ clj -Sdeps '{:deps {cider/cider-nrepl {:mvn/version "0.17.0-NO-SNAPSHOT"} }}' -e '(require (quote cider-nrepl.main)) (cider-nrepl.main/init ["cider.nrepl/cider-middleware"])'
nREPL server started on port 43749 on host 0:0:0:0:0:0:0:0 - nrepl://0:0:0:0:0:0:0:0:43749
Removed tools.nrepl from my ~/.clojure/deps.edn file.

dominicm06:05:46

Quite pleased we figured that out 🙂

bozhidar06:05:19

So, so the new snapshot should be up in a bit.

arrdem06:05:28

Okay. I can take a poke at all that for real. Gimme a sec here.

arrdem06:05:36

Totally worked on my machine when I PR’d it 😉

bozhidar06:05:04

@arrdem 😄 Sure. Ideally I’d love for this to work in 0.17. 🙂

arrdem06:05:25

Yeah for sure.

arrdem06:05:54

A lot of my problems with the content-types work have been around the pprint middleware which makes stupid assumptions about being the only thing which will transform the result of eval.

arrdem06:05:29

I suspect that a /real/ fix will involve reworking the pprint middleware and the cider-repl.el side so they always play nicely.

arrdem06:05:57

middleware ordering was definitely something which rendered my local testing unreliable :c

arrdem06:05:47

It’s not hard - the pprint middleware right now assumes that it needs to intercept the :value produced by eval and destroy it so that a REPL doesn’t see both the :value response and the :pprint output.

arrdem06:05:38

I’ve already got some logic in cider-repl.el to let it be smarter about whether to display the result of evaluation based on whether it’s seen pretty-printing or content-type results.

arrdem06:05:05

so really this is just dumb old non-composable behavior I just need to clean up.

dominicm06:05:24

Nrepl itself has something which strings the value too.

arrdem06:05:38

Which is why the content-type middleware doesn’t use the :value key, it uses :body or something else dumb to sidestep that.

bozhidar06:05:27

Well, certainly the pprint stuff will have to be adjusted at some point. I think the only reason why this is in its current state was to accommodate for the fucked up way in which response handling works right now. This huge single handler function that handles every type of response made sense before we used custom middlewares, but now it’s just making everything complex.

bozhidar06:05:38

In practical terms the problem was that we needed to pretty-print results without discarding them, as traditional pretty-printing works (you don’t want to print a string and have a result that’s nil - you want to keep the result, just show it pretty).

dominicm07:05:13

looks like refactor nrepl is broken with the cider snapshots, something about util/cljs?

bozhidar07:05:17

Only the snapshot of refactor-nrepl works with the snapshot of cider-nrepl right now.

dominicm07:05:07

maybe my snapshot hasn't updated 🙂

bozhidar07:05:26

For some reason they don’t do releases often these days - the last one was about a year ago.

bozhidar07:05:42

I see that 2.3.1 is compatible with cider-nrepl 0.14 🙂

dominicm07:05:34

yeah, I noticed 😛

bozhidar07:05:03

I’m not involved in refactor-nrepl and that’s problematic as I might accidentally change something which I don’t even know they are using. Let’s just keep dreaming of 1.0 and the stability that will come with it. 😉

bozhidar07:05:00

Also - refactor-nrepl still doesn’t defer middleware loading, so as soon as you add it to your deps the repls start 20 times slower.

bozhidar08:05:22

@dominicm I just noticed that the dep change broke all the cljs tests. 😄

bozhidar08:05:23

Maybe we should add this dep to the :test-cljs profile?

bozhidar08:05:19

I’m puzzled why this is happening as piggieback depends on tools.nrepl, so I would not have expected to have to declare the dep explicitly. Perhaps some leiningen bug?

bozhidar08:05:00

Not to mention cider-nrepl depends on tools.nrepl as well. All of this profile interactions feel like magic half the time.

dmitryn09:05:51

does cider-format-buffer use same rules as cljfmt?

bozhidar09:05:11

It just uses cljfmt internally.

dmitryn09:05:30

Thanks, good to know

r0man09:05:07

hello, it looks like the latest cider doesn't find my tests anymore. When I run a test via cider-test-run-test, it tells me: "No assertions (or no tests) were run.Did you forget to use ‘is’ in your tests?" Any ideas?

r0man09:05:29

the same happens also with cider-test-run-ns-tests

bozhidar09:05:41

Likely a regression from @dominicm’s changes yesterday.

bozhidar09:05:55

Damn, now it seems we’re really not doing 0.17 today. 😞

r0man09:05:40

ok, thanks, I'll try rolling back

bozhidar09:05:16

And the CI keeps failing… Guess it wasn’t meant to be… I’ll call it a day and revisit this release at some later point in time…

dominicm09:05:27

I shall fix my changes 🙂 I've spent about 4 hours enjoying some sunny reading, and it's a public holiday

dominicm10:05:43

Hum, the test stuff still works for me.

nicola10:05:49

tests are broken

dominicm10:05:51

Or at least, test-all does.

nicola10:05:57

nice monday 🙂

nicola10:05:39

no assertions..... 😞

dominicm10:05:52

@nicola what way are you running tests.

nicola10:05:52

can you revert 17-snapshot to working version?

dominicm10:05:11

I don't think there's any tests around the test op, I have a suspicion about what is up

nicola10:05:25

cider-test-run-….

dominicm10:05:19

Okay. I suspect cider-test-run-loaded-tests is the one to run for now. Just going to test my hypothesis, I can have my fixes within the hour I suspect

nicola10:05:49

yes looks like run-loaded-tests only works, will wait - thank you

bozhidar10:05:45

> I’ve spent about 4 hours enjoying some sunny reading, and it’s a public holiday

bozhidar10:05:12

It’s a public holiday here as well, but I’m actually both working and trying to ship this damn release. 😄

dominicm10:05:43

@bozhidar I was counting on nil a bit too much in the test op is all 🙂 A slight alteration to the generation of the var query will sort this 🙂

nicola10:05:06

getting worse - cider-connect -

direct connection to .... failed

dominicm10:05:20

Later I can throw up some tests around test

bozhidar10:05:59

@nicola Nothing has been changed for cider-connect recently.

bozhidar10:05:30

@dominicm Can you also try to run lein with-profile +1.9,+test-cljs test? For me locally it hangs so it’s hard to test anything. I’m not sure whether moving tools.nrepl out of dev or updating piggieback broke this and the CI takes 1.5h…

dominicm10:05:26

@bozhidar no problem

dominicm10:05:09

For me, the test op works.

dominicm10:05:36

or rather, the ns form with only ns works fine. The other form does not.

dominicm10:05:36

I'm thinking that cider sends [] in that case perhaps?

bozhidar10:05:39

I think it’s encoded as [] 🙂 Unfortunately in Elisp there’s no boolean type and nil and an empty list are the same.

bozhidar10:05:03

I have to check, but I can tell you it was something fucked up for sure.

bozhidar10:05:36

Some of the odd middleware interactions stem from this nil/empty list problem.

dominicm10:05:43

Makes sense, if you send an empty list, I treat it as an empty list in coercion. This is potentially difficult, I suppose it doesn't make much sense to send an empty :exactly.

dominicm10:05:26

My approach has been to not add a key if it doesn't make sense.

dominicm10:05:31

Same for booleans like load?

bozhidar10:05:03

Perhaps we should just use "false" or :false in such cases.

dominicm10:05:00

because load? is either [] or 1 (or whatever you encode true as).

dominicm10:05:05

Both of which are truthy in clojure.

dominicm10:05:43

Either way, quick fix is to put a seq around the coercion to :exactly. We can figure out how to fix cider.el later 😉

bozhidar10:05:50

I guess so.

bozhidar10:05:54

Looking at this bit of code it needs a lot of work, as all those whens can backfire.

dominicm10:05:23

There are a number of emacsisms that I have encountered in cider-nrepl tbh

dominicm10:05:34

it might be fine to just accept that [] is false

bozhidar10:05:05

Not really - there are cases where you’re really just passing an empty list. 🙂

dominicm10:05:17

there's that too 🙂

bozhidar10:05:22

In certain cases it’s fine - if you know you’re expecting a boolean, but not all the time.

bozhidar10:05:15

I think I was generally writing code like if something is nil, false or [] else whatever, but I didn’t write all the code.

bozhidar10:05:38

And as I said - this works only if you know you’re expecting a boolean. 🙂

dominicm10:05:40

If you like, I can handle the [] case in the coercion for the load? key, and that path can be fixed.

dominicm10:05:35

I've done it, can be removed in the review if you dislike it 🙂

dominicm10:05:29

just running some final tests to make sure I've got it covered 🙂

dominicm10:05:57

PR opened for fixing test op 🙂

dominicm10:05:28

I'll have a look at the errors in the cljs test cases, then go shower and see what I come up with

bozhidar10:05:34

Much appreciated!

dominicm11:05:23

If I revert 08e02870302d96ae2ebda0c4bca2735c3384ed75 (cider piggieback version bump) then the tests pass

dominicm11:05:53

Reverting cdd9450c7e034065085dc458c827a0d6307e775b is also recommended, as it doesn't fix anything.

dominicm12:05:09

Okay, I think I've got it

dominicm12:05:18

@bozhidar the issue seems to be that cider piggieback 0.3.1-2 doesn't support node cc/ @bhauman

dominicm12:05:53

Every message results in:

{:ex "class java.io.IOException", :id "4d36caef-b1a3-4dc8-89d6-6cfbe53369fc", :root-ex "class java.io.IOException", :session #{"b54a5b1f-78f5-4673-942d-1abe29cc17ab"}, :status #{"eval-error" "done"}, :err "java.io.IOException: Stream closed\n\tat java.io.BufferedWriter.ensureOpen(BufferedWriter.java:116)\n\tat java.io.BufferedWriter.write(BufferedWriter.java:221)\n\tat java.io.Writer.write(Writer.java:157)\n\tat cljs.repl.node$write.invokeStatic(node.clj:36)\n\tat cljs.repl.node$write.invoke(node.clj:35)\n\tat cljs.repl.node$node_eval.invokeStatic(node.clj:57)\n\tat cljs.repl.node$node_eval.invoke(node.clj:52)\n\tat cljs.repl.node.NodeEnv._evaluate(node.clj:211)\n\tat cljs.repl$evaluate_form.invokeStatic(repl.cljc:518)\n\tat cljs.repl$evaluate_form.invoke(repl.cljc:452)\n\tat cider.piggieback$eval_cljs.invokeStatic(piggieback.clj:124)\n\tat cider.piggieback$eval_cljs.invoke(piggieback.clj:123)\n\tat cider.piggieback$do_eval.invokeStatic(piggieback.clj:150)\n\tat cider.piggieback$do_eval.invoke(piggieback.clj:131)\n\tat cider.piggieback$evaluate.invokeStatic(piggieback.clj:173)\n\tat cider.piggieback$evaluate.invoke(piggieback.clj:171)\n\tat clojure.lang.Var.invoke(Var.java:381)\n\tat cider.piggieback$wrap_cljs_repl$fn__9356$fn__9358$fn__9359.invoke(piggieback.clj:202)\n\tat cider.piggieback$enqueue$fn__9336.invoke(piggieback.clj:106)\n\tat clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__1055.invoke(interruptible_eval.clj:190)\n\tat clojure.lang.AFn.run(AFn.java:22)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat java.lang.Thread.run(Thread.java:748)\n"}

dominicm12:05:21

The cider clojurescript tests all pass when I switch node for nashorn.

bozhidar12:05:18

I’ll switch back to 0.3.0, so the tests would pass and we can fix this later. Guess some of the refactorings in 0.3.1 broke the node repl.

dominicm12:05:48

Yeah, I presume so.

bozhidar12:05:05

And were you able to run the tests locally? I really wonder why the hang for me…

dominicm12:05:36

I was able to run them locally, yep

dominicm12:05:41

Do you have node installed? 😄

bozhidar12:05:13

I do, but now when I tried to run it I got a funny error

bozhidar12:05:24

dyld: Library not loaded: /usr/local/opt/icu4c/lib/libicudata.60.1.dylib
  Referenced from: /usr/local/bin/node
  Reason: image not found

bozhidar12:05:44

Guess some package upgrade broke my node and lead to my mysterious issues

dominicm12:05:47

that's why 🙂

bozhidar13:05:41

Yep, I can confirm this. 🙂 Reinstalling node fixed my test issues.

bozhidar13:05:12

@nicola @r0man Would you mind testing the latest snapshot of cider-nrepl before I make it the new stable? 🙂

nicola13:05:48

:+1: will do

r0man13:05:59

yes, 1 sec

nicola13:05:27

works like a charm! thank you

dominicm13:05:42

What is 0.17 called? 🙂

r0man13:05:12

@bozhidar Running all tests in a namespace seems to work fine now. But running a single tests case doesn't work for me.

r0man13:05:28

I'm seeing this stacktrace in my nrepl-server buffer:

dominicm13:05:33

I don't suppose cider sends non-namespaced vars when it's testing a single var?

bozhidar13:05:09

I think it sends the ns and the var separately.

bozhidar13:05:37

(defun cider-test-run-test ()
  "Run the test at point.
The test ns/var exist as text properties on report items and on highlighted
failed/erred test definitions.  When not found, a test definition at point
is searched."
  (interactive)
  (let ((ns  (get-text-property (point) 'ns))
        (var (get-text-property (point) 'var)))
    (if (and ns var)
        ;; we're in a `cider-test-report-mode' buffer
        ;; or on a highlighted failed/erred test definition
        (progn
          (cider-test-update-last-test ns var)
          (cider-test-execute ns (list var)))
      ;; we're in a `clojure-mode' buffer
      (let ((ns  (clojure-find-ns))
            (def (clojure-find-def)))
        (if (and ns (member (car def) cider-test-defining-forms))
            (progn
              (cider-test-update-last-test ns (cdr def))
              (cider-test-execute ns (cdr def)))
          (message "No test at point"))))))

bozhidar13:05:56

Yep, it’s a ns and a symbol (var).

bozhidar13:05:29

A single element list to be precise.

dominicm13:05:50

I can update cider-nrepl. Sorry, I've guessed how this op works to some degree. I can update the test op to union the namespace to the symbols automatically.

dominicm13:05:52

diff --git a/src/cider/nrepl/middleware/test.clj b/src/cider/nrepl/middleware/test.clj
index 46eeb72..ed57451 100644
--- a/src/cider/nrepl/middleware/test.clj
+++ b/src/cider/nrepl/middleware/test.clj
@@ -298,7 +298,7 @@
    (merge msg {:var-query {:ns-query {:exactly [ns]}
                            :include-meta-key include
                            :exclude-meta-key exclude
-                           :exactly tests}})))
+                           :exactly (map #(str ns "/" %) tests)}})))
 (defn handle-test-all-op
   [{:keys [load? include exclude] :as msg}]
This is the diff, it's really that simple.

dominicm13:05:52

There's not much API surface to test & test-all, so I can't imagine there's anything else that could go wrong here (famous last words? 😁)

bhauman13:05:32

@dominicm so was the issue with piggieback and node a false alarm?

dominicm13:05:11

@bhauman no false alarm, it's real. We've rolled back for the tests.

bhauman14:05:10

@dominicm which test suite do I need to run?

dominicm14:05:59

@bhauman Running cider-nrepl's test suite with lein with-profile +test-cljs,+1.9 test

mattly18:05:02

I’m attempting to connect via nrepl and I get this, has anyone seen it before?

dpsutton18:05:29

Have you upgraded emacs recently

mattly18:05:27

emacs-mac 26.1-rc1

mattly18:05:31

installed this morning

mattly18:05:47

I can’t run anything older for reasons not worth getting into

bozhidar18:05:00

Seems something has changed in the queue.el lib we’re using in CIDER. Damn!

bozhidar18:05:30

The fix should be simple I guess, but our bad luck today is epic!

mattly18:05:40

ok. LMK if there’s any info I can provide

mattly18:05:53

I’ve done a lot of emacs debugging already today

dpsutton18:05:47

try deleting your elc files or just reinstalling cider

dpsutton18:05:15

i'm on emacs 27 and it works fine. you probably just need to have emacs 26 compiled bytecode

bozhidar18:05:06

But do you know what’s your version of queue.el?

dpsutton18:05:27

;; Version: 0.2

dpsutton18:05:47

i think it's changes in cl- style elisp

bozhidar18:05:45

Seems version 0.2 is pretty old, so it’s more likely that something in Emacs broke it.

bozhidar18:05:57

I’m on 25.3 and it works fine.

dpsutton18:05:07

yeah my thinking is they just changed what the compiled code looks like. such that a defstruct under 25.x is not what a defstruct under 26.x expects

dpsutton18:05:51

and i see this commit: Make cl-defstruct use records. CommitDate: Tue Apr 4 08:23:46 2017 +0200. 056548283884d61b1b9637c3e56855ce3a17274d

bozhidar19:05:32

Ouch. It’s probably true.

bozhidar19:05:02

That’s going to be one painful transition…

dpsutton19:05:19

I've run into it with recompiling emacs and I've solved it with just blowing away elc file

dpsutton19:05:14

@mattly was recomping the dir or deleting the .elc files sufficient for you?

mattly19:05:27

I haven’t had time to try that yet

mattly19:05:42

I recomped this morning tho

dpsutton19:05:37

@bozhidar go drop that release in #announcements.

bozhidar20:05:02

Thanks to all of your for your contributions, feedback and for just being there! You’re the heart and soul of CIDER! ❤️

bozhidar20:05:35

Hopefully 0.17 won’t be very buggy and will be followed by smaller releases in the next few months.

bhauman21:05:17

I'm currently fixing the node repl problem with piggieback, had to prevent the initializing call to cljs.repl/repl to not tear down the node setup, and needed to bind out and err for initialization

bhauman21:05:24

Well it looks like piggieback is fixed, sorry to miss the release. @bozhidar @dominicm

bhauman21:05:42

and congrats on the release!

b2berry23:05:01

Congrats on the release!

b2berry23:05:41

I’ve spent most the working day trying to make heads or tails of this issue I’m experiencing, it’s nearly identical to this https://clojurians.slack.com/archives/C0617A8PQ/p1525088568000045 except that the resolution here didn’t work for me.

b2berry23:05:52

When attempting to start cljs-repl via cider-jack-in-clojurescript I get the following error

error in process filter: cider-cljs-repl-form: Symbol's function definition is void: nil
error in process filter: Symbol's function definition is void: nil
Context: OSX, Spacemacs, Cider 0.17.0 .dir-locals.el
((nil
  (cider-default-cljs-repl . figwheel+harmonium) ;; Was "Harmonium" tried also harmonium without figwheel+ and error persists
  (cider-cljs-repl-types
   ("Harmonium" "(do (require 'figwheel-sidecar.repl-api)\n            (figwheel-sidecar.repl-api/start-figwheel! \"login\" \"imageviewer\" \"harmonium\")\n            (figwheel-sidecar.repl-api/cljs-repl))"))))

b2berry23:05:07

Any tips would be very appreciated

artenator23:05:42

started getting “namespaces are in a bad state” errors today when trying to refresh namespaces in cider. Anyone else run into this issue? I just updated to the 5-7-18 build from melpa