Fork me on GitHub
#clj-kondo
<
2019-11-25
>
borkdude08:11:37

All JVM tests are succeeding now on Windows 😄 Ran 121 tests containing 1207 assertions. 0 failures, 0 errors.

parrot 2
👏 2
sogaiu14:11:13

nice! thanks to @ales.najmann for help 🙂

borkdude15:11:16

Yes, thank you both!

👍 1
borkdude09:11:26

There is one remaining native test error:

lein test clj-kondo.main-test
Unexpected error. Please report an issue.
lein test :only clj-kondo.main-test/deprecated-var-test
FAIL in (deprecated-var-test) (main_test.clj:1945)
config
expected: (clojure.core/= (clojure.core/count maps__8060__auto__) (clojure.core/count res__8061__auto__))
  actual: (not (clojure.core/= 1 0))
Unexpected error. Please report an issue.
Unexpected error. Please report an issue.
Unexpected error. Please report an issue.
Unexpected error. Please report an issue.

borkdude09:11:41

This only seems to occur when you run all native tests, not when I run this test in isolation

borkdude09:11:50

Might we worth investigating.

borkdude09:11:50

Oh, it looks like I ran the wrong test (typo) in isolation.

borkdude09:11:11

FAIL in (deprecated-var-test) (main_test.clj:1945)
config
expected: (clojure.core/= (clojure.core/count maps__761__auto__) (clojure.core/count res__762__auto__))
  actual: (not (clojure.core/= 1 0))
Ran 1 tests containing 15 assertions.
1 failures, 0 errors.
Tests failed.
This is only when testing the native version.

sogaiu19:11:47

I commented out what looked like what was leading to that:

#_(testing "config"
    (assert-submaps
     '({:file "corpus/deprecated_var.clj",
        :row 10,
        :col 1,
        :level :warning,
        :message "#'foo.foo/deprecated-fn is deprecated"})
in my output, there are the following lines right after the FAIL, whether the section is commented or not:
Unexpected error. Please report an issue.
Unexpected error. Please report an issue.
Unexpected error. Please report an issue.
Unexpected error. Please report an issue.

sogaiu19:11:55

did you see this?

borkdude19:11:27

ah then those might be caused by something else... hm

borkdude19:11:48

in main.clj this line is printed. maybe you can include (.getMessage e) in the println

sogaiu19:11:18

i can try that -- though in my main.clj there is something about printing the stacktrace. is there some reason i'm not seeing any info about the stacktrace?

borkdude19:11:09

yes, because that text is going to *err*

borkdude19:11:14

and normal println is going to stdout

borkdude19:11:53

which is not consistent in this case, but stderr is not visible I think in the tests (which could be a nice for debugging)

sogaiu19:11:42

thanks for the explanation -- build with patch complete, waiting for test output 🙂

sogaiu19:11:14

lein test clj-kondo.main-test
Unexpected error. Please report an issue. clojure.lang.Symbol cannot be cast to java.lang.String
Unexpected error. Please report an issue. clojure.lang.Symbol cannot be cast to java.lang.String
Unexpected error. Please report an issue. Unsupported character: \\.specs$
Unexpected error. Please report an issue. Unsupported character: \\.specs$

borkdude19:11:42

aha... interesting. maybe you can print the entire stacktrace to stdout? which is an argument to .printStackTrace

sogaiu19:11:24

sounds good, just need to figure out the details of how -- will bug you if i have trouble 🙂

borkdude20:11:05

Probably just (.printStackTrace e System/out)

sogaiu20:11:03

i did something different -- but may be that would have been enough. too much output so adding to issue.

sogaiu20:11:20

github seems to not like my comment 😞

sogaiu20:11:49

i guess they are still having issues

borkdude20:11:25

I forgot to compile with clojure 1.10.1 which I do in linux.

borkdude20:11:41

maybe it has to do with that, but clj-kondo also should be able to run with clojure 1.9.0, so I don't know

sogaiu20:11:16

i'm puzzled as to why the test framework doesn't explain better which test(s) it's experiencing these issues with.

borkdude20:11:18

maybe it has to do with how I process the result from the shell output

sogaiu20:11:01

may be so -- but i feel like blaming the test framework 🙂

sogaiu20:11:29

ok, i guess i'll change compile.bat to include: call lein with-profiles +clojure-1.10.1 do clean, uberjar then?

borkdude20:11:48

I already pushed it now

sogaiu20:11:52

i allocated another core to my vm to cope with slowness..it's gradually eating away at my real computing resources 🙂

sogaiu20:11:23

curious...`lein test` leads to this: https://pastebin.com/7a9GwMqj

borkdude21:11:56

I found this out:

Could not parse config! Unsupported character: \\.* "{:linters {:unused-binding {:level :off}, :unresolved-symbol {:level :off}, :refer-all {:level :off}, :type-mismatch {:level :off}, :deprecated-var {:exclude #:foo.foo{deprecated-fn {:namespaces [foo.bar bar\\\\.*], :defs [foo.baz/allowed foo\\\\.baz/ign\\\\.*]}}}}}"

borkdude21:11:22

> Caused by: java.lang.ClassNotFoundException: clojure.core$with_meta__5420 That's usually caused by stale target files

borkdude21:11:29

lein clean solves it

borkdude21:11:36

(already added it)

borkdude21:11:38

somehow the backslashes get doubled

sogaiu21:11:34

wherever we turn, there is the windows escaping beast...there is no escape...or should i say, there is always escape?

sogaiu21:11:37

i don't understand, lein clean was in the original compile.bat file -- i pulled and tried building + test, and i still see an exception. am i misunderstanding?

borkdude21:11:58

the most recent commit is 3efab387e2842c0f5b138bdf30ededccb6b68abd

borkdude21:11:10

it should work with that one. see appveyor (the error was there as well, but is now fixed)

sogaiu21:11:43

ah, running lein clean after building but before lein test? that seems to be working.

sogaiu21:11:11

ok, now i get it, thanks

sogaiu21:11:49

i think i'm still seeing the same exceptions during the test

sogaiu21:11:58

i am on 3efab387

sogaiu21:11:23

this is the quote thing then?

borkdude21:11:34

what exceptions do you see?

sogaiu21:11:03

i think it's the same as before, but here is a fresh copy-paste: https://pastebin.com/m0ntm5fB

borkdude21:11:29

it's the same issue:

"{:linters {:unused-binding {:level :off}, :unresolved-symbol {:level :off}, :refer-all {:level :off}, :type-mismatch {:level :off}, :unused-namespace {:exclude [.*\\\\.specs$]}}}"
cannot be parsed as EDN, somehow the backslashes get doubled by ProcessBuilder

sogaiu21:11:14

ok, thanks

borkdude21:11:42

at least that's what I think

borkdude21:11:16

lint-native! that is where the EDN config gets passed as a string

borkdude21:11:35

to the conch library and this uses the ProcessBuilder to make the program call

borkdude21:11:16

but hey, this worked in the normal JVM tests

borkdude21:11:27

so that's kinda weird isn't it

sogaiu21:11:39

it is weird

borkdude21:11:53

oh well, no, then the string isn't handled via the processbuilder

sogaiu21:11:07

this has nothing to do with regexp, right?

sogaiu21:11:32

ok, because there seems to be something wonky about native-image and asterisk

sogaiu21:11:39

that's a different thing

borkdude21:11:47

oh is that it maybe?

sogaiu21:11:58

no idea -- but we used {0,} as a substitute for * to get native-image to succeed in working with building at some point recently, right?

sogaiu21:11:29

i think that's a separate thing

borkdude21:11:42

if you put this into an EDN file:

{:linters {:unused-binding {:level :off}, :unresolved-symbol {:level :off}, :refer-all {:level :off}, :type-mismatch {:level :off}, :unused-namespace {:exclude [.*\\\\.specs$]}}}
you will get: user=> (edn/read-string (slurp "/tmp/foo.edn")) Execution error at user/eval3 (REPL:1). Unsupported character: \\.specs$

borkdude21:11:54

and it seems the slashes are doubled somehow

borkdude21:11:21

wait, the dollar sign is also weird

borkdude21:11:47

ah, it has to do with quotes probably

borkdude21:11:51

the quotes are missing

sogaiu21:11:01

i don't seem them, yes

sogaiu21:11:23

isn't the whole thing (from left curly to right curly) in quotes?

borkdude21:11:00

let me just print the config before it goes through

👍 1
borkdude22:11:48

ok, so this works:

(edn/read-string "{:linters {:unused-binding {:level :off}, :unresolved-symbol {:level :off}, :refer-all {:level :off}, :type-mismatch {:level :off}, :deprecated-var {:exclude #:foo.foo{deprecated-fn {:namespaces [foo.bar \"bar\\\\.*\"], :defs [foo.baz/allowed \"foo\\\\.baz/ign\\\\.*\"]}}}}}")

borkdude22:11:01

but somehow the quotes disappear

borkdude22:11:03

not sure if that's related but maybe the quotes have to be doubled for Windows?

sogaiu22:11:07

could be - am reading - fwiw, there is a quoting tips section in the clj-on-windows tools.deps wiki page

sogaiu22:11:28

i roll my eyes at the response at the bottom of JDK-8131908

sogaiu22:11:56

how is anyone supposed to know what is sitting inside processbuilder on windows?

sogaiu22:11:53

nor "escape"

sogaiu22:11:04

sorry, getting carried away 🙂

sogaiu22:11:37

didn't lread summarize some quoting issues related to windows in some jira issue?

sogaiu22:11:41

so this whole thing is because a string is being passed on the command line, right?

borkdude22:11:01

yes, I now tried something

borkdude22:11:17

but I didn't test it on Windows yet, so pushed it to appveyor

borkdude22:11:48

it worked 🙂

borkdude22:11:59

all tests passing now!

borkdude22:11:45

will do come clean up tomorrow and then merge

sogaiu22:11:47

awesome! > If an odd number of backslashes is followed by a double quotation mark, one backslash is placed in the argv array for every pair of backslashes, and the double quotation mark is "escaped" by the remaining backslash, causing a literal double quotation mark (") to be placed in argv. i guess you did this?

borkdude22:11:08

I did this:

config (if windows? (str/replace config "\"" "\\\"")
                    config)

```

borkdude22:11:16

(damn slack with their new markup thing)

sogaiu22:11:33

i was hoping there was a way to revert -- i guess not

sogaiu22:11:54

time for rest perhaps 🙂

sogaiu22:11:24

oh, only a few more entries on this thread

sogaiu22:11:29

and i can reach 100

borkdude22:11:44

how many now?

borkdude22:11:58

ah nice, we broke a record!

sogaiu22:11:18

lol, i can only easily tell that it's over 100

eraserhd14:11:32

It turns out that my editors lint support saves the buffer to a temporary directory before linting, so the feature of finding config from the path doesn't actually help me. :/

borkdude14:11:53

oh shit. would it be better to revert your commit?

borkdude14:11:48

I guess it still works as before since it fallbacks on the cwd behavior right?

borkdude14:11:20

some plugins allow the current working directory to be configured based on the file that is being linted

borkdude14:11:25

like IntelliJ

eraserhd14:11:47

Well, I don't have evidence, but I suspect the feature is still useful generally. And yeah, it doesn't hurt anythin.g

eraserhd14:11:13

I'm asking a question in the editor's IRC about how we want to proceed. Perhaps we'll change how the linter works.

delaguardo15:11:39

https://github.com/marketplace/actions/setup-clj-kondo setup-clj-kondo for hosted runners in github actions is here) including latest clj-kondo.exe for running on windows

borkdude15:11:36

@eraserhd if I'm not mistaking flycheck has the same behavior actually. because you don't have to save the file for linting, it makes a copy of it somewhere and then lints that file

borkdude15:11:53

using the cwd of the file (I think the dir the file lives in)

borkdude15:11:04

so it might actually slow things down a bit, but probably not noticable

eraserhd15:11:04

Ok, I'll look into that. If this is something that several linters need, I'll fix the editor.

borkdude15:11:23

maybe the linting behavior of kakaune should be the same, setting the cwd to the parent dir of the file

borkdude15:11:05

I forgot how much nicer Parallels is than VirtualBox

borkdude15:11:08

and so far no crashes

borkdude15:11:16

@eraserhd but this behavior could still be useful for VSCode, I'll have to manually test it. Please keep me updated if we should keep this behavior and if nobody is using it (and it's not useful for VSCode) we can maybe deprecate it again

borkdude22:11:03

@eraserhd fwiw, I think emacs does the following: it saves the intermediate state of a file to a tmp file and then invokes the linter piping the input from the tmp file to stdin

borkdude22:11:20

so clj-kondo only sees the path "stdin", not even the filename

borkdude22:11:04

so in that case the new feature isn't very beneficial. however I think in the case of VSCode marco morains plugin, it's invoked on save and then using the filename, so it works for that case

borkdude22:11:05

(thanks again @ales.najmann and @sogaiu for mentally supporting me and providing useful additions)

👍 5
👏 3
clj-kondo 1