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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature inquiry: optionally exit with non-zero exit code if any deps are outdated #39

Closed
aviflax opened this issue Mar 4, 2020 · 16 comments

Comments

@aviflax
Copy link

aviflax commented Mar 4, 2020

I’d like to set up a nightly CI job that would run depot and would fail if any deps are out of date. The CI service I use will automatically fail a step if a command exits with a non-zero exit code, so I’d love to be able to invoke Depot with a new option that would cause it to exit if any deps are found to be out of date.

Something like: clojure -A:outdated --fail

I’d be happy to try to contribute this feature via a PR (assuming I can find the time) but I thought I’d run it by the project maintainers first to check if there’s be interest in accepting such a contribution.

Thanks!

@Olical
Copy link
Owner

Olical commented Mar 4, 2020

Oh this is a really nice idea, I don't have a need for it personally but I'm okay with the concept.

I had a think about the arg and I think I agree with --fail, since --ci is a bit too specific and making it always happen will potentially break existing tools or scripts that rely on this in interesting ways. Fail is just opting into return codes for the status.

I don't think it'd be too complex to implement, presumably a (System/exit ...) in -main. If I'm not mistaken, you can just check if new-versions is empty or not?

@aviflax
Copy link
Author

aviflax commented Mar 4, 2020

Oh this is a really nice idea, I don't have a need for it personally but I'm okay with the concept.

Great!

I had a think about the arg and I think I agree with --fail, since --ci is a bit too specific and making it always happen will potentially break existing tools or scripts that rely on this in interesting ways. Fail is just opting into return codes for the status.

Great.

I don't think it'd be too complex to implement, presumably a (System/exit ...) in -main. If I'm not mistaken, you can just check if new-versions is empty or not?

Aha, that’s very helpful! Yeah, that looks right. I’ll get right on it. Thanks!

@aviflax
Copy link
Author

aviflax commented Mar 11, 2020

I’ve got this “code complete” locally but I haven’t had a chance to test it yet. Hopefully next week — I’m very distracted right now as my daughter (7) is quarantined at home due to COVID-19.

@Olical
Copy link
Owner

Olical commented Mar 16, 2020

Well done, I'll take a look at it soon and see if I have any feedback too. I'm sure it's good 😄 I was snowboarding over the past week but I'm isolating now too.

You focus on your family, that's far more important, we'll get this sorted whenever, no rush. I hope your daughter and the rest of your family are completely okay!

@aviflax
Copy link
Author

aviflax commented Mar 16, 2020

Thanks! We are okay, thanks. 🤗

@Wegi
Copy link

Wegi commented Jul 22, 2020

Hey, any new on this issue? Would really appreciate this.

@hoxu
Copy link
Contributor

hoxu commented Sep 7, 2020

I want this feature as well. Non-zero exit status seems to be somewhat standard for similar tools (npm outdated for example).

@aviflax @Olical: is someone working on this still? What is missing? Does it need automated tests? I noticed the fork repository is archived.

I noticed this line in b98e1fe :

exit-code (if fail (count new-versions) 0)]

This is probably a bad idea. Different exit codes denote different things, and especially high ones have standard uses. It's better to just exit with 1 always when there are updates, and document that under exit codes.

@Olical
Copy link
Owner

Olical commented Sep 7, 2020

Agreed with the exit code part, and no, I don't think anyone is working on this right now.

I'm actually erring on finally giving this repository the makeover it kind of sorely needs. I accepted PRs that were good but not how I would've done them, and as a result I feel a little alienated from the codebase so I'm hesitant to try and make any changes.

I've been kind of mentally overloaded until pretty recently so I'm going to be getting my OSS gears spinning again, I have a LOT I want to do on Conjure but a rewrite of Depot could be pretty welcome since it doesn't look like anyone else has tackled this problem since I did?

Food for thought, a future version that has the following properties:

  • Exits 1 on outdated.
  • Uses tools.deps and DOES NOT parse deps.edn manually in any way. Yes this means you'll have to provide aliases and things if you want those to be checked. I think.
  • Prints a human readable list of changes by default, leaves it up to you to apply them.
  • It never modifies your deps.edn, I've never used this feature in any tool, I know a lot of people like that but I think it's a bit tooooo trusting that every version will work and I'm not okay with promoting that ethos. I did my time in npm land.
  • With a flag it will format the version changes as a best effort diff that you can apply if you so choose through git/patch CLI tools.

Basically the goal would be to make it so simple and targeted towards a goal that it's hard to get it wrong. Yes I'd be removing bells and whistles, but I don't understand those bells and whistles right now so I'd rather leave them in a stale version since I don't know how to improve / work on them anyway.

Thoughts?

@hoxu
Copy link
Contributor

hoxu commented Sep 7, 2020

I'm not aware of better tools for this task.

I personally use Depot with --every --write, so losing those features would not be nice. Although I can appreciate simplicity and easier maintainability in a tool.

What would the command to get equal functionality to --every look like?

@Olical
Copy link
Owner

Olical commented Sep 7, 2020

I'm not sure yet, I need to give this some long slow thought really to see if I can find a way to do both. I think updating everything is a little scary in a lot of cases. I just don't want to be processing the EDN anymore, I think it's too hard to do consistently with tools.deps, I'd rather lean on tools.deps for as much as possible.

Maybe I can think of a way to use tools.deps but do a little EDN digging to find all alias names as well as work out how to put the changes back in the right places. I think it'll be really hard to do, but I'll give it some time with the slow brain.

@hoxu
Copy link
Contributor

hoxu commented Sep 8, 2020

Here is a workaround for getting the exit value 1 when old versions are found:

clj -A:outdated |awk -v ret=0 '/^  /{ret=1;} /^/{print} END {exit ret}'

@hoxu
Copy link
Contributor

hoxu commented Sep 18, 2020

Correction, the above did not account for the "All up to date!" being indented by two as well:

Checking for old versions in: deps.edn
  All up to date!
clj -A:outdated |awk -v ret=0 '/^  .*}$/{ret=1;} /^/{print} END {exit ret}'

Should match the following:

Checking for old versions in: deps.edn
  org.foo/foo {:mvn/version "1.2.2"} -> {:mvn/version "1.2.3"}

@hoxu
Copy link
Contributor

hoxu commented Apr 3, 2021

@Olical any update on getting that exit code feature in depot? Could someone help by creating a PR? Any tips?

@Olical Olical closed this as completed in 6291d23 Apr 5, 2021
@Olical Olical reopened this Apr 5, 2021
@Olical
Copy link
Owner

Olical commented Apr 5, 2021

Didn't mean to close that, I just pushed a change that adds a --fail flag, let me know how it is.

@hoxu
Copy link
Contributor

hoxu commented Apr 6, 2021

@Olical tested by cloning and running mvn install. Seems to be working fine to me, looking forward to having this in a release!

$ clj -M:depot --fail; echo $?
...
Checking for old versions in: deps.edn
  com.impossibl.pgjdbc-ng/pgjdbc-ng {:mvn/version "0.8.6"} -> {:mvn/version "0.8.7"}
  io.netty/netty-transport-native-kqueue {:mvn/version "4.1.60.Final"} -> {:mvn/version "4.1.63.Final"}
  org.clojure/clojurescript {:mvn/version "1.10.773"} -> {:mvn/version "1.10.844"}
  org.flywaydb/flyway-core {:mvn/version "7.7.0"} -> {:mvn/version "7.7.2"}
1
$ clj -M:depot --every --write; echo $?
...
Updating old versions in: deps.edn
  com.impossibl.pgjdbc-ng/pgjdbc-ng {:mvn/version "0.8.6"} -> {:mvn/version "0.8.7"}
  io.netty/netty-transport-native-kqueue {:mvn/version "4.1.60.Final"} -> {:mvn/version "4.1.63.Final"}
  org.clojure/clojurescript {:mvn/version "1.10.773"} -> {:mvn/version "1.10.844"}
  org.flywaydb/flyway-core {:mvn/version "7.7.0"} -> {:mvn/version "7.7.2"}
  clj-kondo/clj-kondo {:mvn/version "2021.03.22"} -> {:mvn/version "2021.03.31"}
  binaryage/devtools {:mvn/version "1.0.2"} -> {:mvn/version "1.0.3"}
  day8.re-frame/re-frame-10x {:mvn/version "1.0.1"} -> {:mvn/version "1.0.2"}
  thheller/shadow-cljs {:mvn/version "2.11.24"} -> {:mvn/version "2.12.1"}
0
$ clj -M:depot --every --fail; echo $?
...
Checking for old versions in: deps.edn
  All up to date!
0

@Olical
Copy link
Owner

Olical commented Apr 6, 2021

Published under v2.2.0!

@Olical Olical closed this as completed Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants