-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Clojure (Leiningen) Support #2769
Conversation
Using `leiningen.change` works but it doesn't preserve formatting of the original `project.clj` file, so creates really ugly diffs. `rewrite-clj` should preserve the original formatting. Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
The Maven::FileParser will tag them as `maven` (obviously) and this results in PRs being tagged as `java` rather than `clojure`! Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
(zip/root-string))) | ||
|
||
(defn generate-pom [{:keys [file]}] | ||
(let [proj (project/read-raw (io/reader (char-array file)))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this execute code that lives in the project.clj
file? I think so? Not sure if that's safe in this context or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's mostly fine, as long as the code that's executed is from the source repository. The same thing is possible in a setup.py
, Gemfile
or mix.exs
file. My main worry would be if an external package would be able to execute code in this context, but I don't think that's the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, downloading POMs (Maven/Clojure dependency manifest) and JARs (dependency files) doesn't execute any code. Only the code that exists in the project would be executed.
Thanks for the PR @CGA1123, overall the approach looks solid, awesome work! Currently the team is stretched a bit thin, and we won't be able to take on reviewing + supporting this new ecosystem, but we hope to have a bit more bandwidth for that in a few weeks. One thing that would be great to see is some test coverage for the bits that are newly implemented, but that's something we can also look into once we have time to properly review + test this. |
@jurre No worries 😄 And yes will be trying to carve out some time myself to get some test coverage |
Side note: If this approach works for Leiningen, you could do something very similar for |
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
Signed-off-by: Christian Gregg <christian@bissy.io>
👋 Thanks for opening this PR, we really appreciate the effort to make Dependabot better for the community. Unfortunately, we are not accepting new ecosystems into core at the moment. We are currently focused on making some improvements into how we handle extensibility before we add any more ecosystems. We will still be improving and upgrading support for existing ecosystems. Please check the Contribution guidelines for more information. |
@brrygrdn Thanks for the update, makes sense. 😄 |
Closing as part of some new years cleaning 😄 If people are interested in running a Clojure dependabot for their org / themselves check out dependabot-lein-runner which achieves that and feel free to open issues and i'll help out as much as time allows 🙂 |
It looks like new ecosystems are now being accepted as of #8844. My team would benefit greatly from first-class Leiningen support. Unfortunately, we're bound by a somewhat restrictive OSS contributions policy, and we don't have Ruby knowledge internally. If this PR were reopened, would it be mergeable as-is, or have the intervening 3.5 years made further changes necessary? I could try to get approval to take ownership of this and muddle my way through the Ruby, if the necessary changes are simple enough. |
👋 This code is not 100% but want to open this PR up for any feedback and to gauge whether there would be any appetite for getting
lein
support upstreamed into the dependabot-core and eventually the product!We've been running this at work for the last couple weeks (via a version of
dependabot-lein-runner
) and its been working a charmThis implementation mostly piggy-back off of the
maven
implementation, with some lein specificFileFetcher
andFileUpdater
(and also aFileParser
, but only to set thepackage_manager
aslein
so that PRs have the proper label.Related: #572 #1162