Hey guys, if implementation ideas can help in the discussions, I think the most straightforward way would be to re-use the "mix deps" code since it already reports lock mismatches. I came up with this draft <https://github.com/luisguilhermemsalmeida/elixir/commit/c0974a8528d129f15fe52fbf3559c5835bb301ae> for now
On Tuesday, April 5, 2022 at 6:41:21 PM UTC+2 [email protected] wrote: > To me, `mix deps.get` should not be modifying the lockfile by default, > with the following exceptions: > > - there is no `mix.lock` file, or > - `mix.lock` does not contain one of the dependencies > > In this case, a `mix deps.get --frozen-lockfile` case would absolutely > catch this for CI cases (uncommited `mix.lock`, etc.). > > I could see as well a `mix deps.get --update` or `mix deps.get > --update-git` that would permit the updating of things which have updates > and fit within the boundaries of the mix specifications, but that still > feels like it’s the realm of `mix deps.update`. > > That‘s why I said in my initial reply: > > This feels like something that either isn’t needed or should be the >> default behaviour, not an opt-in. > > > If we need something that can be run in CI and fail if mix.lock would > change for any reason, perhaps it should be a different command that would > *only* be used on CI or release builds. Maybe `mix deps.install`? > Otherwise, IMO the fundamental promise of "lock" in `mix.lock` isn’t really > being fulfilled. > > -a > > On Tue, Apr 5, 2022 at 12:23 PM 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> >> . >> > > > -- > 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/809594a1-992a-470a-8a41-d06239fe8314n%40googlegroups.com.
