antq icon indicating copy to clipboard operation
antq copied to clipboard

When using antq as a lein plugin it does not seem to find outdated deps in github actions

Open lread opened this issue 9 months ago • 13 comments

Hello @liquidz! I just noticed a thing while helping out with cheshire.

Problem

When using antq as a lein plugin, it finds outdated deps in project.clj but does not seem to find them in github actions config.

Minimal Scenario

Here's a wee repro, from an empty dir:

project.clj

(defproject foo "1.2.3-SNAPSHOT"
  :description "Testing antq as lein plugin"
  :dependencies [[rewrite-clj/rewrite-clj "0.6.8"]]
  :plugins [[com.github.liquidz/antq "2.11.1276"]])

.github/workflows/test.yml

name: Test

on:
  push:
    branches:
      - master
  pull_request:
    branches:
      - master

jobs:
  testing:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout
      uses: actions/checkout@v3

    - name: Setup Java
      uses: actions/setup-java@v4
      with:
        distribution: 'temurin'
        java-version: 23

    - name: Install Clojure Tools
      uses: DeLaGuardo/[email protected]
      with:
        lein: 'latest'

Actual Behaviour

When I run antq as a lein plugin, I only see outdated deps for project.clj:

$ lein antq
[##################################################] 4/4
| :file       | :name                   | :current | :latest |
|-------------|-------------------------|----------|---------|
| project.clj | nrepl/nrepl             | 1.0.0    | 1.3.1   |
|             | rewrite-clj/rewrite-clj | 0.6.8    | 1.1.49  |

Available changes:
- https://github.com/nrepl/nrepl/blob/v1.3.1/CHANGELOG.md

Expected Behaviour

I would expect to see outdated deps for all supported files. For example, when I run antq from clojure in the same dir, I see all outdated deps (including those for github actions):

$ clojure -Sdeps '{:deps {com.github.liquidz/antq {:mvn/version "2.11.1276"}}}' -M -m antq.core
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[##################################################] 5/5
| :file                      | :name                    | :current | :latest |
|----------------------------|--------------------------|----------|---------|
| .github/workflows/test.yml | DeLaGuardo/setup-clojure | 13.1     | 13.2    |
|                            | actions/checkout         | v3       | v4.2.2  |
| project.clj                | rewrite-clj/rewrite-clj  | 0.6.8    | 1.1.49  |

Available changes:
- https://github.com/DeLaGuardo/setup-clojure/compare/13.1...13.2
- https://github.com/actions/checkout/blob/v4.2.2/CHANGELOG.md

Does that make sense?

lread avatar Mar 20 '25 21:03 lread

@lread Leiningen plugin does not check any other sources for now. (cc @vemv ) https://github.com/liquidz/antq/blob/0e8d29f47977179bb076a74a0ee7ff576b0281fd/src/leiningen/antq.clj#L13

I'll add a note to the README.

liquidz avatar Mar 20 '25 22:03 liquidz

Ah. There is probably a good reason for this limitation; I just don't know it!

lread avatar Mar 21 '25 02:03 lread

It may have been the case that GHA processing was a feature devised after the Lein plugin was implemented?

vemv avatar Mar 21 '25 02:03 vemv

Ah, thanks @vemv. If it is just that, and there is no technical barrier, would you like me to attempt a PR @liquidz?

lread avatar Mar 21 '25 15:03 lread

@lread No problem! PR is very welcomed :)

liquidz avatar Mar 21 '25 20:03 liquidz

I'm a deps.edn guy, but I think lein users should have a great antq experience too.

Current State

Antq can be used with lein in 2 different ways:

  1. When using antq as a lein plugin, it only analyzes project.clj When doing so, it uses lein's loaded version of project.clj Upgrade of deps is not supported for the lein-evaled project.clj.

  2. When using antq as a main program, we get the same behaviour as using antq via tools cli. We get static analysis of project.clj with deps upgrade support.

    1. One awkwardness is that a confusing message is emitted by lein when there are outdated dependencies:
      Error encountered performing task 'run' with profile(s): 'antq'
      Suppressed exit
      
    2. Another awkwardness is the uninitialized logger messages, but this is addressable by https://github.com/liquidz/antq/blob/main/doc/avoid-slf4j-warnings.adoc

My Thoughts

I think plugins are the natural preference of lein users.

It is interesting that the antq lein plugin analyzes the lein-loaded project.clj. This does have value; for example, I learned that lein itself is using an outdated version of nrepl. I wonder if the lein antq plugin could upgrade non-dynamic dep targets.

An alternative for the antq lein plugin might have been to have it only carry out antq's full static analysis (without the lein-loaded project.clj analysis support). But since the antq lein plugin already does the lein-loaded analysis of project.clj, I think we probably need to keep this feature.

If the antq lein plugin were to have feature parity with running antq as main, then lein users would probably never run antq as main.

Proposal

Have the antq lein plugin support all files and all options.

For project.clj, continue the current analysis on lein-loaded deps, but marry this to a static analysis of project.clj to allow upgrades on deps that are safe to upgrade. Emit warnings for deps that antq cannot upgrade.

Technical

I'll look into how lein plugins work as I dig in.

Sound good?

Let me know what you think. Happy to discuss/adapt/evolve/tweak the plan.

lread avatar Mar 22 '25 15:03 lread

Sounds great to me!

The Lein plugin was contributed because Lein can do eval in a number of ways (~, plugins, middleware) so static analysis could miss out.

This has the tradeoff of missing the source code, that has valuable info for a complete 'upgrade' recipe for the user.

Probably there's no inherent reason why it can't analyse deps.edn, GHA, etc.

About doing a dynamic+static hybrid approach, it would sound good to me as long as it doesn't end up being an overly demanding task, implementor side. I mainly think of the cost/benefit, given the declining popularity of Lein.

vemv avatar Mar 22 '25 16:03 vemv

Thanks for chiming in @vemv; much appreciated!

You make a good point about leiningen popularity. Although it is in decline, it is still quite popular, at least as of the last clojure survey.

Image

lread avatar Mar 22 '25 17:03 lread

Sounds good to me :)

liquidz avatar Mar 22 '25 20:03 liquidz

Cool, I will get started on this soon!

lread avatar Mar 23 '25 15:03 lread

@liquidz, the lein plugin can supply antq options via a map under :antq in project.clj. I was thinking that because antq validates cli options, these should be validated too.

  1. I noticed antq uses malli and started to code up options validation with malli. I then noticed that malli is a dev-only dep. What do you think of adding malli as a default dep to antq? It is currently babashka compatible and clojure 1.11+, I think antq is clojure 1.10+. I could code up antq lein plugin options validation without malli. Lemme know.
  2. I noticed that clojure tools usage (antq.tool) doesn't yet seem to validate options. We could probably reuse options validation code for this.
  3. Would it also be interesting to validate options at the antq.api entry points?

Items 2 & 3 are out of scope for this issue but are worth thinking about as a general options validation strategy.

lread avatar Mar 27 '25 16:03 lread

I noticed antq uses malli and started to code up options validation with malli. I then noticed that malli is a dev-only dep. What do you think of adding malli as a default dep to antq? It is currently babashka compatible and clojure 1.11+, I think antq is clojure 1.10+. I could code up antq lein plugin options validation without malli. Lemme know.

The current malli schema in antiq is incomplete, so it was included as a dev-only dependency. So I think it makes sense to make it a default dependency once the schema is properly defined. (I’ve prioritized other projects for now, so fixing that has been postponed 🙇 )

I noticed that clojure tools usage (antq.tool) doesn't yet seem to validate options. We could probably reuse options validation code for this.

Yes, exactly. The same kind of validation should be in place here too!

Would it also be interesting to validate options at the antq.api entry points?

I think docstrings are sufficient for the API, so I personally don’t feel it’s necessary.
However, I do think it wouldn’t hurt to have it.

liquidz avatar Mar 28 '25 21:03 liquidz

Thanks for the reply @liquidz. If I understand:

  1. You'd like to introduce malli as a default dep to antq, but only after you've fleshed out the schema.
  2. After that, you are fine with using malli to validate clojure tools cli and lein antq plugin options entrypoints
  3. Validating options at API entrypoints is not a real priority.

That all sounds good to me. If we are to use malli to validate for point 2, I think it depends on point 1.

Another idea. I've used babashka.cli on clj-watson to validate options for both clojure -M and clojure -X syntaxes. I could explore using it to validate options for point 2. To consider: the babashka.cli min clojure version is, like malli, 1.11 (I don't see this as an issue for antq, but worth a mention).

In any case I think maybe validating options (point 2) deserves its own separate issue. I'll create one.

lread avatar Mar 29 '25 15:03 lread