-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make lein plugin a separate, optional artifact #42
Comments
IIUC, this refers to
It seems like inline_deps.clj could just use leiningen.core.main's What's not so obvious is what to do about the use of Any thoughts on these things? |
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? |
Yes, thanks for the elaboration. |
How does the idea of introducing a couple of dynamic variables in A partial sketch:
So replace Then in
(Also, in the function |
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:
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. |
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... |
Cool, since it is the easiest, maybe I can start with option 2.
I think the advantage is you inherit lein's logging behaviour. |
Yes, that way the |
yeah, you are right |
Actually, Option 2 has the con that it doesn't give the user control over logging. |
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 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: Cheers - V |
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:
The pros:
|
quite unlikely if you don't |
Ya, same page, that's what I meant. Other stuff might be looking at |
@benedekfazekas are you ok with Option 6?
|
sounds good to me, thanks! |
We now replicate, instead of use, lein's logging. And add in multi-threaded support. This decouples us from lein for general use, and should allow API analysis on cljdoc to pass. Closes benedekfazekas#42
We now replicate, instead of use, lein's logging. And add in multi-threaded support. This decouples us from lein for general use, and should allow API analysis on cljdoc to pass. Closes benedekfazekas#42
We now replicate, instead of use, lein's logging. And add in multi-threaded support. This decouples us from lein for general use, and should allow API analysis on cljdoc to pass. Closes benedekfazekas#42
* Replace lein logging with our own internal impl We now replicate, instead of use, lein's logging. And add in multi-threaded support. This decouples us from lein for general use, and should allow API analysis on cljdoc to pass. Closes #42 * changelog
reopened this, as this is rather unlocked now by @lread 's good work, but there are still a few things to do
|
Ah, ok, sounds good.
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?
This would be for non-lein users, yeah? 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? |
Eastwood takes the single-artifact approach and it hasn't caused problems (which of course shouldn't - namespaces aren't |
Ah good, thanks @vemv! |
We could use the same approach used in 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 Annotate q4 - respecify projects to inline as MrAnderson arg?
Thoughts
|
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 |
yeah i like @vemv approach as well if the clojure tooling does not block it |
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. |
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: cmd-line q2: overrides in a deps worldWhile looking at how current config translates to the command line, I wondered about In the lein world, deps always come from a maven repo. For I was thinking of a repeatable option like so:
But we might have So perhaps we might express overrides in some other way for deps.edn projects. cmd-line q3: shell escaping friendlinessIf MrAnderson is never going to support Windows, I might not concern myself too much with this. cmd-line q4:
|
Re: marking deps to inline in |
@benedekfazekas would love some feedback on cmd-line if/when you have a moment. So an existing :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 ;; 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 If you are still interested in meaningful cmd-line support, separating it out into its own issue would probably be helpful. |
there is a separate issue for cmdline support #35 |
let me think about this a bit. obviously these three things:
so let me have a think about this but also keep sharing any ideas you have pls ;) |
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.
Supporting tools.deps resolution could be interesting if:
I think we could probably treat this as a separate issue to start with.
We've already unhooked ourselves explicitly depending on lein.
My take? Expressing MrAnderson config in |
almost all
leinigen
specific code is already inleiningen.inline-deps
, howevermranderson.util
still references leiningen for logging. if this latter is eliminated leiningen could be excluded when mranderson is used directlyrelated to #35
The text was updated successfully, but these errors were encountered: