Fork me on GitHub
#tools-deps
<
2019-11-04
>
vlaaad11:11:45

Another use-case for https://clojure.atlassian.net/browse/TDEPS-116 — I want to create a library that has different code for different jdk versions. I do it using 2 aliases:

{:jdk8 {:extra-paths ["src/jdk8"]}
 :jdk11 {:extra-paths ["src/jdk11"]}}
It would be much easier to depend on such library when you can specify extra alias when declaring a dependency to it

vlaaad11:11:24

I think I found a bug: :paths in aliases are not picked up when there are also :deps present. Repro: deps.edn:

{:paths ["src" "jdk11"]
 :aliases {:jdk8 {:paths ["src" "jdk8"]
                  :deps {org.clojure/clojure {:mvn/version "1.10.1"}}}}}
clj -Srepro -Sforce -Spath output:
src:jdk11:/home/vlaaad/.m2/repository/org/clojure/clojure/1.10.1/clojure-1.10.1.jar:/home/vlaaad/.m2/repository/org/clojure/spec.alpha/0.2.176/spec.alpha-0.2.176.jar:/home/vlaaad/.m2/repository/org/clojure/core.specs.alpha/0.2.44/core.specs.alpha-0.2.44.jar
clj -A:jdk8 -Srepro -Sforce -Spath output:
src:/home/vlaaad/.m2/repository/org/clojure/clojure/1.10.1/clojure-1.10.1.jar:/home/vlaaad/.m2/repository/org/clojure/spec.alpha/0.2.176/spec.alpha-0.2.176.jar:/home/vlaaad/.m2/repository/org/clojure/core.specs.alpha/0.2.44/core.specs.alpha-0.2.44.jar
notice: second output does not have jdk8 folder on a classpath

vlaaad11:11:08

and sometimes :deps are not picked up? Given this deps.edn:

{:paths ["src" "jdk11"]
 :aliases {:jdk8 {:paths ["src" "jdk8"]}
           :deps {cljfx {:mvn/version "1.5.1"}}}}
`clj -A:jdk8 -Srepro -Sforce -Spath` output has jdk8 folder, but not dependency:
src:jdk8:/home/vlaaad/.m2/repository/org/clojure/clojure/1.10.1/clojure-1.10.1.jar:/home/vlaaad/.m2/repository/org/clojure/spec.alpha/0.2.176/spec.alpha-0.2.176.jar:/home/vlaaad/.m2/repository/org/clojure/core.specs.alpha/0.2.44/core.specs.alpha-0.2.44.jar
EDIT: diregard this message, it was a typo

stijn13:11:05

@vlaaad you have a typo in the deps.edn, :deps is not inside the :jdk8 alias.

vlaaad13:11:20

what, where? it is!

vlaaad13:11:02

oh, you are right

dominicm12:11:55

In maven that's a classifier I think. Although jdk11 had a system for having jdk-specific files added.

dominicm12:11:06

@vlaaad jep238: multi release jars

dominicm12:11:51

It would be cool if deps.edn supported MRJAR for fetching dependencies.

Alex Miller (Clojure team)13:11:33

I don’t think mrjars have any effect on deps? That’s really a JVM thing?

Alex Miller (Clojure team)13:11:59

On the problem above, what version of clj are you on?

vlaaad13:11:58

latest, 1.10.1.148

vlaaad13:11:11

@dominicm will jep 238 work with clj files as well? is it okay to make library muti-release?..

dominicm13:11:54

I see no reason why not. Try it and report back!

vlaaad13:11:39

I think jep238 might not be a fitting solution for me, because I also need different dependencies for different java versions...

Alex Miller (Clojure team)13:11:59

Yeah, won’t do that, but should work for clj files as they are loaded as resources, just like class files

vlaaad13:11:23

yeah, makes sense

Alex Miller (Clojure team)13:11:14

This is really where maven uses classifiers and jdk properties

dominicm13:11:33

I was thinking of fetching the appropriate pom.xml from the jar based on jdk version

Alex Miller (Clojure team)13:11:08

I don’t think that’s a thing

Alex Miller (Clojure team)13:11:52

Although I have not looked at all at the maven support for mrjars

dominicm13:11:48

It doesn't look like it is. It could be for local/root on a jar, but for maven a separate fetch is made for the pom, completely unaware of the jdk

vlaaad13:11:48

is there a way to exclude dependency using alias?

vlaaad13:11:05

sad, I'm running out of ideas how to add jdk8 support without breaking git dependencies to cljfx 😞

vlaaad13:11:20

maven is easy, I can build poms with different deps and source paths and publish them either as different artifacts, or with different classifiers/activations

vlaaad13:11:32

but I already have jdk11-specific deps in root deps.edn, and it seems there is no way to exclude them when using git dependency?

Alex Miller (Clojure team)13:11:26

you could try using a lib with nil coordinate - that will merge over, but I don't remember how tools.deps handles it (could easily not work, I just don't remember)

vlaaad14:11:26

using :override-deps?

vlaaad14:11:21

doesn't seem to work: given this deps.edn:

{:paths ["src" "jdk11"]
 :deps {org.openjfx/javafx-base {:mvn/version "13"}}
 :aliases {:jdk8 {:paths ["src" "jdk8"]
                  :override-deps {org.openjfx/javafx-base nil}}}}
running clj -A:jdk8 -Srepro -Sforce -Spath still includes javafx-base:
src:jdk8:/home/vlaaad/.m2/repository/org/clojure/clojure/1.10.1/clojure-1.10.1.jar:/home/vlaaad/.m2/repository/org/openjfx/javafx-base/13/javafx-base-13.jar:/home/vlaaad/.m2/repository/org/clojure/spec.alpha/0.2.176/spec.alpha-0.2.176.jar:/home/vlaaad/.m2/repository/org/clojure/core.specs.alpha/0.2.44/core.specs.alpha-0.2.44.jar:/home/vlaaad/.m2/repository/org/openjfx/javafx-base/13/javafx-base-13-linux.jar

vlaaad14:11:12

and using :deps in alias seems to interfere with :paths so jdk8 path is not seen, as reported earlier

Alex Miller (Clojure team)14:11:33

yeah, I'm going to look at that in a bit once I'm past my morning calls

👍 4
Alex Miller (Clojure team)14:11:42

that's surprising to me

Alex Miller (Clojure team)14:11:04

could you also post me your clj -Sdescribe and whether you have anything in ~/.clojure/deps.edn?

Alex Miller (Clojure team)14:11:35

wait, you said latest but 1.10.1.148 is not a thing, should be 1.10.1.478

vlaaad14:11:03

I'm using -Srepro, so ~/.clojure/deps.edn shouldn't be an issue?

vlaaad14:11:32

I'm using 1.10.1.478, it was a typo

vlaaad14:11:38

clj -A:jdk8 -Srepro -Sforce -Sdescribe

{:version "1.10.1.478"
 :config-files ["/usr/local/lib/clojure/deps.edn" "deps.edn" ]
 :config-user ""
 :config-project "deps.edn"
 :install-dir "/usr/local/lib/clojure"
 :config-dir "/home/vlaaad/.clojure"
 :cache-dir ".cpcache"
 :force true
 :repro true
 :resolve-aliases ""
 :classpath-aliases ""
 :jvm-aliases ""
 :main-aliases ""
 :all-aliases ":jdk8"}

Alex Miller (Clojure team)14:11:25

oh, this is the expected behavior

Alex Miller (Clojure team)14:11:44

the :paths and :deps in an alias are new with the newest release and REPLACE the project :paths and :deps

Alex Miller (Clojure team)14:11:07

if you want to add, you need :extra-paths or :extra-deps

vlaaad14:11:28

I want to replace both

Alex Miller (Clojure team)14:11:50

I thought your complaint above was that the project :paths was not included?

Alex Miller (Clojure team)14:11:22

re-reading what you said above...

vlaaad14:11:22

No, replacement paths are not picked up

Alex Miller (Clojure team)14:11:31

ah, it is a bug and I see the fix

Alex Miller (Clojure team)15:11:41

and there is a test that should have caught this, but the test is missing the is, so it didn't fail :)

😄 16
Alex Miller (Clojure team)15:11:55

programming is hard, y'all

💯 16
andy.fingerhut15:11:36

eastwood will probably warn about such tests, if it works on your code base at all.

Alex Miller (Clojure team)15:11:18

@vlaad logged as https://clojure.atlassian.net/browse/TDEPS-142, fixed, and released in tools.deps.alpha 0.8.584, the rest is working its way through to clj

vlaaad16:11:57

thanks a lot!

Alex Miller (Clojure team)16:11:31

thanks for a concise and useful report! :)

Alex Miller (Clojure team)16:11:03

I didn't that see that with clj-kondo, but I'm not running latest version

borkdude16:11:22

depends on the example. what's the code?

borkdude16:11:09

found it. yes, that's a case I want to improve. currently it only checks the direct children of (deftest ...) like (deftest foo (= 1 2)), but these cases were inside a let: https://github.com/borkdude/clj-kondo/issues/462

andy.fingerhut16:11:41

There are also typically many is/are expressions inside of testing subexpressions inside of deftest, or let/testing/let/... etc.

borkdude16:11:22

yeah. does eastwood test for that btw?

andy.fingerhut16:11:47

Eastwood uses tools.analyzer to analyze everything, then looks for all expansions of is forms, I believe.

andy.fingerhut16:11:08

It is one way to do it, but can be painful to recognize the expansions of is, which is an odd macro.

borkdude16:11:35

but the point is to detect that is was forgotten. so it warns when you have a deftest without a single occurrence of is?

borkdude16:11:54

globally? or does it also point to the location where the is was expected?

andy.fingerhut16:11:16

I'd have to go back to the code to check, but I think it warns about all expressions that are not likely to be called only for their side effects, that do not have is or are around them.

andy.fingerhut16:11:27

I'll follow up in #eastwood channel after checking the code for a little bit.

borkdude17:11:14

I guess even clojure.test could have built-in warnings like: you're defining a deftest but you never called is. Or: you called testing, but you never called is.

Alex Miller (Clojure team)17:11:26

But it’s difficult to verify that beyond lexical scope

borkdude17:11:32

I'm not sure how clojure.test works internally, but is there not some kind of registry to which test an assertion belongs?

borkdude17:11:16

There are all kinds of hooks. Maybe it's possible to verify during one of those hooks that nothing got actually checked when running a test.

Alex Miller (Clojure team)17:11:57

I’ve also written tests with negative assertions in code that shouldn’t run in the past. In those cases, even no assertions is correct

Alex Miller (Clojure team)17:11:15

But those are prob rare

borkdude18:11:07

that's meta: assert on a hook that the number of assertions is zero

Alex Miller (Clojure team)18:11:31

you could also write those kind of tests as a zero? check on a "reached" count in an atom I suppose so there would always be an assertion

Alex Miller (Clojure team)18:11:01

@U08E8UGF7 the build here is a Maven build (as with all contrib builds) so not trivial to integrate

Alex Miller (Clojure team)18:11:50

ironically of course, given it's tools.deps

borkdude18:11:56

it seems the feature we discussed for clojure.test is something kaocha also does, checking at runtime when the test ends using a hook: https://github.com/lambdaisland/kaocha/blob/c97a2cbfbb029604edd42e1340c66bd4704007cf/src/kaocha/type/var.clj#L13

borkdude18:11:19

I would like to see this in clojure.test, but I think the chances are fairly slim it would ever get in

borkdude18:11:53

but as a library it could work

kszabo18:11:07

this is one of the many reasons why we use kaocha, of course a static analysis check would be better as well. The negative assert feature could be fixed with dedicating another name for it like (clojure.test/never-reached!) which macros could check for. If I got the negative assert idea correct

kszabo19:11:58

cool! so productive, @U04V15CAJ 🙂 Can you pretty please add a deps.edn as well for us tools.deps users.

borkdude19:11:59

@U08E8UGF7 it's in there now. I'm trying to make it work under CLJS as well, but registering the multimethod doesn't seem to work

borkdude20:11:21

Now also works in ClojureScript.

borkdude22:11:04

Found an error in one of my own projects! https://twitter.com/borkdude/status/1191482400599158785

🎉 4