Calculate classpath using tools.deps and provide nvd-clojure as a regular -X function?

[ivarref]

Hi @vemv (and others)

And thanks for a great project!

I'm wondering if it is worthwhile to support calculating the classpath using tools.deps, thus avoiding the need for an extra script.
With the changes in this pull request, adding the following alias to an existing project:

:nvd-check      {:replace-deps  {nvd-clojure/nvd-clojure {:local/root "/home/ire/code/nvd-clojure"}} ; real version / change the path to a proper folder on your machine
                              :replace-paths []
                              :jvm-opts      ["-Dclojure.main.report=stderr" "-Dorg.slf4j.simpleLogger.log.org.apache.commons=error"]
                              :exec-fn       nvd.task/check} 

enables to execute the nvd check using simply clojure -X:nvd-check.

I've verified that the new code in nvd.task produces identical classpath to the output of clojure -Spath.

(There could of course be something else I've missed, I'm not a classpath and/or tools.deps expert or anything like that.)

What do you think?

Thanks and kind regards.

[vemv]

Hi @ivarref, thank you for a sensible PR!

I'm aware of this technique and it is indirectly mentioned in the docs.

However, in principle I do not wish to support it.

For instance, how do you guarantee that users will specify :replace-deps instead of :extra-deps? Same for :replace-paths.

And how do you guarantee that having tools.deps as a dependency does not alter nvd-clojure's transitive dependency tree?

That kind of reasoning lead me to support first just one way to run this for clojure users, and later a Tools offering.

The Tools offering is highly idiomatic and reusable. The other one is a bit more surprising, but I'd love for people to see that a single-file directory and a just-data, tiny deps.edn are extremely cheap things to have.

Both techniques allow us maintainers to discard whole categories of bugs.

Last but not least, probably there's nothing stopping you from distributing your own program somewhere and compose it with nvd-clojure. Simply keep in mind that should you encounter a bug, it should be reproducible with one of the officially supported methods.

Cheers - V

[vemv]

This section, although tiny, represents a duplication of what CLI clojure usage does internally.

Having that duplication of knowledge represents a surface area for bugs/misunderstandings that would not otherwise exist.

---

...I'm aware that this reasoning might be considered overly cautious in 9 out of 10 projects, but nvd-clojure is the one-in-ten project that does deserve squeezing the last drop of correctness.

I'd want the project to guarantee that it's doing what it says it does - our users deserve no less, if we're promising security.

[ivarref]

Thanks for the thoughtful response @vemv !

For instance, how do you guarantee that users will specify :replace-deps instead of :extra-deps? Same for :replace-paths.

The deps.edn file can be read and verified that no [:aliases :extra-deps] map contains rm-hull/nvd-clojure.
This can be done using the code already written, i.e. with tools.deps. I believe that should handle the case of multi module projects (which I've personally never used) as well.

And how do you guarantee that having tools.deps as a dependency does not alter nvd-clojure's transitive dependency tree?

It's possible to inspect this. I pushed a namespace for this:

(in-ns 'nvd.tools-deps-conflict-test)
Loading test/nvd/tools_deps_conflict_test.clj... done
(show-diff 'org.clojure/tools.deps)
com.google.errorprone/error_prone_annotations deps.edn: 2.18.0 vs org.clojure/tools.deps: 2.11.0
com.google.guava/guava deps.edn: 32.1.2-jre vs org.clojure/tools.deps: 31.1-android
com.google.j2objc/j2objc-annotations deps.edn: 2.8 vs org.clojure/tools.deps: 1.3
commons-io/commons-io deps.edn: 2.13.0 vs org.clojure/tools.deps: 2.11.0
org.apache.commons/commons-lang3 deps.edn: 3.13.0 vs org.clojure/tools.deps: 3.12.0
org.checkerframework/checker-qual deps.edn: 3.33.0 vs org.clojure/tools.deps: 3.12.0
org.slf4j/jcl-over-slf4j deps.edn: 1.7.28 vs org.clojure/tools.deps: 1.7.36
org.slf4j/slf4j-api deps.edn: 2.0.7 vs org.clojure/tools.deps: 1.7.36

We can see that introducing tools.deps does alter the tree, bumping org.slf4j/jcl-over-slf4j from 1.7.28 to 1.7.36.
Otherwise the classpath is unchanged (deps.edn of nvd-clojure contains newer libs than the ones referred by tools.deps).
I don't think this particular bumping is anything to worry about.

Code duplication

I suppose it also would be possible to call make_classpath2 directly.

All that said, I get the impression that you've made up your mind about this, and that is perfectly fine.
Personally though I fell it's unnatural to install a tool, write shell scripts, etc. in a CI setting, which is the context I want to use -X in.

Did any of the above change your mind? If not I'm happy to discard the pull request. If you are now more convinced than before that this is a good idea, I could try:

• writing a more proper test to verify/detect classpath changes brought by tools.deps.
• write something that detect using this library inside :extra-deps { / is used properly using :replace-deps.
• making use of make_classpath2.

Let me know what you think.
Thank and kind regards.

[ivarref]

I'll make a separate post as I've learned more about tools.deps today (thanks to a colleague).

First it seems that -T also supports :exec-fn in an alias. I have not really seen that documented anywhere in writing, but it does work. Which is great! Thus my need/desire for -X disappears.

My project's deps.edn can then be like this:

             :nvd-check      {:deps     {nvd-clojure/nvd-clojure {:local/root "/home/ire/code/nvd-clojure"}}
                              :jvm-opts ["-Dclojure.main.report=stderr" "-Dorg.slf4j.simpleLogger.log.org.apache.commons=error"]
                              :exec-fn  nvd.task/check}}

Clojure CLI does the right thing regardless if you use :extra-deps, :deps or :replace-deps as long as you use -T.

[ivarref]

PS: If it's decided that it's OK to include tools.deps, it is also possible to print a dependency tree with highlighted problematic dependencies. I've done some work in that direction at the following repo: https://github.com/ivarref/finddep

Best regards

[vemv]

Thanks for the ping!

I'll get back to this soon.

I'm not sure I'll be convinced but I'll try to give it a read open-mindedly.

Cheers - V