babashka / neil

A CLI to add common aliases and features to deps.edn-based projects
MIT License
360 stars 26 forks source link

neil dep upgrade inserts git/url into upgraded dep #230

Closed borkdude closed 3 weeks ago

borkdude commented 3 weeks ago

When upgrading this dep:

{:deps {io.github.nextjournal/markdown {:git/sha "6683c48dfdb23404a23057817b6ac3acf0310bca" :neil/pinned true}}}

a git/url is inserted, which shouldn't happen.

cc @teodorlu if you're interested in having a look

teodorlu commented 3 weeks ago

cc @teodorlu if you're interested in having a look

Yes please - seems there was a case I didn't catch in #228.

teodorlu commented 3 weeks ago

I'm unable to reproduce this problem on latest master per now (6333aa6).

$ cat deps.edn
{:deps {io.github.nextjournal/markdown {:git/sha "6683c48dfdb23404a23057817b6ac3acf0310bca"
                                        :neil/pinned true}}}
$ neil-dev dep upgrade
$ cat deps.edn        
{:deps {io.github.nextjournal/markdown {:git/sha "6683c48dfdb23404a23057817b6ac3acf0310bca"
                                        :neil/pinned true}}}

Same behavior on neil 0.3.67:

$ cat deps.edn 
{:deps {io.github.nextjournal/markdown {:git/sha "6683c48dfdb23404a23057817b6ac3acf0310bca"
                                        :neil/pinned true}}}
$ neil --version
neil 0.3.67
$ neil dep upgrade
$ cat deps.edn
{:deps {io.github.nextjournal/markdown {:git/sha "6683c48dfdb23404a23057817b6ac3acf0310bca"
                                        :neil/pinned true}}}

I wrote a test covering the case, protecting against future regressions:

https://github.com/babashka/neil/pull/232

teodorlu commented 3 weeks ago

I might have misunderstood.

This deps.edn contains :pinned true:

{:deps {io.github.nextjournal/markdown {:git/sha "6683c48dfdb23404a23057817b6ac3acf0310bca" :neil/pinned true}}}

Is the issue that :git/url is added when the dep is not pinned?

If that is the case, I can reproduce. Removing :neil/pinned true and running neil dep upgrade gives me a dep with a :git/url:

{:deps {io.github.nextjournal/markdown {:git/sha "2a622669f52ce3119e563ec022f321d98e9d41b2"
                                        :git/url "https://github.com/nextjournal/markdown"}}}
borkdude commented 3 weeks ago

yes, when it's not pinned

teodorlu commented 3 weeks ago

With #233 on main, can this issue be closed?

borkdude commented 3 weeks ago

Thanks