Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clojure (Leiningen) Support #2769

Closed
wants to merge 19 commits into from
Closed

Conversation

CGA1123
Copy link

@CGA1123 CGA1123 commented Nov 17, 2020

馃憢 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 charm

This implementation mostly piggy-back off of the maven implementation, with some lein specific FileFetcher and FileUpdater (and also a FileParser, but only to set the package_manager as lein so that PRs have the proper label.

Related: #572 #1162

CGA1123 and others added 13 commits November 17, 2020 09:28
Signed-off-by: Christian Gregg <christian@bissy.io>
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)))]

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.

https://github.com/technomancy/leiningen/blob/830d19d5b9f9e2b0e3411428d83422791ca2f814/leiningen-core/src/leiningen/core/project.clj#L1094-L1110

Copy link
Member

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?

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.

@jurre
Copy link
Member

jurre commented Nov 18, 2020

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.

@CGA1123
Copy link
Author

CGA1123 commented Nov 18, 2020

@jurre No worries 馃槃

And yes will be trying to carve out some time myself to get some test coverage

@danielcompton
Copy link

Side note: If this approach works for Leiningen, you could do something very similar for deps.edn files. clj -X:deps mvn-pom will generate a POM given a deps.edn file.

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>
@brrygrdn brrygrdn added F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. T: new-ecosystem Requests for new ecosystems/languages on-hold 馃洃 Feature is proposed but hasn't been scheduled labels Dec 18, 2020
@brrygrdn
Copy link
Contributor

brrygrdn commented Dec 18, 2020

馃憢 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.

@CGA1123
Copy link
Author

CGA1123 commented Dec 18, 2020

@brrygrdn Thanks for the update, makes sense. 馃槃

@CGA1123
Copy link
Author

CGA1123 commented Dec 30, 2020

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 馃檪

@CGA1123 CGA1123 closed this Dec 30, 2020
@colinphill-mdsol
Copy link

colinphill-mdsol commented May 28, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: language-support Issues specific to a particular language or ecosystem; may be paired with an L: label. on-hold 馃洃 Feature is proposed but hasn't been scheduled T: new-ecosystem Requests for new ecosystems/languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants