make lein plugin a separate, optional artifact

[benedekfazekas]

almost all leinigen specific code is already in leiningen.inline-deps, however mranderson.util still references leiningen for logging. if this latter is eliminated leiningen could be excluded when mranderson is used directly

related to #35

[sogaiu]

IIUC, this refers to info, warn, and debug.

warn does not appear to be used anywhere -- may be that can just be removed?

info is used in:

• util.clj's apply-jarjar!
• inline_deps.clj's lein-project->ctx

debug is used in:

• inline_deps.clj's lein-project->ctx

It seems like inline_deps.clj could just use leiningen.core.main's info and debug directly.

What's not so obvious is what to do about the use of info in apply-jarjar! as that is used in mranderson.core's mranderson.

Any thoughts on these things?

[benedekfazekas]

the point of this issue is not to depend on leiningen as a dependency so projects using mranderson like conjure don't need to pull in leiningen at all. the way to go about it would be to pass is some kind of log-info-fn, debug-log-fn or something along those lines so when leiningen is not around it can be just a println or noop or whatever.

does this make sense?

[sogaiu]

Yes, thanks for the elaboration.

[sogaiu]

How does the idea of introducing a couple of dynamic variables in mranderson.core for debug and info output sound?

A partial sketch:

(def ^:dynamic *log-debug* println)

(def ^:dynamic *log-info* println)

;; inlined from leiningen source
(defn- print-dep [dep level]
  (*log-info* (apply str (repeat (* 2 level) \space)) (pr-str dep)))

So replace u/info with *log-info*, and u/debug with *log-debug* in the rest of the file.

Then in inline-deps in leiningen.inline-deps, use binding:

...
  (let [{:keys [pprefix] :as ctx} (lein-project->ctx project args)
        paths                     (initial-paths target-path pprefix)
        source-dependencies       (filter u/source-dep? dependencies)]
    (binding [c/*log-debug* lein-main/debug
              c/*log-info* lein-main/info]
      (c/mranderson repositories source-dependencies ctx paths))))

(Also, in the function mranderson, since apply-jarjar! is called, may be it could take an extra argument for info and *log-info* can be passed to in.)

[lread]

I think I might want to deal with this to help move #65 along.

So, we are hooking into leiningen's logger fns because their behaviour is appropriate when run from leiningen. But we also want to be invoked when leiningen is not part of the equation and not on the classpath.

Some ideas/options:

1. @sogaiu's dynamic var binding proposal above (hi @sogaiu, I miss ya!)
2. always use leiningen logger fns when found on classpath else println logging. @vemv did something like this here.
3. pass in a logger abstraction to fns that need it. This would be a leiningen logger when run under leiningen else println logger (or a user-defined logger I suppose).
4. use a logger lib, maybe timbre. Write a wee appender for leiningen's logger, use that when called form leiningen, use default timbre logging support otherwise.
5. Don't use leiningen logger fns. Control wether to be silent or show debug logging as options specific to MrAnderson rather than inherited from leiningen.

For option 1 maybe we'd abstract at a logger level rather than the fn level? That would be more future-proof, I think.

I think option 2 is viable and might be good enough.

Option 3 feels awkward, although passing in a logger feels functional, I think it is uncommon to take this approach.

Option 4 is interesting to me, but it does bring in more deps. Not sure how you feel about that, but I could explore this one if you are open to it.

Option 5 seems viable to me, but it might be because I am naive. It might be inappropriate/odd behaviour for leiningen plugin. There is also the matter of stdout vs stderr which I am currently ignoring. Lein logs warnings to stderr for some reason.

Prior to hearing what you think, I am gravitating to explore option 4, but think 2 and 5 are also interesting.

[benedekfazekas]

I like 1 and 2 most I think. I don't necessarily veto 4. if you want to go that way, but 1 or 2 sounds simpler. re. 5 tbh I can't remember why it is appropriate to use lein's own logger when in lein mode...

[lread]

Cool, since it is the easiest, maybe I can start with option 2. We can revisit other options in the future if there is interest.

I can't remember why it is appropriate to use lein's own logger when in lein mode...

I think the advantage is you inherit lein's logging behaviour. If the user wants to silence logging (or enable debug logging), they do so via lein and the plugin picks up that choice.

[vemv]

Yes, that way the LEIN_SILENT / DEBUG env vars will be honored

[benedekfazekas]

yeah, you are right

[lread]

Actually, Option 2 has the con that it doesn't give the user control over logging. I'll play with Option 1 first and see how that feels.

[vemv]

dyn vars are kind of old-fashioned by now - while I'm fond of them they still give the occasional headache (or incompatibility)

I'd suggest going for 2 because logging is a very minor concern in something that isn't a production app.

If anything you'd be giving people a chance to swipe useful output under the rug, which later has to be second-guessed (if remembered at all!) in issue reports :)

As a last resource you might want to simply honor the same env vars Lein does: LEIN_SILENT, DEBUG.

Cheers - V

[lread]

Thanks for chiming in @vemv!

I think MrAnderson currently uses debug and info logging.

I like your Option 6: which I think might be: replicate lein debugging and use that everywhere.

So we'll still log the way we do but not reach out to lein to do so.

The cons:

• we won't pick up changes in logging behaviour from lein, but I'm guessing this won't be an issue
• slightly awkward for non-lein use is LEIN_SILENT... but nobody will use that, so moot point? and DEBUG is generic enough... but might turn on other logging, but that's probably fine.

The pros:

• an easy solution that uncouples logging from the lein implementation.

[vemv]

but might turn on other logging

quite unlikely if you don't export the env var altogether e.g. DEBUG=true my-command

[lread]

Ya, same page, that's what I meant. Other stuff might be looking at DEBUG and enable its logging.

[lread]

@benedekfazekas are you ok with Option 6?

• stop using lein logging fns
• replicate lein logging behaviour that we do use
• respect LEIN_SILENT and DEBUG env vars as lein does

[benedekfazekas]

sounds good to me, thanks!

[benedekfazekas]

reopened this, as this is rather unlocked now by @lread 's good work, but there are still a few things to do

• make lein an optional artifact (maybe a separate deployable as a lein plugin and an other one that is just a lib)
• add a CLI to run mranderson from a shell (optianal but would be real nice)

[lread]

Ah, ok, sounds good.

make lein an optional artifact (maybe a separate deployable as a lein plugin and an other one that is just a lib)

Thinking out loud: do we need a separate artifact? So long as the cli does not reference the leiningen ns would we be good? Or is that a bad idea?

add a CLI to run mranderson from a shell (optianal but would be real nice)

This would be for non-lein users, yeah? I think we could work something out here.

General mranderson config should be straightforward.

Cmdline ease of use could be helped by babashka.cli.

Any ideas on how the user would mark which deps should be inlined? I suppose it could just be a respecified list. But I wonder if we could somehow include the info alongside the deps in deps.edn? Worth exploring options.

[vemv]

Thinking out loud: do we need a separate artifact? So long as the cli does not reference the leiningen ns would we be good?

Eastwood takes the single-artifact approach and it hasn't caused problems (which of course shouldn't - namespaces aren't requireed out of the blue)

[lread]

Eastwood takes the single-artifact approach and it hasn't caused problems

Ah good, thanks @vemv!

[lread]

Any ideas on how the user would mark which deps should be inlined? I suppose it could just be a respecified list. But I wonder if we could somehow include the info alongside the deps in deps.edn? Worth exploring options.

We could use the same approach used in project.clj. Antq has done this. Example usage. Because antq deals with versions it annotates the version. I suppose we might want to annotate the project?

Annotate q1 - annotate the project?

{:deps {^:inline-dep rewrite-clj/rewrite-clj {:mvn/version "1.1.47"}}} 

Annotate q2 - annotate the version?

{:deps {rewrite-clj/rewrite-clj ^:inline-dep {:mvn/version "1.1.47"}}} 

Annotate q3 - namespace the annotation?

Perhaps we should be using ^:mranderson/inline-dep?

Annotate q4 - respecify projects to inline as MrAnderson arg?

:inline-deps [rewrite-clj/rewrite-clj]

Thoughts

1. Not sure about q1 vs q2, any opinions? Any cons to choosing q1?
2. For q3 it seems polite to add the mranderson qualifier, but we do have a precedent of using an unqualified ^:inline-dep in project.clj. Perhaps accept both but warn on unqualified?
3. I think we can ignore q4 to start with, but separating the list of deps to inline from the deps could be convenient for users who want to specify/experiment-with multiple inlining configs.

[vemv]

This would feel the most natural to me:

{:deps {rewrite-clj/rewrite-clj {:mvn/version "1.1.47"
                                 :mranderson/inline true}}} 

It seems safe to assume that the clojure team will honor an open-world assumption.

[benedekfazekas]

yeah i like @vemv approach as well if the clojure tooling does not block it

[lread]

Thanks for responding @vemv and @benedekfazekas, much appreciated! I like your proposal @vemv, I think it reads, to us humans, better than metadata. One upside to using metadata is that a program would not see the metadata unless it explicitly looks for it and it therefore would be less likely to trip up any naive tooling.

I'll ask on Slack for feedback.

[benedekfazekas]

thx @lread for picking this up

[lread]

thx @lread for picking this up

You're welcome @benedekfazekas, I do get distracted, but I usually find my way back!

I am also exploring command-line support.

cmd-line q1: Is this to support deps projects only?

I'm a bit leiningen naive, so I have to ask: Would lein projects continue to rely on mranderson lein plugin support only? Or is command line expected to support both lein and deps projects?

cmd-line q2: overrides in a deps world

While looking at how current config translates to the command line, I wondered about overrides.

In the lein world, deps always come from a maven repo. This is not necessarily the case for deps.edn. Deps can refer to maven, local, git and pom.

For {:overrides {[mvxcvi/puget fipp] [fipp "0.6.15"]}}

I was thinking of a repeatable option like so:

--overrides mvxcvi/puget=fipp@0.6.15 --overrides fipp=fipp@0.6.15

But we might have {:overrides {[mvxcvi/puget fipp] [fipp/fipp {:git/url "...git repo.." :git/sha "...some sha..."}.

So perhaps we might express overrides in some other way for deps.edn projects.

cmd-line q3: shell escaping friendliness

If MrAnderson is never going to support Windows, I might not concern myself too much with this. But if there is even a glimmer of a future where you would want MrAnderson to also support being run on Windows, we should think about this now to try to avoid a hellish user-experience for our friends running Windows.

cmd-line q4: -X vs -M

I am thinking of -M support. Which means a conventional cmd line.

clojure -M -m mranderson.main --some-opt somevalue

Clojure deps also supports -X style cmd lines.

clojure -X:exec-alias :some-key somevalue

I suppose we should also consider -X invocation?

cmd-line q5: Clojure Tools support

When you were thinking of the cmd line were you also implying Clojure Tools support? This implies -X invocation and some thought about if invoking MrAnderson as a Clojure Tool makes sense.

[lread]

Re: marking deps to inline in deps.edn, I asked on Slack and the recommendation was not to do that. Respecifying deps to inline in an alias or other file is the recommended approach. Which is slightly less convenient maybe, but not horrible.

[lread]

@benedekfazekas would love some feedback on cmd-line if/when you have a moment. One option is not to expressly support the cmd-line and just support in-file config. This may be reasonable for a tool like MrAnderson where the config can be complex and is unlikely to change very often.

So an existing project.clj config like cider-nrepl's:

  :dependencies [[nrepl "1.0.0"]
                 ^:inline-dep [cider/orchard "0.11.0" :exclusions [com.google.code.findbugs/jsr305 com.google.errorprone/error_prone_annotations]]
                 ^:inline-dep [mx.cider/haystack "0.0.3"]
                 ^:inline-dep [thunknyc/profile "0.5.2"]
                 ^:inline-dep [mvxcvi/puget "1.3.4"]
                 ^:inline-dep [fipp ~fipp-version] ; can be removed in unresolved-tree mode
                 ^:inline-dep [compliment "0.3.14"]
                 ^:inline-dep [org.rksm/suitable "0.4.1" :exclusions [org.clojure/clojurescript]]
                 ^:inline-dep [cljfmt "0.9.2" :exclusions [org.clojure/clojurescript]]
                 ^:inline-dep [org.clojure/tools.namespace "1.3.0"]
                 ^:inline-dep [org.clojure/tools.trace "0.7.11"]
                 ^:inline-dep [org.clojure/tools.reader "1.3.6"]]
  :exclusions [org.clojure/clojure] ; see Clojure version matrix in profiles below

  :plugins [[thomasa/mranderson "0.5.4-SNAPSHOT"]]
  :mranderson {:project-prefix "cider.nrepl.inlined.deps"
               :overrides       {[mvxcvi/puget fipp] [fipp ~fipp-version]} ; only takes effect in unresolved-tree mode
               :expositions     [[mvxcvi/puget fipp]] ; only takes effect unresolved-tree mode
               :unresolved-tree false}

Might be re-expressed in a deps.edn file like so (forgetting, for the moment, about any config changes that make sense for deps.edn projects)?:

{:deps ;; plain old deps without any mranderson config... 
 {nrepl/nrepl {:mvn/version "1.0.0"}
  cider/orchard {:mvn/version "0.11.0" :exclusions [com.google.code.findbugs/jsr305 com.google.errorprone/error_prone_annotations]}
  mx.cider/haystack {:mvn/version "0.0.3"}
  ...etc...}
 :aliases 
 {:mranderson 
  {:extra-deps {mranderson/mranderson {:mvn/version "..."}}
   :exec-fn mranderson.core/exec
   :exec-args
   {:project-prefix "cider.nrepl.inlined.deps"
    :overrides {[mvxcvi/puget fipp] [fipp "0.6.26"]} ; only takes effect in unresolved-tree mode
    :expositions [[mvxcvi/puget fipp]] ; only takes effect unresolved-tree mode
    :unresolved-tree false
    :inline-deps [cider/orchard 
                  mx.cider/haystack 
                  thunknyc/profile
                  mvxcvi/puget 
                  fipp
                  compliment 
                  org.rksm/suitable
                  cljfmt 
                  org.clojure/tools.namespace 
                  org.clojure/tools.trace
                  org.clojure/tools.reader]}}

Expressing config like this does imply -X cmd-line support, but we could expressly not concern ourselves with -X cmd-line ease of use.

If you are still interested in meaningful cmd-line support, separating it out into its own issue would probably be helpful.

[benedekfazekas]

there is a separate issue for cmdline support #35

[benedekfazekas]

let me think about this a bit. obviously these three things:

• use other tool than pomegranate
• don't depend on lein/support tool.deps too
• have a cli are kind of separate but related issues. I also wonder (thinking out loud) how much we need a project file (either lein or tool.deps flavour) or rather what exactly we need from the project file apart from the list of deps to inline. jar name? source dir(s)?

so let me have a think about this but also keep sharing any ideas you have pls ;)

[lread]

there is a separate issue for cmdline support #35

Ah thanks, missed that! Personally, I'm not sure what problem cmdline support would resolve for users of MrAnderson. If we can express the problem we are trying to solve, we could rationalize implementing it.

• use other tool than pomegranate

Supporting tools.deps resolution could be interesting if:

• tools.deps lib resolution is different enough to be interesting to users
• supporting deps from other sources from maven repos would be benefit users (git deps, local deps, etc).

I think we could probably treat this as a separate issue to start with.

• don't depend on lein/support tool.deps too

We've already unhooked ourselves explicitly depending on lein. (If that's whatcha mean here).

I also wonder (thinking out loud) how much we need a project file (either lein or tool.deps flavour) or rather what exactly we need from the project file apart from the list of deps to inline. jar name? source dir(s)?

My take? Expressing MrAnderson config in project.clj or deps.edn is convenient. Having the config close to the actual project deps & sources config probably make errors/typos less likely. But... a separate mranderson.edn is also viable. But... since current MrAnderson users are already specifying their config in project.clj, maybe we don't want to change the strategy and break usage for them.