[Proposal] - Add an option to deps.get to force failure on lock mismatch

[Luis Guilherme]

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 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 for node.js and I think it would be nice to use mix deps.get --strict on the elixir one as well

[Austin Ziegler]

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

--
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 elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/8918d9ca-2fcb-4abd-b28e-f7bf2a00ead1n%40googlegroups.com.

--

Austin Ziegler • halos...@gmail.com • aus...@halostatue.ca
http://www.halostatue.ca/ • http://twitter.com/halostatue

[Jon Rowe]

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

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAJ4ekQugHWnXrhtjPkipgbvPy%3DWzWWpzKLi7WCrJT3d_4AuJ3A%40mail.gmail.com.

[Ben Wilson]

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.

[José Valim]

A PR is welcome assuming that adding the feature is straight-forward. :)

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/02c62eb1-e75c-404a-9ecd-ae2d7165eaacn%40googlegroups.com.

[José Valim]

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 <christ...@gmail.com> 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.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/1f1888b6-2d41-42fa-b1af-154e8f6a61b9n%40googlegroups.com.

[Austin Ziegler]

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 <christ...@gmail.com> 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:

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/1f1888b6-2d41-42fa-b1af-154e8f6a61b9n%40googlegroups.com.