Clojurians Log v2

compared to the https://github.com/babashka/babashka/pull/1478#issuecomment-1407181862 from mac, linux has a very different error:

lein test babashka.bb-edn-test
=== deps-root-tests

Error building classpath. ../../home/lispyclouds/code/repos/babashka (Is a directory)
java.io.FileNotFoundException: ../../home/lispyclouds/code/repos/babashka (Is a directory)
        at java.base/java.io.FileInputStream.open0(Native Method)
        at java.base/java.io.FileInputStream.open(FileInputStream.java:219)
        at java.base/java.io.FileInputStream.<init>(FileInputStream.java:158)
        at java.base/java.io.FileReader.<init>(FileReader.java:75)
        at $slurp_edn.invokeStatic(io.clj:42)
        at clojure.tools.deps$slurp_edn_map$fn__664.invoke(deps.clj:39)
        at clojure.tools.deps$slurp_edn_map.invokeStatic(deps.clj:39)
        at clojure.tools.deps$slurp_deps.invokeStatic(deps.clj:96)
        at clojure.tools.deps.script.make_classpath2$read_deps.invokeStatic(make_classpath2.clj:163)
        at clojure.tools.deps.script.make_classpath2$run.invokeStatic(make_classpath2.clj:174)
        at clojure.tools.deps.script.make_classpath2$_main.invokeStatic(make_classpath2.clj:228)
        at clojure.tools.deps.script.make_classpath2$_main.doInvoke(make_classpath2.clj:195)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.lang.Var.applyTo(Var.java:705)
        at clojure.core$apply.invokeStatic(core.clj:667)
        at clojure.main$main_opt.invokeStatic(main.clj:514)
        at clojure.main$main_opt.invoke(main.clj:510)
        at clojure.main$main.invokeStatic(main.clj:664)
        at clojure.main$main.doInvoke(main.clj:616)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.lang.Var.applyTo(Var.java:705)
        at clojure.main.main(main.java:40)
Tests failed.

some level of deps-root testing is there on my PR though

it seems it tries to read the deps.edn file as a directory?

(the other way around)

its the same test im running here too, not sure if there is a better way to test this that doenst depend on where its running

I'm not sure what you are doing. Is this up somewhere?

In the PR?

this code here: https://github.com/babashka/babashka/pull/1478#issuecomment-1407181862

im running this in the same deftest i added to the PR

didnt push it yet as it breaks

push it with [skip ci] and I'll have a look locally

pushed

I'm getting:

lein test babashka.bb-edn-test
=== deps-root-tests

Error building classpath. Local lib local/dep not found: /Users/borkdude/dev/babashka/local-dep

that's weird, is the deps-root in the temp dir for you in https://github.com/babashka/babashka/blob/rel-deps/src/babashka/impl/deps.clj#L96 ?

this error should come without my changes

the issue is that the tests set a default config and deps-root so I made this change in the test code:

args (cond-> args
               (and *bb-edn-path* (not (some #(= "--config" %) args)))
               (->> (list* "--config" *bb-edn-path*))
               (and *bb-edn-path* (not (some #(= "--deps-root" %) args)))
               (->> (list* "--deps-root" ".")))

but in the normal bb runs the deps-root is the parent dir of the config

which should be okay right? as long as the deps root is passed to the deps/dir with my changes?

I pushed a wip commit and commented out the tests that don't yet work

You can run that test with: lein do clean, test :only babashka.bb-edn-test/unix-task-test

sorry, this one: lein do clean, test :only babashka.bb-edn-test/deps-root-test

Now I'm also getting:

Error building classpath. Can't create directory: /private/Users/borkdude/.clojure/.cpcache

yeah i suppose theres gonna be differing behaviour due to this

I don't see any reference to *dir* here in deps.clj:

config-dir
        (or (*getenv-fn* "CLJ_CONFIG")
            (when-let [xdg-config-home (*getenv-fn* "XDG_CONFIG_HOME")]
              (.getPath (io/file xdg-config-home "clojure")))
            (.getPath (io/file (home-dir) ".clojure")))

oh I see

no, I don't

user-cache-dir
          (or (*getenv-fn* "CLJ_CACHE")
              (when-let [xdg-config-home (*getenv-fn* "XDG_CACHE_HOME")]
                (.getPath (io/file xdg-config-home "clojure")))
              (.getPath (io/file config-dir ".cpcache")))

oh I see:

cache-dir
          (if (.exists (io/file deps-edn))
            (.getPath (io/file *dir* ".cpcache"))
            user-cache-dir)

hehe finally 😛

in bb.edn we set deps-file to ""

in order to suppress loading a local deps.edn

and this causes that behavior

there's quite a few knobs to be aware about

pushed another fix

Can you explain this test to me?

(testing "custom deps-root path"
        (let [out     (bb "--config" config "--deps-root" "/tmp" "cp")
              entries (cp/split-classpath out)]
          (is (= 1 (count entries)))
          (is (= "/tmp/src" (first entries)))))

oh I get it yes, but you have to make this more cross platform, "/tmp" doesn't work on Windows I think

use another tmp dir

yeah that was one of my doubts, looking at it now

also don't use "/foo/" string comparisons, use babashka.fs

lein test :only babashka.bb-edn-test/deps-root-test
FAIL in (deps-root-test) (bb_edn_test.clj:498)
custom deps-root path
expected: (= "/tmp/src" (first entries))
  actual: (not (= "/tmp/src" "C:\\tmp\\src"))
=== deps-race-condition-test

I guess my arg fixes weren't necessary since the last --deps-root argument wins

I'll remove those

pushed

but this test:

(testing "default deps-root path is same as bb.edn"
        (let [out     (bb "--config" config "cp")
              entries (cp/split-classpath out)]
          (is (= (fs/parent f) (fs/parent (first entries))))))

I would expect it to fail, considering what it's saying

so maybe the arg fix is necessary after all

this was my dilemma too in https://clojurians.slack.com/archives/C02FBBU61A9/p1674842023949889?thread_ts=1674551849.533369&cid=C02FBBU61A9

oh no, this is good since we don't set *bb-edn-path* in this test, we don't use with-config

so these are the printed args :

("--config" "/var/folders/j9/xmjlcym958b1fr0npsp9msvh0000gn/T/4a86539f-dff2-4c9e-8c4d-f8931232a5902671065622392340497/bb.edn" "cp")
:entries ["/var/folders/j9/xmjlcym958b1fr0npsp9msvh0000gn/T/4a86539f-dff2-4c9e-8c4d-f8931232a5902671065622392340497/src"]

yep, all good

looks good to me

we could force *bb-edn-path* to nil, just to be sure

should it be set just for this test right?

yes

since in this test you're spitting your own bb.edn and setting config + deps-root yourself

ok, it all makes sense now. when you fix the windows issue, I'll merge

yep

we finally have more tests too

Tests are still failing after I removed the arg fixes

yes the linux error i mentioned above

you can see it in the CI

that's what I meant, CI

yeah i mean a not my machine specific error 😅

does it pass for you?

No:

Ran 213 tests containing 567 assertions.
4 failures, 0 errors.

ah this is something else

some JDK19 issue (because I switched JDK for your issue)

ahh

not a real issue, but error messages being different

and then some string comparisons fail, I will test with my regular graalvm

Ran 213 tests containing 567 assertions.
0 failures, 0 errors.

but if you run just this test, does it pass?

yes

interesting, does it only fail on linux somehow then?

I'll make another check in deps.clj to not read deps.edn when it's not a file

but why is it even going to ../../home/circleci/repo?

dunno. maybe you can dig into this, I'm going for a run

yeah, good that i can reproduce this here

it might be good to insert some printlns etc in the deps.clj (use the -debug function, as out and err are captured in the tests)

yep

i was doing it in places til borkdude.deps/deps is called.

i'll try to dig in a few hours, need to finish off some stuff!

ok, I returned from my run. I can try on my linux machine

some stuff i found: deps-edn is an empty string in deps clj and its being set to config-project and (relativize config-project) gives that weird path

due to https://github.com/borkdude/deps.clj/blob/master/src/borkdude/deps.clj#L807 the surrounding shell cmd is failing

ah ok

I can repro on linux too

so somehow it cant find the deps.edn in the *dir*?

https://github.com/borkdude/deps.clj/blob/master/src/borkdude/deps.clj#L645

ah so *dir* is the dir where the local-dep dir is and that has the deps.edn. so it cant find it

not sure why its different in mac?

setting deps-file to an empty string is a workaround to ignore the local deps.edn

which should be the behavior for bb.edn

brb in some time

sure

I pushed another "fix"

This "fix" seems to work, but it will fail if there is a "nil" file ;)

you can repro this with:

bb -Sdeps '{:deps {borkdude/deps.clj {:mvn/version "1.11.1.1208"}}}' -m borkdude.deps -Sdeps-file uuuuxx

bb -Sdeps '{:deps {borkdude/deps.clj {:mvn/version "1.11.1.1208"}}}' -m borkdude.deps -Sdeps-file /tmp

I think the -Sdeps-file workaround can be entirely removed now though since we always override deps and paths

hmm, that makes a test fail

fixed it by using a file name that should not fail ;)

merged

yay! nice!

checking out the "fixes" in deps

it wasn't "fixed" there (the latest change)

heh

I just "fixed" it like this in bb:

"-Sdeps-file" "__babashka_no_deps_file__.edn"

ah yeah

__don_t_create_a_file_with_this_name_or_you_will_be_fired__

well fun stuff. im happy we found new stuff and had more tests than we started

yep, thanks

definitely makes me more comfortable with the codebase now

Hmm interesting issue. Might be solvable in deps.clj perhaps

By changing the working dir of the java process

i can take a stab at it hopefully soon

Logging an issue would be a good first step

i should do it in the bb repo right?

I wonder how it interacts with the cache, I don’t think the working dir is reflected in there as the cache is simply a hash of deps.edn

Yeah

So that would be something to write down in the issue as well

https://github.com/babashka/babashka/issues/1473

deps.clj already has https://github.com/borkdude/deps.clj/blob/e9318a759628221c48b9259bc8885d4aee1d7262/src/borkdude/deps.clj#L41 so I think it would be just a matter or setting that to the correct dir

small change probably

instead of rewriting the paths after the fact here: https://github.com/babashka/babashka/blob/64dcb3335d6eb35bc8a59e36b5d6104ebd46e1fe/src/babashka/impl/deps.clj#L63 I think we can just bind the *dir* var to the deps-root directory and then it will work for both paths and local/roots

o wait, no paths isn't shelling out to deps.clj, so let's not touch that part

so use with-bindings + cond-> here and add *dir* when deps-root is there https://github.com/babashka/babashka/blob/64dcb3335d6eb35bc8a59e36b5d6104ebd46e1fe/src/babashka/impl/deps.clj#L93

got it

so would the user need to specify the deps-root as well here?

the user?

no, deps-root is set to --config by default, unless the user sets it otherwise

got it. yeah i meant should i need to explicitly specify --deps-root

no, just use the :deps-root like it's used in the paths handling

https://github.com/babashka/babashka/pull/1478

looking into the tests

you can run the tests locally before you push with lein do clean, test

also you can write a test locally before you push :)

you can pull the :deps-root into a let binding maybe

I think the problem might be that you need to bind it to an absolutized path

or something

or fix it here: https://github.com/borkdude/deps.clj/blob/e9318a759628221c48b9259bc8885d4aee1d7262/src/borkdude/deps.clj#L571

good points, will get back to it soon. now my local stuff is working fine, should be better 😄

I'll take a look at a fix in deps.clj right now

I hope absolutizing both files is a good solution

user=> (binding [deps/*exit-fn* identity deps/*dir* ".."] (deps/-main "-M" "-e" "(System/getProperty (str 'user.dir))"))
"/Users/borkdude/dev"
nil
user=> (binding [deps/*exit-fn* identity deps/*dir* "../.."] (deps/-main "-M" "-e" "(System/getProperty (str 'user.dir))"))
"/Users/borkdude"

I can think of one other edge case: in Windows when you invoke the script from another disk drive

let me also test this...

yep, this should work:

user=> (binding [deps/*dir* "/C:/foo"] (deps/relativize "/D:/foo/bar"))
#object[sun.nio.fs.UnixPath 0x1d23ff23 "../../D:/foo/bar"]

I'm copying half of babashka.fs in there...

ok pushed a commit to deps.clj which should help

right, will pull in the changes

tests pass, pushed

• [ ] test • [ ] changelogs

i was using the test-utils/bb for a test like:

(testing "relative paths in :deps are relative to bb.edn"
  (test-utils/with-config '{:deps {babashka/process {:local/root "./process"}}}
    (is (= (str (fs/cwd) "/process/src")
           (first
             (bb "-e" "(do (require '[babashka.classpath :as cp])
                        (cp/split-classpath (cp/get-classpath)))"))))))

or i guess there is a better testing suggestion in general? 😅

also not sure if that (str (fs/cwd) "/process/src") works as expected on windows

this doesn't change the deps-root right, so how does this test the change that the test is for?

I'd look for a test that tests the deps-root feature and add another test like that but with a local/root the refers to a relative dir

cant seem to find a test for deps-root, the only mentions of it in context of tests i can see are: • https://github.com/babashka/babashka/blob/master/test/babashka/test_utils.clj#L52 • https://github.com/babashka/babashka/blob/master/test/babashka/test_utils.clj#L92 any pointers of where could i find it? i thought https://github.com/babashka/babashka/blob/master/test/babashka/bb_edn_test.clj could have it

I guess you're right

there is no test 😨

well, adding that too 😎