Populate managers when checking out dep

[wojtekmach]

Fixes elixir-lang/elixir#6570

Regression was introduced in #372

cc @josevalim

[ericmj]

😍

[josevalim]

Thanks @wojtekmach!

The issue is that returning the lock was done by design: #372 (comment)

deps.get should not change the lock and this change would cause the lock to be modified. Is there any reason why the manager is not set on the lock returned by the remote converger?

[josevalim]

If computing the managers in the converger is a big change, one alternative is to check if the lock in opts[:lock] is an incomplete lock, i.e. a lock that was just returned from the converger and not a lock that came from mix.lock. You can probably check if the lock is incomplete if the managers field is not filled.

[ericmj]

We can't compute the the managers in the converger because the dependency is not fetched yet and this information is only in the package metadata.

I like @josevalim's idea, I think we can set the managers to :incomplete in the converger and check for this value in the SCM. Would you like to try this @wojtekmach?

[wojtekmach]

sounds good. I can work on this next week (feel free to pick it up in the meantime)

[wojtekmach]

Ok, I reverted the change and working on a different approach.

I was able to manually reproduce the issue with my previous change where mix.lock got rewritten with this:

1. create a project with :mime dep and Hex v0.15
2. use f7e936c
3. add :plug and run mix dips.get

So yes, with f7e936c mix.lock entry for :mime got unnecessarily updated. Do you have some suggestions how to put this into the test suite? Some other scenarios were already being tested: https://github.com/hexpm/hex/blob/wm-fix-managers/test/hex/mix_task_test.exs#L551

[wojtekmach]

Okay, I think this is ready for a 2nd pass.

I tried making the change in the converger but I got stuck. Anyhow, pattern matching on [] = managers seems reasonable because afaik managers should always be non-empty for Hex deps.

[ericmj]

Thanks for investigating. Build tools should always be non-empty so this looks good.

[ericmj]

Just leave a comment here about why we need this weirdness.

[ericmj]

Do the pattern match here also, so we don't automatically fall through if we forget to update when we change the lock format.

defp maybe_use_updated_lock({:hex, _, _, _, [] = _managers, _, _}, updated_lock), do: updated_lock
defp maybe_use_updated_lock({:hex, _, _, _, _managers, _, _} = old_lock, _updated_lock), do: old_lock

[wojtekmach]

@ericmj fyi: I got a crash when testing on https://github.com/wojtekmach/shipit. Will fix in a day or two.

** (FunctionClauseError) no function clause matching in Hex.SCM.maybe_use_updated_lock/2

    The following arguments were given to Hex.SCM.maybe_use_updated_lock/2:

        # 1
        {:hex, :ex_doc, "0.15.0", "e73333785eef3488cf9144a6e847d3d647e67d02bd6fdac500687854dd5c599f", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, optional: false]}]}

        # 2
        {:hex, :ex_doc, "0.15.0", "e73333785eef3488cf9144a6e847d3d647e67d02bd6fdac500687854dd5c599f", [:mix], [{:earmark, "~> 1.1", [repo: "hexpm", hex: :earmark, optional: false]}], "hexpm"}

    Attempted function clauses (showing 2 out of 2):

        defp maybe_use_updated_lock({:hex, _, _, _, [], _, _}, updated_lock)
        defp maybe_use_updated_lock({:hex, _, _, _, _managers, _, _} = old_lock, _updated_lock)

    (hex) lib/hex/scm.ex:154: Hex.SCM.maybe_use_updated_lock/2
    (mix) lib/mix/dep/fetcher.ex:64: Mix.Dep.Fetcher.do_fetch/3
    (mix) lib/mix/dep/converger.ex:185: Mix.Dep.Converger.all/9
    (mix) lib/mix/dep/converger.ex:121: Mix.Dep.Converger.all/7
    (mix) lib/mix/dep/converger.ex:106: Mix.Dep.Converger.all/4
    (mix) lib/mix/dep/converger.ex:51: Mix.Dep.Converger.converge/4
    (mix) lib/mix/dep/fetcher.ex:16: Mix.Dep.Fetcher.all/3
    (mix) lib/mix/tasks/deps.get.ex:27: Mix.Tasks.Deps.Get.run/1
    (mix) lib/mix/task.ex:315: Mix.Task.run_task/3
    (mix) lib/mix/cli.ex:80: Mix.CLI.run_task/2
    (elixir) lib/code.ex:646: Code.require_file/2

[ericmj]

fyi: I got a crash when testing on https://github.com/wojtekmach/shipit. Will fix in a day or two.

👍

Use Hex.Utils.lock/1 if you want to handle potentially old lock entries.