Yeah, we are thinking more about answering the question if the lock file changed. Because the git ref is in the lock, then We will check that. If hex dep is yanked, then deps.get fails. If changed, we would update the lock too, so it would catch that as well.
On Tue, Apr 5, 2022 at 18:23 Christopher Keele <[email protected]> wrote: > I took a really quick look to see what the most naive approach would look > like (literally before/after compare the mix.lock file, or rather the map > representing it before final write-back). > > I originally had a whole write-up documenting how this simple > lockfile-change-checking strategy would behave given path, umbrella, and > git dependencies, but there's really only one edge case worth discussing: > > * For a git dependency, if a branch/tag specification stays the same, but > resolves to a different ref, should a "strict CI installation" run be > invalidated?* > > This is the behaviour we would get from a simple lockfile-change-checking > strategy, since post-resolution all that is placed in the mix.lock file > is the ref. > > However, this means that CI runs would fail without the dependency > specification changing, or a developer forgetting to check in lockfile > changes, when an upstream branch or tag is moved. > > So either we: > > *1)* Build a solution that does more than just inspect the mix.lock file > for changes, ex special-casing git dependencies. > *2)* Build the simple lockfile-change-checking solution that is > surprisingly "strict" for git dependencies, mandating that the exact same > source code is installed, but not "strict" in other dependencies in similar > ways (ex: we cannot make the same guarantee for path or umbrella deps which > do not enter the lockfile, we do not check to see if a hex dep has been > yanked and republished in the short window to do so). > > Note that *1)* is not as simple as *ignoring* git dependencies during the > check, since presumably a git dependency specified with an explicit ref > *should* be validated to be up-to-date in the lockfile. > > Either way, I think --strict is a poor choice of flag, given that its > meaning is ambiguous and up to interpretation. In *1)* we are not > "strictly" ensuring the lockfile is up to date and in *2)* we are being > "strict" non-uniformly across dependency specification types. If we really > wanted to be explicit: > > - If we implemented 1) it'd be called something like > --enforce-fully-qualified-dependencies-unchanged and partially-qualified > dependencies would be allowed to change. > - If we implemented 2) it'd be called something like > --enforce-lockfile-unchanged and the actual effect would vary based on > mix's approach to storing different specification types in a lockfile > during dependency resolution. > > The fact that we are entering such nebulous waters suggests to me that > this feature is probably not worth the lift. At the end of the day, any > implementation here effectively forms a new public contract around how mix > resolves dependencies and records the results of resolution for various > internal purposes, and I'd rather keep that as an implementation detail so > we can change its behaviour more freely. > On Saturday, April 2, 2022 at 2:59:58 AM UTC-4 José Valim wrote: > >> A PR is welcome assuming that adding the feature is straight-forward. :) >> >> On Sat, Apr 2, 2022 at 06:24 Ben Wilson <[email protected]> wrote: >> >>> Yup, I also see the value in a human check. I think it's analogous to >>> `mix format --checked` where the option explicitly exists to allow systems >>> to enforce expectations. >>> >>> +1 from me. >>> >>> On Friday, April 1, 2022 at 1:51:12 PM UTC-4 [email protected] wrote: >>> >>>> If I'm understanding the original post correctly its a check for >>>> preventing human error, e.g. they've run an update or a get but forgotten >>>> to check in the changes to `mix.lock`, it's not something that needs to be >>>> a default because that works fine, but just a nicety to prevent dirty >>>> source code checkouts in a CI environment. >>>> >>>> Personally I don't see the harm in that, its just an improvement for >>>> developer experience in setting up CI, I could equally see that this could >>>> be moved to "well your ci should fail if you care about that" (it wouldn't >>>> be that hard to write a step after `mix deps.get` that checked the file >>>> hadn't changed). Overall if its easy for mix to do I'd say "why not", if >>>> its problematic due to implementation reasons and would cause additional >>>> maintenance burden I'd be ok to say "yeah nah". >>>> >>>> Just my 2¢. >>>> >>>> Cheers >>>> Jon >>>> >>>> On Wed, 30 Mar 2022, at 4:43 PM, Austin Ziegler wrote: >>>> >>>> This feels like something that either isn’t needed or should be the >>>> default behaviour, not an opt-in. >>>> >>>> Where I feel it may not be needed is because if there is a mismatch >>>> while I am developing, it is a *deliberate* change that I have made >>>> and want the implicit update behaviour. It also only happens if there’s a >>>> version mismatch (e.g., the mix.exs file contains *~> 3.2* but the >>>> mix.lock file is *3.1.2*). Otherwise, mix.lock is frozen. That is, if >>>> mix.exs contains *~> 3.2* and the mix.lock is *3.2.2* but *3.5.2* is >>>> available, there will not be an update applied. >>>> >>>> Where I feel it may be better as the default behaviour is that I think >>>> that mix deps.get --update-changed might be better as you explicitly >>>> tell the tooling that you expect an update. >>>> >>>> I’m not happy in the Node ecosystem that you have to use npm ci or yarn >>>> install --frozen-lockfile in order to not have volatile lockfiles. The >>>> behaviour in the Node ecosystem is that a transitive dependency *may* >>>> update >>>> with a normal npm install or yarn install. >>>> >>>> -a >>>> >>>> >>>> On Wed, Mar 30, 2022 at 10:31 AM Luis Guilherme < >>>> [email protected]> wrote: >>>> >>>> If dependencies in the mix.lock do not match those in mix.exs, *mix >>>> deps.get --strict* will exit with an error, instead of updating the >>>> mix.lock file. >>>> >>>> This is inspired by npm ci >>>> <https://stackoverflow.com/questions/52499617/what-is-the-difference-between-npm-install-and-npm-ci> >>>> and >>>> aims to solve a rather common problem of people updating mix.exs but >>>> forgetting to update the mix.lock file. >>>> (there are non-obvious situations if you have path dependencies, where >>>> updating a dependency version will cascade to every other mix project using >>>> it) >>>> >>>> npm ci is used on the official github action >>>> <https://github.com/actions/starter-workflows/blob/main/ci/node.js.yml> >>>> for node.js and I think it would be nice to use mix deps.get --strict on >>>> the elixir one as well >>>> >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "elixir-lang-core" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/elixir-lang-core/8918d9ca-2fcb-4abd-b28e-f7bf2a00ead1n%40googlegroups.com >>>> <https://groups.google.com/d/msgid/elixir-lang-core/8918d9ca-2fcb-4abd-b28e-f7bf2a00ead1n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>>> >>>> >>>> -- >>>> Austin Ziegler • [email protected] • [email protected] >>>> http://www.halostatue.ca/ • http://twitter.com/halostatue >>>> >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "elixir-lang-core" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/elixir-lang-core/CAJ4ekQugHWnXrhtjPkipgbvPy%3DWzWWpzKLi7WCrJT3d_4AuJ3A%40mail.gmail.com >>>> <https://groups.google.com/d/msgid/elixir-lang-core/CAJ4ekQugHWnXrhtjPkipgbvPy%3DWzWWpzKLi7WCrJT3d_4AuJ3A%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>>> >>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "elixir-lang-core" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> >> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/elixir-lang-core/02c62eb1-e75c-404a-9ecd-ae2d7165eaacn%40googlegroups.com >>> <https://groups.google.com/d/msgid/elixir-lang-core/02c62eb1-e75c-404a-9ecd-ae2d7165eaacn%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- > You received this message because you are subscribed to the Google Groups > "elixir-lang-core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/elixir-lang-core/1f1888b6-2d41-42fa-b1af-154e8f6a61b9n%40googlegroups.com > <https://groups.google.com/d/msgid/elixir-lang-core/1f1888b6-2d41-42fa-b1af-154e8f6a61b9n%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "elixir-lang-core" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4J9WuWUvqYLfd2zr6dWV2t7Nr7G_XdHqwWPdt8ET-xDmw%40mail.gmail.com.
