deps.clj icon indicating copy to clipboard operation
deps.clj copied to clipboard

Reconsider granularity of API

Open cursive-ghost opened this issue 1 year ago • 3 comments

I recently fixed cursive-ide/cursive#2867, which is due to some internal changes in deps.clj (use of the $HOME env var to find the user home). However as part of debugging this I also found another related issue, due to changes in how the cache checksum is calculated. Most of what I need is exposed via the API, but the recent API changes have required code like this:

get-cache-dir (let [ns (find-ns 'borkdude.deps)]
                (or (ns-resolve ns 'get-cache-dir*)
                    (ns-resolve ns 'get-cache-dir)))
cache-result (get-cache-dir {:deps-edn   deps-edn
                             :config-dir config-dir})
cache-dir (if (map? cache-result)
            (:cache-dir cache-result)
            cache-result)
cache-dir-key (if (map? cache-result)
                (:cache-dir-key cache-result))
checksum (deps/get-checksum
           (cond-> {:cli-opts     cli-opts
                    :config-paths config-paths}
             cache-dir-key (assoc :cache-dir-key cache-dir-key)))

i.e. figure out whether to use the old or the new means of getting the cache dir, and then switching on the type of the return to calculate the checksum. I can't see any other way to have this work such that the version of Cursive doesn't explicitly depend on a specific version of deps.clj without doing this sort of workaround.

I don't have a specific proposal here, but it would be nice if we could figure out a way to evolve the API without the need for this sort of thing on the client side. In this case, it looks like the idea was that get-cache-dir would continue to work, but unfortunately without cache-dir-key I can't get consistent checksums, which I need to pass to get-basis-file. If I continued to call get-cache-dir (as opposed to get-cache-dir*), then I wouldn't pass cache-dir-key to checksum, and would get errors like:

java.io.FileNotFoundException: /Users/colin/dev/cursive-bugs/polylith-kaocha/.cpcache/DDAB581C3D901BDA5B33C69859866E56.basis (No such file or directory)

I had thought that the checksum could continue to produce consistent results if cache-dir-key wasn't passed to it, but of course that won't work, and the script would still produce different values if one client (Cursive) passed it and the other (deps.clj itself) did not. Basically, the checksum will have to be updated from time to time if new internal state is added, but that will be difficult to expose through the API in a backwards compatible way. Perhaps a good way to achieve this would be to add the basis file path to the -Sdescribe output? That would not be consistent with the Clojure CLI tools, of course. Or perhaps add a new flag which outputs extra data? The basic idea would be to add a higher-level API (which -Sdescribe really is) which doesn't depend so much on the internal details of how deps.clj works, currently the API is very granular and requires clients to be quite tied to the internals of the script.

cursive-ghost avatar Jan 20 '24 09:01 cursive-ghost

Sorry for the "breakage" but yeah, the idea was to preserve backward compatibility. This change was necessary since the newest clojure CLI checks if the current working directory is writeable, and if not, it will choose a directory in home that is writeable for the cache key.

I'd be fine adding extra data to -Sdescribe. PR welcome in a way that suits you. Other proposals also welcome.

borkdude avatar Jan 20 '24 09:01 borkdude

@cursive-ide

@puredanger is considering adding -Sbasis to the CLI. Would this solve your problem? I'm not sure if -Sbasis is going to print the file which contains the basis of the contents of that file

borkdude avatar Feb 15 '24 14:02 borkdude

@borkdude I think that would be fine, since I only want the path to get the basis itself IIRC. I'll check the code today. In retrospect, I can't believe I didn't think of this option!

cursive-ghost avatar Feb 15 '24 18:02 cursive-ghost